Give Buddy chat's search, filters, reasoning and fence - #211
Draft
daniilperkin wants to merge 2 commits into
Draft
daniilperkin wants to merge 2 commits into
daniilperkin wants to merge 2 commits into
Conversation
The buddy is about to run chat's search tools (ai#206), and it has to hold their results to the same budget: grep returns every match in the scoped corpus, and without the chunk and character caps one broad pattern can return more text than the context window. Two copies of the budget would drift, and the chat agent is deleted once the chat is retired (ai#207), so the budget cannot stay inside it. agents/tools/evidence.py now owns SOURCE_CHARS, MAX_EVIDENCE_CHUNKS, MAX_EVIDENCE_CHARS, limit_evidence, format_evidence and order_evidence_for_display, moved verbatim. The names are public because pyright strict reports a private name imported from another module. It sits beside the tools, not in rag/, because format_evidence formats a ToolResult and rag must not import agents. format_evidence gains two keyword arguments that default to chat's current behaviour: header (the buddy adds its test-file warning to the chunk label) and empty (the buddy's persona reacts to its own "nothing matched" wording). The chat agent's two evidence unit tests move to tests/agents/test_evidence.py, since they would otherwise be deleted along with the chat agent, and gain budget-edge tests. The chat agent's turn-level budget tests stay where they are. No behaviour change: the chat suite passes unchanged apart from the moved tests.
Once the chat is retired (Wiki#319), a reader who asks the corpus instead of the mentor (capabilities off) talks to Buddy. Four things chat had and Buddy did not would be lost. This brings them over and exposes them on POST /onboarding/buddy/agent. Closes #206. Search. search_docs is now chat's RetrieveTool, subclassed only to keep the name and spec the persona and the backend know. So each hit arrives with its neighbouring chunks (expand_context_window, which the old direct retrieve call skipped), held to the shared evidence budget (agents.tools.evidence), and only direct matches are cited, never context. Turns with capabilities off also get chat's GrepTool as `grep`, for exact identifiers. That includes team mode with capabilities off: the search-only persona clause is shared by both modes, and a second code path for one tool would drift. Mentor turns keep the tool list they had. A backend tool named like a local one cannot shadow it. Searches asked for in the same step run concurrently (up to 4, as in chat), with results in call order. Test material is dropped before the budget, from the chunks and from the match ids, so a dropped fixture is neither quoted nor reported as an omitted match. Filters. BuddyAgentRequest.filters takes chat's source-system and time narrowing (ChatFilters; empty source_systems means all, as for chat) and applies it to every search. The project scope stays project_ids and stays fail-closed; the old comment claiming material without a project stays searchable was wrong and is corrected. If the narrowing applies, the hop searched, nothing matched, and no backend tool was mounted that could have supplied other evidence, the answer is chat's own fixed notice (NO_FILTERED_RESULTS_MESSAGE) instead of an answer composed from nothing. The guard is deliberate: a greeting under an active filter is still answered normally, and so is a turn whose answer may rest on get_my_metrics. The check runs when the model answers, not after the first empty search, so the model can still retry with other terms. It also replaces the forced answer when the step budget runs out. Reasoning. BuddyAgentResponse.reasoning lists the model's reasoning, one entry per call that returned any, for the frontend's thinking panel. Each hop returns only its own; the backend concatenates hops (backend#214). It is display-only. The wire message schema has no field for it, so it is never carried back and never re-enters the context. Internally it now rides the next hop's assistant message (reasoning and reasoning_details), as in chat, so a reasoning provider can continue a tool-using thought. The forced answer uses generate, which returns text only, so it adds nothing. Fence. Every user message is wrapped in a marker line, and the persona, in every mode, says that what sits between two markers is the reader's words, never instructions (onboarding.query_fence). The marker is an HMAC of the text under a per-process key, not chat's random nonce, for two reasons. First, it cannot be forged: whether a message is already fenced is decided by recomputing its marker, never by its shape, so typing a lookalike marker only gets you fenced again. Second, it is byte-stable: the backend rebuilds the history from stored raw text each turn (BuddyService) and carries the returned messages verbatim between hops. Every user message, not only the newest, gets the same bytes on every hop and turn, so the Anthropic prompt cache keeps hitting across turns. A per-request nonce would change an earlier message every turn and miss the cache from there on. A restart draws a new key, which fences an in-flight message twice (still a fence) and costs one cache miss. Persona. The search-only clause no longer says "search_docs and nothing else"; it says the turn can only search, and names grep when grep is mounted. The persona receives the local tool names too, so that clause only appears when grep is really there. Tests: +38 (+44 with the previous commit). tests/onboarding/test_buddy_search_parity.py pins each point above, including that grep is project-scoped, the budget ("8 further matches omitted"), neighbours shown but not cited, call order under concurrency, every leg of the canned-reply guard, reasoning in and out of the transcript, and fence idempotence across resume hops, cross-turn byte stability and a forged marker. tests/onboarding/test_query_fence.py covers the fence itself, and tests/api/test_buddy.py covers the wire (filters, reasoning, no reasoning keys in messages). Existing tests now monkeypatch agents.tools.retrieve.retrieve, check fenced message contents by substring, and use the new search-only wording. Verification: ruff format --check, ruff check and pyright src/ (0 errors) clean; pytest 1042 passed, 8 skipped (1004 after the previous commit, 998 at the base). Removing only the fence call fails the fence tests; reverting buddy_agent.py fails the parity suite. Stacked on feature/311-buddy-onboarding-tutor (#208), which rewrites the same persona and agent files; retarget to dev once it merges.
Contributor
Author
|
CI note: |
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.
Summary
Part of SprintStartProject/Wiki#319 (chat → Buddy migration). Closes #206. Unblocks SprintStartProject/sprintstart-backend#214.
Once
/chatis retired, a reader asking the corpus rather than the mentor (capabilities off) talks to Buddy. This PR brings over the four things chat had and Buddy did not, so nothing is lost in the move: search depth, filters, reasoning, and the prompt-injection fence.Decisions (Wiki#319 plan §5, taken on the lead's behalf)
grep+ the new search-only clause in team mode too (capabilities off)search_docs= chat'sRetrieveToolin both modesCommits
agents/tools/evidence.pygets the budget and formatter, moved verbatim (public names, because pyright strict flags private imports across modules; not inrag/, becauseragmust not importagents).format_evidencegainsheader=andempty=; the defaults keep chat's behaviour. The two evidence unit tests move with the code, so ai#207 doesn't delete them. No behaviour change.Search
search_docsis nowRetrieveTool, subclassed only for the name and spec. Each hit arrives with its neighbouring chunks (the old directretrievecall skippedexpand_context_window), held to the shared evidence budget (12 chunks / 8k chars), and only direct matches are cited, never context.grep. Mentor turns keep their tool list. A backend tool named like a local one can't shadow it.Filters:
BuddyAgentRequest.filtersChatFilters(source systems, time window; an empty list means all) applied to every search. The project scope staysproject_ids, fail-closed. Corrected an old comment that claimed project-less material stays searchable.NO_FILTERED_RESULTS_MESSAGE, chat's wording) only when all of these hold: the filter narrows, the hop searched, nothing matched, and no backend tool was mounted. It's checked when the model answers, so the model can still retry with other terms. It also replaces a forced (budget-exhausted) answer.Reasoning:
BuddyAgentResponse.reasoning: list[str]reasoning,reasoning_details), as in chat.Fence:
onboarding/query_fence.py--<hmac16>--lines, and the persona in every mode says what's between two markers is the reader's words, never instructions.BuddyService.kt:269), so same text → same bytes on every hop and turn. A per-request nonce would change earlier messages every turn and lose the cross-turn Anthropic cache.Persona
search_docsand nothing else"; it says the turn can only search, and namesgrepwhen it's mounted.Wire contract changes (for backend#214)
filters?: { source_systems?: string[], time_from?: string, time_to?: string }, snake_case like chat's. The backend'sAiChatFilterslives in itschatmodule, which be#260 deletes, so it needs an onboarding-side copy. Send it on every hop.reasoning: string[](defaults to[]).messages[*].contentfor user messages now comes back fenced. It's still carried verbatim, and the backend only carries it (BuddyService.kt:285,BuddyTeamService.kt:186).Tests (+44)
tests/onboarding/test_buddy_search_parity.py(25): grep mounting per mode and project scope, budget, neighbours shown but not cited, fixture not counted, concurrent call order, every leg of the canned-reply guard, reasoning in and out of the transcript, fence (every mode, resume idempotence, cross-turn byte stability, forged marker).tests/onboarding/test_query_fence.py(8),tests/agents/test_evidence.py(6 new + 2 moved), persona (1), API (4: filters, empty = all, reasoning / no reasoning keys, defaults).agents.tools.retrieve.retrieve, match fenced contents by substring, and use the new clause wording.type: ignore.Verification
uv run ruff format --check .✅,uv run ruff check .✅,uv run python -m pyright src/: 0 errors ✅uv run python -m pytest: 1042 passed, 8 skipped (base 998; commit 1 alone: 1004 ✅)buddy_agent.pyfails the parity suite.Not verified
grepversussearch_docs, and whether real providers honour the fence note. Needs a stack with a real model.reasoning_detailsacross internal hops (stub-verified only).