test(engine): GPU pass fusion stack 3/5 — pipeline coverage - #2168
test(engine): GPU pass fusion stack 3/5 — pipeline coverage#2168yuto-trd wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains in the current changeset. Important Files Changed
Reviews (52): Last reviewed commit: "test(engine): GPU pass fusion stack 3/5 ..." | Re-trigger Greptile |
Code Review BotDocumentation driftFound 1 possible documentation drift(s) (partially analyzed) — see the bot's latest pull request review for details. These are advisory. Reviewed 67 file(s); skipped 60. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d7ef41fd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1d7ef41 to
b9c4092
Compare
854ac82 to
020111b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 020111b100
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
020111b to
32c8828
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32c882896e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
32c8828 to
5a3bd6b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a3bd6b5ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5a3bd6b to
f443ed4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f443ed4272
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
f443ed4 to
f43579e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f43579e009
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
f43579e to
3aa56c8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3aa56c8833
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
3aa56c8 to
8c058e7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c058e7269
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8c058e7 to
a0aeddb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc6bb7704b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 622748e454
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dac32643c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d496c8fd2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Review summary 4834781878 is addressed in 6861e47. The parity fixture retries non-finite output, hard-fails deterministic invalid 1x locations and deterministic scaled-only defects, and treats only run-varying software-Vulkan artifacts as inconclusive. The final hardware-GPU run passed all affected cases. |
|
Review summary 4835820674 is addressed in 6861e47. The SKSL border guard now requires finite output, at least one expected opaque red border pixel, preserved visible interior source pixels, and the SSIM change threshold, so a blank output cannot satisfy the test. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 064d807e0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Documentation drift
Code Review Bot flagged 1 possible documentation drift(s) (partially analyzed). These are advisory.
AGENTS.md(info):The 'Project overview' section under 'Tests' provides an explicit list of test projects (e.g., 'tests/Beutl.UnitTests/'), but the newly added 'tests/Beutl.Benchmarks/' project is missing from this list.— suggested:In the 'Project overview' section under 'Tests', add 'tests/Beutl.Benchmarks/' to the list of test projects.
Results are for commit 064d807. On newer commits, the bot's summary comment reflects the latest run.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9c1a74366
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| EngineAssemblyVersion = counters.SourceProvenance, | ||
| Command = command, | ||
| BenchmarkDotNetResultFile = Path.GetFileName(resultPath), | ||
| BenchmarkDotNetResultSha256 = Sha256File(resultPath), |
There was a problem hiding this comment.
Bind the manifest to the result bytes actually analyzed
When a BenchmarkDotNet result file changes during the 100,000-iteration analysis, ReadBenchmarkResults makes acceptance decisions from the earlier byte snapshot, but this line re-reads the path and records the replacement file's SHA-256; HashDirectory then repeats the same race for the artifact inventory. The accepted manifest can therefore authenticate samples that were never analyzed. Carry the initially read bytes and their hash through BenchmarkResultRun instead of reopening the result file when creating the manifest.
Useful? React with 👍 / 👎.
| string[] untracked = RunGit(root, "ls-files", "--others", "--exclude-standard", "-z", "--", ".") | ||
| .Split('\0', StringSplitOptions.RemoveEmptyEntries); |
There was a problem hiding this comment.
Authenticate the harness that produced the executed binary
When tracked harness sources are modified after the binary is built or while the benchmark runs, this post-run git ls-files scan hashes the current working-tree bytes even though the executed assembly still reports the unchanged HEAD informational SHA. Although harness hashes are now recorded, fresh evidence is that only untracked files are rejected and no build-time source hash or tracked-file cleanliness check connects these bytes to the executed binary, so an accepted manifest can claim a workload implementation that never ran.
Useful? React with 👍 / 👎.
| options.OutputPath, | ||
| FileMode.Create, | ||
| FileAccess.Write, | ||
| FileShare.None)) |
There was a problem hiding this comment.
Prevent the manifest output from overwriting its evidence inputs
When --output names an existing result, stdout, counter, output-blob, or other evidence file, this unconditional FileMode.Create truncates that input after analysis and replaces it with the manifest. The manifest then references and hashes raw evidence that no longer exists at the recorded path, and a simple argument mix-up can permanently destroy a completed benchmark run; reject aliases with every input path and use create-only output semantics.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
7d705b5 to
d3d92a6
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No TODO comments were found. |
Minimum allowed line rate is |
d3d92a6 to
b4274c4
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No TODO comments were found. |
Planning, recording, fusion, cache, failure, and golden coverage for the stack-2 renderer (T014/T093/T104-T109), the Rgba16fGoldenStore, the benchmark harness (RenderPipelineBenchmarks and its scenes/config), the Baseline tests (T007/T016), SelectedDrawableRenderTests (T028), DrawableBrushThumbnailTests (T029), and GpuPassFusion3DBoundaryTests (T104). The paired-benchmark analyzer, visual-evidence exporter, provenance writer, and their archive tests are stack-4 deliverables and are not carried here; the migration census pins 234 test Process overrides. The FrameProviderImpl retention heuristic test is excluded with the separately shipped retained-target change; the ownership-transfer contract tests are excluded with the removed transfer seam.
b4274c4 to
398d98a
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No TODO comments were found. |
|
Superseded by #2221, which merges every layer of this stack into one branch, propagates the s2 reduction through layers 3–5 (this stack no longer built above s2), and drops the 61.5 MB evidence archive. |
Description
Stacked-PR slice 3/5 of
speckit/004-gpu-pass-fusion(tests and benchmark harness coverage).This slice adds renderer-wide recording, planning, fusion, resource-lifetime, failure-matrix, cache, ROI, nested-target, diagnostics, benchmark-contract, and GPU golden suites. It deliberately excludes the built-in Shader migration tests, which land with their production changes in slice 5, and the immutable evidence corpus, which begins in slice 4.
The review follow-up hardens the evidence producer and analyzer before the corpus is introduced: output archives and feature-harness provenance are mandatory, setup and measured RGBA16F blobs are authenticated independently, logical geometry is compared for every case, and baseline and feature revisions must be distinct. The affected S3 fixtures were also migrated to the final S2 named-resource, deep-state, allocation-budget, and phase-dependent cache contracts.
Stack
speckit/004-s1-spec— specification and contractsspeckit/004-s2-engine— record-then-plan engine and consumer migrationspeckit/004-s4-evidence— benchmarks, paired evidence, and acceptancespeckit/004-s5-shader-migration— built-in Shader migration, excluding Blur and DropShadowAffected areas
Beutl.Engine(rendering / scene / track)Beutl.ProjectSystem(project / document persistence)Beutl.Editor,Beutl.Editor.Components,Beutl.Controls)Beutl.Extensibility(plugin abstractions)Beutl.NodeGraph(node editor)Beutl.FFmpegIpc/Beutl.FFmpegWorker(media IPC boundary)Beutl.Api(server API client)Breaking changes
None in this slice.
Review follow-up (2026-08-09)
MixedSpatialColor, and strengthened effect and primary-stage non-vacuity controls without lowering tolerances.All 17 unresolved inline review threads were answered and resolved. The two review-summary findings about deterministic non-finite output and blank SKSL output were also addressed and answered.
Test plan
Beutl.UnitTests: 6,319 passed, 16 expected skips, 0 failed.Beutl.HeadlessUITests: 250/250 passed.Beutl.PublicApiContractTests: 194/194 passed.SourceGeneratorTest: 30/30 passed.Beutl.Graphics3DTests: 14/14 passed on Apple M3 / MoltenVK.dotnet format --verify-no-changesandgit diff --check: passed.Fixed issues / References
Review follow-up (2026-08-10)
Validation: fresh solution build 0 warnings / 0 errors; focused contracts 60/60; Geometry ownership 33/33; recording/census 16 passed / 1 expected evidence skip; Apple M3/MoltenVK parity 30/30; PublicApiContractTests 212/212; SourceGeneratorTest 30/30; full UnitTests 6,325 passed / 16 expected skips / 0 failed; format and diff checks passed. The InnerShadow fixture also passed Linux arm64 SwiftShader; Linux x64 SwiftShader remains delegated to PR CI.