Skip to content

Support: restore selected args payloads in L0 replay - #1567

Open
indigo1973 wants to merge 1 commit into
hw-native-sys:mainfrom
indigo1973:l0_0727
Open

Support: restore selected args payloads in L0 replay#1567
indigo1973 wants to merge 1 commit into
hw-native-sys:mainfrom
indigo1973:l0_0727

Conversation

@indigo1973

Copy link
Copy Markdown
Contributor
  • Add task func-id and argument-slot filters for level-3 dumps.
  • Restore selected before-dispatch tensor bytes in L0 replays.
  • Expose the msprof timeout and document oversized CAModel cases.
  • Limit payload consumption to a2a3/a2a3sim while the A5 host-shadow defect remains tracked by [Bug] a5sim dump-args level 2 writes zero tensor payloads #1560.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: eb7ce1a6-cd51-4ec3-b561-ed91b5153a59

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds selective level-3 tensor payload capture and --restore-arg replay support. The change extends the payload-filter wire contract, dump collection and manifests, replay materialization, CLI validation, platform guidance, and automated coverage.

Changes

Selective dump and replay

Layer / File(s) Summary
Payload filter contract and propagation
src/common/task_interface/..., python/bindings/task_interface.cpp, python/simpler/worker.py, simpler_setup/scene_test.py
Adds validated function-ID and slot selectors to CallConfig, Python bindings, mailbox decoding, and SceneTest execution.
Filtered collection
src/common/platform/..., src/a2a3/..., src/a5/...
Passes filters through device runners and collectors, selectively copies tensor payloads, and records filter and binary-output metadata in manifests.
Replay restoration and tooling
simpler_setup/tools/l0_swimlane.py, simpler_setup/tools/dump_viewer.py, conftest.py
Adds --restore-arg, payload validation/materialization, generated memcpy initialization, zero-fill fallback, extra include handling, and configurable profiler timeouts.
Validation coverage
tests/ut/..., tests/st/...
Covers wire layouts, selector validation, filtered payload capture, manifest output, restoration failures, strided tensors, CLI conflicts, and generated replay code.
Documentation
.claude/skills/..., docs/dfx/...
Documents selector usage, restoration constraints, SPMD behavior, oversized replays, output schemas, and A5 payload limitations.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

Possibly related PRs

Poem

A rabbit found bytes in a tensor-lined trail,
Chose only the slots that the filters unveil.
With memcpy hops and zeroes in tow,
The replay now knows which real controls flow.
“A5,” said the hare, “wait for your fix—
For now, metadata does the tricks!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.88% 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: restoring selected args payloads in L0 replay.
Description check ✅ Passed The description matches the PR scope, covering payload filters, replay restoration, msprof timeout, and A5 limitations.
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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
simpler_setup/scene_test.py (1)

1703-1715: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

--rounds > 1 disable leaks stale payload-filter CLI args into dispatched children, crashing them.

This block clears args.dump_args and args.dump_args_payload_filter when rounds > 1, but not args.dump_args_payload_func_id / args.dump_args_payload_slot. Those two raw attributes are what _dispatch_test_phases_standalone's common list construction (Lines 1899-1904) actually forwards to spawned child subprocesses — independently of args.dump_args_payload_filter. Since if args.dump_args: is now false, --dump-args is omitted from common, but --dump-args-payload-func-id/--dump-args-payload-slot are still appended (their own guards only check the raw attrs). Each child re-parses argv and calls _resolve_dump_args_payload_filter(0, <leaked func-id>, <leaked slots>), which raises ValueError("... requires --dump-args 3")parser.error()SystemExit(2), failing every L2 fan-out job instead of silently disabling dump-args as intended (and as already happens correctly on the pytest side, and on the single-group inline-execution path which reuses the already-cleared args.dump_args_payload_filter).

🐛 Proposed fix
         if args.rounds > 1 and args.dump_args:
             logger.warning("Dump args disabled: --rounds > 1")
             args.dump_args = 0
             args.dump_args_payload_filter = None
+            args.dump_args_payload_func_id = None
+            args.dump_args_payload_slot = []
🤖 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 `@simpler_setup/scene_test.py` around lines 1703 - 1715, When disabling
dump-args for args.rounds > 1 in the argument-normalization block, also clear
the raw args.dump_args_payload_func_id and args.dump_args_payload_slot
attributes used by _dispatch_test_phases_standalone when building its common
child-argument list. Keep the existing dump_args_payload_filter reset so spawned
children receive no stale payload-filter options and dump-args remains fully
disabled.
🤖 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.

Inline comments:
In `@docs/dfx/l0-swimlane-profiling.md`:
- Around line 683-685: Update the replay-data statement in the profiling
documentation to say that only unselected tensor payloads are zero-filled;
preserve the clarification that scalar values and tensor descriptors retain
dump-derived values.
- Around line 71-72: Update the tensor restoration scope statement in the
profiling documentation to include both a2a3 and a2a3sim, while preserving the
existing availability of a5 metadata-only and zero-filled replays.

In `@src/a5/platform/sim/host/device_runner.cpp`:
- Around line 694-697: Update the A5 initialization path in device_runner around
dump_collector_.initialize to prevent selective payload capture: pass the
disabled payload-filter value instead of dump_args_payload_filter_, or reject
enabled filters with a clear error before initialization. Preserve the existing
collector setup for all other dump options.

---

Outside diff comments:
In `@simpler_setup/scene_test.py`:
- Around line 1703-1715: When disabling dump-args for args.rounds > 1 in the
argument-normalization block, also clear the raw args.dump_args_payload_func_id
and args.dump_args_payload_slot attributes used by
_dispatch_test_phases_standalone when building its common child-argument list.
Keep the existing dump_args_payload_filter reset so spawned children receive no
stale payload-filter options and dump-args remains fully disabled.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d4123f79-b2b6-4407-8c63-b7d6ec6bba38

📥 Commits

Reviewing files that changed from the base of the PR and between 321954b and d077787.

📒 Files selected for processing (30)
  • .claude/skills/l0-swimlane/SKILL.md
  • conftest.py
  • docs/dfx/args-dump.md
  • docs/dfx/l0-swimlane-profiling.md
  • python/bindings/task_interface.cpp
  • python/simpler/worker.py
  • simpler_setup/scene_test.py
  • simpler_setup/tools/dump_viewer.py
  • simpler_setup/tools/l0_swimlane.py
  • src/a2a3/platform/onboard/host/device_runner.cpp
  • src/a2a3/platform/sim/host/device_runner.cpp
  • src/a5/platform/onboard/host/device_runner.cpp
  • src/a5/platform/sim/host/device_runner.cpp
  • src/common/platform/include/common/args_dump.h
  • src/common/platform/include/host/args_dump_collector.h
  • src/common/platform/onboard/host/device_runner_base.cpp
  • src/common/platform/onboard/host/device_runner_base.h
  • src/common/platform/shared/aicpu/args_dump_aicpu.cpp
  • src/common/platform/shared/host/args_dump_collector.cpp
  • src/common/platform/sim/host/device_runner_base.cpp
  • src/common/platform/sim/host/device_runner_base.h
  • src/common/task_interface/args_dump_config.h
  • src/common/task_interface/call_config.h
  • tests/st/a2a3/tensormap_and_ringbuffer/dfx/args_dump/test_args_dump.py
  • tests/ut/cpp/a2a3/test_args_dump.cpp
  • tests/ut/cpp/common/test_args_dump_collector.cpp
  • tests/ut/cpp/types/test_call_config.cpp
  • tests/ut/py/test_chip_worker.py
  • tests/ut/py/test_l0_swimlane.py
  • tests/ut/py/test_scene_test_dump_args.py

Comment thread docs/dfx/l0-swimlane-profiling.md
Comment thread docs/dfx/l0-swimlane-profiling.md Outdated
Comment thread src/a5/platform/sim/host/device_runner.cpp
@indigo1973
indigo1973 force-pushed the l0_0727 branch 2 times, most recently from b9a5155 to c00bb84 Compare July 29, 2026 08:13
- Add task func-id and argument-slot filters for level-3 dumps.
- Restore selected before-dispatch tensor bytes in L0 replays.
- Disable payload selectors with dump args for multi-round fan-out.
- Preserve A5 selective capture while hw-native-sys#1560 makes its payload and restored
  L0 traces untrustworthy.
- Expose the msprof timeout and document oversized CAModel cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant