Skip to content

fix(tutor): stop ordering a tool that was deleted months ago - #605

Merged
mrviduus merged 3 commits into
mainfrom
fix/tutor-dead-tool
Sep 11, 2026
Merged

fix(tutor): stop ordering a tool that was deleted months ago#605
mrviduus merged 3 commits into
mainfrom
fix/tutor-dead-tool

Conversation

@mrviduus

Copy link
Copy Markdown
Owner

The Tutor's system prompt tells the model:

On a word the learner missed (or a hard context-stage card), call get_example_sentence to ground it in a real sentence from their reading.

That tool was deleted with the retrieval spine. ToolRegistry.SchemasFor skips an unknown name silently — deliberately, so a half-configured agent still runs — so the model was never offered it, nothing failed, and nothing logged. Every planning run since has paid for an instruction that could only be ignored, or attempted and refused. An instruction that cannot be obeyed is worse than a missing feature.

Removed from AllowedTools and from the prompt, plus the eval's FixedJsonTool stub for it and two doc comments still describing it as live.

Grounding is not lost, and needs no tool to come back

VocabularyWord.Sentence already holds the sentence each word was saved from, and get_due_vocabulary already reports hasSentence per card. If the planner should see the sentence itself, it belongs in that same projection — one round-trip fewer than the dedicated tool cost. Written down on the field rather than left for the next reader to rediscover.

The guard matters more than the fix

AgentAllowedToolsExistTests reflects over every agent's AllowedTools and asserts each name is implemented by a real ITool. It reads names off the concrete types rather than booting the API's composition root, so it needs no database or LLM client.

Two things make it a real test rather than a green tick:

  • Mutation-verified. Putting get_example_sentence back turns it red with a message naming the tool and explaining that SchemasFor drops it silently.
  • It asserts the reflection found something. A reflection-driven theory that matches nothing passes vacuously — renaming AllTools would do exactly that — so there is an explicit non-empty check.

It also covers agents added later, since it discovers them rather than listing them.

Still open, and it is a product call

exerciseType (recognition / recall / context) is calibrated server-side from the card's SRS stage, carried to the client with everything it would need — sentence, distractors, hint — and then thrown away: buildPlanCard hardcodes reviewMode: 'context', blankSentence: null, options: null, so every card renders as the same flashcard and the type is only a badge. Making it differentiate would cut against a deliberate decision recorded in CLAUDE.md (one card shape on the wire; review style is a client preference), so it is not being changed here.

  • dotnet test tests/TextStack.UnitTests — 1238 pass
  • dotnet build textstack.sln, dotnet format --verify-no-changes — clean

🤖 Generated with Claude Code

https://claude.ai/code/session_011rgEMvYYi4Egj99dVtvm6E

mrviduus and others added 3 commits September 11, 2026 00:27
The system prompt told the model to call get_example_sentence on a miss. That
tool went with the retrieval spine, and ToolRegistry.SchemasFor skips an unknown
name silently — by design, so a half-configured agent still runs — so the model
was never offered it, nothing failed, and nothing logged. Every planning run paid
for an instruction that could only be ignored.

Removed from AllowedTools and the prompt, with the eval's stub for it and two doc
comments that still described it as live.

Grounding needs no tool to come back: VocabularyWord.Sentence already holds the
sentence each word was saved from, and get_due_vocabulary already reports
hasSentence per card — the sentence belongs in that projection, one round-trip
fewer than a dedicated tool.

The guard is the point: AgentAllowedToolsExistTests reflects over every agent's
AllowedTools and asserts each name is implemented, reading names off the concrete
ITool types rather than booting the composition root. Mutation-verified — putting
the dead name back turns it red — and it asserts the reflection found agents at
all, since a theory that finds nothing passes vacuously.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011rgEMvYYi4Egj99dVtvm6E
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011rgEMvYYi4Egj99dVtvm6E
@mrviduus
mrviduus merged commit e608bc8 into main Sep 11, 2026
10 checks passed
@mrviduus
mrviduus deleted the fix/tutor-dead-tool branch September 11, 2026 04:56
mrviduus added a commit that referenced this pull request Sep 12, 2026
Found by an adversarial pass over work shipped 2026-09-11. All three are mine,
and all three are in the half nobody could look at: mobile has no component test
runner at all.

1. Mark-as-finished was a silent no-op on a device clock behind the server. The
   helper added to make web and mobile write the same locator also sent
   updatedAt from the device; the server reads that as a last-write-wins guard
   and answers 200 with the row untouched. The shelf had already flipped
   optimistically. Web's markAsRead never sent one. Removed, with a payload test —
   and the ordinary queued progress write keeps its timestamp, which is what the
   guard is actually for.

2. Apple sign-in never warned about a dropped guest merge — #609 wired three of
   the four entry points. An iOS reader was landed in a new account with no
   indication their work stayed behind. A literal scan now covers all four.

3. The Tutor kept ordering get_example_sentence from BuildGoal on every re-plan
   turn; #605 removed it from AllowedTools and SystemPrompt and missed that copy.

The guard from #605 could not have caught (3): it reflects over a string array
while the instruction is prose. It now reads the words. Two false starts are in
its docblock — matching against REGISTERED tools left the mutation green, because
a deleted tool is registered nowhere; and a naive scan failed on a correct file,
because these prompts wrap and a name lands across a concatenation. Both
mutations are red now.


Claude-Session: https://claude.ai/code/session_011rgEMvYYi4Egj99dVtvm6E

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant