Support: restore selected args payloads in L0 replay - #1567
Conversation
indigo1973
commented
Jul 29, 2026
- 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.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI 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:
📝 WalkthroughWalkthroughAdds selective level-3 tensor payload capture and ChangesSelective dump and replay
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
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 > 1disable leaks stale payload-filter CLI args into dispatched children, crashing them.This block clears
args.dump_argsandargs.dump_args_payload_filterwhenrounds > 1, but notargs.dump_args_payload_func_id/args.dump_args_payload_slot. Those two raw attributes are what_dispatch_test_phases_standalone'scommonlist construction (Lines 1899-1904) actually forwards to spawned child subprocesses — independently ofargs.dump_args_payload_filter. Sinceif args.dump_args:is now false,--dump-argsis omitted fromcommon, but--dump-args-payload-func-id/--dump-args-payload-slotare 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 raisesValueError("... 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-clearedargs.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
📒 Files selected for processing (30)
.claude/skills/l0-swimlane/SKILL.mdconftest.pydocs/dfx/args-dump.mddocs/dfx/l0-swimlane-profiling.mdpython/bindings/task_interface.cpppython/simpler/worker.pysimpler_setup/scene_test.pysimpler_setup/tools/dump_viewer.pysimpler_setup/tools/l0_swimlane.pysrc/a2a3/platform/onboard/host/device_runner.cppsrc/a2a3/platform/sim/host/device_runner.cppsrc/a5/platform/onboard/host/device_runner.cppsrc/a5/platform/sim/host/device_runner.cppsrc/common/platform/include/common/args_dump.hsrc/common/platform/include/host/args_dump_collector.hsrc/common/platform/onboard/host/device_runner_base.cppsrc/common/platform/onboard/host/device_runner_base.hsrc/common/platform/shared/aicpu/args_dump_aicpu.cppsrc/common/platform/shared/host/args_dump_collector.cppsrc/common/platform/sim/host/device_runner_base.cppsrc/common/platform/sim/host/device_runner_base.hsrc/common/task_interface/args_dump_config.hsrc/common/task_interface/call_config.htests/st/a2a3/tensormap_and_ringbuffer/dfx/args_dump/test_args_dump.pytests/ut/cpp/a2a3/test_args_dump.cpptests/ut/cpp/common/test_args_dump_collector.cpptests/ut/cpp/types/test_call_config.cpptests/ut/py/test_chip_worker.pytests/ut/py/test_l0_swimlane.pytests/ut/py/test_scene_test_dump_args.py
b9a5155 to
c00bb84
Compare
- 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.