Run results carry per-step document identity (closes #105) - #112
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings remain across result producers, public APIs, failure handling, and LSP publication.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR upgrades run results to v2, preserving referenced document identity across language ports and LSP diagnostics.
Changes:
- Adds
failure.docPathand referenced-document hashes. - Updates runners, adapters, serializers, and conformance fixtures.
- Routes shared-document failures to the correct editor locations.
File summaries
| File | Summary |
|---|---|
typescript/packages/vitest/tests/run-results-wire.test.ts |
Adds v2 referenced-failure fixture. |
typescript/packages/vitest/tests/reporter.test.ts |
Tests referenced document hashes. |
typescript/packages/vitest/src/runtime.ts |
Collects document metadata and host lines. |
typescript/packages/vitest/src/reporter.ts |
Writes referenced-document metadata. |
typescript/packages/runner/src/results.ts |
Builds v2 result payloads. |
typescript/packages/lsp/src/server.ts |
Publishes diagnostics per document. |
typescript/packages/lsp/src/run-results.ts |
Resolves results for referenced documents. |
typescript/packages/lsp/src/run-results.test.ts |
Tests cross-document diagnostics. |
typescript/packages/core/tests/run-diagnostics.test.ts |
Tests document-aware projections. |
typescript/packages/core/tests/failure-step-span.test.ts |
Updates result version. |
typescript/packages/core/src/run-diagnostics.ts |
Filters diagnostics by document. |
typescript/packages/core/src/result.ts |
Defines the v2 result schema. |
typescript/packages/core/src/failure.ts |
Records failure document paths. |
typescript/packages/core/src/failure-anchor.ts |
Carries document identity on errors. |
typescript/packages/core/src/execute.ts |
Injects referenced document locations. |
rust/runner/src/results.rs |
Persists referenced document hashes. |
rust/core/tests/run_results_wire_test.rs |
Adds the Rust v2 wire fixture. |
rust/core/tests/failure_test.rs |
Tests spliced failure identity. |
rust/core/src/result.rs |
Adds v2 result fields and serialization. |
rust/core/src/failure.rs |
Extracts document paths. |
rust/core/src/execute.rs |
Records referenced step locations. |
rust/cargotest/src/lib.rs |
Records referenced sources and host lines. |
ruby/packages/runner/lib/varar/runner/results.rb |
Persists v2 results. |
ruby/packages/rspec/lib/varar/rspec.rb |
Supplies referenced sources and host lines. |
ruby/packages/minitest/lib/varar/minitest.rb |
Supplies referenced sources and host lines. |
ruby/packages/core/spec/varar/core/run_results_wire_spec.rb |
Adds the Ruby v2 wire fixture. |
ruby/packages/core/lib/varar/core/result.rb |
Adds v2 result fields. |
ruby/packages/core/lib/varar/core/failure.rb |
Records document paths. |
ruby/packages/core/lib/varar/core/failure_anchor.rb |
Carries document identity. |
ruby/packages/core/lib/varar/core/execute.rb |
Injects referenced locations. |
python/packages/unittest/src/varar_unittest/__init__.py |
Records referenced sources and host lines. |
python/packages/runner/src/varar_runner/results.py |
Persists v2 results. |
python/packages/pytest/tests/test_results.py |
Updates the version assertion. |
python/packages/pytest/src/varar_pytest/plugin.py |
Records referenced sources and host lines. |
python/packages/core/tests/test_run_results_wire.py |
Adds the Python v2 wire fixture. |
python/packages/core/src/varar_core/result.py |
Adds v2 result fields. |
python/packages/core/src/varar_core/failure.py |
Records document paths. |
python/packages/core/src/varar_core/failure_anchor.py |
Carries document identity. |
python/packages/core/src/varar_core/execute.py |
Injects referenced locations. |
java/runner/src/main/java/dev/varar/runner/Results.java |
Persists referenced document hashes. |
java/kotest/src/main/kotlin/dev/varar/kotest/OathSpec.kt |
Supplies referenced sources and host lines. |
java/junit/src/main/java/dev/varar/junit/OathFileDescriptor.java |
Supplies referenced sources. |
java/junit/src/main/java/dev/varar/junit/ExampleDescriptor.java |
Filters host lines. |
java/core/src/test/java/dev/varar/core/RunResultsWireTest.java |
Adds the Java v2 wire fixture. |
java/core/src/main/java/dev/varar/core/Result.java |
Adds v2 result fields. |
java/core/src/main/java/dev/varar/core/FailureAnchor.java |
Carries document identity. |
java/core/src/main/java/dev/varar/core/Failure.java |
Records document paths. |
java/core/src/main/java/dev/varar/core/Execute.java |
Injects referenced locations. |
go/runner/results.go |
Persists referenced document hashes. |
go/runner/results_wire_test.go |
Adds the Go v2 wire fixture. |
go/gotest/gotest.go |
Records referenced sources and host lines. |
go/core/result.go |
Adds v2 result fields. |
go/core/failure.go |
Records document paths. |
go/core/failure_test.go |
Tests spliced failure identity. |
go/core/execute.go |
Names source documents in locations. |
dotnet/Varar.TestAdapter/VararAdapter.cs |
Records referenced sources and host lines. |
dotnet/Varar.Runner/Results.cs |
Persists referenced document hashes. |
dotnet/Varar.Core/ResultJson.cs |
Serializes v2 fields. |
dotnet/Varar.Core/Result.cs |
Adds v2 result fields. |
dotnet/Varar.Core/FailureAnchor.cs |
Carries document identity. |
dotnet/Varar.Core/Failure.cs |
Records document paths. |
dotnet/Varar.Core/Execute.cs |
Injects referenced locations. |
dotnet/Varar.Core.Tests/RunResultsWireTests.cs |
Adds the .NET v2 wire fixture. |
doc/adr/0016-reuse-is-a-link.md |
Marks run-result support complete. |
doc/adr/0014-run-results-are-a-cross-port-contract.md |
Documents the v2 schema. |
conformance/run-results/README.md |
Documents the new fixture case. |
conformance/run-results/expected.json |
Pins the v2 payload. |
conformance/adapter/smoke.sh |
Requires version 2 results. |
Review details
Suppressed comments (9)
doc/adr/0014-run-results-are-a-cross-port-contract.md:69
- The published reference page still declares
OathResults.versionas1and says consumers may rely onversion: 1(typescript/packages/website/src/content/docs/reference/run-results.mdx:43,223-236). Updating the ADR to v2 without updating that existing user-facing reference leaves the documented wire contract inconsistent with every writer in this change.
- `version` is `2`. Version 2 adds the per-step document identity reference
blocks need (ADR 0016): `failure.docPath` names the document a failure's
`line`, `cells` and `anchor` are offsets into — absent means the oath
itself — and a top-level `documents` array carries a hash per other oath
whose steps this run spliced in, so a consumer can tell a stale failure
from a live one exactly as `sourceHash` does for the oath. `lines` holds
only the running oath's own lines.
doc/adr/0016-reuse-is-a-link.md:647
- The published website reference at
typescript/packages/website/src/content/docs/reference/run-results.mdx:42-47still documentsversion: 1and omitsdocuments/failure.docPath, while this ADR now declares the v2 contract complete. Update the user-facing reference and examples alongside the wire-format change so consumers are not directed to the obsolete schema.
- ~~**Run-result v2 (ADR 0014).**~~ **Done** — `.varar/<oath>.json` is version 2:
`failure.docPath` names the document its offsets address, `documents` carries a
hash per referenced oath, and `lines` holds only the running oath's own lines.
The LSP publishes a spliced failure against the referenced document's URI (a
shared oath has no result file of its own, so its failures previously had no
dotnet/Varar.Core/FailureAnchor.cs:50
Attachexplicitly skips read-onlyException.Data, but the new path writer does not. For a framework exception with read-onlyDatain a referenced step, this assignment throws and replaces the original step failure instead of falling back to a line-only result. Mirror the same null/read-only guard here.
if (error is not null)
{
error.Data[DocPathKey] = docPath;
go/runner/results.go:93
Recordis exported, and this change removes its existing three-argument call shape; downstream Go adapters now fail to compile unless they know the new metadata argument. Preserve the old method and add a metadata-aware method, or make the new map optional, as the Java/C#/Ruby/Python collectors do.
func (r *Results) Record(oathPath, source string, result core.ExampleResult, referencedSources map[string]string) {
rust/runner/src/results.rs:73
Results::recordis public and re-exported by the runner crate, but this new required argument removes the existing three-argument call shape, so downstream Rust adapters no longer compile. Preserve the old method and add a separate metadata-aware method (or another defaulting API) for this feature.
typescript/packages/core/src/failure.ts:44- When a referenced step throws a non-
Errorobject,augmentStackstill attaches its anchor anddocPathbut returns without injecting afile:lineframe. This expression then falls back to the running oath'sfallbackLine, so the v2failure.lineis not a line indocPath(often it is 0 because spliced steps are excluded fromlines). Use the attached anchor'sstartLineas the fallback for a documented failure.
typescript/packages/lsp/src/run-results.ts:108 oathUris()only helpspublishAll(), but result-file watchers callpublishFor()only for the URI returned byingest/remove. When a referring oath's result changes from failed/referenced to clean (or is deleted), its former shared URI is never republished, so the old diagnostic remains in that shared document. Track and republish the old/new document URIs on result updates (or callpublishAll()).
typescript/packages/lsp/src/run-results.ts:42- The new
documentsfield is used as an iterable/map inresultsFor()andoathUris(), butisOathResults()accepts any version-2 object without checking thatdocumentsis an array of{ path, sourceHash }. A malformed.vararfile such asdocuments: {}is ingested and then makesresultsForthrow instead of being ignored. Validate the optional field before storing it.
typescript/packages/runner/src/results.ts:55 localeCompareis locale/runtime-dependent, while the other ports serializedocumentsusing ordinal ordering (sorted,BTreeMap,TreeMap, or<). With mixed-case or non-ASCII referenced paths, the array order can differ across ports, which breaks the deep-equality run-results corpus. Use an explicit locale-independent ordering shared by all ports.
- Files reviewed: 68/68 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // serialized OathResults. | ||
| export type OathResults = { | ||
| readonly version: 1 | ||
| readonly version: 2 |
There was a problem hiding this comment.
Fixed in b3f79ea: both browser producers (run-oath.ts and the error fallback in run-worker.ts) now stamp version: 2. There is no editor-mount.ts in the tree — those were the only two. The store's reader still accepts a v1 record on disk, on purpose, so a result written before the upgrade still renders.
| /** Records one example's outcome, for {@link #after} to persist. */ | ||
| void recordResult(Result.ExampleResult result) { | ||
| results.record(oathPath, content, result); | ||
| results.record(oathPath, content, result, referencedSources()); |
There was a problem hiding this comment.
Fixed in c3a4f45: the referenced sources are read once per file, in before(), under the same ordering guarantee that caches the example runs; recordResult reuses the map.
| const relevant = runResults?.resultsFor(uri) ?? [] | ||
| let run: LspDiagnostic[] = [] | ||
| const results = runResults?.get(uri) | ||
| if (results) { | ||
| if (relevant.length > 0) { | ||
| let source = documents.get(uri)?.getText() | ||
| if (source === undefined) { |
There was a problem hiding this comment.
Fixed in c84fefc: ingest() and remove() now return every URI a result change affects — the oath's own, the documents the new record names, and the documents the record it replaces named — and the watcher republishes each. A clean rerun that dropped a reference therefore clears the shared oath's stale squiggle, and a new failure inside a shared section reaches that file immediately. Pinned by two new store tests.
| const documents = pendingDocuments.get(path) | ||
| if (documents && Object.keys(documents).length > 0) fileMeta[VARAR_DOCUMENTS_META] = documents |
There was a problem hiding this comment.
Fixed in 70603a7: the entry is deleted on attach, one-shot per collection like the baseline beside it.
…it was written in A step spliced in by a reference block (ADR 0016) has spans in the oath it was WRITTEN in, but the run result named only the oath that ran it. The language server therefore placed the failure in the wrong file — against offsets that address some other sentence — or dropped it when the source hash didn't match. The run-result payload is version 2: - `failure.docPath` names the document `line`, `cells` and `anchor` are offsets into. Absent (the common case) means the oath itself. - `documents` carries a hash per referenced oath that contributed steps, so a consumer can tell a stale failure from a live one exactly as `sourceHash` does for the oath. - `lines` holds only the running oath's own lines: a spliced step's line is not in this file, and a line-wash renderer would decorate an unrelated sentence. The executor's stack frame names the same document, so an editor resolving a failure from the frame lands there too. The LSP publishes a spliced failure against the referenced document's URI — a shared oath has no result file of its own, so its failures previously had no way to reach the editor at all. conformance/run-results/expected.json grows a fourth example covering it, which is what the other six ports now have to satisfy. Ports-deferred: py, java, ruby, rust, dotnet, go — payload lands per port next Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSCLupVART3c5PjiffmahX
…it was written in Ports run-result payload v2: failure.docPath names the document line, cells and anchor are offsets into; documents carries a hash per referenced oath that contributed steps; lines holds only the running oath's own lines. The executor's note names the same document, and both adapters pass the referenced sources through to the collector. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSCLupVART3c5PjiffmahX
…it was written in Ports run-result payload v2: ExampleFailure.DocPath names the document Line, Cells and Anchor are offsets into; OathResults.Documents carries a hash per referenced oath that contributed steps; Lines holds only the running oath's own lines. attachLocation names the document a spliced step was written in, so ToFailure no longer discards the precise line and anchor when the path differs from the oath — a location for another document now MEANS a spliced step rather than a stray one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSCLupVART3c5PjiffmahX
…t it was written in Ports run-result payload v2: ExampleFailure#doc_path names the document line, cells and anchor are offsets into; OathResults#documents carries a hash per referenced oath that contributed steps; lines holds only the running oath's own lines. The executor attaches the document path to the error alongside the anchor, and both adapters pass the referenced sources through to the collector. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSCLupVART3c5PjiffmahX
…t it was written in Ports run-result payload v2: ExampleFailure.doc_path names the document line, cells and anchor are offsets into; OathResults.documents carries a hash per referenced oath that contributed steps; lines holds only the running oath's own lines. attach_location names the document a spliced step was written in, so to_failure no longer discards the precise line and anchor when the path differs from the oath — a location for another document now MEANS a spliced step rather than a stray one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSCLupVART3c5PjiffmahX
…ent it was written in Ports run-result payload v2: ExampleFailure.DocPath names the document Line, Cells and Anchor are offsets into; OathResults.Documents carries a hash per referenced oath that contributed steps; Lines holds only the running oath's own lines. The executor attaches the document path to the exception alongside the anchor, and the VSTest adapter passes the referenced sources through to the collector. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSCLupVART3c5PjiffmahX
…t it was written in Ports run-result payload v2 to Java and Kotlin, the last of the seven: ExampleFailure.docPath names the document line, cells and anchor are offsets into; OathResults.documents carries a hash per referenced oath that contributed steps; lines holds only the running oath's own lines. The executor's synthetic stack frame names the same document, so Failure.toFailure scrapes the line out of the right frame, and both adapters pass the referenced sources through to the collector. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSCLupVART3c5PjiffmahX
ADR 0014's payload gains docPath and documents for the per-step document identity reference blocks need; ADR 0016's open list loses the gap that kept a failure inside a shared section out of the editor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSCLupVART3c5PjiffmahX
…s for A .varar/<oath>.json speaks for its oath and for every document its steps were spliced in from (ADR 0016), but the file watcher republished only the oath's own URI. A new failure inside a shared section never reached that file until something else republished it, and a clean rerun left the old squiggle behind. The store now reports every URI a result change affects — the oath, the documents the new record names, and the documents the record it replaces named — and the server republishes each. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQVAKGfMEKPbT919FqAChP
…ection, not carried over The map handing a file's referenced-document hashes to the reporter was read but never cleared, unlike the baseline beside it. In watch mode an oath whose reference was removed collected again without setting a new entry, so the stale map was attached — and its hashes written — a second time. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQVAKGfMEKPbT919FqAChP
…once per example recordResult() rescanned the plan and reread every referenced oath for each example it recorded — O(examples × references) disk reads per file. The sources are now read once, in before(), under the same ordering guarantee that caches the example runs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQVAKGfMEKPbT919FqAChP
The run-result contract moved to version 2 (ADR 0014); the browser runner and its error fallback still stamped version 1, which no longer matched the public OathResults type. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQVAKGfMEKPbT919FqAChP
0119df7 to
b3f79ea
Compare
Closes #105. Stacked on #104 — this branch is based on
reuse-is-a-link, so review that one first; the diff here is just the eight commits on top.A step spliced in by a reference block (ADR 0016) has spans in the oath it was written in, but the run result named only the oath that ran it. The language server therefore placed the failure in the wrong file — against offsets that address some other sentence — or dropped it when the source hash didn't match. A mismatch inside a shared section never reached the editor at all, because a shared oath has no result file of its own.
The payload is version 2
{ "version": 2, "oathPath": "varar/reuse.md", "sourceHash": "fnv1a:…", "documents": [{ "path": "varar/shared/an-overdue-loan.md", "sourceHash": "fnv1a:…" }], "examples": [{ "name": "…", "status": "failed", "lines": [11], "failure": { "line": 10, "docPath": "varar/shared/an-overdue-loan.md", "cells": [{ "from": 301, "to": 306, "actual": "£2.50" }] } }] }failure.docPathnames the documentline,cellsandanchorare offsets into. Absent (the common case) means the oath itself.documentscarries a hash per referenced oath that contributed steps, so a consumer can tell a stale failure from a live one exactly assourceHashdoes for the oath.linesholds only the running oath's own lines. A spliced step's line is not in this file, and a line-wash renderer would decorate an unrelated sentence.Each port's executor also names that document in the frame/note/location it injects, so an editor resolving a failure from the stack lands in the right file too.
Verified end to end
I broke a step inside the dogfood shared section and checked the written record:
docPathset,line: 10(the line in the shared file), cells at 301–306 — which slice to exactly£9.99in that document — and the hash underdocuments.linesstayed[11, 17], the referencing oath's own.How to review
0c6c40ad(TypeScript) is the reference: the payload, the anchor channel carrying the doc path, and the LSP change. The six port commits are mechanical against it, with one wrinkle worth a look:Go and Rust had a test pinning the opposite behaviour. Both asserted that a location naming a document other than the oath is ignored (fall back to the line). That guard predates references; a differing path now means a spliced step. I rewrote both tests to assert the new meaning rather than deleting them — worth confirming you agree that's the right reading.
Gate
conformance/run-results/expected.jsongained a fourth example covering a spliced failure — that fixture is what made the other six ports go red until they followed.conformance/adapter/smoke.shnow assertsversion == 2.make checkgreen across all seven ports.🤖 Generated with Claude Code
https://claude.ai/code/session_01MSCLupVART3c5PjiffmahX