Settle the ParsedResult information model before the port (#391) - #435

Fifteen tasks, each ending in a green test run and a commit, following the
spec's phasing: extract a backend-neutral port, add the modern reader, prove
parity differentially, then flip selection on.

Two ordering decisions carry most of the risk reduction. The UUID normalizer
and a pre-refactor baseline capture land before the large renderer refactor,
so "behaviour-preserving" is something the implementer diffs rather than
asserts. And the differential harness lands with the migration rather than
after it, because it only works while xcresulttool still supports both
formats.

Fixtures are regenerated on every CI run, so checked-in golden HTML is
impossible; every comparison in the plan is between two renders produced
within one run. Both assertions that could pass on empty input are paired
with an explicit non-vacuity guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The differential test was the weak point. It only checked that declared
markers appeared on legacy and vanished on modern, which says nothing about
lines nobody declared — an undeclared regression would have passed. Checking
the marker strings against HTMLTemplates.swift also showed three of them did
not exist: activity durations render as a bare "(0.00s)" suffix and attachment
names as bare text, with no class to key on.

Replaced with masking: strip exactly the declared losses from both renders and
require what remains to be byte-identical. That is implementable against the
real markup and is a stronger claim than the marker match ever was. Each rule
maps 1:1 to an allow-list entry, and the masked comparison asserts every test
title survives masking so an over-broad mask cannot make it pass vacuously.

Baseline capture no longer skips missing fixtures. It previously continued past
one, which would let Task 5 diff two partial directories and report them
identical — the same vacuous-verification shape.

Also from review: the attachment export leaked its temp directory (a full copy
of every screen recording) on every run; attachment comparison used Set<Data>,
which collapses duplicates; testNodes and testRuns were non-optional and would
throw keyNotFound on a bundle where xcresulttool omits the key; a failed
activities query returned an empty list with no fault, so a visibly gutted
report would exit 0; and Task 5 quoted a stale expected test count.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Three more from review. The --result-reader option was declared on
SummaryOptions but never passed into Summary, so it would have parsed,
validated, and done nothing; the end-to-end check now renders through both
readers and requires the wrapper-group counts to differ, which fails if the
flag is not reaching Summary.

ResultBackend.resolved() only consulted the version string, so an explicit
--result-reader legacy on a post-removal toolchain would have selected a
backend that cannot work. It now demotes to modern with a warning, which is
the degradation rule the spec already stated. The accompanying test is
conditional on toolchain capability rather than asserting .legacy -> .legacy
outright, since that assertion would start failing exactly when the fallback
becomes load-bearing.

The spec listed read() as throwing while the plan returned an optional. Settled
on the optional, matching the existing getInvocationRecord() contract that
Summary.init already guards with a fault, and documented why the sub-level
failures are the ones that needed a new fault kind.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The plan recorded .missingActivities when an activities query fails, but
nothing asserted the fault reaches summary.faults. Since a failed query
degrades to an empty activity list rather than aborting, unproven plumbing
means the CLI could exit 0 on a report whose tests have no activities — the
exact outcome the fault exists to prevent.

Added an XCResultToolInvoking seam so a client that fails only on `activities`
can be injected, and a test that reads through it and asserts both halves: the
read still succeeds, and the fault lands on the caller's collector. The
collector is the one Summary.init owns, so the existing exit-3 path covers the
rest.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Three more from review, two of them substantive.

The modern reader was sourcing failure text from the activities document. The
tests tree's Failure Message nodes are strictly better: measured on
TestResults, they give "FirstSuite.swift:66: XCTAssertTrue failed - Test
failed" where the activity title gives only "XCTAssertTrue failed - Test
failed". The plan was reading from the lossier of two available sources and
the spec described that loss as unavoidable. Both corrected; skip reasons ride
the same node.

The modern reader also collapsed every destination to devices.first, where
legacy emits one run per ActionRecord. Every fixture boots a single simulator,
so no test would have caught the difference — and the differential's zip()
truncates to the shorter sequence, so it would have compared the runs that did
exist and passed. Now one run per device, with an explicit run-count assertion
before the zip, and the coverage gap stated in both documents rather than
implied to be tested.

Also: the legacy-capability probe never drained stderr, which deadlocks if the
pipe fills — the same bug the codebase already documents in TestSupport and
XCResultToolClient.run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Four amendments to the design spec and implementation plan, none of which
change the strategy.

**Milestone.** Both documents targeted 3.0, inherited from #391's label.
3.0.0 shipped 2026-08-07, so the breaking changes here — the `--json` schema
and the declared output diff — land in 4.0. Notes that the report redesign is
a sibling workstream in that same major rather than a later release, since
`activityType`'s removal breaks the visual contract either way, and that the
two are strictly sequenced: templates stay frozen until the differential is
proven.

**#430 replaces Task 1's normalizer, but does not retire it.** Task 1 had
rediscovered #411 independently and solved it in the harness with a UUID
regex. #430 fixes it in the product via `IdentifierPath`. The subtlety worth
recording: identifiers are a digest of each element's *structural path*, and
the two backends disagree on structure — the modern tree drops the "All tests"
and "<bundle>.xctest" wrapper levels — so cross-backend identifiers still
diverge. The normalizer survives, retargeted from RFC-4122 to `[0-9a-f]{32}`;
a regex left matching UUIDs would silently match nothing and the differential
would compare raw digests and fail on every run.

Task 1 also collided with #430 on `ReproducibilityTests.swift`. It now appends
to that file instead of creating it, and #430 is a stated prerequisite.

**Task 2 captures raw renders.** Same-backend renders are byte-identical after
#430, so normalizing the baseline is unnecessary — and harmful: Task 5 moves
the renderer onto `ParsedResult`, and a refactor that perturbed the tree would
move every affected digest, which a normalized baseline would hide. Phase 1's
gate is correspondingly strengthened to exact equality with no normalization.

**HTMLTemplates.swift is not generated.** Both documents asserted it was.
Its `DO NOT EDIT … autogenerated by createTemplates.sh` header is stale —
that script was deleted in #295 — and both linters exempt the file on that
basis, which is how `HTML/*.html` drifted 28 hunks behind while the "generated"
file was hand-edited. No consequence for this work, which touches neither; it
is a live trap for the redesign workstream.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`ParsedResult` is the one artifact this migration could build twice. Shaped so
the current templates render unchanged, it is not backend-neutral — it is
legacy-shaped, and `ModernResultReader` spends its life supplying nil for
fields that exist only because the old UI reads them. The proposed model
already shows the pattern: `activityType`, `finish`, `name`, and
`uniformTypeIdentifier` are each documented as "nil on the modern backend",
and `statusRawValue` would have the modern reader emit legacy spellings it
never saw.

Adds Task 2.5 between the baseline capture and the model: a decision task, not
a code task, with eight questions and a recommended answer for each. The
redesign's visual work stays a sibling workstream and does not gate this one —
but its information model is exactly what the port encodes, and that is an
afternoon rather than a design phase.

Every answer either removes a field from the port or an entry from the
differential allow-list, because holding the legacy backend down to the modern
backend's capability makes the two agree and an unmasked diff proves more than
a masked one. On the recommended defaults, `activityTypeClasses` and
`durations` leave the allow-list entirely: with no `finish` and no
`activityType` in the model, there is no divergence left to mask. The cost is
one-way and stated — the legacy backend stops rendering some things it could
have — but that is a 4.0 behaviour change made once and visible in the model.

Two answers go the other way for reasons worth keeping: Swift Testing
`Arguments` is added now because it is a reshape *inside* the tree and
expensive to retrofit, while insights and metrics are left out because they
attach at the top level beside `runs` and are cheap to add when something
renders them.

Also records the rule the task exists to enforce: no reader code whose only
purpose is to satisfy the render-level diff. The current templates are a
verification scaffold with a retirement date, not a compatibility target.

Updates both #430 prerequisite notes — merged 2026-08-11 as a28b131.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Applies Task 2.5's eight recommendations as written and propagates them
through the port, the readers, the renderer task, and the differential.

Three fields leave ParsedResult (activity finish, activity type, attachment
UTI), one arrives (Swift Testing arguments), one is retyped (status raw string
becomes a neutral enum), and ObjectClass is deleted rather than threaded
through. The direction is the point: each removal makes the two backends agree
by construction instead of by mask, so the differential allow-list drops from
five entries to three. An unmasked diff proves more than a masked one, and the
masked region is exactly where a regression can hide.

Two constraints surfaced while applying the answers, both recorded rather than
assumed away. Answer 6's `arguments` is unexercised: `Arguments` is in the
published TestNodeType enum, but SwiftTestingSuite has no parameterized case
and all three fixtures contain zero such nodes, so Task 8 gains a step that
adds one and a test that fails until it lands. Answer 4 cannot use
UTType(filenameExtension:), which is macOS 11+ against a 10.15 floor, so the
mapping keeps an explicit table.

Rebasing onto #430 also exposed stale guidance in Task 5: it had Activity.uuid
becoming UUID().uuidString, which would compile and silently undo the
reproducibility #430 just established. Activity now mints from IdentifierPath
like every other model.

Removing the activity-type and duration fields leaves two template
placeholders fed with empty strings rather than deleted, because
createTemplates.sh — the generator named in HTMLTemplates.swift's own
DO-NOT-EDIT header — is not in this repository. That leaves a cosmetic empty
paren on activity rows, noted for the redesign workstream, and is still better
than rendering a fabricated (0.00s).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

An xhigh multi-agent review found 30 distinct defects in #435, most of them
downstream of two wrong decisions. Correcting the decisions and re-deriving,
rather than patching the symptoms.

Decision 3 becomes replace-not-delete. Deleting ObjectClass empties ITEM_CLASS
and renders <div class=" failed"> on every row, which breaks the report's own
"show only failures" filter (showElementsWithSelector on .test-summary.failed
and siblings), group expand/collapse (querySelectorAll on .test-summary-group),
the stylesheet rules keyed on both classes, and four test call sites -- one in
CoreTests and three in ReproducibilityTests. The raw IDESchemeActionTest*
values were the legacy part and still go; the emitted class names are the
report's own contract and stay, behind a neutral renderer-side NodeKind. The
port needs no field, since ParsedNode already distinguishes group from case.

Decision 1 still removes the field, with two corrections. `start` is now named
as the replacement ordering key: the sort it fed interleaves failure rows among
activities so a failure renders where it occurred, and deleting it rather than
re-keying it would silently append every failure to the end. And the `durations`
allow-list entry is restored -- deleting it assumed removing `finish` removed
all duration divergence, but the surviving divergence is in group durations.
Verified: durationInSeconds is null on every Test Suite, Test Plan and test
bundle node in all three fixtures, while legacy reports FirstSuite 0.699s,
SecondSuite 0.126s, ThirdSuite 0.132s, SampleAppUnitTests 0.213s. wrapperGroups
only drops wrapper lines, so real suite headings were diverging unmasked.

Task 5 is split. It conflated a pure refactor with mandated behaviour changes
and then demanded a byte-identical gate, so the gate could only ever be waived
-- leaving the migration's largest refactor with no behaviour check. 5a moves
the renderer onto ParsedResult and must be byte-identical; 5b applies the
decisions that change output against an enumerated three-shape diff.

Also fixed, from the same review: failure rows were double-counted on modern
because both documents describe the same failure; the failureTitlePrefix mask
stripped only the legacy shape so every failing test still differed after
masking; read() handed every device the same testables array; an empty devices
list returned a non-nil empty result that exited 0 where legacy exits 3;
PayloadProviding omitted three members the call sites use, including the
downsize path from #428; Task 5 deleted three accessors LegacyResultReader
calls; Summary.init dropped the resultIndex/actionIndex seeding #430 needs;
ValidationError was thrown from a non-throwing init in a target without
ArgumentParser; legacyCapability returned false where the type wanted .unknown;
run??.activities double-chained a flattened optional; and Task 1 called a
render helper that does not exist.

Two refinements beyond the brief. The durations mask is necessarily
over-broad -- the duration sits on a different line from the group class, so it
cannot be scoped -- which also hides XCTest case durations that do agree, so a
targeted model-level assertion restores that coverage. And the empty-read rule
is stated for both readers rather than fixed only on the modern one, since
legacy has the identical shape.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>