feat(chat): validation under an explicit model, per-model quality in chat, demo run quarantined - #839
Merged
Merged
Conversation
`chat_service` routes every KB citation through `page_locator`. `chat_tools.search_knowledge_base` — the path the release notes call the product — read `metadata["page"]` straight off the chunk, reintroducing both bugs `page_locator` exists to prevent: An interpolated OCR page was stated as exact. OCR returns no page structure, so boundaries are estimated by spreading the page count evenly across the text; on a 400-page package with a 40-page budget table that is off by an unbounded amount. `page_approximate` marks it, and this path dropped the flag. And a chunk was cited by the page it starts on, so a passage past a break inside the chunk was cited one page early. Now: `cited_pages` for the page of the segment that actually answers the question (or the range when that is ambiguous), `annotate_chunk_pages` so the model can see where the page changes, and `page`/`page_end`/ `page_approximate` on both the results and the citations. The frontend already renders through `formatPageLocator` and has since #603 — it was never being sent the flag, so no UI change was needed. The tool result also leads with the two labels the classic path carries, for the reason recorded in its comment: a tilde the model has not had explained gets normalised away and the estimate is restated as fact. Both are emitted only when they apply. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HG84HkR8QhgBsmBdFRfCFZ
Review of this PR found both shape changes it makes to the tool's return
value break a consumer I didn't check.
The `{"note": ...}` prepended to the array is rendered as a passage. The
UI counts every element for "Found N relevant passages", previews the
first three, and copies them — so five passages read as six, the note
took a preview slot as a blank "Source ·" row with no content, and "Copy
passages" led with an empty [Source] block. It fires whenever a source is
scanned, which is the common success path for this feature. The guidance
rides on each entry now; the array stays homogeneous.
And the annotated text leaked into document matching. `entry["content"]`
is both what the model reads and what the UI uses to derive its
click-to-highlight phrase, and `pickHighlightPhrase` takes the first ~60
characters — so a chunk whose page break falls inside that window put an
injected "[p. 3]" into the search phrase. That marker appears nowhere in
the document, so clicking the passage opened the viewer on a highlight
that could never match. `content_verbatim` carries the unannotated text
for anything matching against the document, and is set only when the two
differ.
Worth noting this branch gets no CI — the workflows only trigger on PRs
into main — so the suite run here is the only verification it will have:
4429 passed, tsc clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HG84HkR8QhgBsmBdFRfCFZ
Review follow-ups. `page_locator` imports only stdlib, so the function-level import was not dodging a cycle; moved to the top with the rest. The comment explaining `content_verbatim` sat inside the dict literal directly above `source_name`, describing a key assigned four lines later. Moved to the assignment it describes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HG84HkR8QhgBsmBdFRfCFZ
…chat, demo run quarantined - run_validation tool takes an explicit model (name or tag), resolved against the configured list before any LLM spend; the confirm gate and response name it, and the response reports both the requested model and the model the run was labeled with. - The chat session's model is NO LONGER passed to validation as an implicit request: once the model-truth fix from main lands in this branch, a passed model force-overrides the template's configuration (including an optimizer-applied model), and the session picker was poised to do exactly that on every chat-triggered validation. Verified red-on-revert. - get_quality_info returns model_comparison (avg score + run count per model over the item's history, unattributed runs shown as such), so "which model does best here?" is answerable in chat. - The onboarding demo's fabricated ValidationRun (score 92) is tagged source="demo_seed" and excluded from the fleet quality summary, timeline, and the chat model comparison - invented numbers stay in the walkthrough. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018f8shnU7x3BV6PiMZV928r
CHANGELOG entries filed under the 5.0.0 section, where branch-only work lives now that #856 emptied [Unreleased].
CHANGELOG entries filed under the 5.0.0 section, where branch-only work lives now that #856 emptied [Unreleased].
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 7 of the ranked model-eval list — targets
major/agentic-chat(the rest of the stack, #831–#838, targets main). Closes the loop that started this work: an admin can now literally ask Vandalizer how a model performs.Changes
run_validationtool takes an explicitmodel(name or tag). It's resolved against the configured model list before any LLM spend (an unknown name errors with the configured options instead of failing mid-run), the confirm gate and preview name it, and the response reports bothmodel_requestedand themodelthe persisted run was labeled with.context.deps.model_nameon every call. Today that's a harmless fallback — but once the model-truth fix (fix(validation): label runs with the model that ran, honor explicit model requests #831) lands in this branch, a passed model force-overrides the template's own configuration, including an optimizer-applied model, so every chat-triggered validation would have silently measured the session picker instead of the template. Now only an explicit user request forces anything. Verified red-on-revert.get_quality_inforeturnsmodel_comparison— average score and run count per model over the item's measured history, unattributed (pre-attribution) runs shown as such — so "which model does best on this template?" is answerable in chat from data that exists. Best-effort like the function's other enrichment lookups.source="demo_seed"and excluded from the summary, the timeline, and the chat comparison — the demo item still shows its numbers where the walkthrough needs them.Merge-order note
Independent of #831–#838 (merges cleanly either way). The
quality_servicedemo-seed filters will union trivially with the stack's changes at the next catch-the-base merge; the by-model endpoint from #833 should also gain thedemo_seedexclusion when it reaches this branch — flagged here so it isn't lost.Verification
New
tests/test_chat_validation_model.py(unknown model rejected pre-spend, tag→name resolution + forwarding, session-model-not-forced verified red-on-revert, comparison grouping + demo exclusion). Full backend suite on the branch: 4660 passed; the 4 failures are pre-existing on branch HEAD without this diff (2 network-dependent URL-validation tests, 1 workflow-task test, and one legacy chat test that now passes with the best-effort guard).🤖 Generated with Claude Code