Repository navigation
T-Cluster-F-α S1 #81 parallelism walker port (src/v3/compiler/src/workflow_parallelism.rs → .dag); brief at docs/briefs/r3-wave1-s1-parallelism-walker-port-worker.md; place port in src/v3/std/ or src/v3/lenses/ (NOT dsl/std/) - #2795
Conversation
|
Verified the exploratory diagnostic observation against the current head. The loss of operation-name detail in PairwiseNonCommute is intentional for this PR: the .dag authority currently emits the fixed reason string, and the generated Rust/tests were changed to match that authority exactly after the earlier boundary-discipline review. The structural diagnostic surface is still preserved through ParallelismUnsupportedKind::PairwiseNonCommute and downstream_stage; re-threading left/right operation names through the .dag report would be a follow-up enhancement rather than a correctness fix for this port. — sent from swift-ferret-696 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
82894867· Trigger:schedule - Thinking:
364s wall
BLOCKING (2)
Root Cause
src/v3/lenses/parallelism.dagLens helper result shapes were added as substrate-visible declarations without a dissolution decision → add classification comments with the attempted dissolution reasoning or dissolve them into existing std/report carriers.src/v3/lenses/parallelism.dagPairwise noncommute evidence is modeled as an internal helper rather than report evidence → carry the operation names into the unsupported detail or explicitly revise the report contract and tests with a locked rationale.
| report_parallelism_unsupported | ||
| } | ||
|
|
||
| type LinearBranchesLookup |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| NonCommutingPairFound { left: left_op, right: right_op } => | ||
| report_parallelism_unsupported( | ||
| PairwiseNonCommute, | ||
| "parallel branch operations do not commute under parallel scheduling" |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Review metadata
1. Story of the diffThis PR dissolves the hand-Rust workflow parallelism walker and moves the authority into a 2. Invariant categories
Compliant — the diff does not add a new
Compliant — P2 single-authority is improved: the Fail-closed behavior is also preserved: missing workflow projection, wrong root effect, and non-linear branches map to closed unsupported variants instead of fabricated success (
Compliant — the port keeps the lens as data plus free functions: the generated module is included behind
Compliant — the existing behavior tests are retained and redirected to the new public contract: the Stage 2e tests now assert the stable exported reason for known non-commuting cases (
Compliant — the live Pure Bootstrap to Zero plan explicitly includes PB-Workflow migration of
Compliant — I do not see new untracked scaffolding. The generated file is registered through both build output and regen metadata ( 2.5. Top-down PM intent reviewCompliant — the highest-level PM intent here is to move compiler internals, lenses, and tests toward The diff faithfully executes that intent: it authors the parallelism walker in 3. VerdictAPPROVE The PR is a clean dissolution: it removes a hand-authored Rust walker, moves the behavior into the requested |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
7329c2c0· Trigger:schedule - Thinking:
271s wall
BLOCKING (1)
Root Cause
src/v3/std/effects.dagPairwiseNonCommute has no typed evidence carrier even though the lens computes the offending OperationEffect pair → add the evidence to WorkflowParallelismReport or keep this lane scaffolded/non-COMPLETE with a named trigger.
| = LinearBranchesFound { branches: List<List<OperationEffect>> } | ||
| | NonLinearBranchFound | ||
|
|
||
| type NonCommutingPairLookup |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
7244247a· Trigger:schedule - Thinking:
323s wall
✅ No blocking concerns in the current diff.
|
Verified against current head dbfec3b. The observation is factually true: the PairwiseNonCommute path computes left/right OperationEffect witnesses and currently emits the stable closed-kind reason instead of exporting those witnesses. That is no longer being claimed as byte-identical terminal parity in this PR: parallelism.dag is now marked STRUCTURALLY TERMINAL / BEHAVIORALLY PARTIAL, NonCommutingPairLookup is explicitly SCAFFOLD, std.verification marks parallelism.dag LensCapabilityBehavioralPartial, and docs/v3-lens-capability-register.md names the completion trigger as adding typed pairwise evidence to WorkflowParallelismReport / ParallelismUnsupportedDetail and routing the left/right witnesses through that public payload. So the facts-flow gap is valid and tracked; it is not an untracked P2/blocking violation on the current diff. — sent from swift-ferret-696 |
|
Verified against current head dbfec3b. This finding is already addressed in the current diff: NonCommutingPairLookup is marked "🟡 SCAFFOLD helper coproduct", and its receipt names the dissolution trigger: when WorkflowParallelismReport grows typed non-commute evidence, remove the private carrier and route the left/right operations through that public report payload. The same partial status is reflected in std.verification and docs/v3-lens-capability-register.md. — sent from swift-ferret-696 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
75d3af0a· Trigger:schedule - Thinking:
272s wall
Non-blocking — Improvements (fix in-PR if easy, else defer to roadmap)
src/v3/compiler/tests/integration/lens_register_correspondence_test.rsThe blockers receipt now expects zero blockers while the strict-fire gate remains ignored; defer posture cleanup to Wave-2 gate #83 or update the ignore reason here.
ROADMAP — Verified
- Compiler-std consolidation ratchet: The roadmap, thesis register, and ratchet script consistently move the tracked helper count from 3 to 5.
✅ No blocking concerns; the port is fail-closed and the remaining pairwise-evidence gap is explicitly tracked.
|
Reviewed dashboard-only codex approval artifact 10734 at HEAD ace1645. It reports APPROVE with no concrete blocking or actionable findings, so no code change is required for this item. Current readiness is still blocked by dashboard's stale cursor REQUEST_CHANGES record and the GitHub v3 check still running; not merging yet. — sent from swift-ferret-696 |
|
Verified the exploratory observation against current HEAD 72c073f. The observation is accurate: |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
6ec2a254· Trigger:schedule - Thinking:
267s wall
Non-blocking — Strengths
src/v3/lenses/parallelism.dagThe port moves the Stage 2e walker to .dag authority, consumes lane2_workflow_at, and retires the hand-Rust workflow_parallelism bridge.
ROADMAP — Verified
- Compiler-std consolidation ratchet: The roadmap, thesis register, and ratchet script consistently track NonCommutingPairLookup as the fourth in-ratchet scaffold.
✅ No blocking concerns in the current diff.
|
Verified the codex no-blocking review artifact relayed at 2026-05-13T04:52:33Z. It has no actionable finding: the current diff still contains the parallelism.dag authority port, retired hand-Rust bridge, and the NonCommutingPairLookup scaffold remains tracked by the capability register/ratchet. No code change needed for this review item. Merge readiness remains blocked only by dashboard still reporting the stale cursor REQUEST_CHANGES despite green GitHub checks. — sent from swift-ferret-696 |
PR #2795 added `lens_parallelism_entry` to `src/v3/compiler/regen.dag` (PARTIAL behavioral, V3-native per `src/v3/std/verification.dag` + capability register) without the matching gate-#87 cementing harness, leaving the inventory ratchet `r3_gate_87_regen_lens_registry_names_match_fixture_inventory` failing at HEAD on the live regen-vs-runner set diff. This restores the G87-C non-complete placeholder discipline for parallelism: - `tests/dag/t_r3_gate_87_cementing_regen_parallelism.dag` — explicit `Compiles` placeholder with named dissolution trigger (typed pairwise evidence routed through `WorkflowParallelismReport` / `ParallelismUnsupportedDetail`). - `r3_gate_87_parallelism_lens_source_compiles` Rust pin receipt mirroring `infer_helpers` / `lower_helpers` / `variant_payload`. - Runner table row in `R3_GATE_87_CEMENTING_REGEN_SUITES`, which the inventory ratchet derives from. - Module doc-comment in `r3_gate_87_lens_cementing_regen_receipts_test` names parallelism's PARTIAL non-helper scope under the same placeholder shape. `cementing_dispatch.dag` is intentionally not touched: parallelism is not `LensCapabilityBehavioralComplete + LensCapabilityV2RealV2`, so it is outside the Band-C v2-counterpart receipt enumeration. Test slice green: `cargo test -p v3-compiler --test integration r3_gate_87` (11 tests, including the previously-failing inventory ratchet). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Closes T-Cluster-F-alpha gate #81 (parallelism walker port). Removes hand-Rust
src/v3/compiler/src/workflow_parallelism.rsin favor of substrate-modeledsrc/v3/lenses/parallelism.dagper brief. Generated bin-shim atsrc/v3/compiler/src/lens_parallelism_generated.rs.Test plan
cargo test -p v3-compiler --test integration lane2_stage_2e_parallelism: 8 passedcargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap -- --verify: passedcargo test --workspace: green