Skip to content

Test guidance provenance read-back against a real sqlite3 (closes #3801) - #3804

Merged
malibio merged 1 commit into
mainfrom
issue-3801-eval-guidance-provenance
Oct 13, 2026
Merged

malibio merged 1 commit into
mainfrom
issue-3801-eval-guidance-provenance

Conversation

@malibio

@malibio malibio commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

The reader this issue describes (_seed.key / _seed.version through the CLI) was replaced by #3803: seeding stamps _seed.key, config_version and guidance_version on each seeded root, the runner reads them from the served database, fails an empty read-back, and aborts on drift between reps.

The remaining acceptance criterion was a test of the read-back against the shape the store holds. This adds one that runs the real sqlite3 against a table shaped like the daemon's, including an unstamped user skill and a non-guidance node that carries a stamp.

🤖 Generated with Claude Code

@malibio

malibio commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Code Review: PR #3804 (issue #3801)

Review Type: Initial review (no prior reviews or comments).

Summary

Test-only PR: one test in scripts/eval/preflight.test.ts runs the real sqlite3 against a node table shaped like the daemon's and checks readGuidanceProvenance. The issue body is stale: the _seed.key/_seed.version reader it describes was already replaced by #3803, and this PR correctly adds only the missing real-database coverage.

Independent verification of the stored shape

  • packages/core/src/markdown/mod.rs prepare_nodes_from_template (~L1451-1467) stamps properties._seed on the root node with key (template title), config_version, guidance_version, tier. The comment at ~L1444-1450 confirms _-prefixed keys are exempt from namespace hoisting, so _seed sits at a fixed top-level path regardless of node type.
  • packages/core/src/services/node_service/mod.rs reconciliation (~L2191-2240) reads the same _seed.config_version / _seed.guidance_version, so the writer and the reconciler agree with the eval reader.
  • packages/core/src/db/schema.rs: node(id, node_type, properties TEXT, ...). The test's reduced table is compatible with the query readGuidanceProvenance runs (json_extract(properties, '$._seed.key'), etc.).
  • Reader: scripts/eval/preflight.ts:527-581 reads _seed.key, config_version, guidance_version for agent-guidance and skill; throws on an empty type. Drift: checkGuidanceDrift (scripts/eval/runner.ts:815) fingerprints per-type, sorted key@config/guidance; runner calls it per rep (runner.ts:1594) and aborts, with unit tests in runner.test.ts:514.
  • bun test scripts/eval/preflight.test.ts: 38 pass, 0 fail.

Requirements Check

  • provenance.guidance lists every seeded guidance and skill node with a version: met by main (Eval preflight: read guidance provenance from the database, refuse to run beside another model (closes #3788) #3803). Every seeded node with _seed.key is listed with its config and guidance version; a type with none fails the run. This PR now proves it through real SQLite.
  • A rep whose guidance differs from rep 1 aborts: met by main (checkGuidanceDrift, runner.ts:1594, tested in runner.test.ts).
  • A test covers the read-back against the daemon's shape: met by this PR. The test covers the nested _seed shape, a user skill without a stamp (excluded), and a non-guidance type carrying a stamp (excluded).
  • Issue wording "delete the _seed reader": satisfied differently. _seed is the correct source and the stale reader no longer exists.

Findings

Recommendation

APPROVE (self-authored; posting as COMMENT). Net-positive, test-only, no production code touched. Merging is the user's call.

@malibio malibio left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review verdict: APPROVE (self-authored, so posted as a comment). Test-only; seeding stamps top-level properties._seed.{key,config_version,guidance_version} as verified in core; all three acceptance criteria are met by main (#3803) plus this PR. Full report in the comment above.

@malibio

malibio commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Address review

Review verdict: APPROVE. No code changes were needed.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: no commits since the review; the PR is a single test addition.

…red _seed shape (closes #3801)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@malibio
malibio force-pushed the issue-3801-eval-guidance-provenance branch from 5c7dd7c to 71437d1 Compare October 13, 2026 21:26
@malibio
malibio merged commit caf40eb into main Oct 13, 2026
@malibio
malibio deleted the issue-3801-eval-guidance-provenance branch October 13, 2026 21:26
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