Run slot extraction on LLM router decisions #17

Closed
claude wants to merge 2 commits from overnight/slot-extract into overnight/embedder-e5
Contributor

The second and last blocker for turning the LLM router on by default. Builds on PR 10 and PR 15.

The bug: when the LLM router decided a turn, slot extraction never ran. Only the classifier path called it. So an LLM act came back with no function to call, and an LLM reminder came back with no parsed time. Turning the router on today would have quietly broken reminders and actions — the intent would be right and nothing would happen.

The fix: one new fillSlots, called only when the LLM router succeeds. It runs the same extractor the classifier path runs and fills only the slots the model left empty — time, key/value, function/args, and text. The model's own slots always win; extraction is a gap-filler, never an overwrite.

One extra step for acts: if the utterance doesn't match the allowlist, the matcher gets retried against the model's verb instead of the raw utterance, which is usually the cleaner string to match on.

A reminder with no usable time still creates nothing. No time is invented. It falls through to the existing reply, не получилось разобрать время напоминания. — same behaviour as before, just now reachable from the LLM path too.

Five tests, no model needed, using the existing mockLLM: reminder gets a time; reminder without a time stays empty; act gets its function; model slots beat extracted ones; a keyless fact gets a key from the parser.

Three things found and deliberately not fixed here:

  1. cmd/mavend/voice.go:588 has its own reminder time-parse fallback gated on Stage == 0. It's now redundant for the LLM path but still the only cover for stage-0 grammars. Two places parse reminder time; that should be one place.
  2. Compound utterances are still dropped — LLMRouter.Route honours only the first act, so "напомни X, и запиши Y" loses the second half. Needs Route to return a slice.
  3. llm/check_prompt_parity.py in the training workspace will now fail: the Go prompt got the reorder and the unknown wording, that copy did not (Vikunja #362).
The second and last blocker for turning the LLM router on by default. Builds on PR 10 and PR 15. **The bug:** when the LLM router decided a turn, slot extraction never ran. Only the classifier path called it. So an LLM `act` came back with no function to call, and an LLM `reminder` came back with no parsed time. Turning the router on today would have quietly broken reminders and actions — the intent would be right and nothing would happen. **The fix:** one new `fillSlots`, called only when the LLM router succeeds. It runs the same extractor the classifier path runs and fills **only the slots the model left empty** — time, key/value, function/args, and text. The model's own slots always win; extraction is a gap-filler, never an overwrite. One extra step for acts: if the utterance doesn't match the allowlist, the matcher gets retried against the model's verb instead of the raw utterance, which is usually the cleaner string to match on. **A reminder with no usable time still creates nothing.** No time is invented. It falls through to the existing reply, `не получилось разобрать время напоминания.` — same behaviour as before, just now reachable from the LLM path too. Five tests, no model needed, using the existing `mockLLM`: reminder gets a time; reminder without a time stays empty; act gets its function; model slots beat extracted ones; a keyless fact gets a key from the parser. **Three things found and deliberately not fixed here:** 1. `cmd/mavend/voice.go:588` has its own reminder time-parse fallback gated on `Stage == 0`. It's now redundant for the LLM path but still the only cover for stage-0 grammars. Two places parse reminder time; that should be one place. 2. Compound utterances are still dropped — `LLMRouter.Route` honours only the first act, so "напомни X, и запиши Y" loses the second half. Needs `Route` to return a slice. 3. `llm/check_prompt_parity.py` in the training workspace will now fail: the Go prompt got the reorder and the `unknown` wording, that copy did not (Vikunja #362).
claude changed target branch from master to overnight/embedder-e5 2026-07-31 10:39:23 +02:00
claude added 2 commits 2026-07-31 10:39:23 +02:00
An LLM-routed reminder came back with no parsed time and an act with no
fn, because only the classifier path ran the extractor. Now the router
runs the same extraction after an LLM decision and fills only the empty
slots. No time in the utterance still means no time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
Owner

is this one even worth reviewing?

is this one even worth reviewing?
Owner

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47.

Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47. Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.
kami closed this pull request 2026-07-31 20:21:57 +02:00

Pull request closed

Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#17