Fix Prefix - #11
Merged
Merged
Conversation
- modelSupportsThinking now treats catalog facts, advertised effort lists, and matching model-family contracts as support evidence instead of denying reasoning when a per-provider regex misses the model name (fixes MiniMax M3 on Bynara: picker offered efforts but reasoning stayed off and nothing was sent) - resolved profiles prefer live catalog-advertised efforts over builtin lists so the picker and the wire always agree per model - meta no longer hardcodes an effort list: advertised efforts win when the provider sends them, otherwise the full scale is offered - learned negatives (reasoning:false, controlDialect, rejectedFields) are never persisted and legacy ones are scrubbed on hydration, so every session starts at slate 0; rejections stay in-memory with the existing retry ladder handling them per session - rejected-field learning is session-scoped
Removing a plan/responder/instructions block used to splice it out of the outgoing timeline, shifting already-sent bytes and busting prefix caches on every append-only provider. Clearing a block now appends a short '(cleared)' tombstone under the same prefix; re-adding it appends a fresh copy. Clearing a block that was never present is a no-op, and predicates/helpers treat tombstones as absent so the latest copy still wins.
Dynamic reasoning capability detection and append-only prefix caching
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run 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. Comment |
There was a problem hiding this comment.
🟡 Changes recommended
The current diff introduces verified correctness issues (test setup string escaping, keyed-block classification impacting tool-history handling, and rejected-field key normalization for free-* models).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates how “keyed” system blocks (responder context, injected instructions/skills, plan context) are cleared to preserve prompt-cache stability, and refines reasoning-capability/effort handling to rely more on catalog + model-family knowledge while keeping negative learnings session-scoped.
Changes:
- Switch keyed-block clearing from deletion to appending a
\n(cleared)sentinel to avoid rewriting previously-sent messages. - Prefer catalog-advertised reasoning efforts (and model-family contracts) when determining reasoning support/effort vocabulary.
- Stop persisting “negative” route capability learnings; keep rejections/rejected fields in-memory per session and scrub legacy persisted negatives.
File summaries
| File | Description |
|---|---|
| test/volatile-suffix-caching.test.ts | Updates expectations to reflect cleared responder context markers instead of deletion. |
| test/turn-request-context-trim.test.ts | Adjusts “stable message” comparison to ignore volatile request-context blocks. |
| test/responder-inband.test.ts | Asserts inbox is explicitly cleared via sentinel rather than absent. |
| test/reasoning-degradation.test.ts | Updates expectation to emit reasoning controls based on family contract. |
| test/project-instructions.test.ts | Updates clearing expectations for injected instruction blocks. |
| test/profiles/catalog-layer.test.ts | Verifies Meta reasoning effort vocabulary is driven by live catalog facts. |
| test/llm/reasoning-capability-scope.test.ts | Adds coverage for family-based reasoning support and effort clamping. |
| test/llm/learned-route-capabilities.test.ts | Verifies only positive learnings persist; negatives/rejections are session-scoped. |
| test/llm/effort-display.test.ts | Updates expectation that rejections aren’t persisted to config. |
| test/conformance/snapshots/request-snapshots.test.ts.snap | Snapshot updates for Meta reasoning effort/summary defaults. |
| test/agent/compaction-candidate.test.ts | Updates compaction expectations for cleared keyed blocks. |
| src/llm/provider-profiles.ts | Prefers catalog-advertised efforts when overlaying observed/learned reasoning data. |
| src/llm/provider-profile-layers.ts | Removes fixed Meta accepted-efforts list in favor of catalog-driven values. |
| src/llm/learned-capabilities.ts | Adds sanitization to avoid persisting negatives/legacy fields; introduces session rejected-fields store. |
| src/llm/capability/state.ts | Resets session rejected-fields; scrubs persisted negatives during load. |
| src/llm/capabilities.ts | Adds catalogAdvertisedEfforts; uses model-family contract for support/evidence; session-scopes rejections. |
| src/agent/responder-context.ts | Appends a cleared sentinel instead of deleting responder context messages. |
| src/agent/plan-tool.ts | Appends a cleared sentinel for plan context instead of deleting. |
| src/agent/injected-blocks.ts | Implements generic keyed-block upsert/clear + helpers for cleared detection/latest lookup. |
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
27
to
+31
| export function isAgentInstructionsMessage(content: string): boolean { | ||
| return content.startsWith(AGENT_INSTRUCTIONS_PREFIX); | ||
| return ( | ||
| content.startsWith(AGENT_INSTRUCTIONS_PREFIX) && | ||
| !isClearedKeyedBlock(content) | ||
| ); |
Comment on lines
34
to
+38
| export function isActiveSkillsMessage(content: string): boolean { | ||
| return content.startsWith(ACTIVE_SKILLS_PREFIX); | ||
| return ( | ||
| content.startsWith(ACTIVE_SKILLS_PREFIX) && | ||
| !isClearedKeyedBlock(content) | ||
| ); |
| const entry = readLearnedRoute(routeCapabilityKey(provider, model)); | ||
| if (!entry?.rejectedFields?.length) return []; | ||
| return negativeIsStale(learnedRouteAt(entry)) ? [] : entry.rejectedFields; | ||
| return sessionRejectedFields.get(routeCapabilityKey(provider, model)) ?? []; |
| it("marks the block cleared when the instruction files disappear", () => { | ||
| const messages = baseMessages(); | ||
| upsertAgentInstructionsMessage(messages, `${AGENT_INSTRUCTIONS_PREFIX}\nrule`); | ||
| upsertAgentInstructionsMessage(messages, `${AGENT_INSTRUCTIONS_PREFIX}\\nrule`); |
MCP:
- schema-aware argument coercion at the call boundary (fixes -32602 storms
when models send stringified JSON/numbers)
- automatic streamable-http <-> legacy SSE transport fallback on 400/404/
405/410/415 (fixes servers like Notion 404ing on the wrong transport)
- compact unresolved-${input:...} error messages instead of dumping
hundreds of variable names into agent context
- unambiguous callable names in mcp.tools / mcp.enable / prompt context
(wire names for native tools, dotted names in text mode) plus an
enable-first hint when selection is off
- built-in catalog of well-known servers (github, notion, context7, fetch,
filesystem, memory, sequential-thinking, brave-search, mongodb,
puppeteer, slack) with guided /mcp add <name> in both UIs and an mcp.add
agent tool; secrets are prompted and stored user-level, env refs adopted
- OAuth device-authorization flow (RFC 8628) for headless/SSH and
browser-less environments, browser-open failure detection, and the
authorization URL surfaced as a notice (classic + opentui) or on stderr
in noninteractive mode
LLM:
- detect rate limiting and quota exhaustion from provider error bodies when
the status is not 429 (quota exhausted, insufficient_quota, free quota,
resource exhausted, too many requests, etc.) so keys rotate and users get
actionable guidance instead of a raw provider dump
MCP robustness overhaul + rate-limit body detection
- tool-history: classify cleared keyed blocks (tombstones from the append-only injected-block change) as benign mid-group system blocks, so repairToolProtocol no longer splits tool groups or injects 'No stored body' placeholders when a tombstone lands between a tool call and its result - learned-capabilities: look up session rejected fields with the same free-<n>/-stripped key the writer uses, so free-* model aliases do not re-send rejected fields within a run - test/project-instructions: use a real newline in the fixture instead of a literal backslash-n
fix: address Copilot review on injected blocks and learned capabilities
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Related Issue
Closes #
Type of Change
How Has This Been Tested?
npm run typecheckpassesnpm testpassesTest Details
Screenshots / Recordings
Checklist