feat(ext): expose the detected session ID to request rewriters by SantiagoDePolonia · Pull Request #612 · ENTERPILOT/GoModel

SantiagoDePolonia · 2026-07-28T17:12:22Z

Summary

Adds SessionID to ext.Input. Session detection (SessionCapture) already runs before RequestRewriteMiddleware, so the user-path-scoped session ID is in the request context when rewriters fire — this just surfaces it.

Motivation: lets a rewriter keep per-conversation decisions stable across messages, e.g. GoModel Pro's content-aware tier router pinning its easy/hard model choice to the session instead of re-deciding (and potentially flip-flopping) on every message.

Stacked on #611 (base: feat/ext-route-selector); rebase to main if it lands first.

Tests

TestRequestRewriteMiddlewareExposesSessionID — stamps a session via ExtraMiddleware (which runs before rewriters, standing in for the detector) and asserts the rewriter observes it.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Request rewriters can now access the detected client session ID via the rewrite input, helping ensure consistent decisions across a conversation.
Tests
- Added a test that verifies the session ID is exposed to request rewriters when present, and is empty when not detected.

Extensions can now steer load balancing for virtual models. A new ext.RouteSelector registers through the registry's single selector slot and is consulted by redirects using the new "adaptive" strategy; it picks among the currently viable targets (catalog-supported, with rate-limit capacity) and observes every upstream attempt — primaries, retries, and failovers — through the llmclient hooks, so it can score targets by real traffic. Session affinity, capacity filtering, failover chains, and retries all remain core's responsibility. Without a registered selector — or when the selector declines, answers outside the pool, or panics (contained and logged) — the adaptive strategy behaves exactly like round_robin, so configs stay portable between core and extended builds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai · 2026-07-28T17:12:47Z

📝 Walkthrough

Walkthrough

ext.Input now exposes the detected session ID to request rewriters. RequestRewriteMiddleware populates it from request context, and a test verifies propagation through session stamping and rewriting.

Changes

Session ID propagation

Layer / File(s)	Summary
Session ID input and middleware propagation `ext/ext.go`, `internal/server/request_rewrite.go`, `internal/server/request_rewrite_test.go`	`ext.Input` includes a path-scoped `SessionID`; request rewriting copies it from context, and table-driven middleware coverage verifies detected and empty session values.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A rabbit twitched its nose with glee,
“A session tag now hops along!”
Through context fields and rewrite paths,
The rewriter finds where it belongs.
“sess-42!” we cheer in song.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: exposing the detected session ID to request rewriters.
Description check	✅ Passed	It covers what changed, why it matters, and the test added, though it uses different headings than the template.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/ext-session-id

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/server/request_rewrite_test.go`:
- Around line 122-154: Convert TestRequestRewriteMiddlewareExposesSessionID into
a table-driven test covering both a populated session ID and an absent session
ID. For each case, configure the session-stamping middleware accordingly,
execute the request, and assert the rewriter observes the expected session
value, including an empty default when no session is present.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ef829019-e33f-432e-920d-8082be09d54a

📥 Commits

Reviewing files that changed from the base of the PR and between c31d624 and 3d71ff7.

📒 Files selected for processing (3)

ext/ext.go
internal/server/request_rewrite.go
internal/server/request_rewrite_test.go

greptile-apps · 2026-07-28T17:20:23Z

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Files Needing Attention: No files require changes.

T-Rex Logs

What T-Rex did

Executed the request rewrite session propagation test suite and saved a log showing a successful run with exit code 0 and a PASS result.
The log records the Go version, the target package, the working directory, and verbose subtest output for review.

_{Ran code and verified through T-Rex}

_{Reviews (2): Last reviewed commit: "test(server): assert the rewriter ran in..." | Re-trigger Greptile}

Panic recovery around selector calls no longer re-enters extension code: the selector's name is captured panic-safe at install time, so a Name that panics mid-recovery can no longer escape the handler and abort the request instead of falling back. The recovered value is no longer logged — it is extension-controlled and could carry request data. Candidate pricing is now defensively copied: selectors received the catalog's own *float64 values, so a misbehaving extension could mutate the prices that cost routing and cost calculation later read. The per-attempt observation contract is corrected to match behavior: hooks fire once per upstream call, and transport-level retries inside the provider client are aggregated into that call's single outcome (final status, duration spanning retry backoff). Moving observation inside the client retry loop would change hook semantics for every consumer (metrics, health tracking) and is out of scope here. Also: registry selector-slot tests are table-driven (incl. nil reset), the delegation test asserts the full candidate set, a regression test covers the double-panic path, and the example config no longer implies the adaptive strategy requires an extension. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

SantiagoDePolonia · 2026-07-28T17:52:43Z

Converted the session-propagation test to table-driven cases (detected session propagates / no session yields empty) in 5a91572, and rebased on the latest #611.

The strategy dropdown was hardcoded to round_robin and cost, so the new adaptive strategy was unreachable from the UI even on deployments with a route selector registered — and hardcoding would have offered it on plain core, where it silently falls back to round robin. /admin/runtime/config now carries VIRTUAL_MODEL_STRATEGIES, the comma- separated strategies this deployment supports; adaptive appears only when a route-selector extension is registered. The backend owns the values, the dashboard owns the labels (unknown values render as their raw value), and the editor keeps the edited row's current strategy selectable even when the deployment no longer offers it, so opening an adaptive virtual model on a plain-core dashboard no longer renders a blank select that could silently rewrite the strategy on save. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/server/request_rewrite_test.go`:
- Around line 133-165: In the test around the capturing stubRewriter, assert
that capturing.calls equals 1 after the request succeeds and before validating
seenSession. This must cover both session and no-session table cases, ensuring
the rewriter was invoked rather than relying only on the default empty
seenSession value.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 848cc916-eae3-4f57-9dd6-c874e641aac6

📥 Commits

Reviewing files that changed from the base of the PR and between 1a3bd5b and edd6ec5.

📒 Files selected for processing (3)

ext/ext.go
internal/server/request_rewrite.go
internal/server/request_rewrite_test.go

The label promised health/latency/cost routing, but core's contract only delegates target choice to the registered extension — describe that, not one extension's policy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Session detection already runs before the rewrite middleware, so the scoped session ID is sitting in the request context when rewriters fire. Surfacing it on ext.Input lets a rewriter keep per-conversation decisions stable — for example pinning a model-tier choice to the session instead of re-deciding on every message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

SantiagoDePolonia · 2026-07-29T15:33:02Z

Addressed in 3662589: the no-session case now asserts the rewriter actually ran (calls == 1) before checking the empty SessionID, so it can't pass vacuously. Rebased on the latest #611.

# Conflicts: # internal/admin/dashboard/static/dist/index.html # web/dashboard/src/pages/models/VirtualModelEditor.svelte

The help condition used the dropdown options, which include an edited row's unsupported value for compatibility — so an adaptive virtual model opened on a deployment without a route selector still showed help claiming a registered extension picks targets. Gate on the server-provided strategy list instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

# Conflicts: # internal/admin/dashboard/static/dist/index.html

…normalization normalizeDashboardRuntimeConfig rebuilt the response field-by-field and dropped VirtualModelStrategies, so /admin/runtime/config never exposed the strategy list and the dashboard could not offer adaptive. Carry the field through and assert it in the handler-level runtime-config test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

codecov-commenter · 2026-07-30T16:10:59Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

coderabbitai Bot reviewed Jul 28, 2026

View reviewed changes

Comment thread internal/server/request_rewrite_test.go

SantiagoDePolonia force-pushed the feat/ext-session-id branch from 3d71ff7 to 1a3bd5b Compare July 28, 2026 17:39

SantiagoDePolonia mentioned this pull request Jul 28, 2026

feat(ext): add route selector extension point and adaptive strategy #611

Merged

test(virtualmodels): cover every pricing pointer in the isolation test

b3c07a4

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

SantiagoDePolonia force-pushed the feat/ext-session-id branch from 1a3bd5b to 5a91572 Compare July 28, 2026 17:50

SantiagoDePolonia force-pushed the feat/ext-session-id branch from 5a91572 to edd6ec5 Compare July 29, 2026 09:50

coderabbitai Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread internal/server/request_rewrite_test.go

SantiagoDePolonia and others added 4 commits July 29, 2026 17:32

fix(dashboard): describe adaptive as extension-defined

fe6a0e6

The label promised health/latency/cost routing, but core's contract only delegates target choice to the registered extension — describe that, not one extension's policy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

test(server): table-driven session propagation coverage

d96c808

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

test(server): assert the rewriter ran in the no-session case

3662589

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

SantiagoDePolonia force-pushed the feat/ext-session-id branch from edd6ec5 to 3662589 Compare July 29, 2026 15:32

SantiagoDePolonia and others added 9 commits July 30, 2026 17:10

Merge remote-tracking branch 'origin/main' into feat/ext-route-selector

38263ff

# Conflicts: # internal/admin/dashboard/static/dist/index.html # web/dashboard/src/pages/models/VirtualModelEditor.svelte

Merge branch 'feat/ext-route-selector' into feat/ext-session-id

abcfe55

Merge branch 'feat/ext-route-selector' into feat/ext-session-id

56b0dcd

Merge remote-tracking branch 'origin/main' into feat/ext-route-selector

c6ec988

# Conflicts: # internal/admin/dashboard/static/dist/index.html

Merge branch 'feat/ext-route-selector' into feat/ext-session-id

2d56255

Merge branch 'feat/ext-route-selector' into feat/ext-session-id

176d0ba

Merge remote-tracking branch 'origin/main' into feat/ext-session-id

9a51eb7

SantiagoDePolonia changed the base branch from feat/ext-route-selector to main July 30, 2026 16:05

SantiagoDePolonia merged commit 5cd22ee into main Jul 30, 2026
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat(ext): expose the detected session ID to request rewriters - #612

feat(ext): expose the detected session ID to request rewriters#612
SantiagoDePolonia merged 17 commits into
mainfrom
feat/ext-session-id

SantiagoDePolonia commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 28, 2026 •

edited

Loading

T-Rex Logs

Uh oh!

SantiagoDePolonia commented Jul 28, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

SantiagoDePolonia commented Jul 29, 2026

Uh oh!

Uh oh!

codecov-commenter commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

SantiagoDePolonia commented Jul 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Tests

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

T-Rex Logs

Uh oh!

SantiagoDePolonia commented Jul 28, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SantiagoDePolonia commented Jul 29, 2026

Uh oh!

Uh oh!

codecov-commenter commented Jul 30, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SantiagoDePolonia commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

greptile-apps Bot commented Jul 28, 2026 •

edited

Loading