[agent, ci, trainer, dist, ops, optim] refactor: add anti-bloat review rules and prune hypothetical defenses

.agents/knowledge/constraints.md

@@ -253,9 +253,7 @@ Core files:
 
 28. **In-place collective reductions in backward must own their gradient buffer**
     - Autograd can pass the same incoming gradient to multiple branches. `.contiguous()` does not copy an already contiguous tensor, so reducing that tensor in place can silently change a sibling branch's gradient and the caller's `grad_outputs`.
-    - `_Gather.backward` uses NCCL reduce-scatter for nonempty real gradients with positive shard sizes. Equal shards use rank-major stacked tensor input to avoid the list API's internal flatten; uneven shards use the list path. Borrowed inputs require a separate output. Owned packed inputs may reuse the local rank's slice only where the old contiguous all-reduce result would also retain full storage; otherwise keep compact local storage. This avoids adding a local output allocation on top of a required full packing buffer. Other backends and complex/empty inputs retain an owned contiguous all-reduce buffer. `_GatherConcatSP.backward` also owns its in-place reduction buffer.
-    - Collective selection must agree across ranks: negative-view flags and strides may differ by rank, so materialize them locally without changing the chosen collective. Preserve scaling before summation (FP16 overflow makes the order observable). The no-sum path scales only the local slice. Regression tests in `tests/parallel/ulysses/test_all_gather.py` cover shared gradients, edge cases, and local output storage with real Gloo/NCCL collectives where available.
-    - A regression's reference collective must also use a contiguous buffer for NCCL. Make only the reference clone contiguous; preserve the layout of the actual incoming gradient so transposed, narrowed and expanded inputs remain covered.
+    - `_Gather.backward` uses out-of-place NCCL reduce-scatter (`reduce_scatter_tensor` for equal shards, `reduce_scatter` for uneven shards) into a freshly allocated local `grad_input` buffer, and clones `grad_output` (`grad_output.clone(memory_format=torch.contiguous_format)`) before running an in-place `dist.all_reduce` on other backends (`_GatherConcatSP.backward` likewise clones before `dist.all_reduce`). Regression tests in `tests/parallel/ulysses/test_all_gather.py` cover shared gradients across layouts for `_Gather`.
 
 29. **Saved RoPE table metadata must stay stable across checkpoint recomputation**
     - FSDP2 can cast fixed vision cos/sin tables to BF16 on the first block forward while non-reentrant checkpoint recomputation sees the original FP32 tables. Saving them unchanged makes checkpoint backward raise a metadata mismatch.

.agents/knowledge/testing.md

@@ -46,34 +46,10 @@ scans over `tasks`, `veomni`, and `tests`; the pytest guards alone do not cover
 that workflow. CPU tests checking device-name forwarding should use a mock
 backend sentinel when no real hardware API is involved.
 CPU-planned worker cases scope `torch.utils.checkpoint.DefaultDeviceType` to
-`cpu` and restore it afterward. On Ascend, CPU-only checkpoint inputs otherwise
-fall back to the registered NPU and AMP capability queries can initialize a
-hidden device. Preserve this scope across fixture setup and teardown as well
-as the test body; accelerator plans and full-visibility collection stay unchanged.
-During that scope, optimizer foreach-capability queries use the canonical
-`torch.utils._foreach_utils` helper. torch_npu 2.10 replaces the optimizer's
-alias with a device-name lookup that initializes NPU even for CPU parameters.
-Restore the prior alias on exit, preserving explicit foreach/fused options.
-The restoration scope includes setup itself, so partial initialization failures
-also undo changes already applied. Unsupported vendor isolation interfaces
-raise an explicit error before the test runs.
-torch_npu 2.10 also replaces the multi-worker DataLoader iterator constructor
-with unconditional NPU synchronization/affinity calls, even for CPU datasets.
-CPU-planned cases temporarily use the vendor module's saved Torch constructor
-and restore the patch after teardown or failure. Only replace the known vendor
-wrapper; preserve unrelated constructor patches and all accelerator plans.
-The vendor Inductor wrapper also registers its NPU backend before compiling
-CPU graphs. CPU cases temporarily disable that registration through the loaded
-vendor registry and restore its prior disabled state after teardown or failure.
-Keep real CPU Inductor compilation and gradient checks enabled; do not replace
-compiled-mask coverage with eager execution to avoid hidden-device queries.
-CPU cases must also register Torch's built-in CPU device-op overrides when
-missing. torch_npu populates the override registry at import; Torch 2.10 then
-skips all built-ins because the registry is nonempty. Torch 2.11 repairs missing
-keys, so testing only that version misses the NPU CI failure. Preserve existing
-CPU/vendor entries and validate the pinned Torch version's startup state.
-A synthetic registered backend alone misses vendor monkeypatches: exercise
-the pinned wheel's optimizer patch in CPU isolation audits too.
+`cpu`, restore Torch's query-free optimizer foreach helper, register the built-in
+CPU device-op overrides when `torch_npu` left them out, and bypass `torch_npu`'s
+DataLoader affinity and Inductor NPU registration hooks while devices are hidden,
+restoring the prior settings afterward.
 
 CPU numerical flakes need fresh-process reproduction, not only warm repeats.
 The QSA CPU fused-kernel mock promotes only `exp` to FP64 and casts back:
@@ -142,11 +118,6 @@ Consequences that cut both ways:
 The accelerator runner cancels running peers on the first failure by default;
 `--keep-going` continues all cases. Foreground logs must identify case progress,
 the original failure and cancelled peers without conflating their outcomes.
-Accelerator runner thread pools must cancel owned worker scopes before waiting
-for callbacks on interruption. Preserve scoped process cleanup and failure
-artifacts; do not terminate unrelated device users or rely on the batch timeout.
-Use future completion to prove callbacks drained: an interrupted `Thread.join`
-can report a stopped thread before its callback finishes cleanup.
 
 ## What the NPU job can actually run
 

.agents/skills/veomni-develop/SKILL.md

@@ -33,6 +33,7 @@ When restructuring code (same behavior, better structure):
 - Data collators in `veomni/data/data_collator.py` are coupled to `DEFAULT_DATA_COLLATE_INFO` — adding new tensor keys requires updating the collate info table.
 - `MainCollator` has **strict SP ordering** (pad → slice → FA kwargs → slice position_ids) — reordering breaks SP correctness.
 - `position_ids == 0` marks segment boundaries for FA varlen — any transform that produces position_ids must preserve this convention.
+- **Avoid the "AI self-inflicted defense loop"**: Do not add defensive code or synthetic unit tests for hypothetical scenarios outside VeOmni's runtime contract (e.g., `asyncio`/multi-threaded concurrency on synchronous trainer state, SP/DP ranks disagreeing on config or shared batch metadata, internal hooks maliciously mutating tensors, 0-token samples, `complex64`/`_neg_view` activations, or hostile OS/import-machinery tampering). Keep fixes minimal and proportional to the real bug or feature; see `/veomni-review`.
 
 ## Documentation
 

.agents/skills/veomni-review/SKILL.md

@@ -65,6 +65,31 @@ branch you took, so the reader knows a review happened or why it didn't.
 5. Run `make quality` before pushing. (It gates every commit anyway, but this
    is the last chance before the diff is public.)
 
+## Anti-Pattern: "AI Self-Inflicted Corner-Case & Defensive Bloat"
+
+A recurring failure mode in AI-authored PRs is the **"self-inflicted defense
+loop"**: during self-review, an agent invents hypothetical edge cases ("What if
+an `asyncio` task or background thread calls this hook across steps?", "What if
+a user's dataset has a key matching our internal private key?", "What if
+`metadata_collate_func` secretly flips `input_ids`?", "What if a sample has 0
+tokens?", "What if rank 0 has a `torch._neg_view` bit flag and rank 1 does
+not?") and then adds a unit test plus hundreds of lines of locks, SHA-256
+hashing, preflight collectives, and tensor-cloning guards for each imagined
+scenario — ballooning a 40-line change into 3,000+ lines.
+
+Both the outer agent and the review subagent must enforce two rules:
+
+1. **Do NOT invent hypothetical corner cases that violate VeOmni's runtime contract**:
+   - Training is **synchronous and single-threaded per rank** (`forward -> backward -> step`). Do not ask for thread locks, `asyncio` task ownership checks, or epoch tokens on trainer/model/callback state.
+   - Ranks in the same SP/DP/EP group run the **same config, same model code, and (within an SP group) slice the same deterministic micro-batch**. Do not ask for cross-rank SHA-256/pickle preflight collectives to verify that ranks agree on config flags or batch metadata.
+   - Internal hooks (`metadata_collate_func`, model/callback hooks) and collators are **trusted in-process code**, not adversarial inputs. Do not clone tensors before/after hooks to check for mutation or guard against user keys colliding with private `_veomni_*` keys.
+   - Dataset preprocessing already filters empty samples and builds monotonic `position_ids` (`0..L-1`) per sample. Do not demand provenance state machines or dynamic-programming solvers for 0-token samples or mid-sample `position_ids == 0` resets.
+   - Activations and gradients in LLM/VLM/DiT training are real floating-point tensors. Do not demand handling for `complex64`, conjugate views, rank-asymmetric C++ `_neg_view` flags, deliberate FP16 pre-sum overflow to `NaN`, or manual `untyped_storage().resize_()` aliasing.
+   - Do not demand defenses for hostile OS/Python environments (custom `sys.meta_path` finders, broken consecutive namespace packages, source files mutated mid-CI run, all-zeros `/proc/sys/kernel/random/boot_id` UUIDs) or wrong Python types (`bool` passed as `int`, generators passed as lists) on internal helpers already behind typed config dataclasses.
+2. **Flag speculative defensive bloat and synthetic tests as `needs-attention` (for deletion)**:
+   - When a diff contains defensive scaffolding, state machines, extra collectives, or synthetic unit tests for any of the hypothetical scenarios above (or meta-tests that parse AST/source text or test another test's `pytest.skip`), flag them as `needs-attention` and require their **removal**.
+   - Never promote a test-invented corner case into `.agents/knowledge/constraints.md` or `.agents/knowledge/testing.md`.
+
 ## Subagent Launch
 
 Launch a subagent with this prompt. Use whatever the running agent calls it —
@@ -85,11 +110,21 @@ You are a code reviewer for VeOmni, a distributed multi-modality training framew
 For each changed file, check:
 
 ### Implementation Quality
-- Hidden risks or edge cases not handled?
-- Simpler alternative that achieves the same result?
+- Hidden risks or edge cases in *real* training workflows not handled? (Do NOT invent hypothetical corner cases outside VeOmni's runtime invariants — see "Anti-Bloat & Proportionality" below.)
+- Simpler alternative that achieves the same result with substantially less code?
 - Boundary conditions (tensor shapes, distributed rank handling, gradient accumulation steps)?
 - Does the fix depend on downstream code to "clean up"?
 
+### Anti-Bloat & Proportionality (CRITICAL)
+- Does the diff add speculative defensive machinery or synthetic unit tests for scenarios that cannot happen in VeOmni's synchronous distributed training runtime? Flag any of the following as `needs-attention` and recommend **deleting** them:
+  - Thread locks, `asyncio` task/thread-ID ownership checks, or epoch tokens on synchronous trainer/callback/op state.
+  - Cross-rank SHA-256 digests, `pickle` serialization, or extra preflight `all_gather` / `all_reduce` collectives defending against SP/DP ranks having mismatched configs or corrupted batch metadata.
+  - Tensor-cloning guards checking whether internal hooks (`metadata_collate_func`, etc.) mutated inputs, or custom subclasses defending against user dataset keys colliding with private internal keys.
+  - Elaborate solvers or provenance tracking for 0-token samples, mid-sample `position_ids` resets, or multi-row packed batches (PackingCollator always packs into 1 row).
+  - Handling or testing `complex64`, `.conj()`, rank-asymmetric `torch._neg_view`, deliberate FP16 overflow to `NaN`, custom `sys.meta_path` finders, or `isinstance(x, bool)` on internal `int` helpers.
+  - Meta-tests that unit-test a test helper's own AST visitor, use `inspect.getsource` to match substrings in trainer methods, or test another test function's `pytest.skip`.
+- Is the diff size proportional to the problem solved? If a 40-line fix is accompanied by 500+ lines of defensive scaffolding and synthetic tests, require stripping it down to the essential change.
+
 ### Multi-file Consistency
 - If a Trainer method changed, do all subclasses need matching changes?
 - If model loading changed, are configs and parallel plans updated?

docs/design/ci_case_scheduling.md

@@ -200,18 +200,16 @@ or the accelerator runtime. Its default env:// port uses the legacy run-scoped
 locked registry, which prevents cooperative reuse but does not hold an OS
 reservation. Distributed spawn launches instead own a live TCPStore from
 allocation through worker cleanup; standalone torchrun agents bind their own
-Stores atomically on port zero. Spawn lifecycle events are retained in each
-batch's `.rendezvous.jsonl` with the owner PID, case, launch ID and endpoint.
+Stores atomically on port zero.
 Common launchers validate actual `nproc`, node count, and child device visibility
-against the plan and lease. Compare observed launch arguments with the baseline
-to verify backend and parallel-topology coverage. Release resources only after scoped
+against the plan and lease. Release resources only after scoped
 cleanup of the launcher's process group and tracked descendants, including
 when its group leader has already exited. By default the first failed pytest
 phase or worker stops new dispatches and cancels running peers in the same
 suite/job. Wait for owned process cleanup, then return failure. `--keep-going`
 continues all cases and batches without cancelling peers.
 If owned processes survive cleanup, stop admission and cancel running peers
-even with `--keep-going`; the affected device lease must not be reused.
+even with `--keep-going`; the affected worker process scope cannot be safely reused.
 
 On Ctrl-C, collection and scheduling cancel their owned subprocess scopes before
 waiting for thread-pool callbacks. Workers then use the existing bounded
@@ -235,7 +233,7 @@ Failure output includes the node ID and pytest traceback summary, followed by
 the failed worker's complete combined stdout/stderr. Actions groups this output
 under the manifest entry ID so training-rank tracebacks remain accessible when
 pytest reports an outer subprocess error. Local runs print the same output
-inline. Stream the existing log in bounded chunks after worker cleanup, while
+inline. Read and write the full worker log in one call after worker cleanup, while
 successful batches retain compact progress output.
 Cancelled batches are distinguished from test failures, and the original
 failure remains the final error even if a cancelled peer exits first. Preserve
@@ -309,7 +307,7 @@ remain part of test collection.
 - Exercise defaults, parameterized plans, CPU workers, sequential/concurrent
   case peaks, mismatches, starvation prevention, fixture isolation, and cleanup.
 - Run the full unit/e2e selections, including conditional coverage. Preserve
-  per-case timings, actual wall time, device assignments, logs/JUnit, source
-  fingerprints, and cache/environment limits.
+  per-case timings, actual wall time, device assignments, logs/JUnit, and
+  cache/environment limits.
 - Run serial and concurrent GPU/NPU adapter checks on their respective hardware;
   record any NPU validation gap explicitly.

scripts/ci/accelerator/README.md

@@ -100,9 +100,6 @@ cooperating callers in one run, including default worker environments and
 remaining in-process env:// fixtures. It does not reserve an OS socket or
 coordinate other runs. New distributed launchers must use a live Store or
 standalone agent instead of probing and closing a socket before startup.
-Spawn endpoint ready/closed events include case ID, launch ID, owner PID and
-port in each batch's `.rendezvous.jsonl` artifact. This fixes rendezvous
-allocation races; Gloo/NCCL/HCCL transport sockets have their own lifecycles.
 Visibility is set before importing torch,
 and launch validation checks nproc and child device assignments against the
 case plan and lease. Every distributed launch starts fresh workers.
@@ -117,8 +114,8 @@ other running batches in this suite/job. The runner waits for bounded process
 cleanup before exiting with failure. It does not wait for those tests to finish.
 `--keep-going` records failures and attempts all cases/batches without cancelling
 peers. Failure to drain owned processes always stops admission and cancels
-running peers, even with `--keep-going`, because the affected device lease
-cannot be safely reused. `--serial` uses the same failure policy with one batch at a time.
+running peers, even with `--keep-going`, because the affected worker process
+scope cannot be safely reused. `--serial` uses the same failure policy with one batch at a time.
 CPU BLAS and compiler threads are bounded.
 Explicit CPU cases also scope checkpoint's fallback device to CPU through
 fixture setup, execution and teardown, restoring the previous value afterward.
@@ -137,9 +134,7 @@ Inductor registry, restoring its previous policy afterward. CPU graphs still
 compile with Inductor; accelerator cases keep normal NPU registration.
 CPU cases initialize a missing built-in CPU device-op override as well. This
 handles Torch 2.10's partially populated registry after importing torch_npu,
-while preserving existing CPU and vendor overrides. Setup is also covered by
-the restoration scope: partial setup failures restore every state already
-changed. Missing vendor isolation interfaces fail explicitly before test execution.
+while preserving existing CPU and vendor overrides.
 
 ## Results and local commands
 
@@ -149,8 +144,8 @@ console log; they do not upload the output directory as a GitHub artifact.
 Full result files are available in the job workspace while it exists.
 Use a fresh `--output-dir` for each local measurement.
 Result files include full case inventory, resolved plans, batches, device
-assignments, per-case timings/outcomes, logs, JUnit XML, observed distributed
-launches, source fingerprints and the suite's elapsed wall time. Sum of batch
+assignments, per-case timings/outcomes, logs, JUnit XML, and the suite's
+elapsed wall time. Sum of batch
 or case durations is not elapsed wall time when work overlaps. Measured timings,
 including `durations.json`, are reporting artifacts and are not scheduling inputs.
 
@@ -190,9 +185,9 @@ Failures print the node ID and pytest traceback summary (up to 200 lines),
 followed by the complete failed worker's combined stdout/stderr. GitHub Actions
 places this output in a collapsible group labelled with the manifest entry ID;
 local runs print it inline. This preserves training-rank tracebacks and torchrun
-errors that precede pytest's outer `CalledProcessError`. Logs are streamed in
-bounded chunks after worker cleanup; successful batches keep compact progress
-output. GitHub Actions also receives an error annotation. Cancelled
+errors that precede pytest's outer `CalledProcessError`. The full worker log is
+read and written in one call after worker cleanup; successful batches keep
+compact progress output. GitHub Actions also receives an error annotation. Cancelled
 batches are labelled separately from failed tests. The final summary reports
 outcomes, failed/cancelled batches, batches never started and the first failure.