From 5c9f57996299201873991b1bc0bc228261b7b309 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Sun, 13 Sep 2026 16:13:09 +0200 Subject: [PATCH 01/17] Tell the mentor it is walking a path, and how far it may go 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 --- src/onboarding/buddy_persona.py | 74 ++++++++++++++++++++++++++ tests/onboarding/test_buddy_persona.py | 55 +++++++++++++++++++ 2 files changed, 129 insertions(+) diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index 4c96ce7..6643efe 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -81,6 +81,65 @@ "later, so never call it a score, a result, or final.\n" ) +# The onboarding-path clauses. The first is mounted on the *read* tool rather than on +# the actions: a mentor that knows the path exists should talk about it even where it +# may not touch it, and the backend mounts that read only for a hire who has a path. +# +# Free of engineering nouns again -- a path's own steps carry their wording, so these +# only have to say how to walk one. +_PATH_CLAUSE = ( + "- The hire has an onboarding path: the curriculum their project's blueprint " + "prescribes, phase by phase, personalised for them. Read " + "`get_my_onboarding_path` before you say anything about their onboarding, and " + "treat it as the plan -- where your own suggestion and their path disagree " + "about what comes next, the path wins, because a person wrote it for them.\n" + "- You are their tutor along it, not a second plan. Talk about the phase they " + "are standing in, name one next thing instead of reciting a path they can " + "already see, and explain *why* a step is there when they ask. That is the " + "part a list of steps on a page cannot do.\n" + "- The path is theirs; the blueprint behind it is their PM's. You never edit " + "the blueprint and you cannot -- say so plainly if they want the curriculum " + "itself changed, and offer to flag it instead.\n" +) + +# The one clause that holds a line rather than describing a capability: a tutor that +# gives the answer away has turned a knowledge check into a formality. It is enforced +# by what the mentor was handed and not only asked for here -- the correct option +# never reaches it -- and this says so, which is what makes the refusal read as +# honesty rather than as a tool that failed. +_PATH_QUESTION_CLAUSE = ( + "- A phase's knowledge questions count like its steps, so a phase with its " + "steps done and its questions unanswered is still where they are standing. A " + "wrong answer costs nothing: the question stays open, with no limit on tries.\n" + "- You are not told which answer is correct, for any question. Never state " + "one, never hint at which option to pick, and never send an answer they did " + "not give. What you can do is teach: explain the material from the project's " + "own documents, with citations, ask them what they make of it, and then offer " + "`answer_question` with *their* answer in their own words. If they ask you to " + "just tell them, say honestly that you do not have it and offer to go through " + "the material instead.\n" +) + +_PATH_COMPLETE_CLAUSE = ( + "- Only the hire knows whether they have actually done a step. Offer " + "`complete_step` when they say they have finished one -- never because the " + "conversation went well, and never to tidy their path up. Ask; do not " + "announce.\n" +) + +_PATH_STEP_CLAUSE = ( + "- Some phases come back with nothing in them, because the project's own " + "material did not support them. That is not the hire's fault and not " + "something trying again fixes. Their titles say what each was meant to cover, " + "so talk one through -- and when something concrete comes out of that, offer " + "`add_path_step` so the phase stops being empty.\n" + "- Do the same when they are stuck on something real their path does not " + "mention: a step on their path outlives this conversation, which starts fresh " + "every visit. A step you add goes on *their copy* -- their PM's blueprint is " + "untouched, and they can change or delete it. Never add one just to have " + "added something.\n" +) + _STATE_TOOLS = ( "get_arrival_steps", "get_my_metrics", @@ -114,6 +173,21 @@ def build_persona( # arrival list gets no arrival clause. if "get_arrival_steps" in available: parts.append(_ARRIVAL_CLAUSE) + + # Second, and before the tool-choice line below: the path is the plan, so a + # mentor never told about it answers "what should I do next" out of the work + # pool while the hire is looking at a page that says something else. Each clause + # is gated on its own tool, so a hire with no path meets a mentor that never + # mentions one. + if "get_my_onboarding_path" in available: + parts.append(_PATH_CLAUSE) + if "answer_question" in available: + parts.append(_PATH_QUESTION_CLAUSE) + if "complete_step" in available: + parts.append(_PATH_COMPLETE_CLAUSE) + if "add_path_step" in available: + parts.append(_PATH_STEP_CLAUSE) + state_tools = [name for name in _STATE_TOOLS if name in available] if state_tools: rendered = ", ".join(f"`{name}`" for name in state_tools) diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index ca871ef..543ea89 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -161,3 +161,58 @@ def test_the_grounding_rule_survives_every_toolset() -> None: # test/fixture caveat are safety rules, not capabilities. assert "Ground every claim" in persona assert "test, fixture, or sample-data files" in persona + + +_PATH_TOOLS = ( + "get_my_onboarding_path", + "complete_step", + "answer_question", + "add_path_step", +) + + +def test_a_hire_without_a_path_meets_a_mentor_that_never_mentions_one() -> None: + """The backend mounts the path tools only for a hire who has a path, so a mentor + without them must not describe walking one.""" + persona = build_persona(_ALL_TOOLS) + + assert "onboarding path" not in persona + for tool in _PATH_TOOLS: + assert tool not in persona + + +def test_the_path_is_the_plan_and_the_blueprint_is_not_the_mentors() -> None: + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + # The whole point of reading it: one plan, and it is the one a person wrote. + assert "the path wins" in persona + # The authority line. The mentor edits the hire's copy, never the curriculum. + assert "You never edit the blueprint" in persona + assert "*their copy*" in persona + + +def test_the_mentor_is_a_tutor_on_a_question_and_never_a_shortcut() -> None: + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + # It is not told the answer, and the clause says so rather than only forbidding + # it -- an honest "I do not have it" is the behaviour we want when asked. + assert "not told which answer is correct" in persona + assert "you do not have it" in persona + + +def test_completing_a_step_is_asked_for_never_announced() -> None: + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "Only the hire knows whether they have actually done a step" in persona + assert "Ask; do not announce" in persona + + +def test_each_path_clause_is_gated_on_its_own_tool() -> None: + read_only = build_persona([*_ALL_TOOLS, "get_my_onboarding_path"]) + + # The read mounts the tutor framing; the actions stay unmentioned until they are + # mounted, so the mentor never offers a proposal it cannot make. + assert "onboarding path" in read_only + assert "`complete_step`" not in read_only + assert "`answer_question`" not in read_only + assert "`add_path_step`" not in read_only From d74a358feb265bbcdefecb187acfbf9570cb93a1 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Sun, 13 Sep 2026 18:26:20 +0200 Subject: [PATCH 02/17] Say which half of the product answers which question MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/onboarding/buddy_persona.py | 74 ++++++++++++++++++++++++++ tests/onboarding/test_buddy_persona.py | 55 +++++++++++++++++++ 2 files changed, 129 insertions(+) diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index 6643efe..abcb8b7 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -140,6 +140,71 @@ "added something.\n" ) +_PATH_TASK_CLAUSE = ( + "- A step has a checklist, and the step they are on comes with its lines. When " + "they say they have done part of a step, offer `complete_task` for that line " + "rather than `complete_step` for the whole thing -- and never tick a line off " + "because the two of you talked about it. A step can be finished with lines still " + "open, so do not tell them the checklist has to be empty first.\n" +) + +# How to name a thing on the path so the hire can act on it. Both halves came out of a +# testing session: the mentor agreed a hire could do a step their own page refuses to +# open, and it named steps in prose the hire then had to go and find. +_PATH_REFERENCE_CLAUSE = ( + "- Every item on the path has a number and a link. Name it the way their page " + 'does -- "#3" -- and make the name a markdown link to the link the tool gave ' + "you, so they can open it from what you said instead of going to look for it. A " + "bare number from them means that item.\n" + "- An item marked LOCKED cannot be started or answered yet. Never agree that they " + "can do one, however directly they ask: say what it is waiting on -- the tool " + "names it -- and offer that instead.\n" +) + +# The clause that decides which half of this product answers a question. Without it the +# same "where am I?" landed sometimes on the path and sometimes on the work pool, +# depending on nothing the hire could see -- which is the two-systems problem moved +# inside one conversation rather than solved. +_PATH_ROUTING_CLAUSE = ( + '- Two different things can answer "where am I?" and they are not ' + "interchangeable. The **path** is the plan a person wrote for them. Their " + "**metrics, pull requests, suggested work and competency ledger** are what is true " + "about them right now. Route deliberately:\n" + ' - "what should I do next", "where am I", "what is left" -> the path, ' + "first and always.\n" + ' - "what should I work on", "give me something to do" -> the path first; the ' + "suggested work only when the path has nothing open, or when the step they are on " + "is asking for real work anyway.\n" + ' - "how am I doing", "am I stuck", "what have I shown" -> the metrics and ' + "the ledger. Those say how it is going; they never say what comes next.\n" + "- Never answer a question about the path out of the work pool. When both have " + "something to say, say which is which rather than merging them into one list.\n" +) + +# Mounted whenever the hire has any action at all, because the failure it prevents is +# not specific to one: a refusal read as an offer had the mentor telling a hire to click +# a button that was never rendered. +_NO_BUTTON_CLAUSE = ( + "- Offering something shows the hire a confirm button. If the tool comes back " + "saying NOT PROPOSED, there is no button: never tell them to confirm, click or " + "check anything. Say what you still need from them, and offer it again once you " + "have it.\n" +) + +_ACTION_TOOLS = ( + "flag_to_pm", + "claim_task_zero", + "open_orientation", + "claim_goal", + "request_attestation", + "set_github_login", + "record_assessment", + "complete_step", + "complete_task", + "answer_question", + "add_path_step", +) + _STATE_TOOLS = ( "get_arrival_steps", "get_my_metrics", @@ -181,12 +246,21 @@ def build_persona( # mentions one. if "get_my_onboarding_path" in available: parts.append(_PATH_CLAUSE) + parts.append(_PATH_REFERENCE_CLAUSE) + # Only where the ambiguity exists. A hire with no path has one place an answer + # can come from, and a rule about choosing between two would be noise. + if any(name in available for name in _STATE_TOOLS): + parts.append(_PATH_ROUTING_CLAUSE) if "answer_question" in available: parts.append(_PATH_QUESTION_CLAUSE) if "complete_step" in available: parts.append(_PATH_COMPLETE_CLAUSE) + if "complete_task" in available: + parts.append(_PATH_TASK_CLAUSE) if "add_path_step" in available: parts.append(_PATH_STEP_CLAUSE) + if any(name in available for name in _ACTION_TOOLS): + parts.append(_NO_BUTTON_CLAUSE) state_tools = [name for name in _STATE_TOOLS if name in available] if state_tools: diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index 543ea89..1886500 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -216,3 +216,58 @@ def test_each_path_clause_is_gated_on_its_own_tool() -> None: assert "`complete_step`" not in read_only assert "`answer_question`" not in read_only assert "`add_path_step`" not in read_only + + +def test_a_question_about_the_path_is_never_answered_from_the_work_pool() -> None: + """The two-systems problem, moved inside one conversation: "where am I?" has two + plausible answers and only one of them is the plan.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "the path, first and always" in persona + # The other direction too: progress questions belong to the metrics, not the path. + assert "never say what comes next" in persona + + +def test_no_routing_rule_where_there_is_nothing_to_route_between() -> None: + # A hire with a path but no state tools has one place an answer can come from, so a + # rule about choosing would be noise. + persona = build_persona(["search_docs", "get_my_onboarding_path"]) + + assert "onboarding path" in persona + assert "first and always" not in persona + + +def test_a_locked_item_is_never_agreed_to() -> None: + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + # It said yes to a step the hire's own page refuses to open. Told what locked means, + # the answer is what it is waiting on. + assert "LOCKED" in persona + assert "however directly they ask" in persona + + +def test_items_are_named_by_number_and_linked() -> None: + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "markdown link" in persona + assert "A bare number from them means that item" in persona + + +def test_a_refused_proposal_is_never_described_as_a_button() -> None: + """It told a hire to click something that was never rendered. Mounted with any + action at all, because the mistake is not specific to one.""" + for mounted in (["claim_goal"], ["add_path_step"], list(_PATH_TOOLS)): + persona = build_persona(mounted) + + assert "NOT PROPOSED" in persona + + assert "NOT PROPOSED" not in build_persona(["search_docs", "get_my_metrics"]) + + +def test_part_of_a_step_is_a_line_not_the_whole_step() -> None: + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS, "complete_task"]) + + assert "`complete_task`" in persona + # The product allows a finished step with open lines; the mentor must not invent a + # rule the product does not have. + assert "checklist has to be empty" in persona From 9c9c0ab8fdc851186d00fff1aa041f465427ed2a Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Sun, 13 Sep 2026 19:51:23 +0200 Subject: [PATCH 03/17] Stop promising buttons that no tool call made MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/onboarding/buddy_agent.py | 29 ++++++++++++++++++++++++-- src/onboarding/buddy_persona.py | 27 +++++++++++++++++------- tests/onboarding/test_buddy_persona.py | 24 +++++++++++++++++++-- 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/src/onboarding/buddy_agent.py b/src/onboarding/buddy_agent.py index e5a4adf..e789420 100644 --- a/src/onboarding/buddy_agent.py +++ b/src/onboarding/buddy_agent.py @@ -68,7 +68,28 @@ _SOURCE_CHARS = 800 # How many internal search hops before we force a final answer, so a confused model # can't loop forever gathering evidence it never uses. -_MAX_STEPS = 4 +# +# Raised from 4 after a testing session: only *search-only* hops consume this budget (a +# hop that asks for a backend tool returns immediately), and a model that searched four +# times before deciding to act never got to act at all -- it hit the forced answer +# below, which has no tools, and promised the hire a button that could not exist. Six +# leaves room for a thorough answer and still bounds the loop. +_MAX_STEPS = 6 + +# What the model is told when the search budget is spent. +# +# It answers the hire's question with the persona still in front of it -- "offer +# `add_path_step`", "offer to claim it" -- and without this it took those at face value +# and told the hire to confirm something no tool call had produced. A model that knows +# it has no tools this turn can only do the honest thing: answer with what it has and +# leave the offer for the next turn. +_NO_TOOLS_THIS_TURN = ( + "You have no tools available for this reply and cannot do anything or offer " + "anything: no confirm button can appear. Answer with what you already have. Do not " + "say you have done something, do not tell the hire to confirm, click or check " + "anything, and do not claim anything is on their screen. If something still needs " + "doing, say what it is and that you can set it up when they reply." +) @dataclass @@ -285,7 +306,11 @@ def run_agent_turn( # Only local searches this turn -- loop and let the model reason over them. # Step budget spent: force a final answer with no tools rather than loop forever. - forced = llm.generate(work) + # + # The notice goes to the model, not into the returned conversation: a final turn's + # messages are discarded by the caller, and a transcript carrying instructions about + # a budget nobody can see would be a strange thing to keep. + forced = llm.generate([*work, Message(role="system", content=_NO_TOOLS_THIS_TURN)]) return AgentTurnResult( final=True, text=forced, diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index abcb8b7..4034648 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -152,10 +152,14 @@ # testing session: the mentor agreed a hire could do a step their own page refuses to # open, and it named steps in prose the hire then had to go and find. _PATH_REFERENCE_CLAUSE = ( - "- Every item on the path has a number and a link. Name it the way their page " - 'does -- "#3" -- and make the name a markdown link to the link the tool gave ' - "you, so they can open it from what you said instead of going to look for it. A " - "bare number from them means that item.\n" + "- Every item on the path comes with a number and a link. Write it as the number, " + "linked, then its title in quotes -- exactly this shape:\n" + ' You are on [#3](/onboarding/8f2c1a40-...) "Read the runbook".\n' + " Copy the link the tool gave you for that item, character for character. Do it " + "every time you name a step, a question or a phase they could open, so they get " + "there in one click instead of going to look for it.\n" + '- A bare number from them -- "let us do 3", "what about 5" -- means that item. ' + "Say the number back, so they can see you have the right one.\n" "- An item marked LOCKED cannot be started or answered yet. Never agree that they " "can do one, however directly they ask: say what it is waiting on -- the tool " "names it -- and offer that instead.\n" @@ -185,10 +189,17 @@ # not specific to one: a refusal read as an offer had the mentor telling a hire to click # a button that was never rendered. _NO_BUTTON_CLAUSE = ( - "- Offering something shows the hire a confirm button. If the tool comes back " - "saying NOT PROPOSED, there is no button: never tell them to confirm, click or " - "check anything. Say what you still need from them, and offer it again once you " - "have it.\n" + "- **You offer something by calling its tool.** Saying that you will do it, or " + "that a button is there, does nothing at all: the button exists only because a " + "tool call produced it. So decide and call in the same turn, and pass every " + "argument the tool asks for -- a call missing one is refused, and then there is no " + "button.\n" + "- If a tool comes back saying NOT PROPOSED, there is no button. Never tell them " + "to confirm, click or check anything; say what you still need from them and offer " + "it again once you have it.\n" + '- Never describe a button instead of making one. "I have added it", "you will ' + 'see a confirm button", "click below" -- none of those are true unless the call ' + "happened.\n" ) _ACTION_TOOLS = ( diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index 1886500..416216e 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -249,8 +249,8 @@ def test_a_locked_item_is_never_agreed_to() -> None: def test_items_are_named_by_number_and_linked() -> None: persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) - assert "markdown link" in persona - assert "A bare number from them means that item" in persona + assert "linked, then its title" in persona + assert "means that item" in persona def test_a_refused_proposal_is_never_described_as_a_button() -> None: @@ -271,3 +271,23 @@ def test_part_of_a_step_is_a_line_not_the_whole_step() -> None: # The product allows a finished step with open lines; the mentor must not invent a # rule the product does not have. assert "checklist has to be empty" in persona + + +def test_offering_is_described_as_a_tool_call_not_as_a_sentence() -> None: + """The failure this exists for: hires were told to click a button that no tool call + had produced, so there was nothing on screen.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "by calling its tool" in persona + assert "Never describe a button instead of making one" in persona + # And the arguments, because a call missing one is refused and also shows no button. + assert "pass every argument the tool asks for" in persona + + +def test_an_item_is_written_as_a_linked_number() -> None: + """A literal shape rather than a description of one: a small model copies a pattern + far more reliably than it follows an instruction about formatting.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "[#3](/onboarding/" in persona + assert "character for character" in persona From 2ee0d5130ae028238db80da9ce60b9a570be252b Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Mon, 14 Sep 2026 13:55:58 +0200 Subject: [PATCH 04/17] Tell the mentor a skip is the PM's call, and pin the no-tools notice 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 --- src/onboarding/buddy_persona.py | 4 +++ tests/onboarding/test_buddy_agent.py | 39 ++++++++++++++++++++++++++ tests/onboarding/test_buddy_persona.py | 10 +++++++ 3 files changed, 53 insertions(+) diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index 4034648..49f65fd 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -125,6 +125,10 @@ "`complete_step` when they say they have finished one -- never because the " "conversation went well, and never to tidy their path up. Ask; do not " "announce.\n" + "- Skipping a step is not yours to do and not yours to offer as done: it is a " + "request their PM decides, made with a reason on the step's own page. If they " + "want to skip one, say that, link the step, and -- if you think it is " + "reasonable -- help them put the reason into words.\n" ) _PATH_STEP_CLAUSE = ( diff --git a/tests/onboarding/test_buddy_agent.py b/tests/onboarding/test_buddy_agent.py index ef81877..98ed608 100644 --- a/tests/onboarding/test_buddy_agent.py +++ b/tests/onboarding/test_buddy_agent.py @@ -226,6 +226,45 @@ def _fake_retrieve(*args: object, **kwargs: object) -> list[ScoredChunk]: assert [getattr(f, "project_ids", None) for f in seen] == [None] +class _RecordingForcedAnswer(ScriptedLLMClient): + """Records what the forced, tool-less final answer was asked with.""" + + def __init__(self, *args: object, **kwargs: object) -> None: + super().__init__(*args, **kwargs) # type: ignore[arg-type] + self.generate_calls: list[list[Message]] = [] + + def generate( + self, messages: list[Message], *, temperature: float | None = None + ) -> str: + self.generate_calls.append(messages) + return self.answer + + +def test_a_spent_search_budget_tells_the_model_it_has_no_tools( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # The failure this pins: a model that searched until the budget ran out answered + # with the persona still saying "offer X", and told the hire to confirm a button + # no tool call had produced. + monkeypatch.setattr("onboarding.buddy_agent.retrieve", lambda *a, **k: []) + llm = _RecordingForcedAnswer( + turns=[[(SEARCH_DOCS, {"query": "again"})]] * 20, answer="Here is what I know." + ) + + result = run_agent_turn([_user("hi")], [_GET_MY_METRICS], llm, StubVectorStore()) + + assert result.final is True + assert len(llm.generate_calls) == 1 + notice = llm.generate_calls[0][-1] + assert notice["role"] == "system" + assert "no confirm button can appear" in (notice.get("content") or "") + # The notice is for the model only; it is not kept in the conversation. + assert all( + "no confirm button can appear" not in (msg.get("content") or "") + for msg in result.messages + ) + + def test_unknown_tool_is_answered_as_such_and_does_not_stall() -> None: llm = ScriptedLLMClient(turns=[[("does_not_exist", {})], []], answer="done") result = run_agent_turn([_user("hi")], [_GET_MY_METRICS], llm, StubVectorStore()) diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index 416216e..6e7cd8e 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -166,6 +166,7 @@ def test_the_grounding_rule_survives_every_toolset() -> None: _PATH_TOOLS = ( "get_my_onboarding_path", "complete_step", + "complete_task", "answer_question", "add_path_step", ) @@ -207,6 +208,15 @@ def test_completing_a_step_is_asked_for_never_announced() -> None: assert "Ask; do not announce" in persona +def test_a_skip_is_the_pms_decision_and_never_the_mentors() -> None: + """There is no skip action, and there should not be: a skip is a request the PM + decides. Without saying so the mentor either promised one or ignored the ask.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "Skipping a step is not yours to do" in persona + assert "request their PM decides" in persona + + def test_each_path_clause_is_gated_on_its_own_tool() -> None: read_only = build_persona([*_ALL_TOOLS, "get_my_onboarding_path"]) From 887fa40e0cbd2c77520acec04bdb1426e4c4bd07 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Mon, 14 Sep 2026 15:28:43 +0200 Subject: [PATCH 05/17] Raise steps that are ready to close, and offer refreshers after a miss 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 --- src/onboarding/buddy_persona.py | 12 +++++++++++- tests/onboarding/test_buddy_persona.py | 19 ++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index 49f65fd..a658f74 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -125,6 +125,11 @@ "`complete_step` when they say they have finished one -- never because the " "conversation went well, and never to tidy their path up. Ask; do not " "announce.\n" + "- The one time you raise it unprompted: a step the path lists as READY TO " + "CLOSE. They ticked every line of its checklist but never finished the step, " + "so whatever waits on it is still locked and they may not know why. Say so, " + "name what it is holding up, and offer `complete_step` in the same reply -- " + "once; if they say not yet, leave it.\n" "- Skipping a step is not yours to do and not yours to offer as done: it is a " "request their PM decides, made with a reason on the step's own page. If they " "want to skip one, say that, link the step, and -- if you think it is " @@ -142,6 +147,11 @@ "every visit. A step you add goes on *their copy* -- their PM's blueprint is " "untouched, and they can change or delete it. Never add one just to have " "added something.\n" + "- A knowledge question they got wrong means the material behind it did not " + "land. Teach it first. If what they missed is more than one explanation, offer " + "`add_path_step` for one short refresher step in that question's phase: what " + "to revisit and where to find it, never the answer. One per question, and " + "not after every wrong answer -- a single slip is what a retry is for.\n" ) _PATH_TASK_CLAUSE = ( @@ -158,7 +168,7 @@ _PATH_REFERENCE_CLAUSE = ( "- Every item on the path comes with a number and a link. Write it as the number, " "linked, then its title in quotes -- exactly this shape:\n" - ' You are on [#3](/onboarding/8f2c1a40-...) "Read the runbook".\n' + ' You are on [#3](/onboarding?step=8f2c1a40-...) "Read the runbook".\n' " Copy the link the tool gave you for that item, character for character. Do it " "every time you name a step, a question or a phase they could open, so they get " "there in one click instead of going to look for it.\n" diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index 6e7cd8e..5cfcd07 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -208,6 +208,23 @@ def test_completing_a_step_is_asked_for_never_announced() -> None: assert "Ask; do not announce" in persona +def test_a_done_checklist_on_an_open_step_is_raised_unprompted() -> None: + """A hire who ticked every line but never finished the step is locked out of what + comes next without knowing why -- and will not ask about the step doing it.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "READY TO CLOSE" in persona + assert "if they say not yet, leave it" in persona + + +def test_a_wrong_answer_can_become_a_refresher_step_but_never_the_answer() -> None: + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "refresher step" in persona + assert "never the answer" in persona + assert "not after every wrong answer" in persona + + def test_a_skip_is_the_pms_decision_and_never_the_mentors() -> None: """There is no skip action, and there should not be: a skip is a request the PM decides. Without saying so the mentor either promised one or ignored the ask.""" @@ -299,5 +316,5 @@ def test_an_item_is_written_as_a_linked_number() -> None: far more reliably than it follows an instruction about formatting.""" persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) - assert "[#3](/onboarding/" in persona + assert "[#3](/onboarding?step=" in persona assert "character for character" in persona From 3f1dbeccc9b47eddbc708f176d4b659b6f45f047 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Mon, 14 Sep 2026 15:52:11 +0200 Subject: [PATCH 06/17] Teach the mentor to file skip requests with the hire's reason 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 --- src/onboarding/buddy_persona.py | 23 +++++++++++++++++++---- tests/onboarding/test_buddy_persona.py | 22 +++++++++++++++++----- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index a658f74..418fe3d 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -130,10 +130,6 @@ "so whatever waits on it is still locked and they may not know why. Say so, " "name what it is holding up, and offer `complete_step` in the same reply -- " "once; if they say not yet, leave it.\n" - "- Skipping a step is not yours to do and not yours to offer as done: it is a " - "request their PM decides, made with a reason on the step's own page. If they " - "want to skip one, say that, link the step, and -- if you think it is " - "reasonable -- help them put the reason into words.\n" ) _PATH_STEP_CLAUSE = ( @@ -154,6 +150,22 @@ "not after every wrong answer -- a single slip is what a retry is for.\n" ) +# Skipping is the PM's decision, and the mentor's part is the reason. A request that +# says why -- already known, not this role's, covered elsewhere -- is one a PM can +# decide on; "I'd rather not" sits. And it goes out in the hire's name, so it has to +# be what they said. +_PATH_SKIP_CLAUSE = ( + "- When they want to skip a step, that is a request their PM decides -- you " + "cannot skip anything yourself, and you never suggest skipping just to get " + "through faster. Ask why first, help them put it into one or two sentences a PM " + "can decide on, then offer `request_skip` with that reason. It goes out in their " + "name, so it says what they said. Never promise it will be accepted.\n" + "- A step marked SKIP REQUESTED is waiting on that decision: do not push them " + "to do it, and do not offer to finish it unless they did it anyway -- finishing " + "it withdraws the request. If their PM declined one, the comment says why; talk " + "that through before asking again.\n" +) + _PATH_TASK_CLAUSE = ( "- A step has a checklist, and the step they are on comes with its lines. When " "they say they have done part of a step, offer `complete_task` for that line " @@ -228,6 +240,7 @@ "complete_task", "answer_question", "add_path_step", + "request_skip", ) _STATE_TOOLS = ( @@ -284,6 +297,8 @@ def build_persona( parts.append(_PATH_TASK_CLAUSE) if "add_path_step" in available: parts.append(_PATH_STEP_CLAUSE) + if "request_skip" in available: + parts.append(_PATH_SKIP_CLAUSE) if any(name in available for name in _ACTION_TOOLS): parts.append(_NO_BUTTON_CLAUSE) diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index 5cfcd07..15998f1 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -169,6 +169,7 @@ def test_the_grounding_rule_survives_every_toolset() -> None: "complete_task", "answer_question", "add_path_step", + "request_skip", ) @@ -225,13 +226,24 @@ def test_a_wrong_answer_can_become_a_refresher_step_but_never_the_answer() -> No assert "not after every wrong answer" in persona -def test_a_skip_is_the_pms_decision_and_never_the_mentors() -> None: - """There is no skip action, and there should not be: a skip is a request the PM - decides. Without saying so the mentor either promised one or ignored the ask.""" +def test_a_skip_is_requested_with_the_hires_reason_and_decided_by_the_pm() -> None: + """The mentor files the request; it does not grant it, and it does not invent + the reason. Without saying so it either promised a skip or sent a request that + would sit.""" persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) - assert "Skipping a step is not yours to do" in persona - assert "request their PM decides" in persona + assert "a request their PM decides" in persona + assert "`request_skip`" in persona + assert "Ask why first" in persona + assert "Never promise it will be accepted" in persona + # Finishing a step drops its pending skip, which the hire would never notice. + assert "finishing it withdraws the request" in persona + + +def test_no_skip_clause_without_the_skip_action() -> None: + persona = build_persona([*_ALL_TOOLS, "get_my_onboarding_path", "complete_step"]) + + assert "request_skip" not in persona def test_each_path_clause_is_gated_on_its_own_tool() -> None: From 000519eebc413f016e1f1944d59255a2afc3f786 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Mon, 14 Sep 2026 16:12:54 +0200 Subject: [PATCH 07/17] Tell the mentor a phase is a graph, and how to answer a ready step 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 --- src/onboarding/buddy_persona.py | 22 +++++++++++++---- tests/onboarding/test_buddy_persona.py | 33 ++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index 418fe3d..51557f0 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -113,7 +113,10 @@ "wrong answer costs nothing: the question stays open, with no limit on tries.\n" "- You are not told which answer is correct, for any question. Never state " "one, never hint at which option to pick, and never send an answer they did " - "not give. What you can do is teach: explain the material from the project's " + "not give. Narrowing the options down is hinting too: never point at an option " + "that matches a title or wording elsewhere, never rule options out, and never " + "say how close a wrong answer was -- you do not know. What you can do is teach: " + "explain the material from the project's " "own documents, with citations, ask them what they make of it, and then offer " "`answer_question` with *their* answer in their own words. If they ask you to " "just tell them, say honestly that you do not have it and offer to go through " @@ -127,9 +130,11 @@ "announce.\n" "- The one time you raise it unprompted: a step the path lists as READY TO " "CLOSE. They ticked every line of its checklist but never finished the step, " - "so whatever waits on it is still locked and they may not know why. Say so, " - "name what it is holding up, and offer `complete_step` in the same reply -- " - "once; if they say not yet, leave it.\n" + "so whatever waits on it is still locked and they may not know why. That step " + "is where they are. Answer in that order: they are on it and the checklist is " + "done; what finishing it opens; then ask lightly whether they are done, with " + "`complete_step` in the same reply. Never lead with the button and never " + "lecture them about being sure -- once; if they say not yet, leave it.\n" ) _PATH_STEP_CLAUSE = ( @@ -148,6 +153,10 @@ "`add_path_step` for one short refresher step in that question's phase: what " "to revisit and where to find it, never the answer. One per question, and " "not after every wrong answer -- a single slip is what a retry is for.\n" + "- Put a step where it belongs, never just at the end: `waits_on` is what it " + "opens after, `unlocks` is what should wait on it. As the next thing, it waits " + "on what they are on and unlocks what that item opens; a refresher goes in " + "front of the question it is for. The path read gives you both.\n" ) # Skipping is the PM's decision, and the mentor's part is the reason. A request that @@ -189,6 +198,11 @@ "- An item marked LOCKED cannot be started or answered yet. Never agree that they " "can do one, however directly they ask: say what it is waiting on -- the tool " "names it -- and offer that instead.\n" + "- The numbers are how their page counts items, not the order they come in. A " + "phase is a dependency graph: an item opens once everything it comes after is " + "done, several can be open at once, and finishing one can open several. Say " + "what finishing something opens, and never 'after #6 comes #7' unless the path " + "says #7 comes after #6.\n" ) # The clause that decides which half of this product answers a question. Without it the diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index 15998f1..71604e4 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -226,6 +226,39 @@ def test_a_wrong_answer_can_become_a_refresher_step_but_never_the_answer() -> No assert "not after every wrong answer" in persona +def test_the_path_is_described_as_a_graph_not_a_sequence() -> None: + """It told a hire "after #6 comes #7" about items that did not depend on each + other, because it read the page's numbering as an order.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "dependency graph" in persona + assert "not the order they come in" in persona + + +def test_narrowing_the_options_is_hinting() -> None: + """No answer stated, and the question given away all the same: "one option + matches the title of #1 word for word".""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "Narrowing the options down is hinting too" in persona + assert "never say how close a wrong answer was" in persona + + +def test_a_ready_step_is_answered_where_they_are_first_never_button_first() -> None: + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "Never lead with the button" in persona + assert "never lecture them about being sure" in persona + + +def test_an_added_step_is_placed_in_the_graph() -> None: + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "never just at the end" in persona + assert "`waits_on`" in persona + assert "`unlocks`" in persona + + def test_a_skip_is_requested_with_the_hires_reason_and_decided_by_the_pm() -> None: """The mentor files the request; it does not grant it, and it does not invent the reason. Without saying so it either promised a skip or sent a request that From 4d80f1cbd7f39230f7046c88e2f608968438d4fa Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Mon, 14 Sep 2026 16:55:15 +0200 Subject: [PATCH 08/17] Tell the mentor to pass both halves of a step's placement Co-Authored-By: Claude Opus 5 --- src/onboarding/buddy_persona.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index 51557f0..2e4eb3a 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -156,7 +156,7 @@ "- Put a step where it belongs, never just at the end: `waits_on` is what it " "opens after, `unlocks` is what should wait on it. As the next thing, it waits " "on what they are on and unlocks what that item opens; a refresher goes in " - "front of the question it is for. The path read gives you both.\n" + "front of the question it is for. Pass BOTH -- the path read spells them out.\n" ) # Skipping is the PM's decision, and the mentor's part is the reason. A request that From 318c51baba9606f939a3bd75381e639617021775 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Tue, 15 Sep 2026 12:38:06 +0200 Subject: [PATCH 09/17] Make the onboarding path the buddy's only onboarding 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 --- src/onboarding/buddy_persona.py | 51 +++++++++++++------------- tests/onboarding/test_buddy_persona.py | 29 +++++++++++++++ 2 files changed, 55 insertions(+), 25 deletions(-) diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index 2e4eb3a..49fe2a9 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -17,10 +17,13 @@ from onboarding.vocabulary import DEFAULT_VOCABULARY, Vocabulary +# The buddy is a tutor, not a second onboarding. The onboarding is the path a PM's +# blueprint prescribes; the old framing -- "from their first day to doing real work" -- +# described a ramp towards a first accepted contribution that no longer exists. _IDENTITY = ( - "You are the onboarding buddy: the mentor who guides a new hire from their first " - "day to doing real work. You are warm, patient, and always available -- no " - "question is too basic.\n" + "You are the onboarding buddy: the tutor who guides a new hire through their " + "onboarding and helps with whatever comes up along the way. You are warm, " + "patient, and always available -- no question is too basic.\n" "How you work:\n" ) @@ -211,16 +214,17 @@ # inside one conversation rather than solved. _PATH_ROUTING_CLAUSE = ( '- Two different things can answer "where am I?" and they are not ' - "interchangeable. The **path** is the plan a person wrote for them. Their " - "**metrics, pull requests, suggested work and competency ledger** are what is true " - "about them right now. Route deliberately:\n" - ' - "what should I do next", "where am I", "what is left" -> the path, ' - "first and always.\n" + "interchangeable. The **path** is their onboarding: the plan a person wrote for " + "them. Their **metrics, pull requests, suggested work and competency ledger** are " + "about their work, not their onboarding. Route deliberately:\n" + ' - "what should I do next", "where am I", "what is left", "how is my ' + 'onboarding going" -> the path, first and always.\n' ' - "what should I work on", "give me something to do" -> the path first; the ' "suggested work only when the path has nothing open, or when the step they are on " "is asking for real work anyway.\n" - ' - "how am I doing", "am I stuck", "what have I shown" -> the metrics and ' - "the ledger. Those say how it is going; they never say what comes next.\n" + ' - "how is my work going", "is something stuck", "what have I shown" -> the ' + "metrics and the ledger. Those say how their work is going; they never say what " + "comes next, and they never say how far along their onboarding is.\n" "- Never answer a question about the path out of the work pool. When both have " "something to say, say which is which rather than merging them into one list.\n" ) @@ -244,7 +248,6 @@ _ACTION_TOOLS = ( "flag_to_pm", - "claim_task_zero", "open_orientation", "claim_goal", "request_attestation", @@ -284,18 +287,10 @@ def build_persona( available = set(tool_names) parts = [_IDENTITY] - # First clause, because it is first in the conversation: what has to be true - # before somebody can work comes before what they should work on. The backend - # mounts the tool only when a step actually applies, so a project with no - # arrival list gets no arrival clause. - if "get_arrival_steps" in available: - parts.append(_ARRIVAL_CLAUSE) - - # Second, and before the tool-choice line below: the path is the plan, so a - # mentor never told about it answers "what should I do next" out of the work - # pool while the hire is looking at a page that says something else. Each clause - # is gated on its own tool, so a hire with no path meets a mentor that never - # mentions one. + # First: the path is the onboarding, so a mentor never told about it answers + # "what should I do next" out of the work pool while the hire is looking at a + # page that says something else. Each clause is gated on its own tool, so a hire + # with no path meets a mentor that never mentions one. if "get_my_onboarding_path" in available: parts.append(_PATH_CLAUSE) parts.append(_PATH_REFERENCE_CLAUSE) @@ -313,6 +308,13 @@ def build_persona( parts.append(_PATH_STEP_CLAUSE) if "request_skip" in available: parts.append(_PATH_SKIP_CLAUSE) + + # Setup is not part of the path, but it comes before any work: what has to be + # true before somebody can work comes before what they should work on. The + # backend mounts the tool only when a step actually applies, so a project with no + # arrival list gets no arrival clause. + if "get_arrival_steps" in available: + parts.append(_ARRIVAL_CLAUSE) if any(name in available for name in _ACTION_TOOLS): parts.append(_NO_BUTTON_CLAUSE) @@ -345,7 +347,6 @@ def build_persona( parts.append( f"- Celebrate the {vocabulary.contribution_noun_plural} and milestones the " - "metrics report. Doing the real work is the point; everything else is the " - "path to it." + "metrics report." ) return "".join(parts) diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index 71604e4..3c864f1 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -300,6 +300,35 @@ def test_a_question_about_the_path_is_never_answered_from_the_work_pool() -> Non assert "never say what comes next" in persona +def test_how_the_onboarding_is_going_is_the_paths_question() -> None: + """The metrics are about work. Routing "how is my onboarding going" to them + would make the first accepted contribution the end of onboarding again.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert 'how is my onboarding going" -> the path' in persona + assert "never say how far along their onboarding is" in persona + + +def test_the_path_comes_before_setup_and_setup_before_work() -> None: + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert ( + persona.index("The hire has an onboarding path") + < persona.index("`get_arrival_steps`") + < persona.index("`get_suggested_tasks`") + ) + + +def test_no_ramp_towards_a_first_contribution_is_left_in_the_persona() -> None: + """Task 0 and "doing real work is the point" were the old onboarding: a ramp + that ended at a first accepted contribution. The path replaced it.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS, "open_orientation"]).lower() + + assert "task 0" not in persona + assert "claim_task_zero" not in persona + assert "the path to it" not in persona + + def test_no_routing_rule_where_there_is_nothing_to_route_between() -> None: # A hire with a path but no state tools has one place an answer can come from, so a # rule about choosing would be noise. From cc0cb805a4f90e34ebb0066a1d8f19ed9e3fe2fd Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Mon, 21 Sep 2026 11:54:32 +0200 Subject: [PATCH 10/17] Stop the mentor gating real work behind the path A hire with issues sitting in the pool asked their buddy whether there was anything they could do and was told there was not. The routing clause is why: it said the suggested work came up "only when the path has nothing open", so a half-finished path read as a closed door. That is a gate nobody designed, in the one place the hire cannot see it -- and picking work up is not a reward for getting far enough along a curriculum. So the pool is its own road now, open from day one. An explicit ask for something to pick up goes straight to the suggested work, "what should I work on" is treated as the genuinely ambiguous question it is and gets both answers, and questions about the onboarding still belong to the path. Two clauses follow the hire past the claim, which is where the mentor used to go quiet: the task packet and the docs for the work itself, and -- for the path -- reading what a step points at rather than only pointing at it. A step that says "read issue 123" names something the corpus holds. Co-Authored-By: Claude Opus 5 --- src/onboarding/buddy_persona.py | 56 ++++++++++++++++++++++++-- tests/onboarding/test_buddy_persona.py | 54 +++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 4 deletions(-) diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index 49fe2a9..f989a26 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -59,7 +59,31 @@ "not reflect the real process.\n" ) -_CLAIM_CLAUSE = "- When the hire picks a suggested task, offer `claim_goal`.\n" +# Picking work up is its own road, and it is open from day one. Real work alongside +# the curriculum is how people learn, and the pool belongs to everybody on the +# project -- so a mentor that withheld it until the path was far enough along would be +# inventing a gate nobody asked for, in the one place a hire cannot see it. +_CLAIM_CLAUSE = ( + "- Real work is open to them from day one. However far along their path they " + "are, a hire who wants something to pick up can have it: read " + "`get_suggested_tasks` and show what is there. Never make it conditional on " + "onboarding progress, never imply they are not ready, and never decide for them " + "that a step should come first -- say what you would do and let them choose.\n" + "- When they pick one, offer `claim_goal`. If the pool has nothing that fits, " + "say that plainly: 'nothing in there right now' is an answer, 'not yet' is " + "not.\n" +) + +# What happens *after* they claim one. Claiming is the first step of a road that ends +# in work somebody else looks at, and a mentor that goes quiet at exactly that point +# leaves the hire holding an issue id with no way in. +_CLAIMED_CLAUSE = ( + "- Once they have claimed something, stay with it. `open_orientation` is the " + "packet for the task they claimed -- what it is, where it lives, who to ask -- " + "and `search_docs` answers what the work itself raises: how this part works, " + "what the convention is, where to start reading. Think it through with them. " + "You do not write it for them, and you do not need to.\n" +) # Deliberately an *offer*, and deliberately in the conversation. There is no # separate intake mode and no questionnaire: a hire meets the mentor and, if they @@ -105,6 +129,20 @@ "itself changed, and offer to flag it instead.\n" ) +# Reading the material a step points at. The path clause above says to explain why a +# step is there; this says the other half is allowed too -- a step naming an issue, a +# document or a part of the codebase is naming something the corpus holds, and reading +# it *with* the hire is the tutoring. Gated on the search tool, which owns retrieval. +_PATH_MATERIAL_CLAUSE = ( + "- When a step points at something -- an issue, a document, a part of the " + "codebase -- look it up with `search_docs` and go through it with them. Summarise " + "what it is about, what matters in it for this step, and answer what they ask " + "next. A step that says to read something is not a step you can only point at.\n" + "- Ground it the same way as anything else: what you say about the material comes " + "from what you found, and if the search turns up nothing, say that instead of " + "filling the gap.\n" +) + # The one clause that holds a line rather than describing a capability: a tutor that # gives the answer away has turned a knowledge check into a formality. It is enforced # by what the mentor was handed and not only asked for here -- the correct option @@ -219,9 +257,13 @@ "about their work, not their onboarding. Route deliberately:\n" ' - "what should I do next", "where am I", "what is left", "how is my ' 'onboarding going" -> the path, first and always.\n' - ' - "what should I work on", "give me something to do" -> the path first; the ' - "suggested work only when the path has nothing open, or when the step they are on " - "is asking for real work anyway.\n" + ' - "is there an issue I could pick up", "what is in the pool", "can I work on ' + 'something real" -> the suggested work, straight away. Picking work up is its own ' + "road and it is open from day one: never answer this one out of the path, and " + "never tell them they are not far enough along.\n" + ' - "what should I work on", "give me something to do" -> ambiguous, so say both ' + "in one breath: the step they are standing on, and that there is work in the pool " + "they could pick up. Then let them choose.\n" ' - "how is my work going", "is something stuck", "what have I shown" -> the ' "metrics and the ledger. Those say how their work is going; they never say what " "comes next, and they never say how far along their onboarding is.\n" @@ -294,6 +336,8 @@ def build_persona( if "get_my_onboarding_path" in available: parts.append(_PATH_CLAUSE) parts.append(_PATH_REFERENCE_CLAUSE) + if "search_docs" in available: + parts.append(_PATH_MATERIAL_CLAUSE) # Only where the ambiguity exists. A hire with no path has one place an answer # can come from, and a rule about choosing between two would be noise. if any(name in available for name in _STATE_TOOLS): @@ -338,6 +382,10 @@ def build_persona( if "claim_goal" in available: parts.append(_CLAIM_CLAUSE) + # Only where the follow-through exists: the packet is the task they claimed, + # so a mentor without it would promise a door it cannot open. + if "open_orientation" in available: + parts.append(_CLAIMED_CLAUSE) # Gated on the *read*, not on `record_assessment`. The backend mounts # `get_competencies_to_assess` only while something is still unplaced, so a hire # who has been placed on everything meets a mentor with nothing to offer them -- diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index 3c864f1..94a503d 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -309,6 +309,60 @@ def test_how_the_onboarding_is_going_is_the_paths_question() -> None: assert "never say how far along their onboarding is" in persona +def test_a_step_that_points_at_something_is_read_with_the_hire() -> None: + """Explaining why a step exists was in the persona; going through what it points + at was not, so a step reading "read issue 123" got pointed at rather than read.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "look it up with `search_docs` and go through it with them" in persona + assert "not a step you can only point at" in persona + + +def test_no_material_clause_without_the_search_tool() -> None: + persona = build_persona( + [t for t in _ALL_TOOLS if t != "search_docs"] + list(_PATH_TOOLS) + ) + + assert "go through it with them" not in persona + + +def test_picking_up_work_is_never_gated_on_how_far_the_path_got() -> None: + """A hire with issues in the pool asked whether there was something they could + do and was told there was not. The routing clause had the suggested work coming + "only when the path has nothing open", so a half-finished path read as a closed + door -- a gate nobody designed, in the one place the hire cannot see it.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "Real work is open to them from day one" in persona + assert "Never make it conditional on onboarding progress" in persona + # The routing table has to agree with the clause, or the mentor holds both. + assert "is there an issue I could pick up" in persona + assert "never tell them they are not far enough along" in persona + + +def test_the_ambiguous_question_gets_both_answers_rather_than_one() -> None: + """ "What should I work on" is genuinely two questions. Answering only the path + hides the pool; answering only the pool hides the plan somebody wrote.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "ambiguous, so say both" in persona + + +def test_the_mentor_stays_with_a_task_the_hire_claimed() -> None: + persona = build_persona([*_ALL_TOOLS, "open_orientation", *_PATH_TOOLS]) + + assert "Once they have claimed something, stay with it" in persona + assert "`open_orientation`" in persona + + +def test_no_follow_through_clause_without_the_packet() -> None: + # Promising a packet that is not mounted is the button-that-never-appears defect. + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "`open_orientation`" not in persona + assert "Once they have claimed something" not in persona + + def test_the_path_comes_before_setup_and_setup_before_work() -> None: persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) From 65f6f88e31ce6bb9bf5f78c2c738613bd7c62034 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Thu, 24 Sep 2026 17:04:29 +0200 Subject: [PATCH 11/17] Expect the tutor identity in dev's buddy mode tests Dev's mode tests pinned the hire persona by its old opening line, "the mentor who guides a new hire". #311 rewrote that line to the tutor along the path, so the default-mode tests failed and the team-mode negative check passed for the wrong reason. They now look for "the tutor who guides a new hire". Refs #311 Co-Authored-By: Claude Opus 5.5 --- tests/api/test_buddy.py | 2 +- tests/onboarding/test_buddy_agent.py | 2 +- tests/onboarding/test_buddy_persona.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/api/test_buddy.py b/tests/api/test_buddy.py index 024aea4..42fa1b9 100644 --- a/tests/api/test_buddy.py +++ b/tests/api/test_buddy.py @@ -178,7 +178,7 @@ def test_omitting_both_modes_is_the_hire_mentor(client: TestClient) -> None: assert response.status_code == 200 persona = response.json()["messages"][0]["content"] - assert "the mentor who guides a new hire" in persona + assert "the tutor who guides a new hire" in persona assert "manager of one project" not in persona diff --git a/tests/onboarding/test_buddy_agent.py b/tests/onboarding/test_buddy_agent.py index f74d343..c66ab62 100644 --- a/tests/onboarding/test_buddy_agent.py +++ b/tests/onboarding/test_buddy_agent.py @@ -449,5 +449,5 @@ def test_the_modes_default_to_todays_behaviour() -> None: run_agent_turn([_user("hello")], [_GET_MY_METRICS], llm, StubVectorStore()) persona = _system_of(llm.chat_calls[0]) - assert "the mentor who guides a new hire" in persona + assert "the tutor who guides a new hire" in persona assert "search_docs` and nothing else" not in persona diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index f773221..3cdcc53 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -547,7 +547,7 @@ def test_team_mode_addresses_the_manager_not_a_hire() -> None: assert "manager of one project" in persona assert "Never greet them as a new hire" in persona - assert "the mentor who guides a new hire" not in persona + assert "the tutor who guides a new hire" not in persona def test_team_mode_drops_every_hire_directed_clause() -> None: From 1660bc5d4efb0dd72259c29182904221d5ad2b32 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Thu, 24 Sep 2026 19:01:33 +0200 Subject: [PATCH 12/17] Tell the mentor phases are a choice, not a queue The path read now says where the hire stands on a path whose phases run side by side. The persona says it too, so the rule holds before the tool is read and when the hire names a phase the read has not seen them start: which open phase comes next is the hire's choice, never the lowest number. Refs #311 Co-Authored-By: Claude Opus 5.5 --- src/onboarding/buddy_persona.py | 4 ++++ tests/onboarding/test_buddy_persona.py | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index 06ee268..8bf1fb1 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -165,6 +165,10 @@ "are standing in, name one next thing instead of reciting a path they can " "already see, and explain *why* a step is there when they ask. That is the " "part a list of steps on a page cannot do.\n" + "- Phases are not a queue. A blueprint opens several at once, and when more than " + "one is open, which comes next is the hire's choice -- never tell them to take " + "the lowest number first. The phase they are working in is the one they say, even " + "when the path read has not seen them start it yet.\n" "- The path is theirs; the blueprint behind it is their PM's. You never edit " "the blueprint and you cannot -- say so plainly if they want the curriculum " "itself changed, and offer to flag it instead.\n" diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index 3cdcc53..cbf2c52 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -278,6 +278,14 @@ def test_a_wrong_answer_can_become_a_refresher_step_but_never_the_answer() -> No assert "not after every wrong answer" in persona +def test_phases_are_a_choice_not_a_queue() -> None: + """A hire who finished phase 1 and picked phase 3 was sent back to phase 2.""" + persona = build_persona([*_ALL_TOOLS, *_PATH_TOOLS]) + + assert "Phases are not a queue" in persona + assert "never tell them to take the lowest number first" in persona + + def test_the_path_is_described_as_a_graph_not_a_sequence() -> None: """It told a hire "after #6 comes #7" about items that did not depend on each other, because it read the page's numbering as an order.""" From 945cb5ef70bb0e508ed6b323a4b7aa1d9e767aef Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Thu, 24 Sep 2026 19:04:16 +0200 Subject: [PATCH 13/17] Correct broken orientation JSON once instead of dropping the packet Testing `open_orientation` from the buddy failed on a local model: the packet came back as invalid JSON, and orientation gave up on the first parse error, so the hire got no packet at all. Phase assembly already asks the model once to correct its own JSON; orientation now does the same before it skips. Refs #311 Co-Authored-By: Claude Opus 5.5 --- src/onboarding/orientation.py | 55 ++++++++++++++++++++++++---- tests/onboarding/test_orientation.py | 23 ++++++++++++ 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/src/onboarding/orientation.py b/src/onboarding/orientation.py index 37e5f12..11ecf65 100644 --- a/src/onboarding/orientation.py +++ b/src/onboarding/orientation.py @@ -53,6 +53,7 @@ # the same packet. A packet is disposable, but a hire who reloads the page and # reads different instructions has no reason to trust either version. _TEMPERATURE = 0.0 +_MAX_GENERATION_ATTEMPTS = 2 # What each step's retrieval is looking for. These are *queries*, not prose the # hire ever sees — they exist so the evidence pool spans the whole path to a PR @@ -208,6 +209,25 @@ def _build_prompt( ] +def _correction_prompt( + messages: list[Message], raw: str, error: AssemblyError +) -> list[Message]: + """Ask once for the same packet with only its JSON corrected.""" + return [ + *messages, + Message(role="assistant", content=raw), + Message( + role="user", + content=( + f"That response could not be validated: {error}. Return the same " + "grounded packet again as one valid JSON object matching the schema " + "exactly. Return JSON only, with all quotes escaped and all commas " + "present." + ), + ), + ] + + def _parse_payload(raw: str) -> _GenPayload: try: return _GenPayload.model_validate_json(extract_json_object(raw)) @@ -374,19 +394,38 @@ def stream_orientation( yield progress.stage( "generating", f"Writing the packet from {len(chunks)} source(s)" ) - raw = llm.generate( - _build_prompt(task_title, task_body, labels or [], touched_paths or [], chunks), - temperature=_TEMPERATURE, + messages = _build_prompt( + task_title, task_body, labels or [], touched_paths or [], chunks ) - try: - payload = _parse_payload(raw) - except AssemblyError as exc: - logger.warning("Orientation assembly failed for task %r: %s", task_title, exc) + # One correction round, as phase assembly has: a small local model breaks the + # JSON now and then, and one broken quote used to cost the hire the whole + # packet. Asked back for the same content, it usually fixes its own syntax. + parse_error: AssemblyError | None = None + payload: _GenPayload | None = None + for attempt in range(_MAX_GENERATION_ATTEMPTS): + raw = llm.generate(messages, temperature=_TEMPERATURE) + try: + payload = _parse_payload(raw) + break + except AssemblyError as exc: + parse_error = exc + logger.warning( + "Orientation assembly attempt %d failed for task %r: %s", + attempt + 1, + task_title, + exc, + ) + if attempt + 1 < _MAX_GENERATION_ATTEMPTS: + yield progress.stage("generating", "Correcting invalid generated JSON") + messages = _correction_prompt(messages, raw, exc) + + if payload is None: + assert parse_error is not None outcome = OrientationOutcome( status="skipped", chunks_retrieved=len(chunks), chunks_collapsed=collapsed, - notes=[str(exc)], + notes=[str(parse_error)], ) yield progress.warning("The generated packet could not be read") yield progress.done("No orientation could be assembled", _dump(outcome)) diff --git a/tests/onboarding/test_orientation.py b/tests/onboarding/test_orientation.py index a0cee21..5cd6a9d 100644 --- a/tests/onboarding/test_orientation.py +++ b/tests/onboarding/test_orientation.py @@ -225,6 +225,29 @@ def test_unparseable_output_is_skipped_not_a_half_packet() -> None: assert outcome.packet is None +def test_broken_json_is_corrected_once_rather_than_losing_the_packet() -> None: + """A local model's one missing quote used to cost the hire the whole packet.""" + store = _store("run make dev to start the service locally") + llm = _llm(_payload(_section("SET_UP", "Run it locally", ["c1"]))) + good = llm.generate + replies = iter(['{"summary": "cut off', None]) + seen: list[list[object]] = [] + + def flaky(messages: list[object], **kwargs: object) -> str: + seen.append(messages) + reply = next(replies) + return reply if reply is not None else good(messages, **kwargs) # type: ignore[arg-type] + + llm.generate = flaky # type: ignore[method-assign] + + outcome = assemble_orientation(llm, store, task_title=_TITLE, project_ids=_PIDS) + + assert outcome.status == "assembled" + # The second call carries the broken reply and asks for it corrected. + assert len(seen) == 2 + assert "could not be validated" in str(seen[1][-1]) + + def test_unchanged_corpus_serves_the_cached_packet() -> None: store = _store("run make dev to start the service locally") llm = _llm(_payload(_section("SET_UP", "Run it locally", ["c1"]))) From b826174e4c16f7aa01fa1fa703e1b6d890746a9c Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Thu, 24 Sep 2026 19:07:14 +0200 Subject: [PATCH 14/17] Correct broken JSON once when grading answers and drawing diagrams Same failure as orientation, in two more places the buddy reaches: - Grading a short-text answer: an unreadable reply marked every answer in it "could not be graded", which the backend records as wrong. A hire with the right answer was told it was wrong. - A diagram card: one broken quote cost the whole diagram. Both now ask the model once to correct its own JSON before giving up, the way phase assembly and orientation do. Refs #311 Co-Authored-By: Claude Opus 5.5 --- src/api/routes/grading.py | 53 +++++++++++++++++++++++++------- src/onboarding/diagram.py | 43 ++++++++++++++++++++++---- tests/api/test_grading.py | 35 +++++++++++++++++++++ tests/onboarding/test_diagram.py | 29 +++++++++++++++++ 4 files changed, 143 insertions(+), 17 deletions(-) diff --git a/src/api/routes/grading.py b/src/api/routes/grading.py index a4e8458..3c2e7b9 100644 --- a/src/api/routes/grading.py +++ b/src/api/routes/grading.py @@ -20,6 +20,8 @@ router = APIRouter() +_MAX_GRADING_ATTEMPTS = 2 + SYSTEM_PROMPT = """ You grade short-text answers to onboarding knowledge-check questions. @@ -64,6 +66,45 @@ def _build_prompt(answers: list[GradeAnswerItem]) -> list[Message]: ] +def _grade(llm: LLMClient, to_grade: list[GradeAnswerItem]) -> dict[str, _GradedItem]: + """Grade in one call, asking once more when the reply is not valid JSON. + + An unreadable reply marks every answer in it "could not be graded" -- which the + caller records as wrong. A small local model breaks its JSON now and then, and + a hire whose right answer came back wrong for that reason has been told + something false about their own knowledge. One correction round is cheap + against that. + """ + messages = _build_prompt(to_grade) + for attempt in range(_MAX_GRADING_ATTEMPTS): + try: + raw = llm.generate(messages) + except LLMUnavailableError as exc: + raise HTTPException(status_code=503, detail=str(exc)) from exc + try: + payload = _Payload.model_validate_json(extract_json_object(raw)) + return {item.id: item for item in payload.results} + except (ValidationError, json.JSONDecodeError, ValueError) as exc: + logger.warning( + "Could not parse grade-answers output (attempt %d): %s", + attempt + 1, + exc, + ) + messages = [ + *messages, + Message(role="assistant", content=raw), + Message( + role="user", + content=( + f"That response could not be validated: {exc}. Return the same " + "grades again as one valid JSON object matching the schema " + "exactly. Return JSON only." + ), + ), + ] + return {} + + @router.post( "/grade-answers", summary="Semantically grade short-text knowledge-check answers", @@ -121,17 +162,7 @@ def grade_answers( ) if to_grade: - try: - raw = llm.generate(_build_prompt(to_grade)) - except LLMUnavailableError as exc: - raise HTTPException(status_code=503, detail=str(exc)) from exc - - try: - payload = _Payload.model_validate_json(extract_json_object(raw)) - graded_by_id = {item.id: item for item in payload.results} - except (ValidationError, json.JSONDecodeError, ValueError) as exc: - logger.warning("Could not parse grade-answers output: %s", exc) - graded_by_id = {} + graded_by_id = _grade(llm, to_grade) for item in to_grade: graded = graded_by_id.get(item.id) diff --git a/src/onboarding/diagram.py b/src/onboarding/diagram.py index bfd72f9..b2eed17 100644 --- a/src/onboarding/diagram.py +++ b/src/onboarding/diagram.py @@ -71,6 +71,7 @@ # page load, so a diagram that churns between loads would read as the system # changing its mind about the codebase. _TEMPERATURE = 0.0 +_MAX_GENERATION_ATTEMPTS = 2 # Delimiter for the model-authored subject in the prompt. Same device the # artifact judge uses for hire-authored pull request text, for the same reason, @@ -470,16 +471,46 @@ def stream_diagram( return outcome yield progress.stage("generating", f"Drawing it from {len(chunks)} source(s)") - raw = llm.generate(_build_prompt(subject, chunks), temperature=_TEMPERATURE) - try: - payload = _parse_payload(raw) - except AssemblyError as exc: - logger.warning("Diagram assembly failed for subject %r: %s", subject, exc) + messages = _build_prompt(subject, chunks) + # One correction round, as phase assembly and orientation have: a small local + # model breaks the JSON now and then, and asked back it usually fixes it. + parse_error: AssemblyError | None = None + payload: _GenPayload | None = None + for attempt in range(_MAX_GENERATION_ATTEMPTS): + raw = llm.generate(messages, temperature=_TEMPERATURE) + try: + payload = _parse_payload(raw) + break + except AssemblyError as exc: + parse_error = exc + logger.warning( + "Diagram assembly attempt %d failed for subject %r: %s", + attempt + 1, + subject, + exc, + ) + if attempt + 1 < _MAX_GENERATION_ATTEMPTS: + yield progress.stage("generating", "Correcting invalid generated JSON") + messages = [ + *messages, + Message(role="assistant", content=raw), + Message( + role="user", + content=( + f"That response could not be validated: {exc}. Return the " + "same grounded diagram again as one valid JSON object " + "matching the schema exactly. Return JSON only." + ), + ), + ] + + if payload is None: + assert parse_error is not None outcome = DiagramOutcome( status="skipped", chunks_retrieved=len(chunks), chunks_collapsed=collapsed, - notes=[str(exc)], + notes=[str(parse_error)], ) yield progress.warning("The generated diagram could not be read") yield progress.done("No diagram could be assembled", _dump(outcome)) diff --git a/tests/api/test_grading.py b/tests/api/test_grading.py index 2b8ce9e..a8d48d4 100644 --- a/tests/api/test_grading.py +++ b/tests/api/test_grading.py @@ -158,6 +158,41 @@ def generate(self, messages: list[Message]) -> str: assert result["correct"] is False +def test_broken_json_is_corrected_once_before_an_answer_counts_as_wrong( + client: tuple[TestClient, StubLLMClient], +): + """An unreadable reply used to record a right answer as wrong.""" + http_client, _ = client + calls: list[list[Message]] = [] + + class FlakyLLM(StubLLMClient): + def generate(self, messages: list[Message]) -> str: + calls.append(messages) + if len(calls) == 1: + return '{"results": [{"id": "q1", "correct": true' + return '{"results": [{"id": "q1", "correct": true, "feedback": "Yes."}]}' + + app.dependency_overrides[get_llm] = lambda: FlakyLLM() + + response = http_client.post( + "/api/v1/grade-answers", + json={ + "answers": [ + { + "id": "q1", + "question": "Q1", + "reference_answer": "a1", + "user_answer": "a1", + } + ] + }, + ) + + assert response.json()["results"][0]["correct"] is True + assert len(calls) == 2 + assert "could not be validated" in str(calls[1][-1]) + + def test_llm_failure_returns_503(client: tuple[TestClient, StubLLMClient]): http_client, _ = client diff --git a/tests/onboarding/test_diagram.py b/tests/onboarding/test_diagram.py index 04c208c..ffb0c76 100644 --- a/tests/onboarding/test_diagram.py +++ b/tests/onboarding/test_diagram.py @@ -121,6 +121,35 @@ def test_assembles_a_diagram_with_the_sources_it_drew_on() -> None: assert outcome.provenance.corpus_fingerprint +def test_broken_json_is_corrected_once_rather_than_losing_the_diagram() -> None: + store = _store(*_TWO_CHUNKS) + llm = _llm( + _payload( + [ + _node("controller", "ReportController", ["c1"]), + _node("repo", "ReportRepository", ["c2"]), + ], + [_edge("controller", "repo")], + ) + ) + good = llm.generate + seen: list[list[object]] = [] + + def flaky(messages: list[object], **kwargs: object) -> str: + seen.append(messages) + if len(seen) == 1: + return '{"summary": "cut off' + return good(messages, **kwargs) # type: ignore[arg-type] + + llm.generate = flaky # type: ignore[method-assign] + + outcome = assemble_diagram(llm, store, subject=_SUBJECT, project_ids=_PIDS) + + assert outcome.status == "assembled" + assert len(seen) == 2 + assert "could not be validated" in str(seen[1][-1]) + + def test_a_node_that_cites_nothing_is_dropped_and_takes_its_edges_with_it() -> None: """The grounding rule, and the reason it has to reach the edges too. From bf99a17e7d648de627059f45ecdeaff926a1d53a Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Thu, 24 Sep 2026 20:02:39 +0200 Subject: [PATCH 15/17] Tidy the JSON retries and point the mentor at a hire's last wrong answer Grading no longer builds a correction prompt it will never send, the diagram retry uses a _correction_prompt helper like orientation and phase assembly, and the question clause tells the mentor to start from the answer the path read now shows. Co-Authored-By: Claude Opus 5.5 --- src/api/routes/grading.py | 2 ++ src/onboarding/buddy_persona.py | 11 ++++++----- src/onboarding/diagram.py | 31 +++++++++++++++++++------------ 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/api/routes/grading.py b/src/api/routes/grading.py index 3c2e7b9..cff64df 100644 --- a/src/api/routes/grading.py +++ b/src/api/routes/grading.py @@ -90,6 +90,8 @@ def _grade(llm: LLMClient, to_grade: list[GradeAnswerItem]) -> dict[str, _Graded attempt + 1, exc, ) + if attempt + 1 == _MAX_GRADING_ATTEMPTS: + break messages = [ *messages, Message(role="assistant", content=raw), diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index 8bf1fb1..245b612 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -202,11 +202,12 @@ "not give. Narrowing the options down is hinting too: never point at an option " "that matches a title or wording elsewhere, never rule options out, and never " "say how close a wrong answer was -- you do not know. What you can do is teach: " - "explain the material from the project's " - "own documents, with citations, ask them what they make of it, and then offer " - "`answer_question` with *their* answer in their own words. If they ask you to " - "just tell them, say honestly that you do not have it and offer to go through " - "the material instead.\n" + "explain the material from the project's own documents, with citations, ask " + "them what they make of it, and then offer `answer_question` with *their* " + "answer in their own words. Where the path shows the answer they last got " + "wrong, start from what it shows they think, not from the top. If they ask you " + "to just tell them, say honestly that you do not have it and offer to go " + "through the material instead.\n" ) _PATH_COMPLETE_CLAUSE = ( diff --git a/src/onboarding/diagram.py b/src/onboarding/diagram.py index b2eed17..f3a14f6 100644 --- a/src/onboarding/diagram.py +++ b/src/onboarding/diagram.py @@ -234,6 +234,24 @@ def _build_prompt(subject: str, chunks: list[ScoredChunk]) -> list[Message]: ] +def _correction_prompt( + messages: list[Message], raw: str, error: AssemblyError +) -> list[Message]: + """Ask once for the same diagram with only its JSON corrected.""" + return [ + *messages, + Message(role="assistant", content=raw), + Message( + role="user", + content=( + f"That response could not be validated: {error}. Return the same " + "grounded diagram again as one valid JSON object matching the " + "schema exactly. Return JSON only." + ), + ), + ] + + def _parse_payload(raw: str) -> _GenPayload: try: return _GenPayload.model_validate_json(extract_json_object(raw)) @@ -491,18 +509,7 @@ def stream_diagram( ) if attempt + 1 < _MAX_GENERATION_ATTEMPTS: yield progress.stage("generating", "Correcting invalid generated JSON") - messages = [ - *messages, - Message(role="assistant", content=raw), - Message( - role="user", - content=( - f"That response could not be validated: {exc}. Return the " - "same grounded diagram again as one valid JSON object " - "matching the schema exactly. Return JSON only." - ), - ), - ] + messages = _correction_prompt(messages, raw, exc) if payload is None: assert parse_error is not None From 8f6adc3086575e4cbc656d1aeca931a1cc35b6e4 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Sat, 26 Sep 2026 14:11:05 +0200 Subject: [PATCH 16/17] Offer flag_to_pm whenever the hire asks for it "Offer flag_to_pm as the last resort" was the only thing the persona said about escalating, and the mentor applied it to the hire too: an explicit "flag this to my PM" was refused as not being a PM matter. A new clause, mounted with the tool, says their asking is the reason. The add-step clause gets the same exception for a step they ask for themselves. Co-Authored-By: Claude Opus 5.5 --- src/onboarding/buddy_persona.py | 17 +++++++++++++++-- tests/onboarding/test_buddy_persona.py | 12 ++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/onboarding/buddy_persona.py b/src/onboarding/buddy_persona.py index 245b612..70b7a4d 100644 --- a/src/onboarding/buddy_persona.py +++ b/src/onboarding/buddy_persona.py @@ -234,7 +234,8 @@ "mention: a step on their path outlives this conversation, which starts fresh " "every visit. A step you add goes on *their copy* -- their PM's blueprint is " "untouched, and they can change or delete it. Never add one just to have " - "added something.\n" + "added something -- but when they ask for a step themselves, offer it: it is " + "their copy.\n" "- A knowledge question they got wrong means the material behind it did not " "land. Teach it first. If what they missed is more than one explanation, offer " "`add_path_step` for one short refresher step in that question's phase: what " @@ -334,6 +335,18 @@ "happened.\n" ) +# "Last resort" is about the mentor's own judgement, never about the hire's. Without +# this the mentor held a hire's explicit "flag this to my PM" against the last-resort +# rule and refused it as "not something for the PM" -- deciding on their behalf what +# they may raise, which is the one thing an escalation path must never do. +_FLAG_ON_REQUEST_CLAUSE = ( + "- When they ask you to flag, raise or pass something to their PM -- a question, " + "a problem, feedback on their path -- offer `flag_to_pm` in that reply. Their " + "asking is the reason: never decide for them that it is not a PM matter, and " + "never make them try the docs first. Say what you know alongside if it helps, " + "but still offer it.\n" +) + _ACTION_TOOLS = ( "flag_to_pm", "open_orientation", @@ -503,7 +516,7 @@ def build_persona( # The escalation offer only makes sense when the hire can actually escalate. escalation = ( - "; offer `flag_to_pm` as the last resort.\n" + "; offer `flag_to_pm` as the last resort.\n" + _FLAG_ON_REQUEST_CLAUSE if "flag_to_pm" in available else ".\n" ) diff --git a/tests/onboarding/test_buddy_persona.py b/tests/onboarding/test_buddy_persona.py index cbf2c52..5903106 100644 --- a/tests/onboarding/test_buddy_persona.py +++ b/tests/onboarding/test_buddy_persona.py @@ -44,6 +44,18 @@ def test_without_an_escalation_tool_the_persona_offers_no_escalation() -> None: assert "rather than inventing an answer" in persona +def test_a_hire_who_asks_to_flag_something_is_offered_it() -> None: + """'Last resort' is the mentor's bar for flagging unasked, never the hire's. + + Read as the only rule, it had the mentor refuse an explicit "flag this to my PM" + as not being a PM matter. + """ + persona = build_persona(_ALL_TOOLS) + + assert "When they ask you to flag" in persona + assert "never decide for them that it is not a PM matter" in persona + + def test_hire_state_tools_are_listed_only_when_mounted() -> None: persona = build_persona(["search_docs", "get_my_metrics"]) From bd740fa54d3b96b281921b935ed32040b0f7653e Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Tue, 29 Sep 2026 13:07:48 +0200 Subject: [PATCH 17/17] Retry grading when a reply leaves an answer ungraded `_Payload.results` defaulted to an empty list, so a valid but incomplete reply (`{}`, or one missing an id) skipped the correction round and every missing grade was recorded as wrong. `results` is now required, and a reply must hold exactly one grade per requested id; a missing, duplicate or unexpected id raises ValueError and gets the same retry as broken JSON. Co-Authored-By: Claude Opus 5.5 --- src/api/routes/grading.py | 21 +++++++++++++++++--- tests/api/test_grading.py | 42 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/api/routes/grading.py b/src/api/routes/grading.py index cff64df..b934f15 100644 --- a/src/api/routes/grading.py +++ b/src/api/routes/grading.py @@ -49,7 +49,7 @@ class _GradedItem(BaseModel): class _Payload(BaseModel): - results: list[_GradedItem] = [] + results: list[_GradedItem] def _build_prompt(answers: list[GradeAnswerItem]) -> list[Message]: @@ -67,14 +67,19 @@ def _build_prompt(answers: list[GradeAnswerItem]) -> list[Message]: def _grade(llm: LLMClient, to_grade: list[GradeAnswerItem]) -> dict[str, _GradedItem]: - """Grade in one call, asking once more when the reply is not valid JSON. + """Grade in one call, asking once more when the reply is unusable. An unreadable reply marks every answer in it "could not be graded" -- which the caller records as wrong. A small local model breaks its JSON now and then, and a hire whose right answer came back wrong for that reason has been told something false about their own knowledge. One correction round is cheap against that. + + Valid JSON is not enough: a reply that leaves an answer out (``{}``, or one id + missing) would record that answer as wrong just the same, so it gets the same + correction round. It must hold exactly one grade per requested id. """ + expected_ids = {item.id for item in to_grade} messages = _build_prompt(to_grade) for attempt in range(_MAX_GRADING_ATTEMPTS): try: @@ -83,7 +88,17 @@ def _grade(llm: LLMClient, to_grade: list[GradeAnswerItem]) -> dict[str, _Graded raise HTTPException(status_code=503, detail=str(exc)) from exc try: payload = _Payload.model_validate_json(extract_json_object(raw)) - return {item.id: item for item in payload.results} + graded_by_id = {item.id: item for item in payload.results} + if ( + len(payload.results) != len(to_grade) + or set(graded_by_id) != expected_ids + ): + raise ValueError( + "grading response does not match the requested answer ids: " + f"expected {sorted(expected_ids)}, got " + f"{[item.id for item in payload.results]}" + ) + return graded_by_id except (ValidationError, json.JSONDecodeError, ValueError) as exc: logger.warning( "Could not parse grade-answers output (attempt %d): %s", diff --git a/tests/api/test_grading.py b/tests/api/test_grading.py index a8d48d4..5c01b9d 100644 --- a/tests/api/test_grading.py +++ b/tests/api/test_grading.py @@ -193,6 +193,48 @@ def generate(self, messages: list[Message]) -> str: assert "could not be validated" in str(calls[1][-1]) +_TWO_ANSWERS = { + "answers": [ + {"id": "q1", "question": "Q1", "reference_answer": "a1", "user_answer": "a1"}, + {"id": "q2", "question": "Q2", "reference_answer": "a2", "user_answer": "a2"}, + ] +} +_BOTH_RIGHT = ( + '{"results": [{"id": "q1", "correct": true}, {"id": "q2", "correct": true}]}' +) + + +@pytest.mark.parametrize( + "first_reply", + [ + "{}", + '{"results": [{"id": "q1", "correct": true}]}', + '{"results": [{"id": "q1", "correct": true}, {"id": "q1", "correct": true}]}', + '{"results": [{"id": "q1", "correct": true}, {"id": "q9", "correct": true}]}', + ], + ids=["empty", "missing-id", "duplicate-id", "unexpected-id"], +) +def test_a_reply_that_does_not_grade_every_answer_is_corrected_first( + client: tuple[TestClient, StubLLMClient], first_reply: str +): + """Valid JSON that leaves an answer ungraded used to record it as wrong.""" + http_client, _ = client + calls: list[list[Message]] = [] + + class IncompleteLLM(StubLLMClient): + def generate(self, messages: list[Message]) -> str: + calls.append(messages) + return first_reply if len(calls) == 1 else _BOTH_RIGHT + + app.dependency_overrides[get_llm] = lambda: IncompleteLLM() + + response = http_client.post("/api/v1/grade-answers", json=_TWO_ANSWERS) + + assert [r["correct"] for r in response.json()["results"]] == [True, True] + assert len(calls) == 2 + assert "could not be validated" in str(calls[1][-1]) + + def test_llm_failure_returns_503(client: tuple[TestClient, StubLLMClient]): http_client, _ = client