fix(prompt): append the tool-policy boundary instead of prepending it (#5704) - #5821
fix(prompt): append the tool-policy boundary instead of prepending it (#5704)#5821ntdatt812 wants to merge 1 commit into
Conversation
…tinyhumansai#5704) `build_system_prompt` put the `## Tool Policy Boundary` block in front of the whole assembled prompt. Every line of that block is session-scoped — agent id, channel, entry point, risk level, allowed permission, the allowed-tool list, a restricted count — so the prompt's first diverging byte sat at offset 0 and the inference backend's automatic prefix cache had nothing to reuse behind it, even when the safety preamble, tool catalogue and workspace sections were byte-identical. That contradicts what the surrounding code already protects: `SystemPromptBuilder::for_subagent` keeps `DateTimeSection` out precisely to avoid one varying value inside the prompt, and the connected-server overview is sorted so it "does not reshuffle between turns and cost its cached prefix". It also restores the archetype/persona as the prompt's opening line. The prepend replaced it with the same constant heading for every agent, which is a silent layout change for anything that identified an agent that way. The composition moved into a pure `append_tool_policy_boundary(prompt, boundary)` so the ordering is testable without standing up a session. Four tests, three of which fail against the prepend: - the boundary lands after the prompt body - the persona stays the opening line - two agents differing only in the boundary share the whole body as a common leading prefix (this is the cache property itself, not a proxy) - no boundary leaves the prompt untouched (passes either way) `cargo test --lib turn::context` 4 passed; reverting the helper to prepend gives 1 passed / 3 failed. `cargo fmt --all` clean.
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
How this change flows0 changed behaviours across 4 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 38 further behaviours left out to keep the diagram readable. flowchart LR
n0["fetch_learned_context"]:::impacted
n1["build_system_prompt"]:::impacted
n2["bound_cached_transcript_messages"]:::impacted
n3["trim_history"]:::impacted
n4["extend"]:::impacted
n5["openhuman"]:::impacted
n0 -->|uses| n5
n1 -->|uses| n5
n2 -->|calls| n4
n3 -->|calls| n4
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. |
tinysweeper
Bot
added
the
priority: p3
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe system prompt builder now appends the optional tool-policy boundary after the assembled prompt. A helper handles absent boundaries. Tests verify ordering, persona placement, shared prompt prefixes, and unchanged output without a boundary. ChangesSystem prompt boundary placement
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change appends the session-specific tool-policy boundary so stable prompt content can retain a shared prefix while preserving the same policy information. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Warning Your free Security trial is over. An organization admin can activate Security or dismiss this notice. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Closes #5704.
build_system_promptput## Tool Policy Boundaryin front of the whole assembled prompt:Every line of that block is session-scoped — agent id, channel, entry point, risk level, allowed permission, the allowed-tool list, a restricted count. Putting it first moves the prompt's first diverging byte to offset 0, so two turns share no cached prefix at all even when the safety preamble, tool catalogue and workspace sections behind it are byte-identical. It renders whenever anything is blocked or hidden, which is the normal state for any scoped toolbelt.
This is the same concern the surrounding code already goes out of its way to protect:
SystemPromptBuilder::for_subagentdeliberately excludesDateTimeSection— "Repeat spawns of the same sub-agent definition must produce byte-identical system prompts so the inference backend's automatic prefix cache can reuse the prefill".connections::connected_overviewis sorted "so a prompt built from this does not reshuffle between turns and cost its cached prefix".One varying value was worth excluding; seven varying values ahead of everything were not caught.
Appending loses nothing — the model reads the whole system message either way — and it restores the archetype/persona as the opening line, which the prepend had replaced with the same constant heading for every agent.
Testability
The placement lived inside a session method, so I pulled the composition into a pure helper:
Everything that decides the ordering is now in those two arguments, so it can be tested without standing up a session.
Tests
Four, three of which fail against the prepend (verified by reverting the helper:
1 passed; 3 failed):The third asserts the cache property directly rather than through a proxy — it walks the two prompts byte-by-byte and requires the shared prefix to cover the entire stable body. That is the thing the issue is actually about, so I wanted the test to measure it rather than to assert an ordering that merely implies it.
The fourth is included deliberately and passes either way; it keeps the set honest about what this change does not alter.
Verification
cargo test --lib turn::context— 4 passed.cargo fmt --all— clean.Summary by CodeRabbit