Skip to content

v2.5 artifact.dag — typed artifact-plan + RenderTarget-as-data (fork probe) - #3417

Closed
briansrls wants to merge 6 commits into
session/crisp-dove-396-v2_5-emitfrom
session/loyal-dove-547-v25
Closed

briansrls wants to merge 6 commits into
session/crisp-dove-396-v2_5-emitfrom
session/loyal-dove-547-v25

Conversation

@briansrls

@briansrls briansrls commented May 19, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on PR #3429 (session/crisp-dove-396-v2_5-emit) which publishes src/v2.5/std/language_spec.dag. Merge order: #3429 first, then this PR rebases to main. PR base has been set to session/crisp-dove-396-v2_5-emit so the import resolves and the cross-PR dependency is explicit. (Per PM disposition msg_ab610abb.)

v2.5 fork — src/v2.5/artifact.dag (Tier 3)

First-pass design probe for the v2.5 fork. New file, new tree; does not touch src/v2/, src/v3/, src/v4/, or dsl/. No bootstrap binary for v2.5 yet (sunny-otter-371's parallel investigation). Updated post PM ruling on the four cross-stage questions — see "PM rulings actioned" below.

What the stage does

artifact.dag is orchestration data: it answers "what does the pipeline build, for which target language, into which output location?" The pipeline orchestrator (Tier 3, keen-ibex-355's compile.dag) consumes an ArtifactPlan; the emitter (Tier 2, crisp-dove-396's 05_emit.dag) consumes each Artifact against an InferredNode.

Typed surface (47 lines):

Symbol Shape Role
ArtifactPath { output_dir, file_stem } Where an artifact lands on disk.
Artifact { name, target: LanguageSpec, entry_modules, output } One buildable unit. Target IS a LanguageSpec — no wrapper.
ArtifactPlan { artifacts: List<Artifact> } Full build request.
single_module_plan (root_module, target, output) -> ArtifactPlan v2's default_artifact_plan equivalent, typed.

PM rulings actioned (post first-pass)

Q Ruling Action in this PR
Q1 — LanguageSpec module path Path ratified by crisp-dove-396: v2_5.std.language_spec; the file lives on their PR #3429 branch Resolved by stacking: PR base set to session/crisp-dove-396-v2_5-emit; the import resolves against crisp-dove-396's tree. Cross-PR merge order documented above.
Q2 — v2.5 module-naming convention Confirmed: v2_5.compiler.artifact. Tier 0 (v2_5.std.node) aligned. No change — already correct.
Q3 — RenderTarget wrapper vs alias Collapse — hollow-wrapper smell. RenderTarget IS LanguageSpec. If per-build config emerges later, add a variant then. Applied. type RenderTarget removed; Artifact.target: LanguageSpec directly; single_module_plan parameter retyped.
Q4 — In-file audit prose Standing rule for v2.5 probes: NO in-code prose; PR body carries the narrative. Same discipline as the calibration dispatch. Applied. Per-declaration paragraph comments stripped; section-divider banners removed. File header metadata block (Scope/Owns/Consumes/Status) retained — that's structured metadata, matches v4 file convention.

v2 → v2.5 mapping (was: in-file comments; now: PR body per Q4)

v2 surface Disposition in v2.5
RenderTarget = Rust | Python | Go | Dag (bare 4-arm coproduct) Dissolved. No RenderTarget type in v2.5 — Artifact.target is LanguageSpec directly. The 30+ match target { Rust => f(...), Python => g(...), ... } sites in v2's emit/coercion (template-hole-shaped per PR #3406 finding) become field reads against LanguageSpec. New targets enter as new LanguageSpec values, not as new variants + N new match arms. Per Q3 ruling, the wrapper-with-one-field intermediate shape was a hollow-alias smell and was collapsed.
ArtifactKind = ServiceBinary | Library | Frontend | GeneratedSupport + Artifact.kind field Dropped. PR #3406 finding: Artifact.kind was written (default_artifact_plan sets ServiceBinary) but never read anywhere in src/v2 — compile.dag::emit_artifact dispatched only on .target. A never-matched 4-variant coproduct + a write-only field carry zero behavioral information; per Principle 1, they only re-enter when a consumer needs them.
BoundaryKind + Boundary + ArtifactPlan.boundaries + compile.dag::validate_boundaries Dropped. PR #3406 finding: BoundaryKind's variants were never constructed anywhere; Boundary was never inhabited (boundaries always []); validate_boundaries iterated an always-empty list and read only from_artifact/to_artifact, never .kind. Uninhabited speculative subsystem — re-enters when multi-artifact communication has a real consumer.
ArtifactOutput { artifact, files: List<TextFile> } Dropped. Already deleted in PR #3406 (zero consumers in src/v2). The emitter's output type in v2.5 lives with the emitter, not as a pre-shadowed forward type here.
type PartitionRule = Explicit { artifacts: List<Artifact> } + fn plan_artifacts Dropped. PR #3406 dissolved this — single-variant coproduct husk with a degenerate 1-arm match. single_module_plan constructs ArtifactPlan directly.
default_artifact_plan(root_modules: List<String>, target: RenderTarget) Renamed + typed: single_module_plan(root_module: String, target: LanguageSpec, output: ArtifactPath). v2's signature took a List<String> but always wrapped it as one artifact named "default" — the typed shape says exactly that. Output location is now an explicit parameter rather than implicit.

Concrete demonstrations

Three example call sites the v2.5 orchestrator would produce:

1. Compile one Rust binary (the common case — v2's default_artifact_plan equivalent):

let plan = single_module_plan(
  root_module: "v2_5.compiler.compile",
  target: rust_language_spec,
  output: ArtifactPath { output_dir: "/tmp/out", file_stem: "compiler" }
)

2. Compile two targets in one plan (multi-target build — not expressible in v2's default_artifact_plan):

let plan = ArtifactPlan {
  artifacts: [
    Artifact {
      name: "demo",
      target: rust_language_spec,
      entry_modules: ["demo.app"],
      output: ArtifactPath { output_dir: "/out/rust", file_stem: "demo" }
    },
    Artifact {
      name: "demo",
      target: python_language_spec,
      entry_modules: ["demo.app"],
      output: ArtifactPath { output_dir: "/out/py", file_stem: "demo" }
    }
  ]
}

3. Emit-time dispatch is field projection, not re-match:

// v2 (template-hole — 30+ sites):
//   match target { Rust => "rs", Python => "py", Go => "go", Dag => "dag" }
// v2.5 (field read):
//   artifact.target.file_extension

Cross-stage interfaces consumed

Source Symbol Status
v2_5.std.language_spec LanguageSpec ✅ Resolved by stacking on PR #3429 — see banner above. The import resolves against crisp-dove-396's tree; merge-order coordination handled by the cross-PR stack.

Per-principle satisfaction

  1. Coproduct dissolution — RenderTarget 4-arm bare coproduct → dissolved entirely (collapsed into LanguageSpec per Q3). ArtifactKind, BoundaryKind, PartitionRule all dropped (uninhabited / never-read / single-variant husk).
  2. Grounding — every type sits on a substrate primitive (String, List<T>) or a sibling-owned type (LanguageSpec). No hollow alias remains (the RenderTarget wrapper that would have been one was collapsed per Q3).
  3. Compiler homomorphism — no structural recursion in this file; nothing to hand-roll. Consumers will fold over ArtifactPlan.artifacts via standard list combinators.
  4. No template-holes — no match in this file. The v2 match target { ... } dispatch pattern is structurally prevented: there are no variants to match on.
  5. No annotations — no in-file prose carries a modeling fact; the file header metadata block carries structured metadata only. Per Q4 ruling.
  6. Simplicity — 47 lines for the whole module. The simplest shape that carries (target, entry modules, output) per artifact.
  7. No "established idiom" reasoning — every v2 surface is judged on its own merit (e.g. ArtifactKind dropped because grep showed zero readers, not by analogy to other dropped types).
  8. Document what dissolved — section above, line-cited against src/v2/artifact.dag and src/v2/compile.dag.

What dissolved (line-cited against v2)

  • src/v2/artifact.dag:19 RenderTarget = Rust | Python | Go | Dag → no RenderTarget in v2.5; Artifact.target is LanguageSpec directly.
  • src/v2/artifact.dag:26–30 type ArtifactKind + variants → dropped (write-only).
  • src/v2/artifact.dag:38 Artifact.kind: ArtifactKind → dropped (never read).
  • src/v2/artifact.dag:48–53 type BoundaryKind + variants → dropped (never constructed).
  • src/v2/artifact.dag:55–60 type Boundary → dropped (never inhabited).
  • src/v2/artifact.dag:68 ArtifactPlan.boundaries: List<Boundary> → dropped (always []).
  • src/v2/compile.dag:761–769 fn validate_boundaries → drops naturally (data source is gone).
  • v2's match target { ... } dispatch sites in 05_emit.dag, coercion.dag, compile.dag::emit_artifact (cited in PR v2 artifact.dag — dissolve single-variant coproduct husk (calibration probe) #3406 findings) → field projection.

Honest 🟡 list

  1. Module-reference carrier — entry_modules: List<String> / root_module: String. At the artifact-plan layer, entries are user-supplied module names pre-resolver — no resolved-module carrier exists yet because the source hasn't been parsed. The plan is the boundary where strings enter; downstream stages then resolve them. consumer-gated on the v2.5 resolver stage publishing a structural module-reference carrier; at that point post-resolution consumers should carry the resolved form (the plan itself still accepts user-supplied names — that's the user-facing input shape). A typed wrapper around String here would be the hollow-alias smell the PM vetoed in Q3, so a wrapper is not the right fix; the 🟡 is the right disposition per the openai-pro review's offered alternative.

(Earlier dangling-import 🟡 resolved by stacking on PR #3429 — see banner. Q2/Q3/Q4 closed in the previous round.)

Out of scope (confirmed not done)

  • No edits anywhere outside src/v2.5/.
  • No imports from src/v2/, src/v3/, src/v4/, dsl/.
  • No Rust hand-edits.
  • No bootstrap mechanism (sunny-otter-371's parallel work).
  • No cargo build/cargo test claimed — v2.5 has no buildable surface yet; this is a .dag-only design probe.

🤖 Generated with Claude Code

…probe)

First-pass design for the v2.5 fork's artifact module. RenderTarget is
first-class data carrying a LanguageSpec (data-not-tag), not a bare
4-arm coproduct consumers re-dispatch on. Drops v2's write-only
ArtifactKind and never-inhabited Boundary subsystem (loyal-dove-547
PR #3406 finding) — they re-enter only when a consumer needs them.

Cross-stage contract: imports LanguageSpec from `v2_5.compiler.emit`
(crisp-dove-396 owns the shape; module path provisional, surfaced to
the PM). No bootstrap binary for v2.5 yet (sunny-otter-371).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briansrls briansrls changed the title v2 stage rework — artifact.dag — artifact-plan + RenderTarget dispatch — apply v4 modeling discipline (coproduct dissolution + grounding + compiler homomorphism); CALIBRATION investigation; sibling-parallel to 7 stage workers + complexity.dag v2.5 artifact.dag — typed artifact-plan + RenderTarget-as-data (fork probe) May 19, 2026
@briansrls
briansrls marked this pull request as ready for review May 19, 2026 22:07
briansrls and others added 3 commits May 19, 2026 22:09
PM rulings on cross-stage questions:
- Q3: RenderTarget was a hollow wrapper around LanguageSpec — collapse.
  Artifact.target is now LanguageSpec directly; type RenderTarget gone.
  If per-build configuration emerges as a real differentiator, it enters
  then (as a variant), not pre-built as an empty carrier.
- Q4: Practice 9 strict for all v2.5 calibration probes — no in-file
  prose; PR body carries the v2→v2.5 mapping narrative. Stripped the
  per-declaration explanatory comments and section banners; file header
  metadata block (Scope/Owns/Consumes/Status) stays — that's structured
  metadata, not prose, and matches the v4 file convention.
- Q1: LanguageSpec module path stays provisional with a RECONCILE-NEEDED
  marker pending crisp-dove-396 sign-off; PM is routing.
- Q2: module-naming v2_5.compiler.artifact confirmed.

95 → 47 lines; one type (RenderTarget wrapper) and three paragraph
comment blocks dissolved.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PM/crisp-dove-396 ratification: LanguageSpec is homed at
`src/v2.5/std/language_spec.dag` with module path
`v2_5.std.language_spec`. Reasoning: std/ tier per multi-consumer
criterion (emit + artifact); mirrors v4's pattern (spec-carrier TYPES in
`src/v4/std/*`, per-target INSTANCES in `src/v4/extdeps/languages/`).

Import path updated; RECONCILE-NEEDED marker cleared from the file
header. Type shape unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per codex review on PR #3417 (APPROVE_WITH_COMMENTS): header was
non-conforming. Fixed both findings:
- Scope: was split across two lines; now one line.
- Status: was carrying process/rationale prose ("fork first-pass — no
  bootstrap binary yet (sunny-otter-371)"); now terse ("fork first-pass").

Matches the v4 file convention (one line per Scope/Owns/Consumes/Status
field).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Addressed in commit 773b5036e — Practice 9 header now conforms:

  • Scope: is one line (was split across lines 2–3).
  • Status: is terse (fork first-pass.); the process/rationale text was removed.

Verified current head of the branch matches the four-line shape.

— sent from loyal-dove-547

@briansrls

Copy link
Copy Markdown
Contributor Author

Thanks for the catch — the finding is right on substance. Updated the PR body without a code change:

  • The Q1 disposition is path agreed, file pending, not "resolved" in the unqualified sense. crisp-dove-396 ratified the path v2_5.std.language_spec, but they own publishing the file and src/v2.5/std/language_spec.dag has not yet landed in the tree — git ls-files src/v2.5/ is exactly the one file you observed.
  • I added a real 🟡 entry: the import at artifact.dag:9 is a dangling reference relative to current repo state, consumer-gated on that publish. Cleared the prior PR-body claim that the 🟡 list was empty.
  • The import path itself stays as-is per the ratified contract — it resolves the moment crisp-dove-396 publishes, no further edit here.

Not pursuing the exploratory isolated-parse smoke in this PR: there's no v2.5 bootstrap yet (sunny-otter-371's parallel investigation), so a hermetic smoke depends on the same publish.

— sent from loyal-dove-547

@briansrls

Copy link
Copy Markdown
Contributor Author

Thanks for the review.

On the dangling v2_5.std.language_spec import — already disposed in the PR body and a prior comment: the path is crisp-dove-396-ratified, the file is theirs to publish, and the import resolves cleanly the moment it lands. Carried as a 🟡 (consumer-gated on that publish).

On the exploratory path-vs-target authority split — that's the intended split, just worth making explicit: ArtifactPath { output_dir, file_stem } carries the per-build location parts, and the file extension is owned by LanguageSpec, not the path. The full output filename is output_dir + "/" + file_stem + spec.file_extension — extension/suffix come from the target, stem from the artifact, no overlap and single-sourced per facet. When the orchestrator (keen-ibex-355) and emit (crisp-dove-396) wire up, that's the contract their compose step relies on.

— sent from loyal-dove-547

@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: 7afc9b48 · Trigger: schedule
  • Thinking: 143s wall

BLOCKING (1)

Root Cause

  • src/v2.5/artifact.dag The fork introduces a v2_5 compiler artifact namespace before the corresponding v2_5 LanguageSpec authority exists → land/reuse the canonical LanguageSpec module in the same scope before importing it.

⚠️ One unresolved module dependency blocks this otherwise small artifact-plan model.

Comment thread src/v2.5/artifact.dag
// Status: fork first-pass.

module v2_5.compiler.artifact

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.

BLOCKING: The import targets v2_5.std.language_spec, but this PR adds no such module and git ls-tree origin/main shows only src/v2/languages.dag for this slice, so the new model has an ungrounded dependency and fails P1/P2 loadability.

@briansrls

Copy link
Copy Markdown
Contributor Author

Acknowledged. The dependency is ungrounded against current main — git ls-files src/v2.5/ shows only artifact.dag — so the BLOCKING is correct on substance.

Per the cross-stage coordination rule (PM-routed only), I've surfaced to sunny-wolf-435 with three dispositions for your call:

  • A. I create src/v2.5/std/language_spec.dag as a minimal stub now (module + placeholder LanguageSpec type) to ground loadability — risks stepping on crisp-dove-396's domain since the PM previously assigned that file to them.
  • B. Remove the import + retype Artifact.target to a primitive — regresses the data-not-tag dissolution this fork demonstrates.
  • C. Wait on crisp-dove-396 to publish — doesn't address the BLOCKING in this PR.

Holding for PM/operator routing; will action whichever path comes back.

— sent from loyal-dove-547

@briansrls

Copy link
Copy Markdown
Contributor Author

Same root cause as the operator's BLOCKING posted at 00:52:57Z — ungrounded v2_5.std.language_spec import; the operator's merge of main into the branch (commit 7afc9b480) didn't change my file. Already escalated to PM with three dispositions (create stub here / remove import / wait on crisp-dove-396); holding for direction. See the prior acknowledgment for the path options.

— sent from loyal-dove-547

@briansrls
briansrls changed the base branch from main to session/crisp-dove-396-v2_5-emit May 20, 2026 00:56
@briansrls

Copy link
Copy Markdown
Contributor Author

Disposition from PM (msg_ab610abb): PATH D — stack this PR on crisp-dove-396's PR #3429, which already publishes src/v2.5/std/language_spec.dag on session/crisp-dove-396-v2_5-emit. Stub-here was vetoed (it would duplicate when #3429 merges — same shape as the recent cool-bee-832/vivid-lynx-807 syntax.dag conflict).

Actioned:

  • PR base changed from main → session/crisp-dove-396-v2_5-emit (gh pr edit 3417 --base ...); the import now resolves against crisp-dove-396's tree.
  • PR body banner documents the stack and the merge order: v2.5 emit: framework + LanguageSpec carrier (std/) + Rust spec instance #3429 merges first; this PR then rebases to main.
  • 🟡 list and Q1 disposition updated to reflect the resolution.

No code change needed in this file — the import path was already the ratified one.

— sent from loyal-dove-547

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: 7afc9b48 · Trigger: manual
  • Comparison: session/crisp-dove-396-v2_5-emit @ c5d13140 ... session/loyal-dove-547-v25 @ 7afc9b48
  • Conversation: View conversation

1. Story of the diff

This PR adds one new .dag authority, src/v2.5/artifact.dag, that models the output side of the v2.5 pipeline. It imports the resolved LanguageSpec, defines an ArtifactPath, an Artifact that binds a name, target language spec, entry modules, and output path, then wraps those in an ArtifactPlan. The only behavior is single_module_plan, a constructor that takes one root module plus a target/output and produces a one-artifact plan where the root module is both the artifact name and sole entry module. The load-bearing direction is good: target selection is data (LanguageSpec) rather than a new render-target wrapper, so target identity is being pulled into the model rather than encoded as a compiler-side enum.

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Finding — src/v2.5/artifact.dag:21, src/v2.5/artifact.dag:32: Boundary Discipline / illegal states unrepresentable. This file is substrate/modeling work, not implementation-only Rust: Artifact and ArtifactPlan are compiler artifact-plan data that downstream pipeline/emission code will consume. The PR correctly models target as LanguageSpec, but module identity is still raw String via entry_modules: List<String> and root_module: String. That leaves the artifact plan unable to structurally prove that entries name existing modules, and forces later consumers to reinterpret names by string convention. Boundary Discipline requires boundaries to carry enough declared information for mechanical consumers and keep facts in one authority, not in a string later re-resolved by consumers. chatgpt-review-074bb0dc-64a7-4e…

Use a canonical module/declaration reference carrier here, or add a bounded 🟡 gate naming the missing module-reference carrier and its dissolution trigger.

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

Finding — src/v2.5/artifact.dag:21: facts flow forward / single authority. entry_modules: List<String> makes the plan carry an unresolved spelling rather than the resolved module fact. That is exactly where facts stop flowing forward: the compiler knows the module graph earlier, but the artifact plan drops back to names and makes the next stage do another lookup. This is PR-relevant because the new file’s purpose is a “typed artifact-plan”; target is typed, but the source module dependency is not. The fix is to make entry modules resolved data in the plan, not source-level strings.

  1. CODING.md.

Compliant. The new behavior is a small pure function over explicit inputs, returning structured data (ArtifactPlan) rather than mutating hidden state; the code is data + function, not object/method scaffolding. The comment/header debt is handled under tracked debt below rather than treated as a separate style nit.

  1. TESTING.md.

N/A — no test surface or executable compiler behavior is added beyond one pure .dag constructor. I would not require a separate test before the model-shape issue above is fixed; once module references are structural, any existing whole-tree .dag parse/compile gate should be enough unless this file gains a consumer or execution claim.

  1. LOCKED DESIGN DECISIONS.

Compliant. The diff does not alter a locked doc, and the core design move is aligned with the Shape A direction: Artifact.target is LanguageSpec (src/v2.5/artifact.dag:20) rather than a new compiler-private target taxonomy. That preserves the thesis distinction that compiler language targets are driven by language specs, while non-language artifacts remain user-program Shape B emitters. chatgpt-review-850ddf84-e608-4e…

  1. TRACKED vs UNTRACKED DEBT.

Finding — src/v2.5/artifact.dag:5: Progress Is Dissolution / scaffold boundaries. // Status: fork first-pass. marks this as provisional, but the PR does not bound the provisional state or name the dissolution trigger. P5 requires scaffolds/intermediate surfaces to land with explicit dissolution paths; a scaffold without a trigger becomes load-bearing by accident. chatgpt-review-074bb0dc-64a7-4e…

Either make this a terminal status, or record a real tracked bridge: documentation of what is provisional, the bound, and the trigger such as “dissolves when artifact-plan consumer X reads structural module refs,” with the corresponding task/PR lane. Practice 9 allows a terse four-line header, but it does not make “first-pass” a valid unbounded debt state. chatgpt-review-75d59b8d-284d-4b…

2.5. Top-down PM intent review

Compliant. At the PM-intent level, this PR is trying to move build outputs into .dag data and collapse target choice onto LanguageSpec, which preserves the target-realization direction rather than adding a new compiler render-target path. I do not see a concrete diff-cited mismatch with Shape A/Shape B, Pure Bootstrap, or locked design intent. The blocking concerns are lower-level modeling discipline: untyped module references and an untracked “fork first-pass” state.

3. Verdict

REQUEST_CHANGES. The direction is sound, especially the LanguageSpec target choice, but this is substrate/modeling data and it currently leaves module identity as raw strings plus an unbounded provisional status. Those two issues should be fixed before merge so the new artifact-plan surface does not become a string-reinterpretation boundary or an untracked fork scaffold.

Per openai-pro REQUEST_CHANGES on PR #3417 (Finding 2 — P5 Progress Is
Dissolution): the "fork first-pass." Status was unbounded provisional
state with no dissolution trigger; a scaffold without a trigger becomes
load-bearing by accident.

Bound it to a concrete consumer event: dissolves when keen-ibex-355's
compile.dag (Tier 3 orchestrator) consumes ArtifactPlan. That's the
first real consumer of this surface; until then it's scaffold-only.

Finding 1 (entry_modules: List<String>) is disposed via the PR body
🟡 list per the reviewer's offered alternative — at the plan layer the
entries are user-supplied names pre-resolver, and a typed wrapper
around String would be the hollow alias the PM already vetoed in Q3.
Consumer-gated on the v2.5 resolver stage publishing a structural
module-reference carrier.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Thanks — both findings addressed.

Finding 2 (P5, unbounded Status: fork first-pass.) — fixed in commit 96718cad1. Status is now scaffold; dissolves when keen-ibex-355's compile.dag consumes ArtifactPlan. — concrete consumer event, no longer an unbounded provisional.

Finding 1 (entry_modules: List<String> / root_module: String) — disposed via PR-body 🟡 per the reviewer's offered alternative, not a typed-wrapper change. Reasoning:

  • The artifact plan is the pre-resolver layer. The user supplies module names; no resolved-module carrier exists yet because nothing has been parsed. Strings are the right shape at this boundary — they're the user-facing input.
  • The single-authority concern about downstream re-resolution is real, but the right place to fix it is in the post-resolve stages carrying the resolved form, not by pre-wrapping the user input.
  • A thin type ModulePath = String here would be the hollow-alias smell the PM already vetoed in Q3 ("if per-build config emerges as a real need later, ADD A VARIANT then; don't pre-build a wrapper for hypothetical future fields"). The reviewer explicitly offered the bounded 🟡 as the alternative; I've taken that — see the new 🟡 entry in the PR body, consumer-gated on the v2.5 resolver stage publishing a structural module-reference carrier.

PR body 🟡 list updated. Banner already documents the cross-PR stack on #3429.

— sent from loyal-dove-547

@briansrls

Copy link
Copy Markdown
Contributor Author

Already disposed — verified against current branch HEAD (96718cad1):

  • Finding 2 (unbounded Status) — fixed in commit 96718cad1. Current line 5 reads: // Status: scaffold; dissolves when keen-ibex-355's compile.dag consumes ArtifactPlan. — concrete consumer trigger, no longer unbounded.
  • Finding 1 (entry_modules: List<String>) — disposed via the new entry in the PR body 🟡 list, per the reviewer's explicitly offered alternative ("add a bounded 🟡 gate naming the missing module-reference carrier and its dissolution trigger"). The plan layer is the pre-resolver boundary; a thin type ModulePath = String wrapper would be the hollow-alias smell the PM vetoed in Q3. consumer-gated on the v2.5 resolver stage publishing a structural module-reference carrier.

See the full disposition reply for reasoning.

— sent from loyal-dove-547

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: 96718cad · Trigger: manual
  • Comparison: session/crisp-dove-396-v2_5-emit @ c5d13140 ... session/loyal-dove-547-v25 @ 96718cad
  • Conversation: View conversation

1. Story of the diff

This PR adds a new src/v2.5/artifact.dag model that makes the compiler’s output plan explicit data instead of an implicit render-target wrapper. The new file defines an output path shape, an Artifact carrying a LanguageSpec, entry module names, and output path, then wraps those in ArtifactPlan; single_module_plan is the constructor for the common “compile one root module for one language target” case (src/v2.5/artifact.dag:9, :12, :18, :26, :31). The load-bearing move is that target selection is now carried by LanguageSpec data (src/v2.5/artifact.dag:20) rather than a parallel RenderTarget taxonomy, so the future compile.dag consumer can read the plan structurally.

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Compliant — this is substrate/modeling surface, not Rust implementation. The diff introduces .dag data types under v2_5.compiler.artifact; the target field is typed as LanguageSpec (src/v2.5/artifact.dag:20) and the file’s scaffold status names the downstream consumer trigger (src/v2.5/artifact.dag:5), so it does not create a second implementation-only target authority.

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

Compliant — P2/P5 single-authority and scaffold discipline are handled by importing LanguageSpec from v2_5.std.language_spec (src/v2.5/artifact.dag:9) and by binding the staged surface to “compile.dag consumes ArtifactPlan” (src/v2.5/artifact.dag:5). No new coproduct, predicate helper, walker, alias, or target-private realization schema appears in the diff; the new data is product-shaped and reads forward into the named future consumer. INVARIANTS requires scaffolds to carry a named dissolution trigger, which this status line does. chatgpt-review-80c22c73-6062-4e…

  1. CODING.md.

Compliant — the new .dag file follows Practice 9’s allowed comment shape: path line plus four-line header, with no per-type explanatory prose (src/v2.5/artifact.dag:1-5). Names are direct and data-first (ArtifactPath, Artifact, ArtifactPlan, single_module_plan), and the constructor is a small pure function rather than hidden compiler behavior. Practice 9’s allowed header/comment shape matches this file. chatgpt-review-2d7013ba-f7cc-42…

  1. TESTING.md.

N/A — this PR adds a data scaffold and a trivial constructor but no current consumer or bug fix. A behavior test becomes meaningful when compile.dag consumes ArtifactPlan; until then, a dedicated test would mostly restate the literal constructor body at src/v2.5/artifact.dag:36-42.

  1. LOCKED DESIGN DECISIONS.

Compliant — the diff preserves the Shape A compiler-target boundary by using LanguageSpec as the target data (src/v2.5/artifact.dag:20) rather than broadening compiler render targets to arbitrary artifact classes. That aligns with the thesis distinction that compiler targets are programming-language targets while Shape B artifacts are user programs walking typed values, not compiler render targets. chatgpt-review-3ddd6669-ac35-4d…

  1. TRACKED vs UNTRACKED DEBT.

Compliant — the only scaffold is explicitly documented and bounded in the file header: scope/owns/consumes identify the surface (src/v2.5/artifact.dag:2-4), and the status line names the dissolution trigger, “when keen-ibex-355’s compile.dag consumes ArtifactPlan” (src/v2.5/artifact.dag:5). I do not see an unbounded TODO, temporary wrapper, or comment-only bridge.

2.5. Top-down PM intent review

Compliant — the PR’s stated intent is “typed artifact-plan + RenderTarget-as-data,” and the diff does that by making the artifact plan itself data while binding the target to LanguageSpec (src/v2.5/artifact.dag:18-27). It does not dilute the higher-level target-realization direction into a new compiler-side target enum or per-target implementation path; the plan remains language-spec-driven, which is the thesis’s intended Shape A cost model. chatgpt-review-3ddd6669-ac35-4d…

3. Verdict

APPROVE

No blocking invariant issue surfaced. The PR is small, but it moves target planning in the right direction: typed data, no new Rust path, no new coproduct/predicate debt, and the only scaffold has a named consumer-trigger dissolution path.

@briansrls

Copy link
Copy Markdown
Contributor Author

Acknowledged — APPROVE verdict on current HEAD (96718cad), no findings to address. Together with codex's APPROVE on the same sha, the two prior REQUEST_CHANGES blockers are superseded.

— sent from loyal-dove-547

@briansrls

Copy link
Copy Markdown
Contributor Author

Closing per operator wrap-up directive 2026-05-20.

@briansrls briansrls closed this May 20, 2026
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