[ChaoWao]

What

Scene tests compared every output against compute_golden with a fixed
torch.allclose at the class RTOL/ATOL. The only escape was the global
--skip-golden CLI flag, which is all-or-nothing across the whole run.

Two shapes had no clean expression:

• Outputs whose valid extent the device decides. A task that writes one
  slot per SPMD block only defines the prefix it actually ran with — the host
  cannot know that width up front.
• Diagnostic outputs with no golden. merge_pipeline_barrier wanted to
  drop its timing tensor from the comparison and resorted to monkey-patching
  the private _compare_outputs through sys.modules.

Change

• CASES[*]["skip_golden"], defaulting to the new class attribute
  SKIP_GOLDEN (itself False). True means the case has no host-computable
  expected output; compute_golden is never called for it. The CLI flag still
  forces it on for every case (benchmark mode). Applied on both the L2 and L3
  run paths.
• compare_outputs(self, test_args, golden_args, output_names, params) is
  now the overridable comparison hook; the default delegates to the existing
  _compare_outputs. Override it to compare a subset, apply a per-tensor
  tolerance, or derive the valid extent from the output itself.
  _compare_outputs keeps its signature — the four prepared_callable tests
  that import it directly are untouched.
• merge_pipeline_barrier uses the override instead of patching module
  internals.
• docs/testing.md documents both hooks under a new "Output Validation"
  section, with the guidance to prefer an override to a skip (a self-describing
  output still gets checked; a skipped case only proves the run didn't crash).

Why now

This is groundwork for making the SPMD grid width runtime-discovered rather
than pinned per case. Once a case no longer declares block_dim, its golden
either doesn't depend on the core count (paged attention, qwen — unchanged) or
must read the launch width out of the output. Both hooks are needed before that
rework can start, and they are independently useful today, so they land on
their own.

Verification

Suite	Result
pytest tests/ut -m "not requires_hardware"	802 passed
New test_scene_test_golden_hooks.py	7 passed
pytest examples tests/st --platform a2a3sim	58/58 cases
pytest examples tests/st --platform a5sim	43/43 cases
merge_pipeline_barrier onboard a2a3 (task-submit)	1 passed
pre-commit (all hooks)	passed

The migrated example doubles as end-to-end coverage of the override: its
timing golden is all zeros while the run writes real counter values, so the
default comparison would fail on that tensor. The case passing is proof the
override ran.

[coderabbitai]

📝 Walkthrough

Walkthrough

Golden validation now supports class-level and per-case skipping plus overridable output comparison. The merge-pipeline test adopts the hook for timing diagnostics, with documentation and unit tests covering precedence and mismatch behavior.

Changes

Golden validation customization

Layer / File(s)	Summary
Golden validation hooks simpler_setup/scene_test.py	Adds SKIP_GOLDEN and overridable compare_outputs, routing L2 and L3 checks through the new behavior.
Timing diagnostics comparison examples/a2a3/tensormap_and_ringbuffer/merge_pipeline_barrier/test_merge_pipeline_barrier.py	Replaces global comparator patching with a class override that reports timing gaps and excludes timing from golden checks.
Validation coverage and documentation tests/ut/py/test_scene_test_golden_hooks.py, docs/testing.md	Documents and tests default comparison, skip precedence, custom filtering, and mismatch reporting.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SceneTestCase
  participant GoldenGeneration
  participant CompareOutputs
  participant TestMergePipelineBarrier
  SceneTestCase->>GoldenGeneration: compute golden outputs when checks are enabled
  GoldenGeneration-->>SceneTestCase: golden_args
  SceneTestCase->>CompareOutputs: compare test outputs with golden outputs
  TestMergePipelineBarrier->>CompareOutputs: report timing gaps and omit timing
  CompareOutputs-->>SceneTestCase: validate remaining outputs

Loading

Possibly related PRs

• hw-native-sys/simpler#1442: Also customizes merge-pipeline timing output comparison.

Poem

I’m a rabbit with tests in my burrow,
Golden checks now skip or compare with care.
Timing gaps hop into view,
Hooks trim outputs as they should do—
Clean validation everywhere!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: per-case scene-test hooks for skip_golden and custom comparison.
Description check	✅ Passed	The description is detailed and directly describes the hook additions, docs update, and migration of the example.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (2)

tests/ut/py/test_scene_test_golden_hooks.py (2)

34-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Annotate framework configuration attributes as ClassVar.

Ruff reports RUF012 for these mutable class attributes. Keep them class-level, but add ClassVar annotations to make the intended configuration contract explicit and clear the warnings.

Also applies to: 106-107, 125-126

🤖 Prompt for 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.

In `@tests/ut/py/test_scene_test_golden_hooks.py` around lines 34 - 35, Annotate
the mutable class-level configuration attributes such as CALLABLE and CASES with
typing.ClassVar in the affected test classes, including the additional
occurrences noted in the review. Keep these values as class-level attributes and
preserve their existing contents while resolving Ruff RUF012.

Source: Linters/SAST tools

---

51-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Add an L3 regression test for the new hooks.

_drive always exercises _run_and_validate_l2, so the separate L3 wiring for skip_golden and compare_outputs is untested. Add a focused stubbed L3 case covering both skip precedence and comparison dispatch.

🤖 Prompt for 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.

In `@tests/ut/py/test_scene_test_golden_hooks.py` around lines 51 - 58, Add a
focused stubbed L3 regression test alongside _drive that exercises the L3
execution path, verifying skip_golden takes precedence and compare_outputs is
dispatched correctly. Reuse the existing _build_chip_task_args and _build_config
patches, configure the instance for level 3, and assert the relevant comparison
behavior without touching device operations.

🤖 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.

Nitpick comments:
In `@tests/ut/py/test_scene_test_golden_hooks.py`:
- Around line 34-35: Annotate the mutable class-level configuration attributes
such as CALLABLE and CASES with typing.ClassVar in the affected test classes,
including the additional occurrences noted in the review. Keep these values as
class-level attributes and preserve their existing contents while resolving Ruff
RUF012.
- Around line 51-58: Add a focused stubbed L3 regression test alongside _drive
that exercises the L3 execution path, verifying skip_golden takes precedence and
compare_outputs is dispatched correctly. Reuse the existing
_build_chip_task_args and _build_config patches, configure the instance for
level 3, and assert the relevant comparison behavior without touching device
operations.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e742646e-7d29-4bac-970f-3000cc76c1b9

📥 Commits

Reviewing files that changed from the base of the PR and between 6284040 and a72a326.

📒 Files selected for processing (4)

• docs/testing.md
• examples/a2a3/tensormap_and_ringbuffer/merge_pipeline_barrier/test_merge_pipeline_barrier.py
• simpler_setup/scene_test.py
• tests/ut/py/test_scene_test_golden_hooks.py