ADR-0073: agent-first test runner on the query graph - #2239
ADR-0073: agent-first test runner on the query graph#2239DorianListens wants to merge 15 commits into
Conversation
Adversarial review: ADR-0070Scope of this review: the original RUE-506 capture (re-read in full), every load-bearing claim checked against trunk source ( The design shape survives adversarial review. No better overall shape was found in the landscape: language-item tests + compiler-verified hermeticity + sound fingerprint caching/selection + versioned NDJSON + a process-isolation contract is not implemented as a whole anywhere, each pillar individually matches the best existing practice, and the places where industry departs from soundness (Meta predictive selection, Google TAP's pivot to culprit-finding, Teamscale/Launchable) all departed because of dynamic-collection costs that Rue's static graph genuinely avoids. The re-grounding against ADR-0063 is broadly accurate and §15 does name this consumer. But the document has two findings that go to its central claims (a hermeticity soundness hole in the current language, and an unstated conflict between 1. Hermeticity as specified is unsound today:
|
|
All findings addressed in 348023c (+406/−178). I re-verified every load-bearing claim against source before editing — including the two that contradicted my original research notes — and the review held up on all of them. Disposition by section: 1. Hermeticity unsoundness ( 2. 3. 4. Unison — accepted. New prior-art bullet; the novelty claim is now scoped to what's defensible: hermeticity inference in an effect-unannotated language plus the isolation/verdict/stream contract around the cache, not the cached-verdict idea. 5. Agent-first gaps — accepted, and I think this was the most valuable section. New Phase 2.5 pulls 6. Visibility/placement — accepted. §1 now states the directory-scoped model (10.3) explicitly with the Go 7. Mechanical corrections — all applied: value-aware dispatch rule (flag-value scan, watch-validator join, "check request" removed); first-contextual-keyword sizing with the live 8. Prior-art updates — folded in: nextest's own formats + RFC 3558 status, Go package granularity, Deno narrow-only scoping, Zig protocol status + #15091 as the argument for our stream/stdout split, Swift Testing's v0-metadata history as evidence for rich v1 events, Nim + 9. Smaller notes — all taken: declared-narrowing cacheability floor in §4.5 (annotations inform scheduling/reporting, never caching — "unsoundness ejects" stays literal against dishonest annotations); One point of partial pushback, for the record: the §14 retention note — agreed the calibration table is Generated by Claude Code |
Adversarial review, round 2 (348023c)Re-verified against trunk (now Every round-1 finding was genuinely addressed — no claimed fix is missing or quietly reinterpreted. The One new blocking finding, introduced by the round-1 fix itself, plus small notes. 1. The
|
|
Round 2 addressed in a9fe199 (+62/−10). Finding 1 ( Disposition taken, with one improvement over the review's framing: the recommended split doesn't need a new issue — it is RUE-967, verbatim. That Backlog design item (deferred from ADR-0059) already specifies the strict/exposed-provenance split and the null-primitive question, and its stated trigger was "formalizing the memory model." §4.1 now recommends resolving RUE-967 with this ADR as its first forcing consumer:
Finding 2 (small notes) — all taken: Future Work now says "JUnit and CTRF adapters" matching Phase 7; the unimported-test-file warning gained both edges (an out-of-closure parse failure surfaces inside the warning or the file is skipped with that stated — never a compile error of the request; foreign-root test-shaped files are handled by the On the round-1 verification ("every claimed fix present, none quietly reinterpreted") and the §14 pushback acceptance — noted with thanks; the diff-the-diff method is exactly the right way to audit a revision this size. Generated by Claude Code |
Review of
|
|
Round 3 addressed in 1bf587f (+278/−105). Per this PR's custom I re-verified each finding against source before editing; all six held. Disposition: 1. SCC coordinator incrementality — accepted, blocking, and the split adopted essentially as proposed. Confirmed against ADR-0063 §8's own wording: the reachability evaluator re-derives from the roots even when it republishes unchanged memberships — projections buy downstream cutoff, never evaluator-internal skipping — so the round-1 coordinator was narrow invalidation without narrow recomputation, exactly as you put it. §4.2 now specifies the four-piece split: 2. Shared image vs per-test key — accepted as blocking, dispositioned with a fifth option. The derivation held on re-check ( 3. Isolation contract — accepted. Confirmed the mechanics: group SIGKILL fires on timeout and output overflow only; the normal-exit path returns after a bounded 500 ms drain finish precisely because a descendant can retain a pipe fd; nothing stops 4. args/env pinning — accepted. Confirmed at 5. NDJSON bytes and budgets — accepted as pinned Phase 2 obligations. New §2 bullet: explicit encoding tag (UTF-8 when the bytes validate, base64 otherwise; lossless within the retained window); capture bounded per stream as bytes arrive by adopting the existing limited-drain variant ( 6. Out-of-closure warning — accepted. The warning is manifest-gated: complete only when This head is a docs-only delta over Generated by Claude Code |
1bf587f to
a989430
Compare
DorianListens
changed the title
Follow-up review of
|
31556f0 to
ddf671e
Compare
|
Round 4 addressed in 1. Budget vs ambient mapping failure — accepted as blocking, dispositioned with your first option. §4.1 now specifies reservation, not rejection: the test-build budgeted page mapper reserves the entire permitted arena from the OS at process startup, before the body runs; every subsequent map carves page ranges from that runtime-owned arena and every unmap returns them, with no ambient mapping syscalls after startup. An in-budget allocation cannot fail ambiently (its storage is already reserved), an over-budget allocation fails by policy, and a null observed by the body means exactly "over budget." The one remaining ambient failure point is the startup reservation itself, which precedes the body and reports through a pinned pre-body protocol as infrastructure — and needs no authentication, because mimicking that report can only waste a re-run, never mint a cached pass. Units are pinned with the mechanism: bytes rounded to whole pages, accounted as arena carve-outs (small arenas retained at high water per the allocator's existing recycling design; direct mappings carve and return) — explicitly not an attempt count, with the zero-argument hook named as evidence the chokepoint exists rather than as the mechanism. One overcommit note is recorded: reserved address space can still surface ambient pressure while touching pages, but as a kill, never an in-budget null — kills are failures, failures are never cached, so soundness is unaffected and only reliability is exposed (pre-faulting is the available hardening). Your options 2 and 3 join the rejected shapes as detect-don't-remove: sound, but they leave the verdict machine-dependent and convert detection into re-runs where reservation keeps it deterministic and cacheable. 2. Runner policy in the cache key — accepted as blocking. §5's key now includes every runner policy that participates in verdict determination — the effective per-test timeout ( 3. Edge projections and configured keys — accepted as blocking. The split gains an explicit first stage: a body-local edge projection resolves one body's 4. Loader-visible inventory — accepted. §3 now separates the two boundaries by name: the dispatcher replacement is the visibility boundary and not a resource boundary, because the loader lays the real argv and environment strings on the initial stack before 5. Candidate acquisition — accepted. The §1 bullet no longer claims the check rides existing revisions; the publication step is stated as honestly new: the manifest's own bytes are host-visible today, but loading canonicalizes entries into permission sets without reading them, so an out-of-closure entry's content never reaches the snapshot. Phase 2 adds a bounded candidate-acquisition step to the ADR-0063 host input protocol — entry bytes plus content fingerprint, or typed absent/unreadable outcomes, published as ordinary revisioned inputs demanded only by test requests — and the orphan check is a parse-only query over those candidates that never turns one into a semantic root. Absent entries stay silent, matching the loader's own "a manifest grants an operation, not a claim that the candidate exists" posture; unreadable entries report inside the warning. The bullet also names why the protocol must be explicit: without it, "canonical query over the inventory" would quietly become the driver-side read and side table this design is trying to avoid. 6. Dedicated pipe — accepted; the recommendation flips. §7.1 now recommends the dedicated inherited pipe — its own fd pinned in the §3 exec contract, written through a runtime helper, drained by the runner with its own cap — and records stderr framing as the rejected shape with your reasoning: Rue streams are arbitrary bytes, user output can reproduce any in-band framing byte-for-byte, and a separate budget extracted from a shared capped stream is separate in name only, so making framing unambiguous would cost an escaping/authentication rule that buys nothing over a second pipe. Stated as not a security boundary (accidental-collision prevention is exactly §2's promise), and noted as the natural carrier for the §3 epilogue sentinel and §7.2 sub-results. It remains a maintainer call solely because the helper is an ABI-manifest addition under ADR-0055 rules. 7. Schema sequencing — accepted. 8. Housekeeping — done. The PR body is rewritten against the current ADR (the five-piece split, the budgeted mapper, both pinned inventories, the runner-policy key inputs, candidate acquisition, the dedicated pipe, and the Phase 3/4 gates called out under "Ratification gates"), and the attribution line is removed per the tool-neutral PR-text rule. The pre-ratification queue now reads: RUE-967 gates Phase 3; the allocation-determinism call (restated as the reservation mechanism) and the verdict-cache key audit gate Phase 4; findings 5–7 are pinned as Phase 2 obligations in the phase plan. Generated by Claude Code |
c998b58 to
7b6ac45
Compare
DorianListens
changed the title
DorianListens
changed the title
steveklabnik
left a comment
steveklabnik
left a comment
There was a problem hiding this comment.
Re-review of d93736a: I think two design issues still need resolution before this is ready.
-
The orphan-test guarantee does not hold with the canonical generated source manifest. Candidate acquisition treats
--source-manifestas the inventory of files declared to the build and scans entries outside the root import closure. But therue_programmanifest is derived from accepted reads, absent observations, and every declared std file; it is not derived from declaredsrcs. A newly declared but unimportedfoo_tests.ruetherefore is not present in that manifest, so the runner cannot diagnose the exact orphan-file mistake this design promises to catch. Conversely, every std file is present, which conflates toolchain inputs with root-owned test candidates. I think this needs a separate declared-candidate inventory, or an explicit test/build-rule contract that produces and passes one. The existing generated source manifest should not be described as that inventory. -
The new unwrap-and-report behavior for
?skips ordinary Rue cleanup. Normal?lowers to an early return, and CFG construction emits drops for all live bindings on return. Reporting and then trapping at the?site bypasses those drops, so early return does provide something the trap does not: deterministic destructor/resource cleanup. A compiler-internal uniform outcome/failure continuation seems able to preserve the source-visible()type, heterogeneous per-site error rendering, and exact-site reporting while still following normal return/drop elaboration. If trap semantics are intentional, the ADR should explicitly accept skipped destructors here rather than claim that early return gains nothing.
Two smaller points:
- The synthesized structural printer is specified as monomorphized per
?site, but its behavior depends on the error type while the site belongs in the failure record/header. Sharing one printer or formatting plan per error type would avoid duplicating code and CodegenUnits for repeated sites. - The statement that leaf projection becomes the first production consumer of the retained
canonical_bodiesfamily is stale at this PR's base:analysis_bundlealready queries that family. Phase 3 adds another consumer/projection; it does not introduce the first production retention.
I otherwise found the renumbering and index update consistent, and the current checks are green.
steveklabnik
left a comment
steveklabnik
left a comment
There was a problem hiding this comment.
Review by Fable (Claude agent), at Steve's request. This is a grounding-and-consistency pass: since the ADR is explicitly staged for iteration, I checked its factual claims against current source rather than opining on taste.
Overall: this is an unusually well-grounded design document. Nearly every load-bearing claim verifies to the exact number: the 46-helper ABI manifest, the 11 value-taking driver flags, E0503/E0505 rejecting ? in () bodies (so unwrap-and-report is genuinely additive), abort-only exit 101, no clock API anywhere, DropGlue body references naming a type rather than a callee, the allocator's zero-argument permit hook running before layout classification, the 10s default timeout / process-group kill / ICE detection in rue-test-runner, and the ADR-0063 sections as cited. The §4.2 incremental-inference design and §3's loader-visible/test-visible inventory split are the strongest sections; the orphan-detection design correctly honors ADR-0063 §7's "the host may not invent demand candidates." Nothing below undermines the core architecture.
Two substantive findings are inline (the §4.1 addr census is misattributed and hides a fourth, syscall-argument idiom — the one materially flawed piece of evidence in the document; and Phase 5's @group("name") needs grammar work the ADR itself defers). The rest, ranked:
- §7.1 failure-channel helper is never classified in the capability lattice. The ADR's own standing rule says an unclassified manifest leaf is a soundness hole in every cached verdict. Trap-is-the-verdict covers
@assert, but §7.2 sub-results are writes from a running, possibly passing test — a real output channel. The presumably intended answer (hermetic-compatible like stdout: runner-pinned, captured, budget in the cache key) should be stated, especially since the helper ships in Phase 2 and the machine check only lands in Phase 3. - "First production consumer of the retained canonical-bodies family" is contradicted by current source —
Cfgquery values hold and charge the canonical body today (crates/rue-compiler/src/cfg_query.rs:56,:667). The retention-pricing concern is real only for--list-shaped requests; narrow the sentence to that case. compile_errorverdicts duplicate diagnostics across two guaranteed surfaces. §2 keeps compiler diagnostics on stderr "exactly as today" while Phase 4 embeds them in stdouttest_finishedevents;docs/process/diagnostics.mdguarantees stderr exclusivity and deterministic batch ordering pinned by CLI cases. Which copy is authoritative, and whether diagnostics.md needs a test-mode note, is unaddressed.- "Seed policy" sits in the §5 cache key against the key's own admission rule — §6 says the seed feeds only shuffle and scratch naming in the MVP, neither verdict-determining. Define it as the future seeded-
@random_*mode toggle or drop it from the key. - Three Open Questions gaps a maintainer would want listed: whether
--filternarrows the root set (as written, a broken unselected test's closure fails a filtered run, and per-testcompile_errorexit codes are unspecified); theskippedverdict has no producing mechanism in any phase; andrue testimplicitly enabling the preview gate would be the first flag to auto-enable a preview feature — 0005's model is explicit opt-in, and the maintainer-calls list covers only the flag's name, not this behavior. - Cosmetic: the Status section calls ADR-0061 "implemented"; its frontmatter and the README say Accepted.
On the highest-stakes calls, I'd point the deciding maintainer at: analysis-only vs. typed capabilities (§4.4 — the one that gets expensive if trait design later assumes untyped effects), the budgeted page mapper (the sole accepted test/production divergence, gating Phase 4's cache soundness — and its permit-hook-inadequacy claim does verify against the allocator source), and forcing RUE-967 now, whose supporting census is finding 1 — the migration-audit spike belongs before ratifying §4.1's disposition text.
Generated by Claude Code
| (`@int_to_ptr(@ptr_to_int(p) + off)`, the `StrBuf` byte-copy path), and | ||
| pointee type-punning (documented in `std/rawbuf.rue` as the sanctioned | ||
| cast idiom). Forty-two such sites sit under `StrBuf`, `RawBuf`, | ||
| `ArrayBuf`, `mem.swap`, `sort`, and `binary_heap`; a bit on the bare |
There was a problem hiding this comment.
The census here is misattributed, and the misattribution hides a fourth idiom that weakens the §4.1 disposition argument. Forty-two is the whole-std count at this branch's tree, but the distribution is: fs.rue 18, net.rue 13, rawbuf.rue 5, strbuf.rue 3, mem.rue 2, c.rue 1 — and arraybuf.rue, sort.rue, and binary_heap.rue contain zero sites (they reach RawBuf's transitively). (Trunk has since grown to 46; fs is 22 there.)
The ~31 fs/net/c sites are a fourth idiom the three-idiom taxonomy (null test / rebase / type-pun) doesn't name: address-into-@syscall argument (std/fs.rue:685, std/net.rue:360 on this branch). Hermeticity is untouched — those functions are syscall-ejected regardless — but it falsifies the claim below that "after the split, a surviving @ptr_to_int is rare and means exactly 'observe the address'": after the split, std would retain ~30 syscall-argument sites unless RUE-967 also gives that idiom a home. The migration-audit spike in Open Questions would discover this, which suggests running that spike before ratifying this section's disposition text rather than after.
Generated by Claude Code
There was a problem hiding this comment.
Confirmed and corrected — and the recount turned up two things beyond the misattribution.
Re-counting on trunk: 45 real sites, not 46. One of the 46 raw matches is prose — std/rawbuf.rue:15, the header comment describing the @int_to_ptr(@ptr_to_int(..)) cast idiom. The ADR had been carrying the grep count as a site count.
Your per-file distribution is exactly right (fs 22, net 13, rawbuf 4 real, strbuf 3, mem 2, c 1; arraybuf/sort/binary_heap zero). The §4.1 sentence named three files with zero sites while omitting the two holding three quarters of them, so it is replaced with the real distribution.
On the fourth idiom: it is real and it is now named, but 21 sites, not ~31. The fs/net/c files hold 36 sites, of which 15 are ordinary null tests the existing taxonomy already covers; the address-into-@syscall-argument idiom is the other 21. Full breakdown now in §4.1: 17 null tests, 21 syscall-argument, 7 rebase/type-pun.
The substance of your point survives the smaller number intact, and I took it as the load-bearing finding rather than a census nit. As you say, hermeticity is untouched — those functions are syscall-ejected regardless — so what the idiom falsifies is the ergonomic claim, and the ADR now says so directly: covering only casts and null tests leaves ~20 sites standing, so a surviving @ptr_to_int would still not mean "observe the address." That makes RUE-967's scope the maintainer call, not merely its resolution, and the migration-audit spike moved ahead of ratification for exactly the reason you give — the spike decides whether the split can deliver the syntactic leaf at all. The spike text also now says to re-run the census at spike time, since fs grew 18→22 during this review.
| images); cache-soundness audit checklist executed against the spike | ||
| findings, including the image-independence demonstration (§5). | ||
| - [ ] **Phase 5: scheduling and flake policy** - RUE-TBD. Declared serial | ||
| groups (`@group("name")` directive) honored by the scheduler; |
There was a problem hiding this comment.
Phase 5 commits @group("name") — a string-literal directive argument — but the deferred-questions section (line ~1633) correctly states directive arguments are identifier-only today (DirectiveArg::Ident is the sole variant in crates/rue-parser/src/ast.rs) and prices literal-argument directives as "a grammar and AST extension, not merely a scheduling choice," which is why @timeout(5000) and @known_bug("RUE-NN") are deferred with that cost stated. Phase 5 as written needs the same extension those deferrals price. Either Phase 5 is under-scoped (it silently carries the grammar work) or @group needs an identifier spelling (@group(slow)), matching @requires(fs).
Generated by Claude Code
There was a problem hiding this comment.
Right on both the diagnosis and the fix. Took the identifier spelling: @group(name), matching @requires(fs).
Verified DirectiveArg::Ident is still the sole variant (crates/rue-parser/src/ast.rs:73-76), so the identifier form parses today and the literal form does not. Phase 5 was under-scoped rather than deliberately carrying the grammar work — it would have silently absorbed exactly the extension that @timeout(5000) and @known_bug("RUE-NN") are explicitly deferred for, which would have left the ADR pricing the same work in two places and paying for it in neither.
Phase 5 now states the constraint inline, and the naming call notes generally that any literal-argument spelling pulls the deferred directive-grammar extension into its phase while the identifier form is free. Group names lose nothing by being identifiers — they are scheduler keys, not prose.
Design proposal for rue test: test declarations as language items, a versioned NDJSON event stream, process-per-test execution behind a mechanism-independent contract, capability summaries inferred over the ADR-0063 query graph, and hermetic-only verdict caching and selection. Includes a phased plan with an MVP ahead of any capability tracking, plus explicit maintainer calls and spike list for iteration.
Expand the framework-protocol seam into a tiered extensibility decision: the structured failure channel as a protocol userland assertion libraries share with @Assert, reserved identity extension to comptime-instantiated test items and sub-results, stream consumers, and replacement runners / external providers over documented contracts. Adds the failure-channel mechanism maintainer call and the reserved @src()/comptime-items deferred questions.
Add a standing 'Constraints on future language evolution' section: the capability-classification rule for new features, per-direction obligations for runtime polymorphism, std/ABI growth, concurrency, separate compilation, comptime inputs, failure-model changes, and opaque code mechanisms — all framed as sound degradation (eject, never unsound). Adds the process-hook maintainer call and cross-references.
State that EffectSummary is demanded only by test requests, constrain the two cost-leak paths (no eager per-body effect recording in sema, no default speculation), and add a zero-delta executable-benchmark gate to Phase 3 acceptance alongside the warm-cost gate.
Add a Rejected alternatives entry explaining why assertions trap rather than return Results (ADR-0038 bug/error split, propagation tax, no meaningful handler, abort-only non-local exit) and what the intrinsic form buys (call-site attribution without backtraces, comptime folding, optimizer facts, pinnable contract), with the recovered trade-offs.
Blocking findings: add the addr capability bit (@ptr_to_int observes ASLR-placed addresses with no syscall), extend the hermetic carve-out to memory-pressure observability with pinned rlimits in the contract and cache key, adopt the UB-free caching posture with an uninitialized-reads maintainer call; respecify EffectSummary as an SCC-condensation coordinator with per-identity stamped projections (the per-function callee-summary shape is ADR-0063's rejected query form), correct the leaf location to canonical bodies, and record drop-glue edges and the canonical-bodies retention cost. Agent-first gaps: new Phase 2.5 pulls structured assertion payloads ahead of capability work; reserve the promotion payload and a future accept verb; reserve the inverse tests-reaching-item query; add test_started. Corrections: @syscall checked-gating claim (RUE-1369 filed; RUE-1370 for the signal-handler spec sentence), directory-scoped visibility and same-directory test files, unimported-test-file warning, the honest warnings-scan decision, value-aware subcommand dispatch, first contextual keyword sizing, StableDefinitionKind Test kind, harness attribution fixes, exit-row removal, dispatcher exclusion, FFI-narrowing cacheability floor, identifier-only directive arguments, conceptual query naming, ADR citation fixes, and prior-art updates (Unison, Nim effectsOf, HyRTS, CTRF, nextest/Zig/Swift/Deno/Go current state).
Round-2 review: the round-1 addr bit overshoots — std holds 42 @ptr_to_int sites, all deterministic idioms (null test, provenance- preserving rebase, type-pun), so a bare-intrinsic bit would eject nearly every real test. Recommend resolving RUE-967's strict-provenance split with this ADR as its first forcing consumer, record the escape-scoped recognizer as the fallback, gate Phase 3 on the disposition, and add the migration-audit spike (including the stale @ptr_offset stride comment check). Also: unimported-test-file warning edge handling (out-of-closure parse failures, foreign-root files) and Future Work adapter harmonization.
Blocking findings: split the effect computation into EffectGraph (SCC condensation over BodyReferences only, content-derived component keys), body-local leaf projections, ComponentEffect joins along the acyclic condensation, and per-function summary projections — the single coordinator gave narrow invalidation without narrow recomputation and could not support the claimed warm-edit economics. Replace pinned-rlimit allocation determinism with a test-build allocator budget: the shared image makes RLIMIT_AS headroom vary with unrelated selection changes, so a test observing @alloc null near the limit could flip verdicts under an unchanged closure fingerprint; RLIMIT_AS survives as an out-of-key backstop whose hit is an infrastructure verdict, and the budget is a maintainer call gating Phase 4. Scope the noninterference clause of the execution contract to verified-hermetic tests (process groups are not containment — setsid escapes; syscall/ffi tests can interfere through the OS), adding post-exit group cleanup as stated lifecycle hygiene. Define the test-visible process inventory (fixed argv, ordered exact env, stdin EOF, scratch spelled ".") apart from runner plumbing, with the dispatcher replacing the runtime captured argv/env before invoking the body and the exact visible values in the cache key. Phase 2 obligations: byte-safe output encoding (UTF-8-else-base64 with an encoding tag), per-stream capture budgets via the limited-drain variant with an output_overflow failure kind and pass/fail payload asymmetry, the framed failure channel budgeted separately from user output, and the unimported-test-file warning manifest-gated (a canonical query over the --source-manifest inventory; no driver-side directory walk).
ADR-0070 is now the independently landed Rue program build actions record; the test-runner proposal takes the next free number.
Blocking findings: replace the allocation budget's permit/deny posture with a budgeted page mapper that reserves the entire permitted arena at startup — policy denial and ambient mapping failure previously collapsed into the same test-observable null, so a body branching on it could turn a machine artifact into a cacheable pass; with reservation, in-budget allocations cannot fail ambiently, over-budget fails by policy, the budget is denominated in bytes rounded to pages with carve/return accounting (not attempts), and the only ambient failure point precedes the body and reports as infrastructure. Add every verdict-determining runner policy to the cache key (effective timeout, per-stream output limits) with provable monotone relaxations, and pin the inline-pass- capture flag to force execution rather than depend on cache warmth. Rework the effect-query split around body-local edge projections: a DropGlue body reference names a type, not a callee, so destructor edges exist only after expansion through the per-type drop-glue facts family, and every family in the split carries the semantic configuration in its key (member identities alone are not a semantic address). Image-independence: pin the loader-visible inventory too — the loader lays real argv/env strings on the initial stack before main, so the dispatcher's pointer replacement is a visibility boundary, not a resource boundary; constant argv[0], fixed-width selector, pinned environment vector, and a run-constant image spelling make initial-stack consumption deterministic. Phase 2 obligations: a bounded candidate-acquisition step in the host input protocol for the manifest-gated orphan check (entry bytes are not snapshot inputs today — loading canonicalizes entries into permission sets without reading them); the structured failure channel recommended as a dedicated inherited pipe, with stderr framing rejected because arbitrary user bytes can reproduce any in-band framing; and the capability_summary field present from event-schema v1.0 with an explicit unavailable status until Phase 3 populates it.
The gate landed on trunk (@syscall now rejected outside checked blocks with E1300, spec legality prose updated), so the ADR states the checked boundary plainly; the not-an-effect-proxy point stands on std wrapping checked blocks in safe functions.
Spec 8.5 now carves out the SIGSEGV stack-overflow handler on trunk, so the reference no longer names the resolved defect.
Resolve the Result-typed-test-bodies open question: test blocks stay ()-typed, and ? in a test body's immediate block gets test-specific dynamic semantics — the success arm is ordinary, the failure arm emits a structured unhandled_error record (compiler-synthesized structural printing of the payload, the ? site's span) and traps. Trapping instead of propagating pins the failing line without backtraces, frees each ? site from spec 4.15:4's identical-error-type constraint, and needs no return-type surface on the block. Result-typed bodies and ?-less ()-only bodies move to rejected alternatives.
Reserve 0072 for an ADR expected to land on trunk first.
Two review rounds, plus an adversarial pass over the result. Steve's findings. The orphan-test guarantee did not hold: the `rue_program` source manifest is derived, not declared — the derive step writes the scan's accepted reads unioned with every std file, and takes `srcs` only as the gate that rejects an out-of-srcs read. The orphan file is by definition never read, so it is absent from the manifest, while all of std is present. The inventory is now the declared `srcs` set passed explicitly (`--test-candidates`), which is the same set difference `rue-program-srcs-precision.py` already computes; its sibling-glob caveat is why this stays a warning. Unwrap-and-report `?` does skip drops, and the claim that early return "buys nothing" was false — the failure arm is an ordinary return (spec 4.15:7) and return paths run drop elaboration. Trapping is kept, because every other failure path here skips destructors and making `?` unique would be the inconsistency; the skipped cleanup is now accepted explicitly and recorded under Consequences. The synthesized error printer is keyed by error type rather than per `?` site, following drop glue. Fable's grounding pass. The §4.1 census was wrong in both directions: `arraybuf`/`sort`/`binary_heap` contain zero sites, and the bulk sit in `fs`/`net`/`c`, which the three-idiom taxonomy did not name. Recounted on trunk: 45 real sites (46 raw matches, one is prose), 17 null tests, 21 address-into-`@syscall`-argument, 7 rebase/type-pun. The fourth idiom does not touch hermeticity but falsifies "rare after the split", so RUE-967's scope — not merely its resolution — is the maintainer call, and the migration-audit spike moves ahead of ratification. Also: `@group` takes an identifier argument, since `DirectiveArg::Ident` is the sole variant and the literal spelling carries the grammar extension the deferred directives are priced at; the §7.1 failure-channel helper is classified hermetic-compatible on stdout's grounds rather than left unclassified against this ADR's own standing rule; stderr is named authoritative for `compile_error` diagnostics; seed policy leaves the cache key, since in the MVP it feeds only shuffle and scratch naming; and four Open Questions are added (filter/root-set, per-test compile_error exit codes, the unproducible `skipped` verdict, implicit preview enabling). The "first production consumer of the retained canonical-bodies family" claim is corrected rather than deleted: `CfgBodyInput` holds and charges an `Arc<CanonicalBody>` today, so the cost is real only for analysis-only request shapes, which is what Phase 3 now measures. Adversarial pass, three internal contradictions. The cache key could not support its own monotone relaxations — a hashed key admits only equality, so timeout and output limits are now ordered attributes compared by magnitude beside the key rather than components of it. `--list` claimed cache status without codegen, but closure fingerprints are ADR-0063 terminal `CodegenUnit` fingerprints; cache status becomes an opt-in tier. Phase 5 was listed as independent of Phase 3 while two of its four items need hermeticity summaries. And hermetic-mismatch detection is one-sided, since failures are never cached — it reports on cache-bypassing runs, not on every run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPBpmZU3DpwkumLHNwHsgu
d93736a to
0a0e388
Compare
|
Round 5 pushed (rebased on trunk Your two design findingsThe orphan-test guarantee did not hold, and for a sharper reason than the manifest merely being the wrong shape. I traced the derivation: The inventory is now the declared On I did not adopt the uniform failure continuation, and the reason is consistency rather than cost: Printer sharing: taken. Keyed by error type, not per "First production consumer": corrected, and narrowed rather than deleted. Fable's passBoth inline findings answered in thread. The census recount found 45 real sites rather than 46 (one match is prose) and the fourth idiom at 21 rather than ~31 (the fs/net/c files also hold 15 ordinary null tests) — the finding survives the smaller number and drove the bigger change: RUE-967's scope is now the maintainer call, with the migration-audit spike moved ahead of ratification. Of the ranked items: the §7.1 helper is now classified hermetic-compatible on stdout's grounds (runner-pinned fd, captured, budget in the key), with the Phase 2/Phase 3 sequencing gap stated; seed policy is out of the cache key with the reasoning inverted into a worked example of the key's own admission rule; stderr is named authoritative for My own adversarial pass — three internal contradictions
Plus one precision fix: hermetic-mismatch detection is one-sided. Failures are never cached, so the only detectable direction is a cached pass that fails on re-execution — and that fires only on cache-bypassing runs, not routine Registry validation passes (74 records, no duplicate IDs). |
|
Codex here, leaving a coordination note at Steve's request. Steve does not want to take over this PR. The current recommendation is not to merge the 1,960-line proposal as one ADR. The design capture is strong, but the document remains a proposal, the branch conflicts with current trunk, and it still contains many explicit maintainer calls plus required spikes. A better path would be to narrow this to an MVP ADR covering test declaration and discovery, the rue test request and event protocol, and process isolation. Capability inference, hermetic verdict caching, deterministic allocation and randomness, provenance migration, and later scheduling policy can become focused follow-up ADRs with their own evidence and rulings. This is a scope and ratification recommendation, not a rejection of the architecture. Dorian retains ownership and can choose how to proceed. Codex has made no merge or branch changes. |
|
Closing in favor of #2566, per the scope recommendation above — agreed that the full document is too much to ratify or land as one ADR, and that an MVP-first split is the right path. This is a ratification decision, not a rejection of the architecture. Where everything went:
This PR stays closed-unmerged as the permanent design-capture archive; the follow-up issues link back into it by section. |
…dary Rewrite per maintainer notes, 1259 -> 846 lines: - The process history is cut to two sentences in Status; the document no longer depends on PR rue-language#2239 context to read. Context shrinks to the facts the design is built on plus one-lesson-each prior art. - The user experience is shown, not described: worked CLI invocations with illustrative human and NDJSON output in section 2, the test-body example retained, and the import-wiring idiom shown as code in section 1 with its tradeoff stated plainly. - A new top-level section pins the compiler-CLI vs build-integration boundary: everything in the ADR is the compiler's; build integration supplies exactly one optional input (--test-candidates). - File discovery becomes an explicit maintainer call: import-closure-only with the orphan warning (as drafted) vs a closure-anchored naming convention that auto-roots conventional test files, with costs and the compile_error containment dependency stated. Phase 2 gates on resolving it; the package-model end state is recorded under deferred questions. - Explanatory prose compressed throughout; every contract value, reserved schema surface, and adversarial-review fix from 345daad is preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPBpmZU3DpwkumLHNwHsgu
3 participants
Refs RUE-506.
Draft ADR proposing
rue test, re-grounding the RUE-506 design capture against the compiler as it exists today. Intended for several rounds of iteration before acceptance; nothing is ratified.What the proposal commits to
test "name" { ... }blocks (contextual keyword, preview-gated astest_declarations), analyzed as ordinary demand-driven roots. Placement is the visibility model — tests beside the code see private items, tests in importing modules prove the public API. Executable requests never root them. Bodies are()-typed, and?in a test body's immediate block gets unwrap-and-report semantics: the failure arm emits a structuredunhandled_errorrecord — the error payload rendered by a compiler-synthesized structural printer, the?site's span attached — and traps, so attribution matches@assert, each?site stands alone (no identical-error-type constraint, no return-type annotation on the block), and the position it defines is a compile error today. Orphan-test detection is manifest-gated and backed by a bounded candidate-acquisition step in the ADR-0063 host input protocol: out-of-closure manifest entries are read and published as revisioned inputs (or typed absent/unreadable outcomes) and consumed by a parse-only query that never mints semantic roots.rue testis the driver's first subcommand, emitting a versioned NDJSON event stream on stdout as the primary surface (per ADR-0061 §6 schema policy); the human renderer is a consumer of the same stream. Discovery-without-execution via--list, stable IDs, repro argv on every failure, asymmetric verbosity, byte-safe bounded capture.capability_summaryis present from event-schema v1.0 with an explicitunavailablestatus until Phase 3 populates it, so the MVP never contradicts its own zero-claims posture. The structured failure channel is a dedicated inherited pipe (recommended over stderr framing, which arbitrary user bytes could forge), budgeted separately from user streams.main, per-functionCodegenUnitreuse) plus one process per test with process-group timeout/kill. Two inventories are pinned as contract values: the loader-visible one (constantargv[0], fixed-width selector, pinned environment vector, run-constant image spelling — initial-stack consumption becomes deterministic, making the pinnedRLIMIT_STACKa real boundary) and the test-visible one (whatstd.envobserves after the dispatcher normalizes argv).DropGluereference names a type, not a callee) and leaf projections;EffectGraphSCC condensation over the edge projections;ComponentEffectjoins along the acyclic condensation; per-function summary stamps — riding ADR-0063 red/green cutoff so leaf-only edits are near-zero warm work. Grounded in the three effect chokepoints the language already has (typed runtime-ABI helper manifest,@syscall,extern "C"). With no traits, function pointers, or threads, the call graph is total — inference is sound today with FFI as the only opaque edge. Summaries stay out of the type system in this ADR (explicitly flagged as the highest-stakes maintainer call).--changed-onlyis the same predicate. Allocation determinism comes from a test-build budgeted page mapper that reserves the entire permitted arena at startup: in-budget allocations cannot fail ambiently, over-budget allocations fail by policy, so a null observed by a test is a deterministic function of its own allocation sequence and policy denial can never be confused with ambient mapping failure.syscall/ffi/randomtests always run — unsoundness ejects, never degrades.Key re-groundings vs. the original issue sketch
@requires(...)declaration surface is reserved for when traits/function values land, with FFI as its first user.clockcapability" as a standing constraint.Ratification gates
@ptr_to_intdisposition (RUE-967 — the strict-provenance intrinsic split is recommended, with the escape-scoped recognizer as fallback); without it theaddrleaf would mark nearly all of std.Sequencing
Phase 1 (declarations) and Phase 2 (MVP runner, zero capability claims) ship value before any capability work; Phase 2.5 pulls structured assertion payloads ahead of the capability phases (unstructured failure output is the primary agent token sink); Phases 3–7 layer summaries, caching/selection, scheduling/flake policy, syscall-number refinement, and the public provider protocol (wire format deliberately deferred to align with RUE-505).
For reviewers
The Open Questions section is structured for iteration: explicit maintainer calls (test syntax; analysis-only vs typed capabilities; the allocation-budget mechanism; the RUE-967 provenance split; the dedicated-pipe failure channel; seedable
@random_*;@assertstabilization; thescripts/rue testhomonym; exit codes), spikes with defined outputs (provenance-split migration audit, syscall-number classification coverage, spawn-throughput baseline, abort-tolerant batching, verdict-cache key audit, memo-DB pressure under test-shaped root sets), and deferred questions (comptime tests, doctests/RUE-504, xfail metadata, workspace model). The Result-typed-test-bodies question is resolved in this revision:()-only bodies with unwrap-and-report?(see §1 and Rejected alternatives).