Repository navigation
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughTool schemas now direct callers to copy runtime-issued identifiers from prior results, source listings, goal lists, or page snapshots. Fabricated identifier examples were removed, and extraction-tool tests verify the updated documentation. ChangesRuntime identifier documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change replaces misleading example identifiers with guidance to obtain real runtime-issued values, without changing runtime behavior; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The pull request replaces fabricated examples with references to valid runtime ID sources across extract handles, goal IDs, memory source IDs, and browser references. The added tests verify that fabricated handle examples are absent. A rabbit reads the schemas bright, Comment |
|
| Filename | Overview |
|---|---|
| src/openhuman/agent/harness/subagent_runner/extract_tool.rs | Removes the res_1 example from the result_id parameter description; adds a code comment recording two live production misuse runs to prevent the example from being re-introduced. |
| src/openhuman/memory/goals/schemas.rs | Replaces "(e.g. 'g1')" with "taken from a goals list you have read" for edit and delete id fields; the sequential-ID risk warning present in tools.rs is not repeated here. |
| src/openhuman/memory/goals/tools.rs | Replaces "(e.g. 'g1')" with instructional text plus an explicit "Ids are sequential, so a guessed one lands/deletes a real goal you never looked at" warning for both edit and delete. |
| src/openhuman/memory/query/cover_window.rs | Replaces the slack:#eng / gmail:abc example with "copied from tree.list_sources" for the source_id field. |
| src/openhuman/memory/query/query_source.rs | Same source_id description fix as cover_window.rs — example replaced with "copied from tree.list_sources". |
| src/openhuman/memory/tree/retrieval/schemas.rs | Two source_id FieldSchema comment strings updated — both drop the slack:#eng / gmail:abc examples in favour of "copied from tree.list_sources". |
| src/openhuman/tools/impl/browser/browser.rs | Changes @ref (e.g. @e1) to "@ref copied from the latest page snapshot" in the selector description, preventing the model from guessing stale element references across navigations. |
Sequence Diagram
sequenceDiagram
participant M as Model
participant S as Schema description
participant T as Tool runtime
participant C as Cache / Store
Note over M,C: Before this PR
S-->>M: result_id handle example res_1
M->>T: extract_from_result result_id res_1
T->>C: lookup res_1
C-->>T: cache-miss
T-->>M: Error
Note over M,C: After this PR
S-->>M: Copy handle from earlier result
M->>T: some_tool returns placeholder result_id res_abc42
M->>T: extract_from_result result_id res_abc42
T->>C: lookup res_abc42
C-->>T: hit
T-->>M: extracted answer
Reviews (2): Last reviewed commit: "fix(prompts): stop showing example ids t..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/openhuman/agent/harness/subagent_runner/extract_tool.rs`:
- Line 192: Update the description of the oversized tool output placeholder
handle to state that emitted handles may expire after cache eviction; when a
handle is no longer valid, rerun the original tool to obtain a new result
instead of sending the expired handle.
- Around line 170-183: Add focused semantic regression tests for the
runtime-identifier model-facing descriptions: in
src/openhuman/agent/harness/subagent_runner/extract_tool.rs:170-183 verify exact
placeholder-handle copying and no fabricated handles, and at 192 verify no-stash
guidance; test source_id guidance in cover_window at
src/openhuman/memory/query/cover_window.rs:43 and query_source at
src/openhuman/memory/query/query_source.rs:31; test the query_source and
cover_window controller schemas in
src/openhuman/memory_tree/retrieval/schemas.rs:106 and 168; test edit and delete
goal-ID guidance in src/openhuman/memory_goals/schemas.rs:94 and 118; test
GoalsEditTool and GoalsDeleteTool in src/openhuman/memory_goals/tools.rs:133 and
185; and test snapshot references plus CSS/text selector support in
src/openhuman/tools/impl/browser/browser.rs:773.
🪄 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 Plus
Run ID: 4f6e103a-c320-4966-9c52-24c5a1e06a35
📒 Files selected for processing (7)
src/openhuman/agent/harness/subagent_runner/extract_tool.rssrc/openhuman/memory/query/cover_window.rssrc/openhuman/memory/query/query_source.rssrc/openhuman/memory_goals/schemas.rssrc/openhuman/memory_goals/tools.rssrc/openhuman/memory_tree/retrieval/schemas.rssrc/openhuman/tools/impl/browser/browser.rs
ef5be20 to
352f770
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…tract Two halves of one rule. The text says a handle is only valid if it appeared in an earlier result, but not that one can stop being valid — the cache holds a bounded number of recent entries, and `execute` already answers an evicted handle with "re-run the original tool". Unsaid, an eviction reads to the model as "wrong handle", which invites exactly the guess this PR removed the sample handle to stop. Now stated where the model reads it. Adds the regression the descriptions had none of: no model-facing text may contain a handle-shaped literal (`res_1` and friends), and `result_id` must still say where a real handle comes from and that it expires. A description is a prompt, and an example in it is an instruction — the removal has to be pinned or the next edit helpfully adds the example back. Two live runs called this tool with `res_1` against a cache that had never issued one. `description` and `parameters_schema` are lifted to free functions so the test can read them without a live cache and model source; the trait methods delegate. extract_tool 5 pass. Reported by CodeRabbit on tinyhumansai#5320. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
There was a problem hiding this comment.
yh928 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Everything else on this head is green, and both review threads are addressed in |
…tract Two halves of one rule. The text says a handle is only valid if it appeared in an earlier result, but not that one can stop being valid — the cache holds a bounded number of recent entries, and `execute` already answers an evicted handle with "re-run the original tool". Unsaid, an eviction reads to the model as "wrong handle", which invites exactly the guess this PR removed the sample handle to stop. Now stated where the model reads it. Adds the regression the descriptions had none of: no model-facing text may contain a handle-shaped literal (`res_1` and friends), and `result_id` must still say where a real handle comes from and that it expires. A description is a prompt, and an example in it is an instruction — the removal has to be pinned or the next edit helpfully adds the example back. Two live runs called this tool with `res_1` against a cache that had never issued one. `description` and `parameters_schema` are lifted to free functions so the test can read them without a live cache and model source; the trait methods delegate. extract_tool 5 pass. Reported by CodeRabbit on tinyhumansai#5320. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
e4df37c to
2a4f0ae
Compare
There was a problem hiding this comment.
yh928 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Correction to my earlier comment: the link check has since passed on this head, so no re-run is needed. It was a transient timeout against external hosts (the failing run reported 0 errors and 10 timeouts), and the branch touches no markdown. |
|
Maintainer review (review only — no changes pushed to your branch). The change is right and I would like to see it land. The argument in the description is the correct one: a schema description is a prompt, an example id in it is an instruction, and adding a "do not guess" rule alongside a guessable sample loses to the sample. Not widening the parser or the cache to accept a fabricated id is also the right call — that converts a wasted turn into a wrong result. The It is 1. The string is identical there, so it is a straight re-target, no re-wording needed. 2. #[cfg(test)]
#[path = "extract_tool_tests.rs"]
mod tests;and move only your two new tests — Everything else in the PR — After the rebase the net diff is 8 files, |
…send Six tool schemas illustrated an id-shaped argument with a sample value. A sample is the shape a model reproduces, so it sent the sample. `extract_from_result` showed `result_id="res_1"`. Two live runs called it with `res_1` against a cache that had never issued a handle, and both got a cache-miss instead of the data they wanted. A handle is minted at runtime and only exists once a result was actually stashed, so no example of one can ever be valid — the fix is to show none and say where the real one comes from. The same shape appeared in five more places: - `memory_goals` edit/delete — `(e.g. 'g1')`. Goal ids are sequential, so a guessed one lands on, or deletes, a real goal the model never looked at. - `memory` query source + cover window, and the memory-tree retrieval schema — `(e.g. `slack:#eng`, `gmail:abc`)`. Source ids are provider-specific strings only `tree.list_sources` can tell you. - the browser element selector — `@ref (e.g. @e1)`. Refs come from the latest page snapshot. Each now names the call that issues the id instead of inventing one. No prohibition rules and no parser tolerance were added: the fix is what the schema shows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
…send\n\nSix tool schemas illustrated an id-shaped argument with a sample value. A\nsample is the shape a model reproduces, so it sent the sample.\n\n`extract_from_result` showed `result_id="res_1"`. Two live runs called it with\n`res_1` against a cache that had never issued a handle, and both got a\ncache-miss instead of the data they wanted. A handle is minted at runtime and\nonly exists once a result was actually stashed, so no example of one can ever\nbe valid — the fix is to show none and say where the real one comes from.\n\nThe same shape appeared in five more places:\n\n- `memory_goals` edit/delete — `(e.g. 'g1')`. Goal ids are sequential, so a\n guessed one lands on, or deletes, a real goal the model never looked at.\n- `memory` query source + cover window, and the memory-tree retrieval schema —\n `(e.g. `slack:#eng`, `gmail:abc`)`. Source ids are provider-specific strings\n only `tree.list_sources` can tell you.\n- the browser element selector — `@ref (e.g. @e1)`. Refs come from the latest\n page snapshot.\n\nEach now names the call that issues the id instead of inventing one. No\nprohibition rules and no parser tolerance were added: the fix is what the\nschema shows.\n\nClaude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy\n
…tract Two halves of one rule. The text says a handle is only valid if it appeared in an earlier result, but not that one can stop being valid — the cache holds a bounded number of recent entries, and `execute` already answers an evicted handle with "re-run the original tool". Unsaid, an eviction reads to the model as "wrong handle", which invites exactly the guess this PR removed the sample handle to stop. Now stated where the model reads it. Adds the regression the descriptions had none of: no model-facing text may contain a handle-shaped literal (`res_1` and friends), and `result_id` must still say where a real handle comes from and that it expires. A description is a prompt, and an example in it is an instruction — the removal has to be pinned or the next edit helpfully adds the example back. Two live runs called this tool with `res_1` against a cache that had never issued one. `description` and `parameters_schema` are lifted to free functions so the test can read them without a live cache and model source; the trait methods delegate. extract_tool 5 pass. Reported by CodeRabbit on tinyhumansai#5320. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
…tract\n\nTwo halves of one rule. The text says a handle is only valid if it appeared in\nan earlier result, but not that one can stop being valid — the cache holds a\nbounded number of recent entries, and `execute` already answers an evicted\nhandle with "re-run the original tool". Unsaid, an eviction reads to the model\nas "wrong handle", which invites exactly the guess this PR removed the sample\nhandle to stop. Now stated where the model reads it.\n\nAdds the regression the descriptions had none of: no model-facing text may\ncontain a handle-shaped literal (`res_1` and friends), and `result_id` must\nstill say where a real handle comes from and that it expires. A description is a\nprompt, and an example in it is an instruction — the removal has to be pinned or\nthe next edit helpfully adds the example back. Two live runs called this tool\nwith `res_1` against a cache that had never issued one.\n\n`description` and `parameters_schema` are lifted to free functions so the test\ncan read them without a live cache and model source; the trait methods delegate.\n\nextract_tool 5 pass.\n\nReported by CodeRabbit on tinyhumansai#5320.\n\nClaude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy\n
2a4f0ae to
6a8918a
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Rebased onto The three moves:
Everything else ( One thing worth noting about the test move. The auto-merge silently resolved the test module to Verified locally: The substance is unchanged, and the |
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0091 · 119,479 in / 934 out · 23,552 cached (20%) · openrouter/openai/text-embedding-3-small, deepseek/deepseek-v4-flash · 429 embedded
critique: $0.0036 · 58,591 in / 329 out · 23,552 cached (40%) · deepseek/deepseek-v4-flash
security: $0.0042 · 46,716 in / 300 out · 0 cached (0%) · deepseek/deepseek-v4-flash
tests: $0.0013 · 14,172 in / 305 out · 0 cached (0%) · deepseek/deepseek-v4-flash
How this change flows2 changed behaviours across 7 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 27 further behaviours left out to keep the diagram readable. flowchart LR
n0["schemas<br/>changed"]:::changed
n1["schemas<br/>changed"]:::changed
n2["vec"]:::impacted
n3["format"]:::impacted
n4["join"]:::impacted
n5["openhuman"]:::impacted
n6["caps_tools_ws"]:::impacted
n7["expansion_tools_for"]:::impacted
n0 -->|calls| n2
n1 -->|calls| n2
n4 -->|calls| n3
n6 -->|calls| n3
n6 -->|calls| n4
n7 -->|calls| n2
n7 -->|uses| n5
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
…solation The full lib suite and scoped lib filters now use `--test-threads=1` to prevent tests sharing process-global registries and configuration from interfering with each other. Scoped filters are additionally run in separate processes so that fixture setup from one domain cannot leak into another, preserving test isolation while still allowing coverage reports to be merged. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
You have reached your Codex usage limits for security reviews. Please try again later. |
The coverage script now runs the `a_build_only_runtime_is_swept_before_it_can_be_invoked` test separately from the rest of the lib suite, because that test installs a process-wide harness context whose narrowed DomainSet could interfere with shared registries and configuration in other tests. The test is still instrumented and contributes to the merged coverage report. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The two llvm_cov calls for the registry suite were passing the test filter and `--test-threads` flag in the wrong position relative to the `--` separator, causing the test runner to misinterpret the arguments. The fix moves `--test-threads=1` after the `--` separator so it is passed to the test binary correctly, and places the test name filter in the proper position for each invocation. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ubagent clarification flow Added an eprintln statement to capture the clarification state before the assertion on turn-2, making it easier to diagnose failures by printing the captured context when the test fails. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Removed an eprintln! call that was left in the test as a debugging aid, keeping the test output clean and avoiding unnecessary noise in test runs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Added an eprintln statement to print the number of captured clarifications during the subagent clarification flow test, making it easier to diagnose failures by showing the actual count when the assertion fails. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Removed an `eprintln!` debug statement that was left in the `subagent_clarification_flow_inner` test, which would clutter test output and was not intended for the final test logic. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a `wait_for_web_chat_idle` helper that polls the web queue status until the active flag clears, with a 10-second timeout. Use it in the subagent clarification flow to wait for the progress bridge to remove the in-flight entry before submitting the answer, preventing the answer from being treated as a same-turn follow-up that could cause the collector to return the first terminal event again. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Remove the second turn of the subagent clarification end-to-end test, which sent a user reply and verified the final answer, because the paused child agent is now resumed through a different mechanism rather than a new web-chat message. The test still validates that the scheduler agent runs, produces a clarification question, and that the question is persisted in the delegated request history. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `wait_for_web_chat_idle` function was only used in the `subagent_clarification_flow_inner` test, where it was removed because the race condition it guarded against is no longer present. The function and its call are now dead code, so they have been cleaned up along with a variable name fix in the assertion. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…t pause The subagent clarification flow test now reflects the correct behaviour where `ask_user_clarification` triggers an early-exit pause in the scheduler agent, rather than being blocked and requiring a second LLM call to return the question as text. The test fixture is reduced from three upstream requests to two, and the comments are updated to describe the actual pause-and-resume flow. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…r messages The test was comparing system messages across requests, but the shared project context makes system prefixes identical for all agents. Changed the assertion to compare the last user message instead, which correctly verifies that the scheduler agent received a distinct delegated prompt. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Replaced the complex multi-step assertion that checked for identical user content and a specific canary string with a single check that the delegated scheduler prompt contains the expected phrase. This makes the test more robust and easier to understand. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ff1958a80
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…pstream calls The test for the subagent clarification flow previously expected only two upstream LLM requests, but the orchestrator now synthesizes the delegated pause into a third request. The test fixture and assertion are updated to reflect this, adding a third text completion mock and raising the minimum expected request count from two to three. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test harness now catches panics from the agent harness thread and prints the panic message to stderr before resuming the unwind, making it easier to diagnose test failures caused by panics in spawned threads. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The subagent clarification flow test was asserting at least three upstream requests, but the early-exit envelope is now surfaced directly on this path rather than requiring a separate parent synthesis step. The test expectation is lowered to two requests, and the third scripted response still covers the parent synthesis path when it is reached. The panic handling in the agent harness e2e thread is also simplified by removing the manual panic message extraction. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
You have reached your Codex usage limits for security reviews. Please try again later. |
… deltas for self-executing CLI The test previously asserted that tool call start and argument delta events were emitted and that the mapper recorded a completed tool call. Since the Claude CLI self-executes tool calls, the mapper now swallows these lifecycle events to prevent the OpenHuman harness from dispatching them a second time. The test is updated to verify that no tool call deltas appear and that the mapper's tool calls list is empty. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 055a644e98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add the new `list_sources` module to the query submodule and register its `MemoryTreeListSourcesTool` in the tool registry, making source listing available to the agent loop. Also simplify the `source_id` field description in `query_source` to remove an implementation detail about how the id is obtained. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The new memory_tree_list_sources tool is added to the capability mapping and the list of optional family memory tools, ensuring it is recognised as a tree-level operation alongside existing memory tree tools. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb12219f19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // previously surfaced to the agent loop. | ||
| Box::new(MemoryFlavourTool::new(config.clone())), | ||
| Box::new(MemoryQueryTool), | ||
| Box::new(MemoryTreeListSourcesTool::new(config.clone())), |
There was a problem hiding this comment.
Add source listing to the memory agent's allowlist
Fresh evidence after the earlier native-surface report is that this commit registers the wrapper globally, but the deep-retrieval agent_memory still has a named allowlist containing memory_tree but not memory_tree_list_sources (src/openhuman/memory/agent/agent/agent.toml:24-36). The subagent runner constructs allowed_names solely from that resolved scope (src/openhuman/agent/harness/subagent_runner/ops/runner.rs:1270-1279), so delegated memory runs neither see nor can execute this new tool. When retrieval requires an exact provider-specific source_id, the agent therefore still cannot obtain it despite the updated schemas directing it to this name; include the tool in the memory agent's allowlist.
Useful? React with 👍 / 👎.
Summary
extract_from_resulthandle loses its example entirely: no example of a runtime-minted handle can ever be valid.Problem
A sample is the shape a model reproduces, so the model sends the sample.
extract_from_resultdocumentedresult_idasres_1. Two live runs called it with the literalres_1against a cache that had never issued a handle, and both got a cache-miss instead of the data they wanted.The same shape appeared in
memory_goalsedit/delete ((e.g. 'g1')— ids are sequential, so a guess deletes a real goal), the memory query source / cover window / memory-tree retrieval schemas ((e.g. \slack:#eng`)— source ids onlytree.list_sourcescan produce), and the browser element selector (@ref (e.g. @e1)` — refs change every navigation).Solution
Each description now names where the real id comes from. No prohibition rules and no parser tolerance were added — the fix is what the schema shows. A rule saying "do not guess" competes with an example that shows a guessable value, and the example wins; widening the parser or the cache to accept a fabricated id would turn a wasted turn into a wrong result.
The
extract_from_resultcase carries a code comment recording the two live runs, so the example is not reintroduced as a readability improvement.Submission Checklist
Closes #NNNin the## RelatedsectionImpact
Related
Closes #5316
Summary by CodeRabbit
Documentation
Tests