fix(context): avoid noisy metadata fallback errors#1544
Conversation
|
Worried about impact? Review this PR in Change Stack to explore blast radius before you approve or request changes. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (3)**/*⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughThis PR replaces unconditional console.error logging with a deduplicated warning helper that logs one warn-level message per (routeId, model) via logForDebugging; a module Set tracks warned keys. getContextWindowForModel now calls the helper before returning the OPENAI_FALLBACK_CONTEXT_WINDOW. Tests assert the 128_000 fallback is returned without console.error and that a single warn is emitted. ChangesDeduplicated unknown model warnings
🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 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 `@src/utils/context.test.ts`:
- Around line 436-451: The test 'unknown openai-compatible model fallback does
not emit console errors' should be extended to assert the dedupe contract by
mocking the debug/warn logger used by the context fallback (replace or spy on
the module's debug logger) and then calling getContextWindowForModel twice with
the same (routeId, model) key, asserting that the warning/debug-warn was invoked
exactly once; restore the original logger after the assertions. Specifically,
locate uses of getContextWindowForModel in the test file and add a mock/spy for
the debug-warn function the module emits (the logger the code uses to emit the
fallback warning), call getContextWindowForModel('another-unknown-3p-model')
twice (same routeId), expect the mocked warn toHaveBeenCalledTimes(1), and
ensure cleanup by restoring the original logger.
🪄 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: a0039d63-6111-4ee1-a096-d61b2d3e21e9
📒 Files selected for processing (2)
src/utils/context.test.tssrc/utils/context.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files and prefer small, readable changes over broad rewrites
Files:
src/utils/context.tssrc/utils/context.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/utils/context.tssrc/utils/context.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/utils/context.tssrc/utils/context.test.ts
**/*.test.{ts,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/context.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/context.test.ts
🔇 Additional comments (1)
src/utils/context.ts (1)
4-4: LGTM!Also applies to: 43-43, 83-95, 129-129
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found one issue that needs to be addressed before this is ready.
Findings
- [P2] Type the debug logger mock before reading its call arguments
src/utils/context.test.ts:466
The new test declareslogForDebuggingasmock(() => {}), so Bun infers a zero-argument mock. The assertion then readslogForDebugging.mock.calls[0]?.[1], andbun run typechecknow reportsTS2493/TS2769on this added line because that tuple has no second argument. Please type the mock with the same parameters aslogForDebuggingor otherwise assert the options from a typed call tuple, so this regression test does not add a new touched-path type error.
* fix(context): avoid noisy metadata fallback errors * test(context): cover metadata warning dedupe * docs(context): explain metadata fallback warning * test(context): type debug warning spy
3 participants
Summary
console.errorto debug loggingImpact
Testing
bun test src/utils/context.test.tsenv -u CLAUDE_CODE_USE_OPENAI -u OPENAI_BASE_URL -u OPENAI_MODEL -u OPENAI_API_KEY bun test src/integrations/runtimeMetadata.test.ts src/utils/context.test.ts src/services/compact/autoCompact.test.tsSummary by CodeRabbit
Bug Fixes
Tests