feat(agents): system_instructions_mode — let an agent config EXTEND the corpus persona instead of replacing it - #2262
Conversation
A selected agent's system_instructions REPLACED the context-derived prompt while its available_tools MERGED. That asymmetry forces a bad trade on any corpus-scoped agent: attaching a tool costs you the corpus persona. The only workaround was to paste the whole persona into system_instructions, which duplicates it into a second row that then drifts from the corpus silently. Adds system_instructions_mode (REPLACE default, so existing configs are unaffected; EXTEND opt-in). EXTEND rides extra_system_context, which the factory drains through AgentConfig.resolve_system_prompt AFTER persona resolution — the mechanism added for #2247 for exactly this case. Assigning system_prompt instead would consume the 'caller supplied no prompt' signal and discard the persona, which IS #2247. Wired in both the document and corpus factories rather than only the one needed, and the websocket consumer honours the mode. Measured on a 4,679-section authority deployment: persona 14,753 chars + config 4,212 chars -> built prompt 20,284 chars, tools intact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Review Nice fix for a real asymmetry (instructions REPLACE / tools MERGE), and the mechanism itself is sound: That said, I think the PR is incomplete relative to its own stated goal — a few things worth addressing before merge: 1. Two other call sites still hard-REPLACE, unaware of the new mode
Given corpus-thread @mentions and delegation are arguably the most common way these 2. The new mode isn't reachable through the app's own API
3. No automated test coverage The PR description mentions a manual verification script ( Minor
Also noting the PR's own caveat: the backend suite wasn't run against this change (blocked by an unrelated |
|
Verified both branches, not just the new one — REPLACE is the default every existing configuration relies on, so it is the more important of the two to prove unchanged: REPLACE excludes the persona and applies the config text (the historical behaviour); EXTEND carries both with the persona first; the merged tool survives either way. Harness is |
ExtraSystemContextFactoryTestCase drives the real factory with the kwarg the consumer passes, and both of its cases FAIL without the change (verified by reverting agent_factory.py to its pre-feature state). Wired for the document factory as well as the corpus one, because an agent config can be selected for a document too. ExtendModeAssemblyTestCase covers add_computed_context ordering — that mechanism predates EXTEND and those tests would pass without it, so they are contract coverage rather than regression coverage, and are labelled as such.
…extend-instructions
|
Added tests to
Covers the document factory as well as the corpus one, since an agent config can be selected for a document too — and that test caught a real gap in my own fixture rather than passing by luck.
Combined run with the diagnostics tests from #2261: 23 passed, exit 0. Broader run across five related files: 81 passed. |
ReviewClean, well-scoped change with real production evidence behind it (measured retrieval deltas, and a mechanism — A few things worth addressing before/soon after merge: 1.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The asymmetry
In
unified_agent_conversation.py, a selected agent'ssystem_instructionsreplace the context-derived prompt, while itsavailable_toolsmerge:That forces a bad trade on any corpus-scoped agent: attaching a tool costs you the corpus persona. You get the persona that knows the corpus, or the agent that can call
search_across_corpora— not both.The only workaround is to paste the entire persona into
system_instructions, which duplicates it into a second row that drifts fromCorpus.corpus_agent_instructionssilently.Measured cost of the workaround
On a 4,679-section authority deployment (21 corpora, 18-member group), scoring answers on whether their quotations exist verbatim in the installed corpus:
The combination is worth roughly +2 and +4 over either alone, and it is the only configuration that reaches case law and Federal Register preambles. There is no way to express it today except duplication.
The change
Adds
AgentConfiguration.system_instructions_mode:REPLACE— the historical behaviour, and the default, so no existing configuration changes.EXTEND— the instructions are appended to the context-derived prompt.EXTEND rides a new
extra_system_contextkwarg, which the factory drains throughAgentConfig.resolve_system_promptafter persona resolution. That is the mechanism added for #2247 for exactly this "append without consuming the persona signal" case — assigning tosystem_prompthere instead would consume the "caller supplied no prompt" signal and discard the persona, which is #2247.Wired into both the document and corpus factories, not only the corpus one I needed.
Verification
test/verify_extend_mode.pyasserts the composition on the built agent, not on the intention — #2247 was silent, so intention is not evidence:With EXTEND the config stores 4,212 chars (only what it adds) instead of 18,721 (persona + additions), and the persona stays single-sourced on the corpus.
Caveat
The repo suite was not run here: under
local.yml, pytest dies atdjango.setup()on a duplicatestoragesapp label before any of this is imported, and standing uptest.ymlrisked evicting a database another session was using mid-measurement. Migration0018is additive with a default, so it is backward-compatible, but CI should exercise it.🤖 Generated with Claude Code