Skip to content

γ - #535

Closed
briansrls wants to merge 12 commits into
mainfrom
session/royal-moth-11
Closed

γ#535
briansrls wants to merge 12 commits into
mainfrom
session/royal-moth-11

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Opened from session-dashboard for session royal-moth-11.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f62927f51

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/v3/compiler/src/lens_testgen.rs Outdated
FieldValue::Literal(LiteralBits::String(file_name)),
),
("predicate".to_string(), predicate),
("requires".to_string(), FieldValue::List(Vec::new())),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Set compile-time resource sentinel in generated TestClaim

TestgenLens::push_claim now always emits requires as an empty list, but src/v3/std/verification.dag in this same change defines compile-time predicates as requiring the bootstrap sentinel { identifier: "compile_time" } and reserves the empty-list case for a later dissolution step. With materialize_obligations forwarding suite.claims unchanged, downstream runners that consume requires will see these generated compile-time claims as dependency-free and skip the intended resource classification/acquire behavior.

Useful? React with 👍 / 👎.

@briansrls

Copy link
Copy Markdown
Contributor Author

ChatGPT review in progress... (view conversation)

Check back in ~30 minutes for the full review.

@briansrls briansrls mentioned this pull request Apr 18, 2026
Merged
@briansrls

Copy link
Copy Markdown
Contributor Author

claude-review · director-mode · γ (#535)

⚠️ Superseded by β (#534) — recommend close.

This PR implements DB-15 R2 (Stage 2c test infrastructure) inline in std/verification.dag. β (#534) also implements DB-15 R2 as part of its XL scope, with a strictly cleaner placement:

Dimension γ (this PR) β (#534)
ResourceReference placement inline in verification.dag separate v3.std.resources module (single authority)
ResourceReference typing { identifier: String } (opaque string) { target: DeclarationRef } (typed handle)
Full resources.dag port no yes — adds ResourceHandle too
TestObligation shape materialize_obligations(suite) -> List<TestClaim> (pass-through) TestObligation { claim_name, resources } + materialize projects to obligation list
Test coverage m1_5 assertions for the two new predicate variants + ResourceReference β adds those + a dedicated lane2_stage_2c_db15_test.rs

Per the substrate principle audit (feedback memo)

Q2 Index/handle: γ's identifier: String is the anti-pattern recovery named by the audit — opaque strings should become typed structures (DeclarationRef here). γ's own scaffold comment acknowledges this: "the string is the bootstrap-local carrier pending the resources-in-v3 port." β does the port.

Q5 Construction authority: γ places ResourceReference in verification.dag, making it local to that module. β makes it a first-class substrate piece in v3.std.resources so TestPredicate::MockBackedInvariant.mock_transport: ResourceReference and any future resource-aware substrate can reference it from a single authority.

Literal incompatibility with β

γ's m1_5_verification_test::bootstrap_loads_verification_authority_types asserts:

assert_eq!(record_fields(&dag, "ResourceReference"), vec!["identifier"]);

β changes ResourceReference.target: DeclarationRef, so this assertion would fail under β. The two PRs cannot both land; the assertion would need to be rewritten either way.

Recommendation

Close this PR as superseded by β. If β lands first, there's nothing left for γ to do on DB-15 R2. If γ has residual work that isn't in β (there wasn't any I could identify), rebase γ onto β's v3.std.resources + DeclarationRef-typed surface.

Not blocking from the dispatch side — this is a coordination-loss artifact (overlapping scope in the Stage 2c lane vs XL 2b+2c lane), not a quality issue on either side.

@briansrls briansrls closed this Apr 18, 2026

@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.

codex · gpt-5.4 · 49bdae50

⚠️ Review (blocking: 2, non-blocking: 0+/0-)

BLOCKING (2)

Root Cause

  • src/v3/std/verification.dag DB-15 extends the legacy source-string claim shape with predicate-local DeclarationRefs instead of choosing one authority for subject identity -> either move subject identity onto TestClaim and derive compilation from it, or split source-backed compile claims from declaration-backed behavioral claims.
  • src/v3/std/verification.dag Per-claim acquisition was added independently of predicate-local resource fields -> model the resource once and derive the runner acquire set from that authority instead of storing both requires and mock_transport as peer inputs.

ROADMAP — Verified

  • DB-15 generated-claim compile_time sentinel: The lens_testgen.rs change plus the new Lane 2 smoke test do close the earlier requires: [] runner-classification hole for generated compile-time claims.

⚠️ The compile-time sentinel fix landed cleanly, but the new verification substrate still splits subject and resource authority in ways that will harden once Stage 2c consumes it.

comparator: ComparisonOp
bound: Int
}
| BehavioralObservation {

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: BehavioralObservation and MockBackedInvariant add declaration-based subject slots while TestClaim still identifies the program under test only via source: String, so a claim can pair arbitrary DeclarationRefs with arbitrary source text and the subject fact no longer has a single authority (principles 2 and 5).

source: String
file_name: String
predicate: TestPredicate
requires: List<ResourceReference>

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: TestClaim.requires is a free list even though MockBackedInvariant already carries mock_transport, so the same runtime dependency can be duplicated or contradicted and the compile-time-vs-runtime resource rule remains convention rather than type-enforced (principles 2, 5, and 6).

@briansrls

Copy link
Copy Markdown
Contributor Author

codex · gpt-5.4 · 49bdae50

⚠️ Review (blocking: 2, non-blocking: 0+/0-)

BLOCKING (2)

Root Cause

  • src/v3/std/verification.dag DB-15 extends the legacy source-string claim shape with predicate-local DeclarationRefs instead of choosing one authority for subject identity -> either move subject identity onto TestClaim and derive compilation from it, or split source-backed compile claims from declaration-backed behavioral claims.
  • src/v3/std/verification.dag Per-claim acquisition was added independently of predicate-local resource fields -> model the resource once and derive the runner acquire set from that authority instead of storing both requires and mock_transport as peer inputs.

ROADMAP — Verified

  • DB-15 generated-claim compile_time sentinel: The lens_testgen.rs change plus the new Lane 2 smoke test do close the earlier requires: [] runner-classification hole for generated compile-time claims.

⚠️ The compile-time sentinel fix landed cleanly, but the new verification substrate still splits subject and resource authority in ways that will harden once Stage 2c consumes it.

@briansrls
briansrls deleted the session/royal-moth-11 branch June 1, 2026 18:42
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