fix(adapters): preserve routed model identity - #895
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change adds ChangesRouted model identity
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ClientAdapter
participant MessageFormatter
participant identifyRoutedModel
participant UpstreamRequest
ClientAdapter->>MessageFormatter: messages and effective wire model ID
MessageFormatter->>identifyRoutedModel: assembled system text and model ID
identifyRoutedModel-->>MessageFormatter: routed identity text
MessageFormatter->>UpstreamRequest: formatted request with transformed system text
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@tests/identity-neutralize.test.ts`:
- Around line 68-72: Add focused regression tests alongside the existing
identifyRoutedCatalogModel tests for an empty model ID and an ID created with
"x".repeat(129). Assert both outputs use the fallback text "configured model",
preserving the existing unsafe-text regression coverage.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bebd700c-f5cc-402b-90f8-80f699960b78
📒 Files selected for processing (5)
src/adapters/identity.tssrc/adapters/openai-chat.tssrc/codex/catalog/sync.tstests/codex-catalog-golden.test.tstests/identity-neutralize.test.ts
|
Updated in 38686e5 after CodeRabbit and two independent review rounds. Identity now follows the adapter-normalized wire model across OpenAI Chat, Anthropic, Google/Antigravity, Kiro, and catalog sync; unsafe IDs fall back, valid OpenRouter aliases are preserved, and creator provenance is no longer guessed from incomplete prefixes. Validation: focused 123/123 (independent review: 200/200); typecheck, privacy scan, and diff check pass. The full prepush run reached 6,966 pass / 8 skip; its 7 failures are the same host-DNS fixture failures reproduced on clean dev. @coderabbitai review |
|
✅ Action performedReview finished.
|
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR metadata and commits =='
gh pr view 895 --repo lidge-jun/opencodex --json headRefOid,baseRefOid,commits,title --jq '{title, headRefOid, baseRefOid, commits: [.commits[].oid]}'
echo '== Changed files =='
git diff --name-status origin/"$(gh pr view 895 --repo lidge-jun/opencodex --json baseRefName --jq -r .baseRefName)"...HEAD
echo '== Diff since requested commit parent =='
git show --format=fuller --stat 38686e51
git diff --find-renames 38686e51^ 38686e51 -- src/adapters src/codex tests
echo '== Source outlines =='
fd -t f . src/adapters src/codex tests | sort | sed -n '1,240p'
ast-grep outline src/adapters/identity.ts --items all 2>/dev/null || true
ast-grep outline src/adapters/openai-chat.ts --items all 2>/dev/null || true
echo '== Routed identity helper call sites and wire-model normalization candidates =='
rg -n -C 4 --glob '*.ts' \
'identifyRoutedCatalogModel\s*\(|neutralizeIdentity\s*\(|wireModel|wire_model|modelName|model_name|requestedModel|requested_model' \
src/adapters src/codex testsLength of output: 1921 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='f9b9440c551e3d7f3e2041098caa2ee4de57698e'
head='38686e51db22fad2e036544e67b675cc24d82aa4'
echo '== Complete PR file list =='
git diff --name-status "${base}...${head}"
echo '== Current commit stat and patch =='
git show --format=fuller --stat "${head}"
git diff --find-renames "${head}^" "${head}" -- src/adapters src/codex tests
echo '== Final source outlines =='
for file in src/adapters/identity.ts src/adapters/openai-chat.ts; do
if [ -f "$file" ]; then
echo "---- $file ----"
ast-grep outline "$file" --items all || true
fi
done
echo '== Identity-helper call sites with context =='
rg -n -C 8 --glob '*.ts' \
'identifyRoutedCatalogModel\s*\(|neutralizeIdentity\s*\(' \
src/adapters src/codex tests || true
echo '== Model normalization and forwarded system-message construction =='
rg -n -C 6 --glob '*.ts' \
'wire[Mm]odel|model[Aa]lias|resolve[A-Za-z]*Model|normalized[A-Za-z]*Model|system.*(?:message|prompt)|(?:message|prompt).*system' \
src/adapters src/codex tests || true
echo '== Identity source and identity test source =='
for file in src/adapters/identity.ts tests/identity-neutralize.test.ts; do
if [ -f "$file" ]; then
echo "---- $file ----"
cat -n "$file"
fi
doneLength of output: 224493 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== OpenAI Chat request construction =='
sed -n '80,175p' src/adapters/openai-chat.ts
sed -n '555,760p' src/adapters/openai-chat.ts
echo '== Anthropic request construction =='
sed -n '430,555p' src/adapters/anthropic.ts
sed -n '630,760p' src/adapters/anthropic.ts
echo '== Google request construction and Antigravity resolver =='
sed -n '115,150p' src/adapters/google.ts
sed -n '290,405p' src/adapters/google.ts
fd -t f 'antigravity-models.ts' src/providers --exec sh -c 'echo "---- $1 ----"; cat -n "$1"' sh {}
echo '== Kiro model normalization and payload construction =='
rg -n -C 8 --glob 'kiro.ts' \
'function normalizeKiroModelId|normalizeKiroModelId\s*\(|buildKiroPayload|modelId\s*:|model\s*:' \
src/adapters/kiro.ts
sed -n '400,535p' src/adapters/kiro.ts
sed -n '700,860p' src/adapters/kiro.ts
echo '== Catalog model identity source =='
sed -n '145,215p' src/codex/catalog/sync.ts
rg -n -C 5 --glob '*.ts' 'deriveEntry\s*\(' tests src | head -n 220
echo '== All final model assignments in changed adapters =='
rg -n -C 3 --glob '*.ts' \
'(?:model|modelId)\s*:\s*(?:parsed\.modelId|modelId|wireModelId|routedModelId|stripBracketedModelSuffix)' \
src/adapters/openai-chat.ts src/adapters/anthropic.ts src/adapters/google.ts src/adapters/kiro.tsLength of output: 77009 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='f9b9440c551e3d7f3e2041098caa2ee4de57698e'
head='38686e51db22fad2e036544e67b675cc24d82aa4'
echo '== Kiro model mapping and final payload field =='
rg -n -C 10 --glob 'kiro.ts' \
'function mapModelId|const modelId = mapModelId|modelId\s*:|model\s*:' \
src/adapters/kiro.ts
sed -n '535,615p' src/adapters/kiro.ts
echo '== Complete PR patch, excluding generated or unrelated files =='
git diff --find-renames --unified=40 "${base}...${head}" -- \
src/adapters/identity.ts \
src/adapters/openai-chat.ts \
src/adapters/anthropic.ts \
src/adapters/google.ts \
src/adapters/kiro.ts \
src/codex/catalog/sync.ts \
tests/identity-neutralize.test.ts \
tests/codex-catalog-golden.test.ts
echo '== Tests that exercise Antigravity model resolution =='
rg -n -C 5 --glob '*.test.ts' \
'resolveAntigravityEffortWireModel|cloud-code-assist|gemini-3\.6-flash-(?:low|medium|high)|gemini-pro-agent' \
testsLength of output: 105019
I verified that the identity text uses the same model identifier as the outbound request in the changed adapters:
The focused tests cover unsafe interpolation, OpenRouter aliases, bracket normalization, and the changed adapter paths. I did not run tests in the inspection sandbox. ✅ Action performedReview finished.
|
There was a problem hiding this comment.
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 `@tests/identity-neutralize.test.ts`:
- Around line 173-180: Add a focused Cloud Code Assist regression test alongside
the existing identity-neutralization test, configuring the provider with
googleMode "cloud-code-assist", baseUrl, apiKey, and project, and using an
effort-mapped model. Exercise the adapter’s request envelope and assert that
envelope.request.systemInstruction identifies the same model ID as
envelope.model, ensuring resolveAntigravityEffortWireModel is covered.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a214308e-4bb6-4a09-a756-64d1cdef3b5d
📒 Files selected for processing (7)
src/adapters/anthropic.tssrc/adapters/google.tssrc/adapters/identity.tssrc/adapters/kiro.tssrc/adapters/openai-chat.tssrc/codex/catalog/sync.tstests/identity-neutralize.test.ts
| test("google/antigravity: systemInstruction is neutralized, no proxy mention", async () => { | ||
| const provider = { adapter: "google", baseUrl: "https://generativelanguage.googleapis.com", apiKey: "key" }; | ||
| const { body } = await createGoogleAdapter(provider).buildRequest(parsed("gemini-3-pro", "google")); | ||
| const sysText = JSON.parse(body).systemInstruction.parts.map((p: { text: string }) => p.text).join(""); | ||
| expect(sysText).toContain(NEUTRAL_IDENTITY_LINE); | ||
| expect(sysText).toContain("identify as gemini-3-pro"); | ||
| expect(sysText).not.toMatch(/opencodex proxy/i); | ||
| expect(sysText).not.toContain(SYS); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Exercise the Cloud Code Assist identity path.
Lines 173-175 omit googleMode: "cloud-code-assist". The test uses the AI Studio branch and never executes the effort-specific resolveAntigravityEffortWireModel path at src/adapters/google.ts lines 298-304.
Add a Cloud Code Assist provider with baseUrl, apiKey, and project. Use an effort-mapped model. Assert that envelope.request.systemInstruction identifies the same model ID as envelope.model.
As per path instructions, tests/** changes must include a focused regression test for the changed adapter behavior.
🤖 Prompt for 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.
In `@tests/identity-neutralize.test.ts` around lines 173 - 180, Add a focused
Cloud Code Assist regression test alongside the existing identity-neutralization
test, configuring the provider with googleMode "cloud-code-assist", baseUrl,
apiKey, and project, and using an effort-mapped model. Exercise the adapter’s
request envelope and assert that envelope.request.systemInstruction identifies
the same model ID as envelope.model, ensuring resolveAntigravityEffortWireModel
is covered.
Source: Path instructions
2 participants
Summary
openai-chat, Anthropic, Google/Antigravity, and Kiro system messages;configured modelfor blank, overlong, or unsafe model IDs;Why
Issue #622 and PR #638 fixed current Codex CLI wording that escaped identity neutralization. That prevents a routed non-OpenAI model from inheriting the false Codex/GPT identity, but the neutral replacement still cannot name the model the user actually selected.
The catalog and adapters already know the concrete upstream model ID. Reusing the final wire value produces an accurate answer to model-identity questions while retaining a strict character allowlist and a safe fallback.
Behavior and safety
CODEX_GPT5_IDENTITY_RE.._/@:+-[]~.configured model.Related
Test plan
bun test tests/identity-neutralize.test.ts tests/codex-catalog-golden.test.ts tests/openai-chat-model-suffix.test.ts tests/google-antigravity-wire.test.ts tests/kiro-adapter.test.ts tests/anthropic-tail-guard.test.ts— 123 passed, 0 failedbun run typecheckbun run privacy:scanbun run prepushreached 6,966 passed / 8 skipped; the only 7 failures reproduce unchanged on cleandevbecause this host resolves reserved/example test hostnames into the benchmark address rangeSummary by CodeRabbit