Repository navigation
Conversation
…boarding generation
The buddy's persona gains four clauses, each gated on the tool it depends on, so a hire with no onboarding path meets a mentor that never mentions one: - the path is the plan, and where the mentor's own suggestion disagrees with it the path wins, because a person wrote it; - a knowledge question is a tutoring moment, not an exam the mentor can shortcut: it is not told which answer is correct, and the clause says so, so "I do not have it" is the honest answer when a hire asks for it; - completing a step is asked for, never announced; - an empty AI-enhanced phase is a subject to talk through, and a step that comes out of that goes on the hire's own copy, never on the PM's blueprint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six things a testing session turned up, five of them the mentor's own judgement and one of them the reason it had no judgement to make. The one that matters most: "where am I?" had two plausible answers — the path a person wrote, and the work pool, metrics and ledger that say what is true right now — and which one it landed on depended on nothing the hire could see. That is the two-systems problem moved inside one conversation rather than solved, so the routing is now stated: path questions to the path, always; the work pool only when the path has nothing open or the current step is asking for real work; metrics and the ledger answer how it is going and never what comes next. Mounted only where both exist. Also: - A LOCKED item is never agreed to, however directly asked. It said yes to a step the hire's own page refuses to open. - Items are named by the number their page prints and linked, so "want to take the check?" arrives as something clickable. - Part of a step is a line of its checklist, not the step. - A refusal that came back NOT PROPOSED has no button: it had been telling hires to click something that was never rendered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two causes, one symptom: the mentor kept telling hires to confirm something that was never on their screen. **The search budget.** Only search-only hops consume it — a hop that asks for a backend tool returns immediately — so a model that searched four times before deciding to act never got to act: it fell through to the forced final answer, which is generated with no tools, while the persona in front of it still said "offer `add_path_step`". It took that at face value. The forced turn is now told plainly that it has no tools and can offer nothing, and the budget is 6 rather than 4 so a thorough answer still leaves room to act. **The mechanics were never stated.** The persona said what to offer and never that offering *is* a tool call. It now says so, that a call missing an argument is refused and shows no button, and that describing a button is not making one. Also: an item is written as a linked number in a literal shape (`[#3](/onboarding/...) "title"`) rather than as a description of one — a small model copies a pattern far more reliably than it interprets an instruction about formatting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There is no skip action and there should not be one: a skip is a request the PM decides, made on the step's own page. The persona now says so, so the mentor neither promises one nor ignores the ask. Adds a test that a spent search budget sends the model the no-tools notice and keeps it out of the conversation, and counts complete_task among the tools a hire without a path never hears about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The mentor brings up a step whose checklist is done but that was never finished, once, with complete_step; and after a wrong answer it teaches first and offers one refresher step when the gap is bigger than one explanation. The link example follows the new /onboarding?step= shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaces "skipping is not yours to offer" with a clause gated on request_skip: ask why first, help put the reason into a sentence or two a PM can decide on, send it in the hire's words, and never promise it will be accepted. A step waiting on that decision is not pushed, and finishing it is not offered without saying that it withdraws the request. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The numbers are the page's count, not an order: an item opens once what it comes after is done, and several can open at once. Added steps are placed with waits_on and unlocks rather than dropped at the end. A ready-to-close step is answered where-they-are first, never button first and never with a lecture. Narrowing a question's options down counts as hinting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t-scoping-in-onboarding-generators Add multi-project scoping to onboarding generators (#167)
The persona still carried the old ramp: a mentor guiding a hire "from their first day to doing real work", Task 0 as an action, and a closing line that called everything else "the path to" a first contribution. Onboarding is the path a PM's blueprint prescribes; the buddy tutors along it. - Identity reworded around the onboarding, not a first contribution. - Path clauses come first; the arrival clause follows as setup, not as part of the onboarding. - Routing: "how is my onboarding going" goes to the path; the metrics answer how their work is going and never how far along they are. - claim_task_zero removed from the action tools. Refs #311 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…pus-revision Use datastore revision for BM25 cache invalidation
"Answer" alone would read "can you answer this: who is stuck?" as a request for answer_escalation. The rule now names a hire's question, and says a question the manager asks is answered directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…epare-changes Tell the team buddy that preparing a change is acting for the manager
"How do I create a project?" is a question about Sprintstart, not about the project the team works on, so search_docs has nothing to say about it. A clause, gated on the backend's new get_app_guide tool and shared by the hire's and the manager's persona, routes those questions to the guide, links pages by their path, and forbids sending somebody to a page the guide does not list as theirs to open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Add BITBUCKET to the chat filter values and the known source systems so chunks keep their source system - Cover the ingest round trip, the case-insensitive normalization and the chat filter in tests
- Add NOTION to the chat filter values and the known source systems so Notion chunks keep their source system - Cover the ingest round trip, the case-insensitive normalization and the chat filter in tests
…oarding-tutor Make the buddy the tutor along the onboarding path
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.
… duplicated LinseCed's review of #211 raised five findings. Four held; the fifth is a deployment assumption now made explicit in code. All five are settled here. 1. Non-ASCII questions crashed the fence (query_fence.py). is_fenced compared the message with hmac.compare_digest, which rejects str holding anything non-ASCII: any question of 42+ characters with an umlaut, an em dash or an emoji raised TypeError inside run_agent_turn -- on the first hop, and again on the resume hop that re-checks a message it already fenced. The comparison runs on UTF-8 bytes now; the fenced bytes themselves are unchanged. Pinned by test_query_fence.py (unit) and a German-question resume round-trip in test_buddy_search_parity.py (end to end). 2. A malformed tool call was hidden from the model (evidence.py, agents/tools/base.py). The buddy's empty= wording ("No indexed material matched this search.") replaced the tool's own summary whenever nothing survived -- including when the call never ran, so the model retried the search instead of fixing its arguments. ToolResult now separates the two: ToolResult.failed carries is_error for the never-ran paths (an unknown tool, arguments that did not validate; both raised by Tool.execute and ToolRegistry.execute), and format_evidence returns an error's summary whatever the caller's wording. Genuine empty results -- including the buddy's own test-material drop -- keep the no-evidence wording the persona reacts to. chat was already right: it passes no empty. 3. The canned filtered-empty notice and its narrowing predicate existed twice (chat route and buddy_agent) and had already drifted: chat treated an empty-string time bound as a narrowing, the buddy did not. Both move to rag/filters.py as NO_FILTERED_RESULTS_MESSAGE and has_narrowing_filters(), and both callers import them. Unified on "is not None", which is how where_filter_for_chroma reads the same field. 4. The run-a-step's-calls loop existed twice, each with its own _MAX_PARALLEL_TOOLS, and a change to the cap or its rationale would have updated only one copy. run_tool_calls() and MAX_PARALLEL_TOOLS now live in agents/tools/base.py; ChatAgent._run_tools/_run_one and the buddy's _run_local are gone. The two concurrency tests (barrier overlap, call order) move to tests/agents/test_tools.py, where the shared seam lives, so one test pins both loops. 5. The fence key was drawn per process, so the byte stability the module promises for the prompt cache only held within a single process. The key now comes from FENCE_KEY when the deployment sets one (hashed to 32 bytes), else it is drawn per process. Nothing changes for the shipped deployment -- one uvicorn process, one key for its lifetime -- but a deployment with more workers or replicas can now share one key, and the docstring and .env.example say that it must, or each process re-fences what another fenced (still safe, a cache miss per crossing). Pinned by a fence_key() test. Gates: ruff format --check, ruff check, pyright src/ (0 errors), pytest (1049 passed, 8 skipped; +7 tests).
…rch-parity Give Buddy chat's search, filters, reasoning and fence
…wledge # Conflicts: # src/onboarding/buddy_persona.py # tests/onboarding/test_buddy_persona.py
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review feedback on the app-guide clause:
- The no-button rule ("never describe a button instead of making one") is
about chat proposals; say so, so the model still names the app's buttons.
- The example link is now a page everybody can open (/board), not a manager
page every hire persona carried.
- A test pins the clause right after the search_docs routing rule it narrows.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…owledge Give the buddy a map of the app: get_app_guide
Bitbucket changes
- Resolve the source system conflicts so BITBUCKET and NOTION are both accepted - Keep the Bitbucket tests from dev and the Notion tests side by side
Notion connector
The chat surface is retired on the frontend (sprintstart-frontend#206) — /chat redirects to the buddy and nothing calls /api/v1/chat any more — so the service side goes in one piece rather than as a route that outlives the UI that stopped asking for it. What goes: the /api/v1/chat route with its SSE path, ChatAgent and ChatOrchestrator, the chat CLI (scripts/chat_cli.py, `sprintstart chat`) that was their only non-HTTP caller, ChatRequest/HistoryEntry and the legacy-field validator in schemas.py (the `prompt`/`context` acceptance existed for pre-rename clients), and the chat-only docstrings that referenced any of it. What stays, and why: the buddy runs on the same retrieval stack and the same SSE event shapes, so `agents/__init__.py` stays as the package root, `ChatFilters` still describes the buddy request's filters, and BuddyStreamEvent is untouched. The shared `chat_stream` client primitive keeps its tests — only its docstrings stopped claiming a chat agent. The README is deliberately left alone: it has an open rewrite (#216), and folding prose about the service's entry points into a removal would make both harder to review.
The golden-retrieval and image-ingest integration tests spoke HTTP to /api/v1/chat because that was the one entry point exercising the full stack. With the endpoint gone, the same coverage calls the retrieval layer directly (rag.retriever.retrieve) — the pipeline the buddy itself runs — so the assertions still watch real behaviour instead of a second, thinner client path. The source-exclusion regression test moves to the buddy route, which is where exclusions are applied now, and gains a case proving a disabled connector's material cannot reach the buddy's context. The chat route's own test file and the agent/orchestrator unit tests go with the code they tested; the dependency-graph test keeps its subject and drops the chat route from its docstring.
- Drop rag.prompt.build_messages, dead since the /chat route went - Re-word stale chat references in the CLI client, chroma_store and tests - Rename test_empty_source_systems_mean_all_as_they_did_for_chat
- Add route-level tests that another project's chunks and chunks with no project are neither seen by the model nor cited (search_docs + grep, with and without filters), and that an empty project_ids admits nothing - Drop the chat() helper from tests/api/test_client.py, the last caller of /api/v1/chat
Remove the chat surface from the AI service
- Turn the agent loop into a generator that streams reasoning, tokens and search announcements per hop - Add POST /onboarding/buddy/agent/stream; the sync endpoint drains the same generator - Hold back text that a filter override could replace, and join hop texts with a blank line
- Add reasoning and reasoning_details to the buddy agent wire message and map them both ways - Fix the resume hop after a backend tool, which a thinking-enabled provider rejected with a 400 - Replace the test that pinned reasoning out of messages with carry and resume tests
Stream the buddy agent turn live from the AI service
Collaborator
Author
|
lgtm |
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
7 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.
No description provided.