feat: phases 2 and 3 of openrouter migration plan. - #148
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The current error-handling logic can misclassify some OpenRouter 400s and unnecessarily retries fallback requests for non-retryable 4xx streaming failures (e.g. 402/429), which can waste requests/credits and delay correct user feedback.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Implements phases 2–3 of the OpenRouter migration by switching the chat runtime to OpenRouter, adding in-drawer API key management, and hardening “apply/proposal” behaviors so editor-update tooling is only enabled on explicit edit intent.
Changes:
- Migrate chat requests/error handling from GitHub Models to OpenRouter (endpoints, status messaging, SSE/fallback behavior).
- Add OpenRouter key storage + drawer UI controls and gate the composer until a key is present.
- Gate editor-update tool usage and apply-actions rendering based on explicit user intent, with updated Playwright coverage and docs.
File summaries
| File | Description |
|---|---|
| src/styles/ai-controls.css | Adds layout + styles for the new OpenRouter key section and compact chat toggle text treatment. |
| src/modules/chat/utils.js | Expands error classification (model access + credential errors) for OpenRouter responses. |
| src/modules/chat/proposals.js | Adds an option to disable markdown fallback proposal parsing when apply actions should be suppressed. |
| src/modules/chat/payload.js | Adds intent detection to decide when editor-update tools should be enabled. |
| src/modules/chat/key-store.js | Introduces localStorage-backed OpenRouter key persistence and masking utility. |
| src/modules/chat/key-controls.js | Implements key add/remove UI behavior and emits key-change callbacks to the drawer. |
| src/modules/chat/drawer.js | Wires key controls into the chat drawer, gates tools/apply-actions by intent, and updates status/error handling. |
| src/modules/chat/api/request.js | Updates request/response error mapping for OpenRouter and removes header-based rate parsing. |
| src/modules/chat/api/constants.js | Switches endpoint + default model and updates fallback model list for OpenRouter. |
| src/modules/chat/api/completions.js | Propagates status into API errors and updates rate metadata handling for OpenRouter. |
| src/modules/app-core/github-workflows.js | Updates PR surface visibility sync function name/behavior. |
| src/modules/app-core/github-pr-context-ui.js | Decouples PR UI visibility from chat visibility and removes chat-close coupling. |
| src/modules/app-core/chat-workflows.js | Passes OpenRouter key UI nodes into chat drawer initialization. |
| src/modules/app-core/app-bindings-startup.js | Updates startup visibility sync call to the renamed PR surface method. |
| src/index.html | Moves chat toggle visibility behavior, adds OpenRouter key UI section, and updates chat prompt input attributes. |
| src/app.js | Wires new DOM nodes for key controls and updates chat/pr visibility integration. |
| playwright/helpers/app-test-helpers.ts | Adds OpenRouter key connection helper and tightens the Chat toggle selector. |
| playwright/github-byot-ai.spec.ts | Updates tests for OpenRouter endpoints, key gating, intent gating, and token separation. |
| docs/openrouter-migration-plan.md | Updates implementation status and verification notes for phases 2–3. |
Review details
Suppressed comments (1)
src/modules/chat/drawer.js:905
- When the streaming request fails with a non-credential, non-model 4xx (notably 402/429), the drawer always retries via the non-stream fallback. That second request can’t succeed and may double-charge / double-hit rate limits while delaying the actual error shown to the user.
setChatStatus(
'Streaming unavailable. Retrying with fallback response...',
'pending',
)
}
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
No description provided.