From c5f0fbaaa496ed4192ba1ea24fadf5e0004b38fc Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 18 Aug 2026 14:18:00 +0200 Subject: [PATCH] test: audit helper-mediated source reads --- .../2026-08-18-r3-test-truth-ledger.md | 119 +++++++++++++----- tools/audit-test-inventory.ps1 | 52 +++++++- 2 files changed, 135 insertions(+), 36 deletions(-) diff --git a/docs/reviews/2026-08-18-r3-test-truth-ledger.md b/docs/reviews/2026-08-18-r3-test-truth-ledger.md index 8236984a..c51113f2 100644 --- a/docs/reviews/2026-08-18-r3-test-truth-ledger.md +++ b/docs/reviews/2026-08-18-r3-test-truth-ledger.md @@ -38,7 +38,7 @@ pwsh ./tools/audit-test-inventory.ps1 The generated JSON lives under ignored `artifacts/test-audit/`; it is not a second 6.8 MB checked-in source of truth. The script and this reviewed ledger are durable, while paths and line numbers regenerate after every batch. The -inventory refreshed for Batch L reports: +inventory refreshed through Batch U reports: | Syntax signal | Current count | |---|---:| @@ -56,7 +56,7 @@ inventory refreshed for Batch L reports: | Methods directly using `Thread.Sleep` / `Task.Delay` | 15 / 14 | | Cancellable-infinite-only / elapsed-time methods | 5 / 24 | | Methods directly reading environment variables | 47 | -| Methods directly reading `.cs` source text | 63 | +| Methods directly / through same-file helpers reading `.cs` source text | 67 / 107 | The three remaining prerequisite-return candidates are all reviewed branch false positives: the two Windows/Linux factory assertions and the launcher's @@ -140,7 +140,7 @@ identifiable in the lane report. | T-009 wall-clock double-click tests | resolved in batch E | Four sleeps were replaced by a deterministic test clock behind an internal factory overload. The production factory still reads `Environment.TickCount64` exactly as before. | | T-010 two useless cases | high-confidence cleanup batch A | Delete `SmokeTest.TestProject_IsWired` and `ChaseCameraTests.ImplementsICamera`; compilation already proves both claims. | | T-011 diagnostic-only methods | resolved in batches C, L, and R | The reviewed current set is 82 methods / 103 cases. All carry `Purpose=Diagnostic`, preserving the apparatus while removing it from release pass totals. Batch R catches investigations whose only assertion validates fixture/DAT availability, which the original mechanical output-only scan could not distinguish from an oracle. | -| T-012 source-text freezes | replacement map complete in batch I; 46 implementation-shape facts remain staged | Twelve whole-tree architecture rules and five cross-artifact contracts stay. The other 46 are campaign-era literal implementation freezes; retire each only beside its semantic/behavioral replacement, with user approval for owner/lifecycle assertions that have no equivalent yet. | +| T-012 source-text freezes | direct-read map completed in batch I and helper-mediated gap corrected in batch U; 85 implementation/test-model facts remain staged | Seventeen whole-tree architecture rules and five cross-artifact contracts stay. The other 85 are literal implementation or test-model freezes; retire each only beside its semantic/behavioral replacement, with user approval for owner/lifecycle assertions that have no equivalent yet. | | T-013 controller self-comparison | high-confidence cleanup batch A | Capture the first controller next to the first body and compare every retry with that reference. | | T-014 seven load-sensitive tests | six mechanisms repaired in batch F; one product defect classified | Virtual/wall-clock mixing, tiered-JIT allocation noise, a live controller clock, and a ThreadPool-start timing oracle were removed without weakening behavioral contracts. `DatSoundCache` #321 is a real in-flight-entry race and now runs as `Status=KnownFailure` pending a product fix. | | T-015 four non-prerequisite skips | resolved in batches A/B | PVS scaffold deleted with rationale preserved; redundant chat/radar generators deleted in favor of the comprehensive Manual lane; tower oracle is `Status=KnownFailure`. | @@ -469,25 +469,33 @@ only methods, and 33 methods with some elapsed-time wait. ## Batch I source-text replacement map -The refreshed inventory finds 63 attributed methods that directly read `.cs` -source text. Reconciliation with historical T-012 assigns every method to one -of three dispositions: +Batch I's first inventory found 63 attributed methods that directly read `.cs` +source text. Batch U later proved that count was incomplete: tests that called +a local `BuilderSource()`, `ReadSource(...)`, or method-group +`File.ReadAllText` helper were invisible to the direct body regex. The +corrected syntax inventory follows same-file helper calls, recognizes invoked +and method-group readers, and only treats actual string content—not comments +such as `GameWindow.cs:5893`—as a C# path signal. It finds 67 direct and 40 +helper-mediated methods, 107 total. + +Manual reconciliation assigns every candidate to one of three dispositions: | Disposition | Methods | Owning test files | |---|---:|---| -| Campaign-era implementation-shape freeze; replace in stages | 46 | `ContentEffectsAudioCompositionTests`, `FrameRootCompositionTests`, `HostInputCameraCompositionTests`, `InteractionRetainedUiCompositionTests`, `LivePresentationCompositionTests`, `SessionPlayerCompositionTests`, `WorldRenderCompositionTests`, `GameWindowLiveSessionOwnershipTests`, `GameWindowRenderLeafCompositionTests`, `GameWindowHostBoundaryTests`, `ResourceCleanupGroupTests`, `RetailPViewPassExecutorTests`, `WorldRenderFrameBuilderTests`, `WorldSceneRendererTests`, `LandblockBuildOriginTests`, `LandblockPhysicsPublisherTests`, `LandblockRenderPublisherTests`, `GameWindowLiveEntityCompositionTests`, `UpdateFrameOrchestratorTests` | -| Whole-tree architecture policy; retain, then make syntax/metadata-aware | 12 | `LinuxPlatformBoundaryTests`, `RenderSceneArchitectureTests`, `RuntimePhysicsOwnershipTests`, `RuntimeEntityOwnershipTests` | +| Implementation-shape or test-model freeze; replace in stages | 85 | Thirty files across composition, input/physics/network wiring, graphical host/render ownership, Runtime gameplay-owner wiring, streaming, and frame orchestration; the exact stage manifest follows below. | +| Whole-tree architecture policy; retain, then make syntax/metadata-aware | 17 | `LinuxPlatformBoundaryTests`, `RenderSceneArchitectureTests`, `RuntimePhysicsOwnershipTests`, `RuntimeEntityOwnershipTests`, plus the whole-tree methods in `RuntimeActionOwnershipTests` and `RuntimeMovementOwnershipTests`, `RuntimeWorldTransitOwnershipTests`, and `RuntimeDatAccessArchitectureTests` | | Source/project/artifact text is the actual cross-artifact contract; retain | 5 | `ParticleBindlessInstanceTests`, `PerformanceToolsTests`, `LauncherProjectBoundaryTests` | -The 46 implementation-shape facts were useful during mechanical ownership -cutovers, but are not durable behavioral evidence. They pin constructor -spellings, local names, exact call fragments, or statement order; comments can -satisfy many of their `Contains` checks, while a harmless equivalent refactor -can fail them. Their intent is often valuable—single ownership, ordering, -handoff, teardown, or absence of a parallel path—so wholesale deletion would -discard real notes and weaken some boundaries. +The 85 implementation/test-model facts were useful during mechanical ownership +cutovers and defect investigations, but are not durable behavioral evidence. +They pin constructor spellings, local names, exact call fragments, regex-shaped +method bodies, or statement order; comments can satisfy many of their +`Contains` checks, while a harmless equivalent refactor can fail them. Their +intent is often valuable—single ownership, ordering, handoff, teardown, +absence of a parallel path, or continued validity of a synthetic test model—so +wholesale deletion would discard real notes and weaken some boundaries. -R3 therefore uses this replacement rule for those 46 facts: +R3 therefore uses this replacement rule for those 85 facts: 1. preserve the ownership/order rationale in the architecture document or this ledger; @@ -496,14 +504,15 @@ R3 therefore uses this replacement rule for those 46 facts: 3. add the semantic replacement first when none exists; and 4. remove the literal source freeze in the same small batch. -The 12 whole-tree rules remain valuable even before conversion: they ban +The 17 whole-tree rules remain valuable even before conversion: they ban platform calls outside owners, enforce assembly/namespace containment, and -prevent duplicate Runtime entity/physics authorities. The five cross-artifact -tests compare two independently consumed artifacts (for example CPU/shader -constants, launcher project/workflow/markup, and CLI route tooling); reading -text is appropriate there. +prevent duplicate Runtime entity, physics, movement, action, transit, and DAT +authorities. The five cross-artifact tests compare two independently consumed +artifacts (for example CPU/shader constants, launcher +project/workflow/markup, and CLI route tooling); reading text is appropriate +there. -No source-freeze fact is removed in this classification batch. The 46 staged +No source-freeze fact is removed in this classification batch. The 85 staged facts are an ambiguous architecture decision and will be presented for user approval in behavior-owner groups rather than deleted from a numeric heuristic. @@ -515,30 +524,37 @@ and safest replacement path divide into six owner groups: | Owner group | Facts | What the literal checks still prove | R3 recommendation | |---|---:|---|---| | Composition-root wiring | 13 | `GameWindow` calls the intended phase objects and no longer contains the extracted construction bodies. Phase-local tests cover ordering and rollback inside each phase, but not every root-to-phase link. | Keep until one symbol-aware composition-graph guard proves the root links. Then remove the literal constructor/local-name checks. | -| Runtime root, host, and lifetime | 12 | One canonical Runtime root/command surface, typed owner handoffs, acquisition order, and native-window-last shutdown. Existing Runtime/lifecycle tests prove much of the behavior, but several sole-owner and transfer claims remain unique. | Replace with direct ownership receipts, terminal-ledger assertions, and metadata dependency guards before removing any source freeze. | -| Render delegation and cleanup | 4 | Rendering is delegated and `GameWindow` does not regain leaf draw branches or resource ownership. The owning render suites already exercise the executor, frame builder, renderer, and resource disposal behavior. | Best first conversion: add one symbol-aware boundary guard for the absence/delegation claim, then remove the four literal checks. | +| Runtime root, graphical host, and lifetime | 24 | One canonical Runtime root/command surface, typed owner handoffs, run/render/status ordering, acquisition order, and native-window-last shutdown. Existing Runtime/lifecycle tests prove much of the behavior, but several sole-owner and transfer claims remain unique. | Replace with direct ownership receipts, terminal-ledger assertions, observable host traces, and metadata dependency guards before removing any source freeze. | +| Render delegation and cleanup | 9 | Rendering is delegated and `GameWindow` does not regain leaf draw branches or resource ownership; several tests also pin exact renderer/preparation statement order. The owning render suites already exercise the executor, frame builder, renderer, and resource disposal behavior. | Best first conversion: add a symbol-aware boundary guard for absence/delegation and observable fake traces for order, then remove the literal checks. | | Streaming publication | 4 | The window does not own render/physics publication bodies, captured build origins flow through the extracted owners, and streamer teardown follows session reset. Publisher/build/origin suites already provide broad behavioral coverage. | Best second conversion: consolidate the absence claims into a whole-tree architecture guard and retain the behavioral origin/teardown contracts. | | Live-entity composition | 2 | Typed sources replace window closures and reset closes every streaming/readiness owner. Lifecycle tests cover convergence, but the exact constructor-shape claim is still literal. | Replace with constructor/field metadata inspection plus the existing teardown ledger; do not delete alone. | -| Frame orchestration | 11 | Exact-once publication, frame-stage order, adapter ownership, and a deliberately thin `OnUpdate`. These are high-value order rules, but exact statement fragments are the most brittle implementation specification in the set. | Replace order assertions with an observable fake event trace and use a symbol-aware dependency guard for the thin-root rule. This is the largest conversion and should be last. | +| Frame orchestration and resource preparation | 14 | Exact-once publication, frame/resource-stage order, adapter ownership, and a deliberately thin `OnUpdate`. These are high-value order rules, but exact statement fragments are the most brittle implementation specification in the set. | Replace order assertions with observable fake event traces and use a symbol-aware dependency guard for the thin-root rule. This is the largest conversion and should be last. | +| Input, physics, and network wiring | 9 | Typed owner entry points, retry/reset wiring, and local/remote branch ordering remain at exact source locations. Behavioral controller suites cover the operations, but not every production composition route. | Add symbol-aware call-graph boundaries and route the behavioral fixtures through the production entry seam before removing the text checks. | +| Runtime gameplay-owner wiring | 9 | UI/session/shutdown consumers borrow exact action, character, inventory, and movement owners without mirrors or second mutations. Runtime ledgers cover terminal ownership but several App consumer links are literal only. | Replace consumer links with metadata dependency checks and exact borrowed-instance integration assertions. | +| Test-model/source correspondence | 1 | The hand-called create-authority drift probe still models the executor's sole production advance site. | Expose an observable executor stage/receipt or re-derive the synthetic model before removing this explicit stale-model alarm. | Approval should authorize this replacement sequence, not wholesale deletion: -render delegation, streaming, live entities, composition roots, Runtime/host -lifetime, then frame orchestration. If the semantic replacement exposes a -claim that exists only as historical campaign wording, preserve the rationale -and bring that individual deletion back for review. +render delegation, streaming, live entities, input/physics/network wiring, +composition roots, Runtime gameplay owners, Runtime/graphical-host lifetime, +frame orchestration, then the one test-model/source pin. If the semantic +replacement exposes a claim that exists only as historical campaign wording, +preserve the rationale and bring that individual deletion back for review. The exact staged file manifest is: | Stage | Facts | Complete owning-file set | |---|---:|---| -| Render delegation and cleanup | 4 | `ResourceCleanupGroupTests`, `RetailPViewPassExecutorTests`, `WorldRenderFrameBuilderTests`, `WorldSceneRendererTests` | +| Render delegation and cleanup | 9 | `ResourceCleanupGroupTests`, `RetailPViewPassExecutorTests`, `WorldRenderFrameBuilderTests`, `WorldSceneRendererTests` | | Streaming publication | 4 | `LandblockBuildOriginTests`, `LandblockPhysicsPublisherTests`, `LandblockRenderPublisherTests` | | Live-entity composition | 2 | `GameWindowLiveEntityCompositionTests` | | Composition-root wiring | 13 | `ContentEffectsAudioCompositionTests`, `FrameRootCompositionTests`, `HostInputCameraCompositionTests`, `InteractionRetainedUiCompositionTests`, `LivePresentationCompositionTests`, `SessionPlayerCompositionTests`, `WorldRenderCompositionTests` | -| Runtime root, host, and lifetime | 12 | `GameWindowLiveSessionOwnershipTests`, `GameWindowRenderLeafCompositionTests`, `GameWindowHostBoundaryTests` | -| Frame orchestration | 11 | `UpdateFrameOrchestratorTests` | +| Runtime root, graphical host, and lifetime | 24 | `GameWindowLiveSessionOwnershipTests`, `GameWindowCrashStatusTests`, `GameWindowRenderLeafCompositionTests`, `GameWindowHostBoundaryTests` | +| Frame orchestration and resource preparation | 14 | `UpdateFrameOrchestratorTests`, `RenderFrameResourceControllerTests` | +| Input, physics, and network wiring | 9 | `C3cF2AutoEntryWiringTests`, `C3cF1ProductionWiringTests`, `Issue270ProductionWiringTests`, `LiveEntityNetworkBranchRoutingTests` | +| Runtime gameplay-owner wiring | 9 | The non-whole-tree methods in `RuntimeActionOwnershipTests` and `RuntimeMovementOwnershipTests`, plus `RuntimeCharacterOwnershipTests` and `RuntimeInventoryOwnershipTests` | +| Test-model/source correspondence | 1 | `CreateAuthorityDriftModelSourcePinTests` | -This manifest accounts for all 46 methods exactly. It excludes the 12 +This manifest accounts for all 85 methods exactly. It excludes the 17 whole-tree policy methods and five genuine cross-artifact text contracts. ## Batch P dormant panel reachability reconciliation @@ -945,3 +961,40 @@ Verification: and - the no-retry complete hermetic Release gate passes 14,379/14,379 with zero skips or failures across all 12 test assemblies. + +## Batch U helper-mediated source-text audit correction + +Preparing the first semantic replacement exposed a material inventory gap: +`RetailPViewPassExecutorTests` and `WorldRenderFrameBuilderTests` contain tests +that read production source through local helpers, but Batch I's direct body +regex counted only the one method in each file that called +`File.ReadAllText(...)` itself. Parameterized helpers such as +`ReadSource("GameWindow.cs")` and method-group reads such as +`.Select(File.ReadAllText)` exposed two more missed forms. + +Batch U corrects the audit tool, not any test or product behavior. It now: + +- detects invoked and method-group `ReadAllText`/`ReadAllLines` syntax; +- follows the same-file helper graph already used for failure and prerequisite + signals; +- follows the `.cs` path signal separately, allowing the caller to supply the + filename while a helper performs the read; and +- derives the path signal only from string/interpolated-string content, so a + comment such as `GameWindow.cs:5893` cannot classify an ordinary JSON-lines + test as a source reader. + +The corrected inventory reports 67 direct and 40 helper-mediated source-text +tests, 107 total. Manual method-by-method reconciliation assigns 85 to staged +semantic replacement, 17 to retained whole-tree architecture policy, and five +to retained cross-artifact contracts. The focused inventory assertions prove a +helper-mediated builder test is included, a method-group whole-tree test is +included directly, and the prior comment/path false positive is excluded. + +Verification: + +- all 1,256 tracked C# test files parse and the focused 67/107 count and three + exemplar assertions pass; +- the complete 44-project Release build reports zero warnings and zero errors; + and +- the no-retry complete hermetic Release gate passes 14,379/14,379 with zero + skips or failures across all 12 test assemblies. diff --git a/tools/audit-test-inventory.ps1 b/tools/audit-test-inventory.ps1 index e2ebbc31..9bdd85bd 100644 --- a/tools/audit-test-inventory.ps1 +++ b/tools/audit-test-inventory.ps1 @@ -10,7 +10,7 @@ signals (including same-file helper calls), diagnostic output signals, high-confidence constant-truth assertions, syntactic self-comparison candidates, wall-clock waits, environment-variable dependencies, and - source-text reads. + source-text reads, including reads reached through same-file helper calls. This is a candidate generator, not a semantic proof. In particular, an output-only candidate can call a failure-capable helper declared in another @@ -192,6 +192,39 @@ function Test-DirectOutputSignal { return $false } +function Test-CSharpPathLiteral { + param([Parameter(Mandatory)]$Method) + + foreach ($literal in @($Method.DescendantNodes() | Where-Object { + $_.GetType().Name -eq 'LiteralExpressionSyntax' + })) { + if ($literal.Token.ValueText -match '(?i)\.cs') { + return $true + } + } + foreach ($text in @($Method.DescendantNodes() | Where-Object { + $_.GetType().Name -eq 'InterpolatedStringTextSyntax' + })) { + if ($text.TextToken.ValueText -match '(?i)\.cs') { + return $true + } + } + return $false +} + +function Test-TextFileRead { + param([Parameter(Mandatory)]$Method) + + foreach ($member in @($Method.DescendantNodes() | Where-Object { + $_.GetType().Name -eq 'MemberAccessExpressionSyntax' + })) { + if ($member.Name.Identifier.Text -in @('ReadAllText', 'ReadAllLines')) { + return $true + } + } + return $false +} + function Get-AssertionAuditSites { param( [Parameter(Mandatory)]$Tree, @@ -391,6 +424,8 @@ foreach ($relativePath in $trackedFiles) { $methodMap = @{} $failureMap = @{} $outputMap = @{} + $textReadMap = @{} + $csharpPathMap = @{} $gateMap = @{} $callMap = @{} @@ -400,6 +435,8 @@ foreach ($relativePath in $trackedFiles) { $methodMap[$key] = $method $failureMap[$key] = Test-DirectFailureSignal $method $outputMap[$key] = Test-DirectOutputSignal $method + $textReadMap[$key] = Test-TextFileRead $method + $csharpPathMap[$key] = Test-CSharpPathLiteral $method $gateMap[$key] = @(Get-EmptyReturnSites $tree $method) $callMap[$key] = @($method.DescendantNodes() | Where-Object { $_.GetType().Name -eq 'InvocationExpressionSyntax' } | @@ -425,6 +462,11 @@ foreach ($relativePath in $trackedFiles) { $key $methodMap $failureMap $callMap @{} $hasOutputSignal = Test-RecursiveSignal ` $key $methodMap $outputMap $callMap @{} + $readsFileText = Test-RecursiveSignal ` + $key $methodMap $textReadMap $callMap @{} + $usesCsharpPath = Test-RecursiveSignal ` + $key $methodMap $csharpPathMap $callMap @{} + $readsSourceText = $readsFileText -and $usesCsharpPath $emptyReturns = @($gateMap[$key]) $helperPrerequisiteReturns = @(Get-RecursivePrerequisiteGateSites ` @@ -518,8 +560,9 @@ foreach ($relativePath in $trackedFiles) { @($waitSites | Where-Object { -not $_.IsCancellableInfiniteDelay }).Count -eq 0 - ReadsSourceText = $bodyText -match '(ReadAllText|ReadAllLines)\s*\(' -and - $bodyText -match '\.cs' + ReadsSourceTextDirectly = [bool]($textReadMap[$key] -and + $csharpPathMap[$key]) + ReadsSourceText = $readsSourceText }) } } @@ -668,6 +711,9 @@ $summary = [ordered]@{ DirectEnvironmentVariableMethods = @($orderedRecords | Where-Object { $_.EnvironmentVariables.Count -gt 0 }).Count + DirectSourceTextReadMethods = @($orderedRecords | Where-Object { + $_.ReadsSourceTextDirectly + }).Count SourceTextReadMethods = @($orderedRecords | Where-Object { $_.ReadsSourceText }).Count }