feat: context manager#259
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
skill-check — worker0 verified, 20 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthroughThis PR adds the Rust ChangesContext manager rollout
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
context-manager/Cargo.toml (1)
37-45: 💤 Low valueRedundant dev-dependencies already present in
[dependencies].
serde_json,uuid, andtokioare already declared in[dependencies](lines 21, 32, 19). Listing them again in[dev-dependencies]is unnecessary — Cargo merges features, but the duplication adds maintenance overhead.♻️ Suggested cleanup
[dev-dependencies] -serde_json = "1" tempfile = "3" which = "8" # BDD test runner. Drives Gherkin .feature files under tests/features/. cucumber = "0.22" futures = "0.3" -uuid = { version = "1", features = ["v4"] } -tokio = { version = "1", features = ["rt-multi-thread", "macros", "sync", "signal", "time"] }🤖 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 `@context-manager/Cargo.toml` around lines 37 - 45, Remove the duplicate dependency declarations from the dev-dependencies section in Cargo.toml. The dependencies serde_json, uuid, and tokio are already present in the main dependencies section and do not need to be redeclared. Delete the lines containing these three packages from the dev-dependencies block to eliminate maintenance overhead, as Cargo will automatically make them available for dev targets.
🤖 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 `@context-manager/src/core/lease.rs`:
- Around line 59-74: The lease acquisition and release logic uses non-atomic
multi-step operations (swap followed by set restore, and get followed by
set(None) clear) that allow race conditions where a newer lease holder can be
overwritten by an older path. Add an atomic compare-by-nonce operation to the
LeaseStore interface (such as clear_if_nonce or a similar CAS variant) that
atomically checks the nonce before modifying or clearing the lease. Replace the
two-step restore pattern in the acquire function (the swap followed by the set
call when is_active is true) with a single atomic operation that compares the
nonce before restoring. Similarly, replace the two-step clear pattern in the
release function (around lines 80-84) with the same atomic operation to ensure
mutual exclusion is preserved under contention.
In `@context-manager/src/core/prune.rs`:
- Around line 112-124: The pruned_tokens calculation on line 112 sums only the
original tokens being removed but fails to account for the tokens consumed by
the replacement placeholder text. This causes token savings to be overestimated
and can bypass the min_free_tokens check when actual net savings are lower. Fix
this by calculating the net token savings: for each item in the queue, determine
how many tokens the placeholder function will consume and subtract that from the
original token count. Update the pruned_tokens calculation to reflect this net
difference, ensuring the min_free_tokens comparison on line 113 validates actual
savings rather than gross removal amounts.
In `@context-manager/src/functions/count_tokens.rs`:
- Around line 18-20: The messages field is declared as Option<Vec<AgentMessage>>
in the struct definitions across multiple files, making it optional in the JSON
schema, but all request handlers require it at runtime via ok_or_else() checks.
Remove the Option wrapper from the messages field type definition in
CountTokensRequest (context-manager/src/functions/count_tokens.rs around line
20), CompactRequest (context-manager/src/functions/compact.rs around line 46),
PruneRequest (context-manager/src/functions/prune.rs around line 35), and
AssembleRequest (context-manager/src/functions/assemble.rs around line 62) to
make messages a required field that matches the runtime validation expectations
and aligns the JSON schema with actual handler behavior.
In `@context-manager/src/types.rs`:
- Around line 187-197: The `has_function_result_block` method's documentation
claims it recursively checks for `function_result` wrappers, but the `scan`
helper function only inspects the top-level blocks returned by `self.content()`
and does not recurse into the `content` field of nested
`ContentBlock::FunctionResult` blocks. Either update the doc comment to clarify
that only top-level blocks are checked (if deep nesting is not expected in
practice), or modify the `scan` helper function to recursively examine the
`content` field of any `FunctionResult` blocks encountered to ensure all nested
`FunctionResult` blocks are detected.
In `@context-manager/tests/common/engine.rs`:
- Around line 37-41: The match statement in the probe loop is incorrectly
retrying on all non-NotConnected errors by using a catch-all arm that continues
to the next iteration. This causes transient errors (like engine::workers::list
being unavailable) to eventually result in false test skips. Fix this by
changing the catch-all error arm at line 40 from continuing to the next
iteration to instead returning Some(iii), treating non-NotConnected errors as
"connected enough" for test registration. Only Err(IIIError::NotConnected)
should trigger a retry via continue; all other errors should be treated as
successful connection.
In `@context-manager/tests/integration.rs`:
- Around line 28-50: In the boot() function, replace the silent error
conversions (.ok()?) on the spawn() calls for both the iii command and worker
command with proper error handling that logs or panics with the actual error
message rather than converting spawn failures to None, which masks real boot
failures as test skips. Additionally, ensure that if the worker Command spawn
fails after iii has been spawned, the iii child process is properly terminated
before returning to prevent process leaks that affect subsequent tests. The same
fixes should be applied to any other spawn calls in the same function or related
test setup code (around lines 56-59) that have the same silent error conversion
pattern.
In `@context-manager/tests/steps/common_steps.rs`:
- Around line 238-239: The filter_map call on line 238 silently drops any
non-u64 values from parts_value, allowing the assertion to pass with incomplete
sums. Replace the filter_map approach with explicit error handling that panics
if any field under parts cannot be converted to u64. Instead of using filter_map
to skip non-numeric values, iterate through parts_value and explicitly convert
each value to u64, ensuring the code fails fast when encountering non-numeric
fields rather than silently excluding them from the sum calculation.
---
Nitpick comments:
In `@context-manager/Cargo.toml`:
- Around line 37-45: Remove the duplicate dependency declarations from the
dev-dependencies section in Cargo.toml. The dependencies serde_json, uuid, and
tokio are already present in the main dependencies section and do not need to be
redeclared. Delete the lines containing these three packages from the
dev-dependencies block to eliminate maintenance overhead, as Cargo will
automatically make them available for dev targets.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2df0ac11-e9a1-468c-9f5e-e36234ba864b
⛔ Files ignored due to path filters (1)
context-manager/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (62)
.github/workflows/create-tag.yml.github/workflows/release.ymlREADME.mdcontext-manager/Cargo.tomlcontext-manager/README.mdcontext-manager/build.rscontext-manager/config.yamlcontext-manager/iii.worker.yamlcontext-manager/src/adapters/mod.rscontext-manager/src/adapters/router.rscontext-manager/src/adapters/state_lease.rscontext-manager/src/config.rscontext-manager/src/core/budget.rscontext-manager/src/core/estimate.rscontext-manager/src/core/lease.rscontext-manager/src/core/mod.rscontext-manager/src/core/prune.rscontext-manager/src/core/selection.rscontext-manager/src/core/summary.rscontext-manager/src/error.rscontext-manager/src/functions/assemble.rscontext-manager/src/functions/compact.rscontext-manager/src/functions/count_tokens.rscontext-manager/src/functions/mod.rscontext-manager/src/functions/prune.rscontext-manager/src/lib.rscontext-manager/src/main.rscontext-manager/src/manifest.rscontext-manager/src/ports.rscontext-manager/src/types.rscontext-manager/tests/bdd.rscontext-manager/tests/common/engine.rscontext-manager/tests/common/fakes.rscontext-manager/tests/common/mod.rscontext-manager/tests/common/workers.rscontext-manager/tests/common/world.rscontext-manager/tests/features/assemble.featurecontext-manager/tests/features/assemble_round_trip.featurecontext-manager/tests/features/compact.featurecontext-manager/tests/features/compact_lease.featurecontext-manager/tests/features/count_tokens.featurecontext-manager/tests/features/engine_roundtrip.featurecontext-manager/tests/features/errors.featurecontext-manager/tests/features/invariants.featurecontext-manager/tests/features/model_and_budget.featurecontext-manager/tests/features/prune.featurecontext-manager/tests/golden/schemas/context.assemble.jsoncontext-manager/tests/golden/schemas/context.compact.jsoncontext-manager/tests/golden/schemas/context.count_tokens.jsoncontext-manager/tests/golden/schemas/context.prune.jsoncontext-manager/tests/integration.rscontext-manager/tests/manifest.rscontext-manager/tests/schemas.rscontext-manager/tests/steps/call_steps.rscontext-manager/tests/steps/common_steps.rscontext-manager/tests/steps/compaction_steps.rscontext-manager/tests/steps/engine_steps.rscontext-manager/tests/steps/history_steps.rscontext-manager/tests/steps/invariant_steps.rscontext-manager/tests/steps/mod.rscontext-manager/tests/steps/model_steps.rscontext-manager/tests/support/mod.rs
Comment on lines
+59
to
+74
| let prior = match store.swap(LEASE_SCOPE, key, claim).await { | ||
| Ok(prior) => prior, | ||
| Err(e) => { | ||
| tracing::warn!(error = %e, key, "lease swap failed; never treating as a win"); | ||
| return None; | ||
| } | ||
| }; | ||
|
|
||
| // We clobbered a still-valid claim (always someone else's — our | ||
| // nonce is fresh): restore it and bow out. | ||
| if is_active(prior.as_ref(), now, ttl_ms) { | ||
| if let Err(e) = store.set(LEASE_SCOPE, key, prior).await { | ||
| tracing::warn!(error = %e, key, "lease restore failed"); | ||
| } | ||
| return None; | ||
| } |
There was a problem hiding this comment.
Make lease mutation nonce-atomic to preserve mutual exclusion.
Lines 59-74 and Lines 80-84 perform multi-step read/modify/write flows (swap→set restore and get→set(None) clear). Under contention, a newer holder can be acquired between those calls and then overwritten/cleared by an older path, which allows overlapping compactions.
Use an atomic compare-by-nonce operation in LeaseStore (e.g., CAS / clear_if_nonce) and use that in both acquire restore logic and release.
Also applies to: 80-84
🤖 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 `@context-manager/src/core/lease.rs` around lines 59 - 74, The lease
acquisition and release logic uses non-atomic multi-step operations (swap
followed by set restore, and get followed by set(None) clear) that allow race
conditions where a newer lease holder can be overwritten by an older path. Add
an atomic compare-by-nonce operation to the LeaseStore interface (such as
clear_if_nonce or a similar CAS variant) that atomically checks the nonce before
modifying or clearing the lease. Replace the two-step restore pattern in the
acquire function (the swap followed by the set call when is_active is true) with
a single atomic operation that compares the nonce before restoring. Similarly,
replace the two-step clear pattern in the release function (around lines 80-84)
with the same atomic operation to ensure mutual exclusion is preserved under
contention.
ytallo
approved these changes
Jun 15, 2026
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@context-manager/architecture/integration.md`:
- Line 13: The TOC link on line 13 references a fragment anchor for "Structural
invariants" that does not match the actual heading slug generated in the
rendered documentation. Locate the actual "Structural invariants" heading in the
document, determine its correct generated heading slug (typically lowercase with
hyphens replacing spaces and special characters), and update the fragment in the
line 13 link from `#6-structural-invariants-what-you-can-rely-on` to match the
actual heading slug to ensure the navigation link works correctly.
In `@context-manager/src/adapters/fs_lease.rs`:
- Around line 155-199: The async functions `get`, `set`, and `swap` are
performing blocking filesystem I/O operations (ensure_loaded, write_file,
remove_file) while holding a std::sync::Mutex, which blocks Tokio worker
threads. Replace std::sync::Mutex with tokio::sync::Mutex to make the
synchronization async-aware, and refactor the blocking file I/O calls to use
tokio::task::block_in_place within the async context so they run on dedicated
blocking threads instead of starving the async runtime.
In `@harness/src/runtime/compaction.ts`:
- Around line 143-148: The model.limits object is being set with
max_output_tokens defaulted to 0 when the metadata value is not a positive
number, which causes the context-manager to calculate incorrect budgets. Modify
the condition to only assign model.limits when both meta.context_window and
meta.max_output_tokens are positive numbers; if max_output_tokens is missing or
not positive, omit the limits assignment entirely so the router catalog can
provide the correct defaults. Apply this fix wherever model.limits is
conditionally assigned based on metadata values to ensure limits are only
attached when both context and output limits are positive.
In `@harness/src/runtime/session.ts`:
- Around line 258-264: The issue is that the `tail_start_entry_id` assignment
using `tailNew ?? tailLegacy` will resurrect a stale `tail_start_id` value when
`tail_start_entry_id` is explicitly `null` (which means "whole head
summarized"). The nullish coalescing operator treats `null` as a fallback
trigger, but an explicit `null` in the new key should be preserved. Fix this by
checking whether the new key `data.tail_start_entry_id` is actually present in
the data object (using `in` operator or `hasOwnProperty` check) rather than just
whether it's a string, and only fall back to the legacy `tail_start_id` when the
new key is entirely absent, not when it's explicitly `null`.
In `@harness/src/turn-orchestrator/assistant-streaming/run.ts`:
- Around line 49-71: The tools parsed from function_schemas are not being
included in the assembleContext call, which means the pruning/compaction logic
cannot account for their size when calculating the budget. This can cause the
final request to overflow the model limits once tool schemas are attached. Pass
the tools variable (or function_schemas) as a parameter to the
ports.assembleContext function call so the assembly budget includes the tool
schemas when pruning and compacting the window.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2a99ea7a-98cc-4945-a0e7-35027a271f47
⛔ Files ignored due to path filters (1)
context-manager/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (95)
console/README.mdconsole/web/src/lib/functions.tscontext-manager/Cargo.tomlcontext-manager/README.mdcontext-manager/architecture/README.mdcontext-manager/architecture/integration.mdcontext-manager/architecture/internals.mdcontext-manager/config.yamlcontext-manager/src/adapters/fs_lease.rscontext-manager/src/adapters/mod.rscontext-manager/src/config.rscontext-manager/src/configuration.rscontext-manager/src/core/lease.rscontext-manager/src/error.rscontext-manager/src/functions/assemble.rscontext-manager/src/functions/compact.rscontext-manager/src/functions/mod.rscontext-manager/src/functions/prune.rscontext-manager/src/lib.rscontext-manager/src/main.rscontext-manager/src/manifest.rscontext-manager/src/ports.rscontext-manager/tests/common/fakes.rscontext-manager/tests/common/workers.rscontext-manager/tests/common/world.rsharness/docs/workers/context-compaction.mdharness/docs/workers/turn-orchestrator.mdharness/iii.worker.yamlharness/src/context-compaction/config.tsharness/src/context-compaction/flat-state.tsharness/src/context-compaction/handler-async.tsharness/src/context-compaction/handler-pipeline.tsharness/src/context-compaction/handler-sync.tsharness/src/context-compaction/iii.worker.yamlharness/src/context-compaction/lease.tsharness/src/context-compaction/main.tsharness/src/context-compaction/model-resolver.tsharness/src/context-compaction/overflow.tsharness/src/context-compaction/prompts/compaction.txtharness/src/context-compaction/prune.tsharness/src/context-compaction/register.tsharness/src/context-compaction/replay.tsharness/src/context-compaction/selection.tsharness/src/context-compaction/strip-media.tsharness/src/context-compaction/summarize.tsharness/src/context-compaction/template.tsharness/src/index.tsharness/src/runtime/compaction.tsharness/src/runtime/lease.tsharness/src/runtime/session.tsharness/src/turn-orchestrator/assistant-streaming/ports.tsharness/src/turn-orchestrator/assistant-streaming/run.tsharness/src/turn-orchestrator/errors.tsharness/src/turn-orchestrator/events.tsharness/src/turn-orchestrator/preflight.tsharness/src/turn-orchestrator/state-runtime/context-view.tsharness/src/turn-orchestrator/state-runtime/ports.tsharness/src/turn-orchestrator/state-runtime/store.tsharness/src/turn-orchestrator/state-runtime/turn-end.tsharness/src/turn-orchestrator/state.tsharness/src/types/agent-event.tsharness/tests/_helpers/fakeSessionManager.tsharness/tests/context-compaction/compact-session-registered.test.tsharness/tests/context-compaction/compact-session.test.tsharness/tests/context-compaction/compaction-done-emit.test.tsharness/tests/context-compaction/e2e/full-session.test.tsharness/tests/context-compaction/handler-async.test.tsharness/tests/context-compaction/handler-sync.test.tsharness/tests/context-compaction/integration/backward-compat.test.tsharness/tests/context-compaction/integration/flow-async.test.tsharness/tests/context-compaction/integration/flow-prune.test.tsharness/tests/context-compaction/integration/flow-sync.test.tsharness/tests/context-compaction/lease.test.tsharness/tests/context-compaction/overflow.test.tsharness/tests/context-compaction/prune.test.tsharness/tests/context-compaction/registration.test.tsharness/tests/context-compaction/replay.test.tsharness/tests/context-compaction/selection.test.tsharness/tests/context-compaction/strip-media.test.tsharness/tests/context-compaction/summarize.test.tsharness/tests/context-compaction/template.test.tsharness/tests/runtime/compaction.test.tsharness/tests/turn-orchestrator/_helpers/mockTurnStore.tsharness/tests/turn-orchestrator/assistant-streaming.test.tsharness/tests/turn-orchestrator/assistant.test.tsharness/tests/turn-orchestrator/context-view.test.tsharness/tests/turn-orchestrator/events.test.tsharness/tests/turn-orchestrator/finish.test.tsharness/tests/turn-orchestrator/function-awaiting-approval.test.tsharness/tests/turn-orchestrator/function-execute.test.tsharness/tests/turn-orchestrator/functions.test.tsharness/tests/turn-orchestrator/preflight.test.tsharness/tests/turn-orchestrator/run-transition.test.tsharness/tests/turn-orchestrator/store.test.tsiii-permissions.yaml
💤 Files with no reviewable changes (44)
- harness/tests/context-compaction/replay.test.ts
- console/web/src/lib/functions.ts
- harness/tests/context-compaction/prune.test.ts
- harness/src/context-compaction/template.ts
- harness/tests/context-compaction/handler-async.test.ts
- harness/src/context-compaction/flat-state.ts
- harness/src/context-compaction/model-resolver.ts
- harness/tests/context-compaction/integration/backward-compat.test.ts
- harness/tests/context-compaction/lease.test.ts
- harness/tests/context-compaction/handler-sync.test.ts
- harness/tests/context-compaction/overflow.test.ts
- harness/src/context-compaction/config.ts
- harness/src/context-compaction/handler-sync.ts
- harness/src/context-compaction/replay.ts
- harness/tests/turn-orchestrator/functions.test.ts
- harness/tests/context-compaction/e2e/full-session.test.ts
- harness/src/context-compaction/prompts/compaction.txt
- harness/tests/context-compaction/compact-session.test.ts
- harness/tests/turn-orchestrator/_helpers/mockTurnStore.ts
- harness/tests/turn-orchestrator/store.test.ts
- harness/src/context-compaction/strip-media.ts
- harness/tests/context-compaction/strip-media.test.ts
- harness/tests/context-compaction/integration/flow-prune.test.ts
- harness/src/context-compaction/lease.ts
- harness/src/types/agent-event.ts
- harness/src/context-compaction/selection.ts
- harness/tests/context-compaction/integration/flow-sync.test.ts
- harness/src/context-compaction/overflow.ts
- harness/src/context-compaction/handler-async.ts
- harness/tests/turn-orchestrator/function-awaiting-approval.test.ts
- harness/tests/turn-orchestrator/context-view.test.ts
- harness/tests/context-compaction/selection.test.ts
- harness/tests/context-compaction/integration/flow-async.test.ts
- harness/tests/context-compaction/template.test.ts
- harness/tests/turn-orchestrator/preflight.test.ts
- harness/tests/context-compaction/summarize.test.ts
- harness/tests/context-compaction/compaction-done-emit.test.ts
- harness/src/turn-orchestrator/preflight.ts
- harness/src/turn-orchestrator/state-runtime/context-view.ts
- harness/src/turn-orchestrator/state-runtime/store.ts
- harness/src/context-compaction/handler-pipeline.ts
- harness/src/context-compaction/prune.ts
- harness/src/context-compaction/summarize.ts
- harness/tests/turn-orchestrator/assistant.test.ts
✅ Files skipped from review due to trivial changes (7)
- console/README.md
- harness/src/runtime/lease.ts
- harness/src/context-compaction/main.ts
- harness/src/turn-orchestrator/state.ts
- harness/src/index.ts
- harness/docs/workers/turn-orchestrator.md
- context-manager/README.md
🚧 Files skipped from review as they are similar to previous changes (15)
- context-manager/src/adapters/mod.rs
- context-manager/config.yaml
- context-manager/src/lib.rs
- context-manager/Cargo.toml
- context-manager/src/error.rs
- context-manager/src/functions/prune.rs
- context-manager/tests/common/workers.rs
- context-manager/src/functions/assemble.rs
- context-manager/src/manifest.rs
- context-manager/src/core/lease.rs
- context-manager/src/functions/mod.rs
- context-manager/src/ports.rs
- context-manager/src/functions/compact.rs
- context-manager/tests/common/world.rs
- context-manager/tests/common/fakes.rs
There was a problem hiding this comment.
♻️ Duplicate comments (2)
context-manager/architecture/integration.md (1)
13-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win** Fix the broken TOC anchor for Structural invariants** — this was already flagged in a previous review and still needs correction.
Line 13's link fragment doesn't match the heading slug generated from the actual "Structural invariants" heading at line 269. The em-dash in the heading (
## 6. Structural invariants — what you can rely on) will render as a different slug in the anchor.🔗 Proposed fix
-[structural invariants](`#6-structural-invariants-what-you-can-rely-on`) · +[structural invariants](`#6-structural-invariants--what-you-can-rely-on`) ·(Note: Markdown renderers typically convert the em-dash to
--in the slug; verify the exact slug in your target renderer and adjust if needed.)🤖 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 `@context-manager/architecture/integration.md` at line 13, The table of contents anchor link on line 13 contains an incorrect fragment reference for the "Structural invariants" heading. Update the anchor fragment in the TOC link from the current value to match the actual slug generated by the heading at line 269. Since the heading contains an em-dash (—) which markdown renderers typically convert to double hyphens (--) in the slug, change the anchor fragment to use the correct slug pattern that corresponds to how the "6. Structural invariants — what you can rely on" heading will be rendered, verifying the exact slug syntax your markdown renderer produces.context-manager/src/functions/count_tokens.rs (1)
20-65:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
messagesis schema-optional but runtime-required — this breaks the API contract.Line 20 declares
messagesoptional, but Line 63-Line 65 rejects missing values as invalid. This allows clients to send schema-valid requests that fail at runtime. Makemessagesrequired inCountTokensRequestand remove the runtimeok_or_elseguard, then regenerate the golden schema so"messages"is required too.Proposed fix
pub struct CountTokensRequest { /// Messages to estimate, oldest first. - pub messages: Option<Vec<AgentMessage>>, + pub messages: Vec<AgentMessage>, @@ ) -> Result<CountTokensResponse, ContextError> { - let messages = req - .messages - .ok_or_else(|| ContextError::InvalidRequest("messages is required".into()))?; + let messages = req.messages;🤖 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 `@context-manager/src/functions/count_tokens.rs` around lines 20 - 65, The messages field in CountTokensRequest is declared as Option<Vec<AgentMessage>> making it schema-optional, but the handle function requires it at runtime using ok_or_else, creating an API contract mismatch. Remove the Option wrapper from the messages field in CountTokensRequest so it becomes Vec<AgentMessage> (required), then remove the ok_or_else guard in the handle function since messages will always be present, and finally regenerate the golden schema to reflect that messages is a required field.
🤖 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.
Duplicate comments:
In `@context-manager/architecture/integration.md`:
- Line 13: The table of contents anchor link on line 13 contains an incorrect
fragment reference for the "Structural invariants" heading. Update the anchor
fragment in the TOC link from the current value to match the actual slug
generated by the heading at line 269. Since the heading contains an em-dash (—)
which markdown renderers typically convert to double hyphens (--) in the slug,
change the anchor fragment to use the correct slug pattern that corresponds to
how the "6. Structural invariants — what you can rely on" heading will be
rendered, verifying the exact slug syntax your markdown renderer produces.
In `@context-manager/src/functions/count_tokens.rs`:
- Around line 20-65: The messages field in CountTokensRequest is declared as
Option<Vec<AgentMessage>> making it schema-optional, but the handle function
requires it at runtime using ok_or_else, creating an API contract mismatch.
Remove the Option wrapper from the messages field in CountTokensRequest so it
becomes Vec<AgentMessage> (required), then remove the ok_or_else guard in the
handle function since messages will always be present, and finally regenerate
the golden schema to reflect that messages is a required field.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ce45f8b-d3ef-4a16-a1c0-a1eda8e7176b
📒 Files selected for processing (21)
context-manager/README.mdcontext-manager/architecture/README.mdcontext-manager/architecture/integration.mdcontext-manager/architecture/internals.mdcontext-manager/src/core/estimate.rscontext-manager/src/functions/count_tokens.rscontext-manager/src/functions/mod.rscontext-manager/src/types.rscontext-manager/tests/common/workers.rscontext-manager/tests/common/world.rscontext-manager/tests/features/count_tokens.featurecontext-manager/tests/features/engine_roundtrip.featurecontext-manager/tests/features/errors.featurecontext-manager/tests/golden/schemas/context.count-tokens.jsoncontext-manager/tests/integration.rscontext-manager/tests/schemas.rscontext-manager/tests/steps/call_steps.rsdocs/sops/binary-worker.mddocs/sops/new-worker.mdtech-specs/2026-06-agentic/context-manager.mdtech-specs/2026-06-agentic/presentation/src/content/workers.ts
✅ Files skipped from review due to trivial changes (5)
- docs/sops/new-worker.md
- docs/sops/binary-worker.md
- context-manager/architecture/README.md
- context-manager/README.md
- context-manager/architecture/internals.md
🚧 Files skipped from review as they are similar to previous changes (9)
- context-manager/tests/schemas.rs
- context-manager/tests/features/errors.feature
- context-manager/src/core/estimate.rs
- context-manager/tests/steps/call_steps.rs
- context-manager/tests/features/engine_roundtrip.feature
- context-manager/tests/common/workers.rs
- context-manager/src/functions/mod.rs
- context-manager/tests/common/world.rs
- context-manager/src/types.rs
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
2 participants
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.
Add context-manager worker
Implements the
context-managerRust binary worker fromtech-specs/2026-06-agentic/context-manager.md: turns a raw conversationhistory plus a target model into a model-ready context (system prompt +
budgeted messages) via four
context::*functions.Functions
context::assemble— count → prune → compact → assemble, fit to the model's usable budgetcontext::compact— summarise the head, keep a recent tail verbatim (ok | busy | empty | overflow)context::prune— replace verbose function outputs with[output pruned: was ~N tokens]placeholderscontext::count_tokens— estimate messages + tools + system prompt vs a modelDesign
src/core/(budget math, chars/4 estimator behind a trait, prune walk, turn-boundary tail selection, summary templating, lease protocol) behindModelResolver/Summarizer/LeaseStore/Clocktraits, with production adapters forrouter::models::get,router::chat(stream channel), andstate::update-backed compaction leases.schemars::JsonSchema; schemas are generated from the structs and snapshotted as golden files.llm-routeris a soft dependency (token counting and pruning work standalone, summarisation/limit-resolution degrade cleanly when absent).function_calland its result;custommessages are excluded from the model-facing list and its token count.Tests
# Prevents:comment naming the failure it guards. Pure scenarios run production handlers over deterministic fakes; 6@enginescenarios validated against a live engine (realstate::*lease cycle, router-absent degraded modes).UPDATE_GOLDENS=1to regenerate),--manifestcontract test, and a subprocess end-to-end test.cargo fmt --check,cargo clippy -D warnings, andcargo test --all-featuresall pass.Wiring
README.mdModules row;create-tag.ymloptions +release.ymltag pattern; consumerREADME.md.Out of scope
harnessoff its embeddedcontext-compactionmodule onto this worker (separate follow-up).Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests