Retry transient model route misses in console chat - #1277
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe chat connection detects qualifying HTTP 404 model-route errors, waits 500 ms with abort support, and retries once. Tests verify recovery through a streamed response. ChangesModel-route retry
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The change narrowly retries one transient missing-model response while preserving the request and selected model; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant MeshConnection
participant Fetch
participant RetryDelay
participant StreamResponse
MeshConnection->>Fetch: Request model stream
Fetch-->>MeshConnection: 404 missing model route
MeshConnection->>RetryDelay: Wait 500 ms or abort
RetryDelay-->>MeshConnection: Delay completes
MeshConnection->>Fetch: Retry request once
Fetch-->>StreamResponse: Return successful stream
StreamResponse-->>MeshConnection: Emit text content
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![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
🧹 Nitpick comments (1)
crates/mesh-llm-ui/src/features/chat/api/mesh-connection.test.ts (1)
450-478: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the recovery test deterministic and verify the recovered text.
This test waits for the real 500 ms retry delay. Use Vitest fake timers and advance them after the retry wait starts.
The current assertion only proves that a text event exists. Assert that the emitted text contains
"Recovered"so an empty or incorrect recovered delta fails.As per coding guidelines, “Keep tests deterministic; avoid relying on real timers unless using fake timers.”
🤖 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 `@crates/mesh-llm-ui/src/features/chat/api/mesh-connection.test.ts` around lines 450 - 478, Update the recovery test around createMeshConnectionAdapter to use Vitest fake timers, advancing the clock after the retry delay begins so it does not wait on the real 500 ms delay. Strengthen the chunks assertion to verify the emitted text content includes “Recovered,” not merely that a text event exists, and restore fake timers after the test.Source: Coding guidelines
🤖 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 `@crates/mesh-llm-ui/src/features/chat/api/mesh-connection.ts`:
- Around line 223-227: Update the retry handling around waitForModelRouteRetry
so that when abortSignal.aborted becomes true during the wait, the function
returns immediately before constructing ApiError. Preserve the existing ApiError
path for non-aborted failures and non-retryable responses.
---
Nitpick comments:
In `@crates/mesh-llm-ui/src/features/chat/api/mesh-connection.test.ts`:
- Around line 450-478: Update the recovery test around
createMeshConnectionAdapter to use Vitest fake timers, advancing the clock after
the retry delay begins so it does not wait on the real 500 ms delay. Strengthen
the chunks assertion to verify the emitted text content includes “Recovered,”
not merely that a text event exists, and restore fake timers after the test.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 75d30edd-3e28-402d-9ab5-c1b531d7be47
📒 Files selected for processing (2)
crates/mesh-llm-ui/src/features/chat/api/mesh-connection.test.tscrates/mesh-llm-ui/src/features/chat/api/mesh-connection.ts
2 participants
Summary
apple/systemmodel-not-found responseWhy
The console can receive a model from
/api/modelsbefore the public gateway has an eligible route for it. The first/api/responsesrequest then returns 404 even though the model is valid and becomes routable moments later. Previously this surfaced as “Message failed to send” and required a manual refresh or model re-selection.The adapter now waits 500 ms and retries once only for a 404 whose error body identifies a missing model. Other errors are still surfaced immediately, and aborts cancel the retry wait.
Validation
just buildpasses.DataModeContextlocalStorage test remains failing in this checkout.Fixes the console-side portion of the public model catalog/routing convergence issue.
Summary by CodeRabbit