Skip to content

T-24 Wave-0: ci.dag affected-set authority — dissolve detect-affected-components.sh (ci.yml emit deferred) - #3853

Merged
briansrls merged 63 commits into
mainfrom
session/wise-otter-34
May 29, 2026
Merged

briansrls merged 63 commits into
mainfrom
session/wise-otter-34

Conversation

@briansrls

@briansrls briansrls commented May 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

T-24 Wave-0: dissolve scripts/detect-affected-components.sh and make src/v4/workflow/ci.dag the single authority for CI component path buckets, git-diff fail-closed behavior, and self-hosted M1 cargo-check parallelism.

  • Deleted scripts/detect-affected-components.sh; added detect-ci-affected-components host transport plus Rust mirrors (v4_ci_component_affected.rs, v4_ci_runner_pool.rs) with explicit dissolution triggers.
  • Extended ci.dag: CiComponentAffected, CiGitDiffReadOutcome, workflow_policy predicates, runner pools, M1 probe wiring; fixed ReadFailClosed match pattern for v4 compile.
  • Aligned .github/workflows/ci.yml / dsl/gunbc/ci_github_actions_workflow.dag affected job and V4_M1_CARGO_CHECK_JOBS projection with modeled facts.
  • Ratchets: claim .dag receipts, smoke test cross-checks ci.dag workflow_policy prefixes against the Rust mirror.

Full YamlStatic emission of ci.yml from ci.dag remains deferred per src/v4/TASKS.md T-24 (committed YAML is still interim authority until emission slice lands).

Test plan

  • cargo test -p v3-compiler --test integration v4_workflow_ci_runner_dag_smoke_test -- --quiet
  • cargo test -p v3-compiler v4_ci_component_affected -- --quiet
  • cargo test -p v3-compiler --bin detect-ci-affected-components (unit tests in module)
  • CI: affected job uses detect-ci-affected-components; Lens-CI v4 compile over ci.dag entry root
  • CI green on HEAD (reviews re-running)

SG-0 / P5 receipt

SG-0 hand-path delta: +3

SG-0 pairing: (c) Net +3 on EXPECTED_HAND_AUTHORED_NON_TEST for T-24 host-transport shims (detect_ci_affected_components.rs, v4_ci_component_affected.rs, v4_ci_runner_pool.rs) with matching INVARIANTS §P5(b) rows. Follow-up dispatch: dissolve when ci.dag eval / generated CI runner owns affected-set and M1 parallelism without parallel Rust path logic (T-24 emission slice; tracked gunbc#3853).

@briansrls
briansrls marked this pull request as ready for review May 28, 2026 23:55
Add v4_ci_runner_pool to ci_changed_path_affects_workflow_policy so the
detect-ci-affected-components host transport matches ci.dag authority.
Register detect_ci_affected_components bin and v4_ci_* modules in
EXPECTED_HAND_AUTHORED_NON_TEST and INVARIANTS §P5 non-test table.

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Addressed both REQUEST_CHANGES findings in f39cf8cd9:

  1. workflow_policy path parity — Added src/v3/compiler/src/v4_ci_runner_pool to ci_changed_path_affects_workflow_policy in v4_ci_component_affected.rs (matching ci.dag:675) and a workflow_policy_paths_match_ci_dag_authority unit test so the Rust host transport cannot drift from the .dag predicates.

  2. P5 / SG-0 census — Registered src/v3/compiler/src/bin/detect_ci_affected_components.rs, src/v3/compiler/src/v4_ci_component_affected.rs, and src/v3/compiler/src/v4_ci_runner_pool.rs in EXPECTED_HAND_AUTHORED_NON_TEST with matching INVARIANTS §P5 non-test table rows (T-24 host-transport receipts). sg0_v3_hand_authored_census passes locally.

— sent from wise-otter-34

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Review metadata

  • Provider / model: codex / unknown
  • Commit: e948ea5f · Trigger: schedule
  • Thinking: 233s wall

BLOCKING (2)

Root Cause

  • src/v4/workflow/ci.dag Git diff acquisition is modeled only as GitDiffNameOnly data with no failure carrier → add a typed read outcome/diagnostic path and make the host transport exit nonzero or mark all components affected on read failure.
  • src/v4/workflow/ci.dag Runner-pool arithmetic lacks cardinality/spread evidence at the API boundary → require a non-empty/non-uniform pool witness or totalize the fanout calculation before exposing ci_m1_probe_cargo_check_jobs_from_pools.

⚠️ The PR is aligned with the workflow-as-data direction, but these fail-closed and totality gaps should be fixed before merge.

"warning: git diff --name-only {range} exited {}; treating as no changes",
out.status
);
Vec::new()

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Already addressed on HEAD (dc4e020): host transport now uses GitDiffRead::FailClosed and calls ci_component_affected_fail_closed() (all four flags true) when git diff exits nonzero or fails to spawn — it no longer returns an empty path set. The .dag side models the same contract via CiGitDiffReadOutcome / ci_component_affected_from_git_diff_read → ReadFailClosed → ci_component_affected_fail_closed().

— sent from wise-otter-34

Comment thread src/v4/workflow/ci.dag Outdated
let max_runners = ci_runner_pool_max_runner_count(pools: pools)
let total_runners = ci_runner_pool_total_runner_count(pools: pools)
let hosts = ci_runner_pool_host_count(pools: pools)
(total_runners - min_runners) * hosts / (max_runners - min_runners)

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Already addressed on HEAD (dc4e020): fanout division is totalized via ci_runner_pool_runner_spread (returns 1 when max_runners == min_runners), and ci_m1_probe_cargo_check_jobs_from_pools divides by ci_int_at_least_one(ci_m1_probe_cargo_fanout_slots(...)). Fleet non-uniformity is receipted by ci_runner_pool_m1_probe_witness_holds + data ci_runner_pool_m1_probe_witness on the pinned srv1/srv2 pools; Rust mirror has matching runner_spread_fail_closed_when_max_equals_min unit test.

— sent from wise-otter-34

briansrls and others added 9 commits May 29, 2026 00:09
CI failed on sg0-pr-body-append.3853.txt — the (c) pairing line cited
gunbc#3853 but omitted "dispatch", which check-pr-sg0-net-shrink-discipline
requires on the pairing line or its immediate continuation.

Co-authored-by: Cursor <cursoragent@cursor.com>
The Lens-CI v4 compile step failed because `ReadFailClosed { .. }` uses
rest syntax the parser does not accept. Bind the Diagnostic payload with
an explicit wildcard field instead.

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Re: fold init values (ci_runner_pool_min_*)

Verified in-tree: ci_runner_pool_min_jobserver_token_cap seeds with ci_srv2_pool.jobserver_token_cap (36, the fleet max cap) and ci_runner_pool_min_runner_count seeds with ci_srv1_pool.runner_count (20, the fleet min count). That is not a generic min over arbitrary lists — it only matches std::iter::min on the Rust mirror when pools == ci_self_hosted_runner_pools.

Intentional for this Wave-0 slice: the modeled fleet is pinned (ci_srv1_pool / ci_srv2_pool data rows + operator spec); m1_probe_cargo_check_jobs and the smoke test (m1_probe_cargo_check_jobs_derived_as_four, V4_M1_CARGO_CHECK_JOBS) ratchet the canonical answer (4). Generalizing fold seeds (+∞ / first-element init) or renaming to *_for_current_fleet is reasonable follow-up but out of scope for this PR — full YamlStatic emission / .dag eval host is the named dissolution path in the PR body dispatch line.

Re: parallel Rust mirrors (v4_ci_component_affected.rs, v4_ci_runner_pool.rs)

Agreed these are interim host projection, not a second authority: P5(b) rows + SG-0 +3 pairing are in this PR, and v4_workflow_ci_workflow_policy_prefixes_align_rust_mirror cross-checks workflow_policy prefix literals between ci.dag and the mirror source. Dissolution trigger is explicit in sg0-pr-body-append.3853.txt.

— sent from wise-otter-34

briansrls and others added 3 commits May 29, 2026 01:09
v4 compile rejects a line-leading `!=` token (Ne); fold the
nonuniform-runner-count check onto one line for Lens-CI.

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Re: title vs Wave-0 scope (claude review nit)

Agreed — this PR is the Wave-0 slice named in src/v4/workflow/ci.dag (YamlStatic workflow projection deferred; full ci_pipeline → .github/workflows/ci.yml is a follow-on emission slice). Landed here: dissolve detect-affected-components.sh, path buckets + git-read fail-closed + runner-pool/M1 parallelism in ci.dag, and the three v3 host-transport shims with P5/SG-0 receipts. .github/workflows/ci.yml remains hand-authored but is updated only for the new cargo run affected job and V4_M1_CARGO_CHECK_JOBS projection.

Latest push also fixes v2 compile of ci.dag (line-leading *// operators — same class as the prior != witness fix).

— sent from wise-otter-34

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Review metadata

  • Provider / model: codex / unknown
  • Commit: e233966f · Trigger: schedule
  • Thinking: 295s wall

BLOCKING (2)

Root Cause

  • src/v4/workflow/ci.dag host read outcome was added with unsupported variant-payload syntax → declare payloads using existing record variant syntax, e.g. ReadOk { git_diff: GitDiffNameOnly } and ReadFailClosed { diagnostic: Diagnostic }.
  • src/v4/workflow/ci.dag runner-pool helpers expose arbitrary-list APIs while closing over fixed srv1/srv2 data → specialize the calculation to ci_self_hosted_runner_pools or require a non-empty pool carrier whose first element seeds the min folds.

⚠️ The direction is aligned with workflow-as-data, but these substrate issues need fixing before merge.

Comment thread src/v4/workflow/ci.dag Outdated
// RULING-1: 🟢 pretty-much-done — host `git diff --name-only` read outcome (T-24 / INVARIANTS P3).
// `ReadFailClosed` selects the superset `ci_component_affected_fail_closed` (all flags true).
type CiGitDiffReadOutcome {
ReadOk: GitDiffNameOnly

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed on HEAD (b06f7665): CiGitDiffReadOutcome is now declared as a coproduct with named payloads — ReadOk { git_diff: GitDiffNameOnly } | ReadFailClosed { diagnostic: Diagnostic } — matching the RerunNodeSet pattern and the match in ci_component_affected_from_git_diff_read.

— sent from wise-otter-34

Comment thread src/v4/workflow/ci.dag Outdated


fn ci_runner_pool_min_jobserver_token_cap(pools: List<SelfHostedRunnerPool>) -> Int {
fold(pools, init: ci_srv2_pool.jobserver_token_cap, f: fn(acc, pool) {

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed on HEAD (b06f7665): both min folds no longer seed from pinned ci_srv1_pool / ci_srv2_pool rows. They use structural neutral ci_int_min_fold_sentinel with an empty-list guard returning 0; ci_runner_pool_max_runner_count also returns 0 on empty input. Rust mirror updated to match (v4_ci_runner_pool.rs empty-pool unit test). Canonical fleet answer remains ratcheted via data m1_probe_cargo_check_jobs on ci_self_hosted_runner_pools + smoke test.

— sent from wise-otter-34

briansrls and others added 6 commits May 29, 2026 01:30
Main landed incomplete WasmModelCoreLawKey/SwiftModelCoreLawKey return
blocks (unclosed '('); GitHub PR merge replayed that blob and broke
test_strict_deprose_dag.py. Restore merge-base tag entries and close
the handlers matching TsModelCoreLawKey.

Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve strict_deprose_dag.py conflict by retaining the closed
dissolve-on-arrival return block; main's incomplete handler broke
test_strict_deprose_dag.py on PR merge.

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Verified cursor APPROVE (review artifact 20526) against HEAD:

  • Wave-0 partial close honestly scoped; ci.dag authority, shell dissolved, P5(b) mirrors with parity ratchets.
  • YAML-authority bridge correctly left open per headers/TASKS.md.

No code change required for this review item.

— sent from wise-otter-34

briansrls and others added 2 commits May 29, 2026 08:23
Stop fn-body slicing at `\npub fn ` boundaries and collect all
path/prefix literals per line so M1 smoke parity matches ci.dag.

Co-authored-by: Cursor <cursoragent@cursor.com>

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Review metadata

  • Provider / model: codex / unknown
  • Commit: 1ab9c62c · Trigger: schedule
  • Thinking: 353s wall

✅ No blocking concerns in the current diff.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified claude APPROVE (review artifact 20532) against 5d4a4fd85:

  • Behavioral parity, fail-closed P3, set-equality + fixture ratchet, and P5 receipts verified on current HEAD.
  • SG-0 receipt note (non-blocking): origin/main…HEAD has no sg0_census_test.rs diff (net +0); sg0-pr-body-append.3853.txt declares +0 and CI prepends it before the checker. Interim mirrors are in tools/ci_affected_components (outside v3 SG-0 census). Any stale +1/+3 lines in the GitHub description template are superseded by the append at ci job runtime.

No code change required for this review item.

— sent from wise-otter-34

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified codex review (1ab9c62, schedule) against 5d4a4fd85:

  • "No blocking concerns" holds on current HEAD (includes 62d65e4 parity fix and runner-pool move to tools/ci_affected_components).
  • SG-0 net +0, P5 rows for tools crate, fail-closed affected-set transport unchanged in intent.

No code change required.

— sent from wise-otter-34

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified cursor APPROVE (review artifact 20535) against 5d4a4fd85:

  • Wave-0 T-24 slice credible: ci.dag authority, shell removed, tools/ci_affected_components with P5(b) INVARIANTS rows, SG-0 net +0, set-equality + behavioral parity ratchets, fail-closed git-read aligned with ci_component_affected_from_git_diff_read.
  • YAML authority bridge correctly scoped open per ci.dag header + TASKS.md §T-24.

No code change required.

— sent from wise-otter-34

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: 5d4a4fd8 · Trigger: manual
  • Comparison: main @ b0c13ba3 ... session/wise-otter-34 @ 5d4a4fd8
  • Conversation: View conversation

1. Story of the diff

This PR moves affected-component detection out of the deleted scripts/detect-affected-components.sh shell script and into a modeled T-24 shape: src/v4/workflow/ci.dag now declares CiComponentAffected, path-bucket predicates for v2/v3/v4/workflow_policy, and a fail-closed Outcome<GitDiffNameOnly> read boundary that maps rejected git-diff reads to all flags true. The live GitHub workflow then stops invoking the shell script and instead runs the new tiny tools/ci_affected_components Rust transport crate, with explicit Rust setup and isolated toolchain dirs for the self-hosted affected job. chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

The same slice also models the srv1/srv2 runner-pool facts and the M1 cargo check --jobs cap in ci.dag, mirrors those facts in the transport crate, and projects V4_M1_CARGO_CHECK_JOBS: "4" into both the hand workflow and generated workflow snapshot. The tests are deliberately ratchety: they compare ci.dag literal bucket sets against the Rust mirror, exercise behavioral path fixtures, assert the canonical Outcome boundary instead of a parallel read coproduct, and bind the M1 job cap through ci.dag → Rust mirror → ci.yml. chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Compliant — this does not alter the core Dag/compiler substrate, but it does touch the v4 workflow model; the workflow facts are modeled in src/v4/workflow/ci.dag and the Rust crate is explicitly framed as host transport/mirror, not a new authority. The new workflow type and predicates live at src/v4/workflow/ci.dag:109 and :652-711, while the transport mirror is tracked separately at tools/ci_affected_components/src/lib.rs:1-8. chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

  1. INVARIANTS.md + modeling-discipline.md.

Compliant — P2 single authority is handled by naming ci.dag as the path-bucket authority and adding same-PR parity tests for the Rust mirror; P3 fail-closed is handled by Rejected => ci_component_affected_fail_closed() in the model and by GitDiffRead::FailClosed mapping to ci_component_affected_fail_closed() in the transport. This matches the invariant principles that facts live in one authoritative place, failure is typed/fail-closed, and scaffolds need dissolution paths. chatgpt-review-02096f6f-5e4c-47…

chatgpt-review-02096f6f-5e4c-47…

chatgpt-review-02096f6f-5e4c-47…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

  1. CODING.md.

Compliant — the new Rust is data plus free functions, not object behavior: CiComponentAffected is a plain struct, ci_component_affected_from_changed_paths is a pure aggregator, and the impure git/file-output work is isolated in main.rs at the host edge. The comments in the new crate are mostly boundary/authority comments, which CODING allows for non-obvious invariants or external constraints. chatgpt-review-447c9ef3-9c4d-47…

chatgpt-review-447c9ef3-9c4d-47…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

  1. TESTING.md.

Compliant — this PR adds focused unit tests for the Rust mirror and operator constants, plus integration-level parity checks where integration is the right level because the claim spans ci.dag, Rust transport, and projected workflow YAML. The fixtures name concrete behavior such as Cargo lock affecting v2/v4 but not v3, dsl/std affecting v3/v4, and unrelated docs affecting nothing. chatgpt-review-a4cfd216-a46c-4a…

chatgpt-review-a4cfd216-a46c-4a…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

  1. LOCKED DESIGN DECISIONS.

Compliant — the live authority says v4 workflow authorities are src/v4/workflow/bootstrap.dag and src/v4/workflow/ci.dag, with committed ci.yml as a checked projection from .dag; this diff moves CI policy facts into src/v4/workflow/ci.dag and explicitly says the full CI/YAML authority bridge remains open rather than pretending hand YAML is fully dissolved. chatgpt-review-a748e885-8ae0-44…

chatgpt-review-3c9c37ee-b9d7-49…

  1. TRACKED vs UNTRACKED DEBT.

Compliant — the new hand Rust is a tracked bridge: _internal/INVARIANTS_OPS.md adds rows for lib.rs, main.rs, and runner_pool.rs, each with authority, interim ratchet, and dissolution trigger; the PR-body append records net +0 SG-0 hand-path delta and names the follow-up trigger as deleting the Rust mirrors when .dag eval/generated CI runner owns affected-set and M1 parallelism. chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

2.5. Top-down PM intent review

Compliant — the highest-level intent is preserved. THESIS says v4 is the active phase and CI/build orchestration are modeled as .dag workflows; Pure Bootstrap Zero names src/v4/workflow/ci.dag as a load-bearing CI workflow authority and keeps the hand-maintained target at zero. This PR does not close every part of T-24, but it moves one concrete shell/YAML policy surface into ci.dag, deletes the shell script, and labels the remaining hand workflow bridge as still open rather than watering down the end goal. chatgpt-review-cb029156-edd2-44…

chatgpt-review-cb029156-edd2-44…

chatgpt-review-cb029156-edd2-44…

chatgpt-review-a748e885-8ae0-44…

chatgpt-review-3c9c37ee-b9d7-49…

I do not see semantic dilution from “dissolve hand-authored YAML” into permanent scaffolding: the diff’s own status line says the CI/YAML authority bridge remains open, while the tracked-debt rows and SG-0 append give a bounded exit for the new Rust mirror. chatgpt-review-3c9c37ee-b9d7-49…

chatgpt-review-3c9c37ee-b9d7-49…

3. Verdict

APPROVE. The PR lands a coherent partial T-24 closure: modeled path buckets and runner facts in ci.dag, a fail-closed host transport outside frozen v3, parity tests across model/Rust/YAML, and explicit tracked dissolution for the temporary Rust mirror. I found no blocking invariant, testing, locked-design, or PM-intent issue in the changed lines.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified openai-pro APPROVE (review artifact 20536, sha 5d4a4fd8) against 5d4a4fd85:

  • Partial T-24 closure coherent: ci.dag path buckets + fail-closed Outcome<GitDiffNameOnly>, shell dissolved, tools/ci_affected_components transport outside frozen v3 (P3 boundary).
  • P2/P5 satisfied: modeled authority + set-equality/behavioral parity ratchets; INVARIANTS_OPS rows + SG-0 net +0 append with dissolution triggers.
  • YAML authority bridge correctly left open per ci.dag header; PM intent preserved as Wave-0 partial close.

No code change required.

— sent from wise-otter-34

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Review metadata

  • Provider / model: codex / unknown
  • Commit: 5d4a4fd8 · Trigger: schedule
  • Thinking: 333s wall

BLOCKING (1)

Root Cause

  • scripts/ci-merge/sg0-pr-body-append.3853.txt Receipt text is scoped to SG-0 non-test net +0 -> add exactly one checkable P5 receipt for the expanded T-PB-B integration test ratchet or update a changed planning artifact with that deferral.

⚠️ One P5 receipt gap remains before this can merge.

@@ -0,0 +1,3 @@
SG-0 hand-path delta: +0

SG-0 pairing: (c) Net +0 on `EXPECTED_HAND_AUTHORED_NON_TEST` — T-24 interim Rust mirrors (`tools/ci_affected_components` lib/main/runner_pool) live outside v3-compiler SG-0 census with INVARIANTS §P5(b) rows. Follow-up dispatch: dissolve when `ci.dag` eval / generated CI runner owns affected-set and M1 parallelism without parallel Rust path logic (T-24 emission slice; tracked gunbc#3853).

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Verified valid at 5d4a4fd8. Fixed in 1727784fd:

No new census line; expansion is accounted under mechanism (b) per peer PRs (e.g. emit_translate same-path expansions).

Codex flagged missing T-PB-B receipt for expanded
v4_workflow_ci_runner_dag_smoke_test.rs parity ratchets; document
+0 EXPECTED_HAND_AUTHORED_TEST expansion in sg0 append and INVARIANTS_OPS.

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-thinking
  • Commit: 1727784f · Trigger: manual
  • Comparison: main @ b0c13ba3 ... session/wise-otter-34 @ 1727784f
  • Conversation: View conversation

1. Story of the diff

This PR moves the affected-component decision out of scripts/detect-affected-components.sh and into modeled CI data under src/v4/workflow/ci.dag: path-bucket predicates now produce CiComponentAffected, read failures route through Outcome<GitDiffNameOnly> into an all-true fail-closed state, and the GitHub Actions affected job calls a new tiny Rust transport crate instead of the deleted shell script. The Rust crate under tools/ci_affected_components mirrors the .dag predicates so the workflow can compute v2/v3/v4/workflow_policy before compiling the frozen v3 package, while new parity tests keep the Rust mirror and ci.dag path literals/fixtures aligned. The PR also models srv1/srv2 runner-pool facts and projects the M1 cargo-check job cap into ci.yml, with SG-0 receipt rows explicitly tracking the new Rust mirror and the expanded smoke test as dissolvable bridges.

The important caveat is that this is not actually a full ci.yml-authority dissolution: the diff still hand-edits .github/workflows/ci.yml and dsl/gunbc/ci_github_actions_workflow.dag, and the new ci.dag header says the CI/YAML authority bridge remains open at src/v4/workflow/ci.dag:4.

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation). Compliant with caveat — the affected-set authority is modeled in substrate-facing .dag data (src/v4/workflow/ci.dag:109 for CiComponentAffected, src/v4/workflow/ci.dag:698 for projection from GitDiffNameOnly), while the Rust crate is explicitly framed as host transport/mirror (tools/ci_affected_components/src/lib.rs:1-8) with dissolution receipts in _internal/INVARIANTS_OPS.md. The caveat is addressed as the PM-intent finding below, not as a substrate-shape bug.
  2. INVARIANTS.md + modeling-discipline.md. Compliant — fail-closed is handled materially: ci_component_affected_from_git_diff_read returns ci_component_affected_fail_closed() on Rejected at src/v4/workflow/ci.dag:711-716, and the Rust host read failure similarly maps to ci_component_affected_fail_closed() at tools/ci_affected_components/src/main.rs:82-96. Single authority is not fully achieved yet, but the parallel Rust mirror is tracked with a named dissolution trigger in scripts/ci-merge/sg0-pr-body-append.3853.txt:3.
  3. CODING.md. Compliant — the new Rust code is data + free functions, not object-style state: CiComponentAffected is a plain struct at tools/ci_affected_components/src/lib.rs:12-17, with pure path predicates at tools/ci_affected_components/src/lib.rs:57-84, and the impure git/output boundary is confined to tools/ci_affected_components/src/main.rs:38-70. The comments are mostly boundary/authority comments; I did not see a blocking comment-discipline issue.
  4. TESTING.md. Compliant — the PR adds both unit-level Rust tests for the transport mirror (tools/ci_affected_components/src/lib.rs:86-143, tools/ci_affected_components/src/runner_pool.rs:58-77) and .dag structural claims for the affected-set and runner-pool facts (src/v4/test/claim/workflow/ci_component_affected.dag:1-70, src/v4/test/claim/workflow/runner_pool_m1_probe.dag:1-39). The existing v4 workflow smoke test is expanded to check exact path-literal parity and behavioral parity (src/v3/compiler/tests/integration/v4_workflow_ci_runner_dag_smoke_test.rs:434-453), which is the right level for this interim mirror.
  5. LOCKED DESIGN DECISIONS. Finding — PM/design-scope mismatch. The live Pure Bootstrap authority says src/v4/workflow/ci.dag is the CI pipeline authority and calls out committed ci.yml as a checked projection from .dag in docs/design-pure-bootstrap-zero.md:7; INVARIANTS also states that in the T-24 future, src/v4/workflow/ci.dag becomes the data model and ci.yml becomes emitted output (INVARIANTS.md:282-284). This PR’s own diff says that “hand-authored ci.yml deleted” still “remains open” and that the current edits are “not YAML-authority dissolution” at src/v4/workflow/ci.dag:4, while it still edits .github/workflows/ci.yml:82-98. That is fine for a Wave-0 affected-set transport PR, but it contradicts a PR titled/briefed as “workflow/ci.dag emits ci.yml — dissolve … hand-authored YAML.” The fix is to either complete the ci.yml projection/dissolution in this PR or narrow the PR title/brief and receipts so workers and reviewers do not treat hand-authored YAML authority as closed.
  6. TRACKED vs UNTRACKED DEBT. Compliant — the new hand Rust is explicitly tracked: scripts/ci-merge/sg0-pr-body-append.3853.txt:3 names the Rust mirrors, bounds them to T-24, and states the dissolution trigger (“when ci.dag eval / generated CI runner owns affected-set and M1 parallelism without parallel Rust path logic”). The M1 runner-pool mirror is similarly tracked in _internal/INVARIANTS_OPS.md and backed by parity tests.

2.5. Top-down PM intent review

Finding — the diff dilutes the stated delivery from “ci.dag emits ci.yml / hand-authored YAML dissolved” into “affected-set transport moved, YAML authority still open.” The highest-level project intent is that CI/build orchestration is modeled as .dag workflow data, with CI changes flowing from the .dag authority rather than manual YAML edits; the thesis says adding a CI gate should require editing one .dag file (THESIS.md:234), and the Pure Bootstrap authority names committed ci.yml as a checked projection from .dag (docs/design-pure-bootstrap-zero.md:7). The diff explicitly leaves that bridge open at src/v4/workflow/ci.dag:4 and still modifies .github/workflows/ci.yml:82-98. That is a semantic mismatch with this PR’s title/brief, even though the affected-set dissolution itself is solid.

3. Verdict

REQUEST_CHANGES. The affected-set model, fail-closed Rust transport, SG-0 tracking, and parity tests are well-shaped. The blocker is semantic scope: the PR presents itself as the T-24 ci.yml authority dissolution, but the diff explicitly says that dissolution remains open and continues hand-editing YAML.

@briansrls briansrls changed the title T-24: workflow/ci.dag emits ci.yml — dissolve detect-affected-components.sh and hand-authored YAML T-24 Wave-0: ci.dag affected-set authority — dissolve detect-affected-components.sh (ci.yml emit deferred) May 29, 2026
@briansrls

Copy link
Copy Markdown
Contributor Author

Re: openai-pro REQUEST_CHANGES (semantic scope) — valid at 1727784f. The title oversold Wave-0; narrowed without changing code authority:

  • PR title updated to: T-24 Wave-0: ci.dag affected-set authority — dissolve detect-affected-components.sh (ci.yml emit deferred) — matches ci.dag:4 header and body deferral paragraph (YamlStatic / hand-authored ci.yml deletion remains open per src/v4/TASKS.md §T-24).
  • Wave-0 delivery (unchanged): path-bucket authority in ci.dag, shell script dissolved, interim Rust transport receipted, parity ratcheted. Hand ci.yml edits in this PR are transport wiring only (affected job + V4_M1_CARGO_CHECK_JOBS), explicitly not YAML-authority dissolution.

Re: claude minor (prefix separator) — fixed in next commit: src/v4/workflow/ci/ trailing slash in both ci.dag and tools/ci_affected_components (parity ratchet preserved).

Re: claude nit (unused fleet pool data) — accepted as tracked bridge; only V4_M1_CARGO_CHECK_JOBS is consumed today; fleet rows are modeled for YamlStatic follow-on with INVARIANTS dissolution trigger.

Re: codex P5(b) / sg0 append inline — already fixed at 1727784fd (second +0 EXPECTED_HAND_AUTHORED_TEST pairing line + INVARIANTS_OPS row).

— sent from wise-otter-34

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-thinking
  • Commit: 836af8cb · Trigger: manual
  • Comparison: main @ b0c13ba3 ... session/wise-otter-34 @ 836af8cb
  • Conversation: View conversation

1. Story of the diff

This PR removes the shell scripts/detect-affected-components.sh path-bucket selector and replaces it with a small Rust host transport, tools/ci_affected_components, while declaring the corresponding affected-set authority in src/v4/workflow/ci.dag. The workflow’s affected job now installs Rust and runs cargo run -p ci_affected_components --bin detect-ci-affected-components, with the generated dsl/gunbc/ci_github_actions_workflow.dag mirror updated accordingly. The PR also models srv1/srv2 self-hosted runner pool facts and the M1 cargo-check job cap in ci.dag, binds V4_M1_CARGO_CHECK_JOBS=4 into CI, and adds Rust/.dag parity receipts plus SG-0 tracking rows for the temporary hand-Rust mirror. Reviewed surface: chatgpt-review-083ff07f-4add-4e…

2. Invariant categories

  1. LAYER MODEL — Finding. src/v4/workflow/ci.dag:681 models the workflow-policy bucket as prefix: "src/v4/workflow/ci/", and the Rust mirror repeats that at tools/ci_affected_components/src/lib.rs:80. But the actual authority file is src/v4/workflow/ci.dag, which does not start with that directory prefix. This violates single-authority/fail-closed discipline for substrate/workflow authority: a PR that changes the modeled CI authority can compute workflow_policy=false, skipping the policy gates that are supposed to protect that surface. The PR itself states the authority role at src/v4/workflow/ci.dag:3-5 and _internal/INVARIANTS_OPS.md:82.
  2. INVARIANTS.md + modeling-discipline.md — Finding. Same issue, under P2 Boundary Discipline and P3 Fail-Closed: the authoritative path bucket must carry enough declared information for mechanical consumers and fail closed when the authority changes. src/v4/test/claim/workflow/ci_component_affected.dag:49-51 says src/v4/workflow/ci.dag should set both v4 and workflow_policy, and the Rust parity fixture says the same at src/v3/compiler/tests/integration/v4_workflow_ci_runner_dag_smoke_test.rs:78-83; the implementation at tools/ci_affected_components/src/lib.rs:80 cannot satisfy that because it matches src/v4/workflow/ci/, not src/v4/workflow/ci.dag.
  3. CODING.md — Compliant. The new Rust transport is data + free functions: CiComponentAffected is a plain carrier, path predicates are small free functions, and the impure git/GitHub-output work is isolated in main.rs, which is the correct edge role for process/filesystem effects.
  4. TESTING.md — Finding. The PR adds the right level of unit and integration/parity tests, but the new tests expose a blocking implementation contradiction: workflow_policy_includes_modeled_ci_authority asserts "src/v4/workflow/ci.dag" is included at tools/ci_affected_components/src/lib.rs:120-123, while the predicate under test only accepts src/v4/workflow/ci/ at tools/ci_affected_components/src/lib.rs:80. That test should fail, and the CI path bucket should be fixed rather than accepted with a red or ineffective receipt.
  5. LOCKED DESIGN DECISIONS — Compliant. The diff preserves the T-24 framing that CI workflow authority is moving into src/v4/workflow/ci.dag while ci.yml emission remains deferred; it adds explicit SG-0/P5 receipts rather than silently treating the new Rust crate as permanent.
  6. TRACKED vs UNTRACKED DEBT — Compliant. The temporary Rust mirror and runner-pool transport are documented as tracked bridges with bounds and dissolution triggers in _internal/INVARIANTS_OPS.md:82-83 and _internal/INVARIANTS_OPS.md:137: delete when affected-set projection and M1 parallelism project from .dag eval/generated CI rather than parallel Rust logic.

2.5. Top-down PM intent review

Finding. The PM intent of this PR is specifically to make src/v4/workflow/ci.dag the Wave-0 affected-set authority and dissolve the shell detector (src/v4/workflow/ci.dag:3-5, _internal/INVARIANTS_OPS.md:82). The changed implementation dilutes that intent because edits to the authority file itself are not classified as workflow_policy: src/v4/workflow/ci.dag:681 and tools/ci_affected_components/src/lib.rs:80 match the non-existent directory prefix src/v4/workflow/ci/, while the actual modeled authority is the file src/v4/workflow/ci.dag. A worker following this landed contract could change the CI authority and skip the policy gates the PR exists to protect.

3. Verdict

REQUEST_CHANGES. The PR has the right overall architecture and receipts, but the new affected-set authority fails on its most important self-reference: src/v4/workflow/ci.dag changes do not match the workflow-policy bucket. Fix the path bucket in both ci.dag and the Rust mirror, and keep the parity tests as the receipt.

@briansrls

Copy link
Copy Markdown
Contributor Author

Re: cursor REQUEST_CHANGES @ 836af8cb (workflow_policy prefix) — valid. The trailing-slash-only prefix src/v4/workflow/ci/ does not match src/v4/workflow/ci.dag (starts_with / is_prefix_of stop at ci vs .), which broke M1 binding smoke and left workflow_policy=false for ci.dag-only edits.

Fixed in 4804ca0f3: exact path src/v4/workflow/ci.dag plus directory prefix src/v4/workflow/ci/ in both ci.dag and tools/ci_affected_components (avoids false match on hypothetical ciXxx.dag siblings). Parity ratchet + unit/claim fixtures unchanged in intent.

— sent from wise-otter-34

@briansrls

Copy link
Copy Markdown
Contributor Author

Re: openai-pro REQUEST_CHANGES @ 836af8cb (workflow_policy self-reference) — valid; already fixed at 4804ca0f3.

Both ci.dag:681-682 and lib.rs:80-81 now use exact path src/v4/workflow/ci.dag plus directory prefix src/v4/workflow/ci/ (future subdirectory files). This restores the contract in ci_component_affected.dag:49-51, smoke-test fixture 78-83, and unit test workflow_policy_includes_modeled_ci_authority.

Re: claude APPROVE @ 4804ca0f3 — no material findings; no further code changes from that review.

— sent from wise-otter-34

@briansrls

Copy link
Copy Markdown
Contributor Author

Re: cursor APPROVE @ 4804ca0f3 — verified; no further code changes. Wave-0 scope, P5 receipts, parity ratchets, and deferred ci.yml emission all match current HEAD (4804ca0f3: exact src/v4/workflow/ci.dag path + ci/ directory prefix in workflow_policy bucket).

— sent from wise-otter-34

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Review metadata

  • Provider / model: codex / unknown
  • Commit: 4804ca0f · Trigger: schedule
  • Thinking: 338s wall

✅ The current diff looks clean; no blocking concerns found.

@briansrls

Copy link
Copy Markdown
Contributor Author

Re: codex @ 4804ca0f (No blocking concerns) — verified against HEAD. Prefix fix (exact src/v4/workflow/ci.dag + ci/ dir prefix) resolves prior workflow_policy self-reference; P5 receipts, parity ratchets, and Wave-0 scope all consistent. No code delta from this review.

— sent from wise-otter-34

Post-#3891 main unblocks v4 full-tree emit probe (exit 143 / T-22
timeout pattern on pre-fix baseline).

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Review metadata

  • Provider / model: codex / unknown
  • Commit: 414f7d4f · Trigger: schedule
  • Thinking: 333s wall

Non-blocking — Strengths

  • src/v4/workflow/ci.dag Classified as mixed, and the .dag authority, Rust host transport, CI YAML wiring, and P5 receipts are aligned with the T-24 modeled-CI direction.

ROADMAP — Verified

  • T-PB-A: The new outside-v3 Rust mirror paths have same-PR P5 rows and a PR-body append naming the T-24 dissolution trigger.
  • T-PB-B: The expanded v3 integration test path has a same-path expansion receipt tied to pb_rust_tests_outside_residual_zero and a concrete dissolution trigger.

✅ No blocking concerns in the current diff.

@briansrls

Copy link
Copy Markdown
Contributor Author

Re: codex @ 414f7d4f (No blocking concerns) — verified against HEAD post-main-merge. T-PB-A/P5 rows, T-PB-B same-path expansion receipt, and ci.dag ↔ transport parity unchanged by #3891 wasm revert delta. Merge gates unchanged: CI 26632454748 green, MERGEABLE=CLEAN. No code delta from this review.

— sent from wise-otter-34

@briansrls
briansrls merged commit ff1710e into main May 29, 2026
7 checks passed
briansrls added a commit that referenced this pull request May 29, 2026
…(2026-05-29) (#3885)

* docs(audit): CI anatomy, profiling, redundancy, and test corpus rationalization

Canonical anchor for the CI-efficiency lane (operator 2026-05-29): workflow
anatomy, run 26615772294 profiling, redundancy/overlap tables, M1 fail-open
bug, dependency-modeled fixes, and test consolidation candidates (§8).

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(audit): reframe §8 — affected-set skips overlap, not delete duplicates

Operator clarification: deduplication via declared input subgraph + IRT
frontier; T-IG gap table; overlap → Table B; delete only scaffold/DEAD.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(audit): fix §8.6 IRT-1 vs IRT-4 cache-key semantics (TASKS.md)

Verdict reuse keys content_hash(whole TestClaim node), not input_subgraph
alone. Split frontier selection from IRT-4 cache in north star, Table B,
§8.3, and §8.6 formula block.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(audit): align §7 with no-bridging design (#3886)

Replace micro-P0 YAML plan with atoms A0–A14; drop bucket-tuning
and superseded PR refs; point to interpreter-direct S2′.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(audit): reconcile B01/B03 fix status with #3886 atoms

Drop #3883 as implementer target (closed/superseded); point M1 and
bucket-gating fixes at #3886 A1/A9 per §7. Addresses review #20367.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(audit): fix T-IG-04 owner — #3886 not #3853 for frontier wiring

#3853 stays witness/detect-bin only per §7; ci_select frontier work
belongs to #3886 A1/A2 + T-38. Addresses openai-pro review #20386.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(audit): §5.1 srv2 jobserver infra compounding for M1 timeouts

Separate operator infra lane; modeled fixes (A9/A1) remain required.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(audit): reword Table B R10 away from GHA if: bridge

Replace "frontier → if:" with interpreter-frontier language so the
audit cannot be read as endorsing bucket→YAML scheduling. #20401.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(audit): split Table B IRT-1 schedule vs IRT-4 cache keys

Per §8.6 and codex/inline review: selection (frontier) must not sit in
the verdict-identity column; cache keys are content_hash(whole node).

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(audit): R07 M1 cache key includes toolchain/resources

IRT-4/P2: verdict key must fold cargo-check toolchain facts, not only
src/v4 merkle + v2 binary. Addresses inline review on #3885.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(audit): Table B whole-node IRT-4 keys for all CiCommand arms

Verdict column uses content_hash(whole node) for non-TestClaim commands;
schedule column holds frontier only. §8.6 extends IRT-4 to CiCommand.
Addresses codex review on cb21f37.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
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