TML-2721: deterministic, fail-closed Drive trace emitter#633
Merged
Conversation
… with parity guard
Move the Drive trace-event arktype schema to skills-contrib/drive-record-traces/schema.ts
as the single source of truth. The existing skills-contrib/drive-diagnose-run/schema.ts
becomes a vendored copy, kept byte-identical (below the one-line banner) by a new
schema-parity test wired into pnpm test:scripts.
- skills-contrib/drive-record-traces/schema.ts — new canonical file with CANONICAL banner
- skills-contrib/drive-diagnose-run/schema.ts — updated with VENDORED banner; verdict field
reformatted to single line to match canonical (no schema behavioural change); OQ transient
comment removed and replaced by self-documenting code
- skills-contrib/drive-diagnose-run/test/schema-parity.test.ts — new parity test; strips
first-line banner, asserts remaining text is byte-identical across both files
- skills-contrib/drive-record-traces/events.md — all verbatim arktype code blocks removed;
replaced with a short pointer to schema.ts; field tables and JSONL examples preserved
- package.json test:scripts — schema-parity.test.ts wired in
Reader (drive-diagnose-run/{load,cli,metrics,assertions}) continues importing ./schema.ts
unchanged.
Signed-off-by: Will Madden <madden@prisma.io>
Add skills-contrib/drive-record-traces/emit.ts: a Node CLI the orchestrator invokes once per trace event. The caller supplies only the event type and the event-specific payload; the emitter owns the envelope (event_id, schema_version, ts), validates the fully-merged event against the canonical Slice1TraceEvent schema before writing, and appends exactly one compact JSON line. Validation is fail-closed: any failure exits non-zero and writes nothing. This replaces the hand-written printf append path with a tool that makes every emitted line schema-conformant by construction. - emit.ts — exported emitEvent() core (build envelope + merge + validate + append) plus a thin arg-parsing main() behind the import.meta.url guard; rejects envelope keys appearing in the payload, naming the offending key - test/emit.test.ts — node --test coverage: success round-trip through the canonical schema, envelope ownership, parent-dir creation, successive appends, and each failure mode (unknown event type, wrong-typed field, envelope-key collision, no write on failure) - package.json — drive:emit script + emit.test.ts wired into test:scripts Signed-off-by: Will Madden <madden@prisma.io>
The use-pathe-for-paths rule globs **/*.ts repo-wide, so emit.ts and its test must import path helpers from pathe rather than node:path. fs imports are unaffected (the rule is path-helpers only). Signed-off-by: Will Madden <madden@prisma.io>
The deterministic emit.ts CLI now owns the envelope, validation, and append, so the protocol prose should describe invoking it rather than hand-building the envelope and hand-appending with printf. - emission.md: § Append protocol rewritten around the emit.ts invocation (real CLI shape, payload-only rule, fail-closed validation rationale); § Schema validation now describes emit-time validation as the first gate with read-time in drive-diagnose-run as the second; Canonical Emit snippet, Existence-check sketch, and Operator checklist all call the emitter; § First emit attributes mkdir -p to the emitter - SKILL.md: documents the shipped emit.ts emitter + prerequisites (Node 24+, arktype) per the drive-diagnose-run precedent; What this skill owns and the Emit blockquote example updated to the emitter invocation dispatch_id / round_id remain payload fields the orchestrator generates; only event_id / schema_version / ts moved to the emitter. No code, schema, reader, or test touched. Signed-off-by: Will Madden <madden@prisma.io>
schema.ts stopped being a hand-transcription of the vocabulary when the canonical schema moved to drive-record-traces/schema.ts and a schema-parity test began enforcing byte-identical sync. Correct the two stale passages: schema.ts is a vendored copy of the canonical drive-record-traces/schema.ts; canonical edits happen there and the parity test fails CI on divergence. Signed-off-by: Will Madden <madden@prisma.io>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR centralizes Drive trace event schema management in ChangesSchema Canonicalization and Deterministic Emission
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
size-limit report 📦
|
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/extension-cipherstash
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/emitter
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
…rom drive-record-traces
drive-diagnose-run no longer keeps a local copy of the trace-event schema.
All code, assertions, and tests now import directly from the canonical
skills-contrib/drive-record-traces/schema.ts. The schema-parity test
that guarded the vendored copy is deleted.
- load.ts, metrics.ts, cli.ts: ./schema.ts -> ../drive-record-traces/schema.ts
- assertions/{index,brief,cascade,invariants}.ts: ../schema.ts
-> ../../drive-record-traces/schema.ts
- test/{invariants,metrics,cascade-brief}.test.ts: same depth change
- drive-diagnose-run/schema.ts: deleted
- drive-diagnose-run/test/schema-parity.test.ts: deleted
- package.json test:scripts: schema-parity.test.ts entry removed
- drive-record-traces/schema.ts: banner updated (no vendoring language)
- drive-record-traces/events.md: § Machine-readable schema updated
- drive-diagnose-run/SKILL.md: removed vendored-copy description
Signed-off-by: Will Madden <madden@prisma.io>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills-contrib/drive-diagnose-run/assertions/brief.ts`:
- Line 1: The import in brief.ts currently uses a TypeScript file extension;
update the import to remove the .ts extension so it reads import type {
TraceEvent } from '../../drive-record-traces/schema' (referencing the TraceEvent
type and the import statement) to comply with the repo rule that TypeScript
imports must omit file extensions.
In `@skills-contrib/drive-diagnose-run/assertions/cascade.ts`:
- Line 1: The import for TraceEvent includes a .ts extension; remove the
extension so the statement imports TraceEvent from
'../../drive-record-traces/schema' (no .ts) to comply with the project guideline
"Never add file extensions to imports in TypeScript."
In `@skills-contrib/drive-diagnose-run/assertions/index.ts`:
- Line 1: Update the import to remove the TypeScript file extension: change the
import that brings in TraceEvent from '../../drive-record-traces/schema.ts' to
use '../../drive-record-traces/schema' (locate the import statement referencing
TraceEvent in assertions/index.ts).
In `@skills-contrib/drive-diagnose-run/test/metrics.test.ts`:
- Line 4: Update the TypeScript import so it omits the .ts extension: change the
import statement that brings in TraceEvent (import type { TraceEvent } from
'../../drive-record-traces/schema.ts') to use '../../drive-record-traces/schema'
instead to comply with the repo's TS import conventions and linter rules.
In `@skills-contrib/drive-record-traces/emit.ts`:
- Line 6: The import in emit.ts includes a .ts extension which violates project
guidelines; update the import statement that references Slice1TraceEvent (import
{ Slice1TraceEvent } from './schema.ts') to remove the file extension so it
reads import { Slice1TraceEvent } from './schema' and ensure any related imports
in the same file follow the same rule.
- Line 81: Replace the bare TypeScript `as` cast on the returned value with the
utility cast function: import castAs from '`@prisma-next/utils/casts`' at the top
of the file and change the return of parsed (the expression `return parsed as
Record<string, unknown>;`) to use castAs<Record<string, unknown>>(parsed) so the
runtime-safe cast utility is used instead of a bare `as`.
In `@skills-contrib/drive-record-traces/test/emit.test.ts`:
- Around line 7-8: The imports in the test use explicit .ts extensions; update
the import statements that reference emitEvent and Slice1TraceEvent (currently
importing from '../emit.ts' and '../schema.ts') to remove the .ts file
extensions (use '../emit' and '../schema') so they comply with the TypeScript
import guideline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 7fecaaea-2a02-496c-8881-b1fa3d8b815a
📒 Files selected for processing (18)
package.jsonskills-contrib/drive-diagnose-run/SKILL.mdskills-contrib/drive-diagnose-run/assertions/brief.tsskills-contrib/drive-diagnose-run/assertions/cascade.tsskills-contrib/drive-diagnose-run/assertions/index.tsskills-contrib/drive-diagnose-run/assertions/invariants.tsskills-contrib/drive-diagnose-run/cli.tsskills-contrib/drive-diagnose-run/load.tsskills-contrib/drive-diagnose-run/metrics.tsskills-contrib/drive-diagnose-run/test/cascade-brief.test.tsskills-contrib/drive-diagnose-run/test/invariants.test.tsskills-contrib/drive-diagnose-run/test/metrics.test.tsskills-contrib/drive-record-traces/SKILL.mdskills-contrib/drive-record-traces/emission.mdskills-contrib/drive-record-traces/emit.tsskills-contrib/drive-record-traces/events.mdskills-contrib/drive-record-traces/schema.tsskills-contrib/drive-record-traces/test/emit.test.ts
Factor the object-shape check into an isRecord type predicate so TypeScript narrows parsed to Record<string,unknown> without a cast. Keeps identical behaviour and error messages. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
pushed a commit
that referenced
this pull request
May 30, 2026
Scaffold the "Drive — Judge + live-experiment harness" project workspace: two-tier correctness-first scorecard, an LLM judge calibrated against an accreting instrumented-run corpus, and an SDK-spawned k=N A/B harness. Four slices (TML-2720 scorecard+vocabulary, TML-2735 golden-case harness, TML-2736 judge, TML-2737 experiment engine); two foundation slices run in parallel, judge + engine stack on top. Trace.jsonl carries the first natively-instrumented project-started/spec-authored/plan-authored events, emitted via the deterministic emitter merged in PR #633. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
This was referenced May 30, 2026
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
2 participants
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.
Drive trace events were hand-emitted: each instrumented skill built the JSON envelope itself and appended the line with
printf '%s\n' >> trace.jsonl. That left every field — UUIDs,schema_version, timestamp, and the whole payload — to the agent's discretion, with no validation before the line hit the file. Malformed and drift-prone trace lines were the predictable result. This PR replaces that path with a deterministic emitter that owns the envelope and validates fail-closed, so every emitted line is schema-conformant by construction.It also resolves a schema-ownership wrinkle that blocked the emitter: the canonical arktype schema lived in the
drive-diagnose-runreader, whiledrive-record-traces(the library skill every emit-site cites) carried a second, hand-maintained markdown copy of the same definitions. The emitter needs to import a real schema to validate against, so the canonical schema moves todrive-record-tracesand the reader keeps a vendored copy guarded by a parity test.Changes
skills-contrib/drive-record-traces/schema.ts): the arktype trace-event schema now lives in the library skill that owns the vocabulary.drive-diagnose-run/schema.tsbecomes a vendored copy; a newschema-paritytest (wired intotest:scripts) fails CI if the two diverge below their one-line banner. The verbatim arktype block inevents.mdis removed in favour of a pointer toschema.ts, ending the dual-maintenance of the same definitions.skills-contrib/drive-record-traces/emit.ts): a small Node CLI invoked once per event. The caller passes--event+ a payload-only--payloadJSON (plus--trace-file/--project-run-id); the emitter generates the envelope (event_id,schema_version,ts), rejects any envelope key smuggled into the payload, validates the merged event against the canonical schema, and only then appends one compact line. On a validation failure it exits non-zero and writes nothing. An exported core (emitEvent) keeps the logic unit-testable without spawning a subprocess;emit.test.tscovers the success round-trip and every failure mode. Adrive:emitscript is added.emission.md, both skills'SKILL.md): the canonical emit mechanic is now the emitter, notprintf. The Append protocol, Canonical Emit snippet, existence-check sketch, and operator checklist all describe invokingemit.ts; emit-time validation is documented as the first gate (read-time validation indrive-diagnose-runis the second). The docs deliberately preserve thatdispatch_id/round_idremain orchestrator-generated payload fields — onlyevent_id/schema_version/tsmoved into the tool.Why
The agent's only legitimate creative input to a trace event is the genuine per-event data (which dispatch, which verdict, how many findings). Everything else — envelope construction, append mechanics, schema conformance — is mechanical and was the source of unreliability. Pushing it into a tool that fails closed turns "the agent usually emits valid JSON" into "the file only ever contains valid events." That reliability is a precondition for trusting the diagnostics computed from these traces.
The schema moves to
drive-record-tracesrather than staying in the reader because the library skill is the canonical owner of the vocabulary, and an emitter that validates needs to import the schema from its own skill — keeping each skill independently installable (the reader vendors a copy under a parity guard rather than importing across the cluster boundary).Trace-file/
project_run_idresolution stays caller-side (the once-per-session lookup inemission.md) rather than moving into the emitter: a--modeflag would re-introduce the direct-change "reuse one timestamp across calls" coordination problem for no real gain.Scope
In: the
drive-record-tracesanddrive-diagnose-runskill files listed above, andpackage.jsonwiring. Out: the event vocabulary and field semantics are unchanged (pure relocation); no reader metric/report behaviour changes; emit-side payload-construction inside each instrumented skill is untouched (the agent still computes payload fields — only the envelope and append moved into the tool).Summary by CodeRabbit
New Features
Documentation
Tests