Golden dataset explorer: the dataset on a page, and what it never tests by vishalkalbi27 · Pull Request #250 · AgamiAI/agami-core

vishalkalbi27 · 2026-09-01T08:00:12Z

Summary

A run report says what broke. Nothing says what the dataset is — how many items exist, how many
can actually gate a run, which have rotted. That is the question asked before a run and after it,
and today it needs reading YAML.

The view nothing produces is what the set does not test. Forty questions that never touch the
revenue metric read as coverage they do not have, and the model change that breaks revenue passes
clean. AH-108 already reads statements into table claims, so that gap is computable rather than a
judgement — which is why the Coverage tab, not the item list, is the reason this page exists.

This is the sixth self-contained page in the same shape as the five before it, and the model
explorer is the one it follows most closely: walk the YAML in a script, render tabs and live search,
queue edits in the page, hand back one block a deterministic parser applies.

Spec: AH-112

Draft: two of the ten success criteria are not met. SC7 (a queued change round-trips) and SC10
(a removal shows its content first) need edit/remove verbs on golden_author.py, which exists
only on #248. Nothing invokes the renderer yet for the same reason — the skill that would is on
that branch. Both are named in the spec's Outcome / Handoff. Ready to review, not to merge.

Changes

render_golden_datasets.py + golden-datasets-template.html — render(*, title, profile, payload) -> str behind the same flags the sibling renderers take. Four tabs (Items · Coverage ·
Lint · Queued), live search, status chips.
Not stdlib-only, unlike the run report beside it. The spec asked for both "stdlib only" and
coverage computed from claims, and those cannot both hold: reading a key needs a SQL parser and
reading the model needs a schema loader. The stdlib reading would have reduced coverage to a
re-render of the author-declared tables_used, which is the blind spot the tab exists to catch.
The run report stays stdlib-only for the opposite reason: it decides nothing, because the run
already wrote every verdict down. The contract said "a stdlib-only renderer" and is amended.
The model is read by reusing render_model_explorer.build_manifest, not re-derived. Two walks
of the same YAML would be two answers to what the model holds, and they would drift until this
page and the explorer disagreed about which tables exist.
What can gate is counted apart from what exists. Forty items with three confirmed keys gates
on three; one number would read as forty items of coverage.
Lint carries the reader's findings verbatim — its text, its locator — because a page that
disagreed with the validator about the same file is worse than no page. Asserted field-for-field
against load_golden_datasets on one fixture.
parse_golden_feedback.py — stdlib-only, parse(text) -> (data, anomalies, needs_judgment),
the same grammar and output contract as parse_model_feedback.py.

The rule this slice is really about

A queued action may weaken a claim and may never grant one. sql_confirmed marks an item as one
that can gate a run, so forging it makes a statement nobody verified into the thing every future run
is measured against — the cheapest possible way to make a failing suite green.

The refusal lives at the parser, not in the absence of a button, so a hand-edited page cannot
route around it. Review found the first version was a denylist of three field names over an op
forwarded whole, and five variants walked past it: the field nested under value, under a key
nobody thought of, spelled SQL_CONFIRMED, spelled with a trailing space, or replaced by
confirmed_by. An op now reaches the write door as the four keys the page emits, so a field the
page never wrote cannot arrive at all. The refusal also discards ops read before it — a block
carrying one of those fields is evidence of hand-editing, and applying the well-behaved half is
worse than refusing the whole.

Withdrawing confirmation is accepted. The asymmetry is the point: weakening needs no evidence.

Decisions worth recording

The page renders the answer key in full and the reader deliberately does not. The validator
never puts a statement in a finding because findings travel; this page is licensed by where it
lands. --out is now refused unless it resolves under the gitignored half — the licence is
enforced rather than left to each caller to remember.
Coverage names the keys it could not read. read_claims returns no tables for a statement it
cannot parse — a UNION, ordinarily — and unioning that silently turned the key's own tables into
the gap, so the page claimed nothing held a table a confirmed key demonstrably reads. That is the
one direction this tab must not be wrong in.
Tables coverage is claimed; metrics coverage is name-matched. A table is one of the claims a
statement is read into. A metric is not, so it is matched against the statement text, kept under
its own key, and labelled as the weaker signal it is.
bounded is not offered as a match level. An item is bounded only together with the band it
is held to, and this page carries no bounds — offering it would queue a change refused every time.
No *.json is written beside the run artifacts, or AH-110's re-run selector reads it back as
a run record.

Tests

44 new (test_render_golden_datasets.py 37, test_golden_feedback.py 20 — 57 total across both).
Full suite 5296 passed, 0 failed. Fixtures are synthetic throughout.

Two seam tests pin the page against the parser: the emitted verb set must equal the parser's allowed
set, and the page must never build a confirmation-granting field. They are the only two files in the
feature that must agree, and neither mentions the other.

One existing test changed deliberately.
test_a_closing_script_tag_in_a_statement_cannot_end_the_block asserted the </ → <\/ escape.
The escape now covers every <, because an unbalanced <!-- from a SQL comment plus a <script in
any other item takes the HTML tokenizer to double-escaped state, where the template's own closing
tag stops closing and the page draws its chrome with no data and no error — indistinguishable from a
profile with no datasets. The new assertion subsumes the old one. The five sibling renderers share
the narrower escape and the same exposure; that is noted for a follow-up, not changed here.

Complexity tracking

Added	Why the simpler option fails
`_last_run_verdicts` re-states AH-110's artifact-selection rules	`_last_failures` returns only failure `item_key`s, and importing `run_golden_eval` to reach it would pull the Claude CLI generator and the SQL executor into a renderer that decides nothing.
It keys dataset → item → verdict rather than flat	An item id is unique within its file and nothing makes it unique across a profile, so a flat map shows one dataset's verdict beside another's question.
A fourth name in the guarded import (`sqlglot_dialect`)	`storage_type` is `PostgreSQL`, which sqlglot rejects; handing it over raw parses nothing and reports every table untouched. This is the repo's one mapper — a local dict would be a second.

…nted apart A run report says what broke; nothing said what the dataset is. This renders every golden dataset for a profile as one self-contained page, item by item, and counts the items that can gate a run separately from the items that exist — forty questions with three confirmed answer keys gate on three. The payload is substituted last, for the reason the run report's is: a question is free text and an answer key is somebody's SQL, so either may carry another placeholder's literal text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…keys read The Coverage tab, and the reason the page exists: a dataset that looks substantial while exercising a third of the model lets the change that breaks the rest pass. The tables come from read_claims — the one reader of a statement here — so a case that declares a table its statement never touches does not close the gap, and the dialect comes from the model rather than a default. Metrics sit under their own key with their own note: a metric is not one of the seven claims, so it is matched by name against the answer key text, and the tab must not present the two as the same evidence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…t inline The Lint tab carries what load_golden_datasets reports verbatim — same text, same locator — because a page that disagreed with the validator about the same file would send a reader to fix something the reader would still refuse. It carries no statement, for the reason the reader carries none: a finding travels. Two derivations of this page's own ride beside them as warnings: a confirmed answer key nobody signed for, and a case with no receipt of what the answer looked like on the day. The verdict is on the item rather than one page away, read from the most recent whole run per dataset — keyed per dataset because an id is unique within its file and nothing makes it unique across a profile. No result file renders as a dataset nobody has run, which is the normal starting state and never an error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…engthen one Six queueable actions — tag, untag, set match, reword, remove, withdraw confirmation — folded over the payload rather than written into it, so the page always says what is on disk and what is only asked for, and Undo is a splice rather than a repair. The block is the model explorer's format unchanged: profile first, the ops under a bare golden-ops: header, done last. The two absences are the point. There is no control that grants confirmation: an unconfirmed case says how confirming actually happens instead, which is by running it and accepting the result. And no statement is editable — the page's one text area is the feedback block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…may not say A queued action may weaken a claim and may never strengthen one, so the refusal lives here rather than in the absence of a button: an op carrying sql_confirmed, expected or sql is evidence the page was hand-edited, and the whole block is refused rather than partly applied. Ticking a box would forge ground truth, and it is the cheapest possible way to make a failing suite green. The block names the profile it targets on its first line and this reports it as None when it is absent, never falling back to the active one, so a page rendered for one profile cannot be applied to another by accident. Two tests pin the seam between the page and this parser, which are the only two files in the feature that must agree and neither of which mentions the other. Spec: AH-112 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… could not read a key Review found the confirmation guard was a denylist of three field names over an op forwarded whole, and five variants walked past it: the field nested under `value`, tucked under a key nobody thought of, spelled SQL_CONFIRMED, spelled with a trailing space, or replaced by `confirmed_by`. An op now reaches the write door as the four keys the page emits, so a field the page never wrote cannot arrive by hand-editing the block. The denylist stays as redundancy rather than as the only line. The refusal also discards ops read before it: a block carrying one of those fields is evidence the page was edited, and applying the half that looked well-behaved is worse than refusing the whole. A non-object entry and a non-string op name are anomalies now rather than a traceback out of the one place a refusal is signalled, and a repeated key is reported so a second block cannot silently suppress the first. Coverage was unioning `read_claims(...).tables` and dropping `.unreadable`, so a confirmed key the reader could not parse — a UNION, ordinarily — contributed nothing and its tables were then reported as the gap. The page said nothing holds a table a confirmed key demonstrably reads, which is the one direction this tab must not be wrong in. The reason the reader already hands back is carried onto the page instead. The payload escape now covers every `<` rather than `</`: an unbalanced `<!--` from a SQL comment plus a `<script` in any other item takes the HTML tokenizer to double-escaped state, where the template's own closing tag stops closing, and the page draws its chrome with no data and no error. `test_a_closing_script_tag_in_a_statement` asserted the old narrower escape and is updated deliberately; the escape it now asserts subsumes it. Also: `--out` must land under the gitignored half, because this is the only rendered surface carrying answer keys in full and its licence is where it lands; `bounded` is no longer offered as a match level, since an item is bounded only with the band it is held to and this page carries none; and the fixture spells one model table CUSTOMERS while every key writes `customers`, so the case fold is load-bearing rather than inert. Spec: AH-112 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds a new “golden dataset explorer” surface: a standalone HTML page renderer plus a deterministic feedback-block parser, with tests that pin payload shape, coverage computation, lint mirroring, last-run verdict display, and the allowed queued-op vocabulary.

Changes:

Introduces render_golden_datasets.py to build a profile-wide payload (datasets/items/coverage/lint/last-run verdicts) and render it into a self-contained HTML page.
Adds golden-datasets-template.html implementing the UI (tabs, search, status filters, queueing + feedback block generation) with guardrails against in-browser confirmation/SQL editing.
Adds parse_golden_feedback.py (stdlib-only) to parse the feedback block into a constrained ops list, enforcing “may weaken, may not grant confirmation,” with comprehensive tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/test_render_golden_datasets.py	New end-to-end tests for payload content, escaping, coverage/lint/verdict behavior, and queue UI vocabulary.
tests/test_golden_feedback.py	Tests the feedback-block grammar, anomaly reporting, and confirmation-grant refusal semantics.
plugins/agami/shared/golden-datasets-template.html	New self-contained UI for exploring datasets + coverage/lint + queued ops with feedback block generation.
plugins/agami/scripts/render_golden_datasets.py	New renderer building payload from golden datasets + model manifest + last-run artifacts, then substituting into template.
plugins/agami/scripts/parse_golden_feedback.py	New deterministic parser for the page’s back-channel block, projecting ops onto an allowlist of fields.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+The explorer page renders one dataset and queues curation actions against it.
+This script does the parse so the skill reads structured output and applies it
+through the golden write path (append-only, validator in front).


… does The tile read "Model tables no item reads" over a number computed from confirmed answer keys alone, so it claimed a stronger fact than the one beneath it: an unconfirmed case reads tables too, and the tab three hundred lines down already says "no confirmed answer key reads". The page disagreed with itself about its own number, in the direction that overstates coverage — which is the direction the comment two lines above the tile exists to prevent. The parser's docstring described a page that renders one dataset. It renders every dataset in the profile, which is the first thing its spec asks for and the reason each op carries its own `dataset`. A contract file describing the wrong shape is worse than one describing none, because it is the file somebody reads to write the applier. The template's payload comment gains `unreadable`, added to coverage earlier and never written down there. Spec: AH-112 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…val is shown first The explorer rendered a page nothing invoked and a parser nothing consumed. This is the consumer: `apply` takes the ops array the parser emits, one dataset per call, and funnels it through the same `_write_items` the import and save doors use — so the append-only preview, the re-read gate and the rollback are inherited rather than copied. A deletion is the most destructive thing any door can ask for, and giving it a write path of its own would be giving it a second copy of those three to drift from. A removal previews as `needs_confirmation_removals`: a `before` and no `after`, because the whole of what somebody agrees to is what disappears. The skill is told to render the questions and answer keys rather than the ids — an id is a slug, and nobody can decide from one. Nothing grants confirmation at any of the three layers. The page has no control, the parser refuses a block that asks, and this door knows no verb that could: `withdraw-confirmation` clears the flag and the signature and keeps the statement, which is the thing somebody actually wrote down. The claim being withdrawn is that anyone verified it. Two ops on one item fold onto the running edit rather than the file's copy, or the second would overwrite the first with a version that never saw it. An item queued for both an edit and a removal is refused, because neither order is obviously right. The success payload gains `removed` beside `added` and `replaced`, and `summary` gains its count. Six assertions that pinned the old two-key summary are widened rather than worked around — the payload grew a field, which is the additive change the run artifact beside it already documents as the only allowed kind. Closes AH-112's last two success criteria: a queued tag change round-trips from the page through the parser to the file, and a queued removal shows its content first. Spec: AH-112 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`apply` looked for `ops` at the top level. The parser prints the standard contract — `{ok, data:{profile, ops}, anomalies, needs_judgment}` — so the skill's own documented flow, which redirects the parser's stdout straight into `--ops`, failed with exit 2 and a missing-key line. Found by walking the steps rather than by the suite. The round-trip test did not catch it because it re-wrapped the ops by hand before handing them over, which tests a shape nothing produces. It now passes the parser's own document through unchanged, which is the seam it was written to pin. The door also reads `needs_judgment` now. The parser refuses at the block level rather than per op, so a caller piping a refusal through would otherwise have applied the well-behaved neighbours of the op that was refused — the one thing the block-level refusal exists to prevent. Spec: AH-112 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ready does The page's entire hand-off is "copy this and paste it back", so the block is the trigger — there is no sentence the user is expected to type around it, and expecting one would make the last step of the flow the one thing nobody was told about. The skill listed prose triggers only, so a bare paste had nowhere to route. The model explorer's skill has named its own back-channel format in `when_to_use` since it shipped, for this reason. This says the same thing about this one. Spec: AH-112 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

vishalkalbi27 and others added 6 commits September 1, 2026 11:14

ashwin-agami requested a lite review from Copilot September 1, 2026 09:03

Copilot started reviewing on behalf of ashwin-agami September 1, 2026 09:03 View session

Copilot AI reviewed Sep 1, 2026

View reviewed changes

vishalkalbi27 and others added 3 commits September 1, 2026 16:53

Merge branch 'main' into AH-112-golden-dataset-explorer

18609fc

vishalkalbi27 force-pushed the AH-112-golden-dataset-explorer branch from 21172d5 to 49a8bd2 Compare September 1, 2026 13:07

vishalkalbi27 marked this pull request as ready for review September 1, 2026 13:19

vishalkalbi27 requested review from ashwin-agami and sandeep-agami as code owners September 1, 2026 13:19

vishalkalbi27 and others added 2 commits September 1, 2026 19:02

vishalkalbi27 merged commit fc02cfd into main Sep 1, 2026
8 checks passed

vishalkalbi27 deleted the AH-112-golden-dataset-explorer branch September 1, 2026 13:50

github-actions Bot locked and limited conversation to collaborators Sep 1, 2026

Navigation Menu

Uh oh!

Golden dataset explorer: the dataset on a page, and what it never tests - #250

Golden dataset explorer: the dataset on a page, and what it never tests#250
vishalkalbi27 merged 11 commits into
mainfrom
AH-112-golden-dataset-explorer

vishalkalbi27 commented Sep 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

vishalkalbi27 commented Sep 1, 2026

Summary

Changes

The rule this slice is really about

Decisions worth recording

Tests

Complexity tracking

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants