Original-span source8 adapter: fresh independent design review

2026-09-23 — delegated Astra critic; local-only artifact, parent owns adoption and publication.

Disposition: proceed with the bounded private adapter, with the clarifications below. The proposed opaque rename batch plus complete-snapshot compositor is compatible with eventual real mixed source8. A wholesale edit-AST replacement is not justified by the inspected rename problem. No P1 defect was established in this unimplemented proposal. The main risks are accidentally promising more ordinary edits than the collision rules admit, treating matching lifetimes/bytes as complete parse provenance, and allowing the narrow slice to stand in for full source8 editing.

Evidence and limits

Reviewed .local/analysis-source8-s3-original-span-adapter-proposal-20260922.md, SHA-256 8f99e8ea1ba522371c1e5df1a19ae94ceedeecfacfc96dc5345e0d24f623a9d; the S2/S3 handoffs; accepted r2 exact-editing requirements in harness/analysis-source8-20260913/analysis-source8-repair-design-r2-20260913.md from the analysis checkout; and the actual source below.

A = 1b1c3e3abb6fc3d0934c9dbfbd21fddc8ae4f594, .local/ai3, clean at inspection.
R = 4ffff0d9d94bd1875e6b316b84ac11e5c5c9657c, requested ordinary-source pin. Root advanced to cc3d6a48751ac5ab08e727cfd31c2b261bf8ade3 during review; independent Git comparison showed no differences from R in source_project.rs, source_project/source_structure.rs, or language.rs, so reads remain valid for R.
Source/design review only. No compiler, runtime, GUI, production changes, or Gitea writes. The proposed falsifiers are unrun; this review is not evidence of a working mixed parser or updater.

Actionable clarifications

P2 — define the supported ordinary edit set before routing it to the new compositor. Proposal lines 13 and 47 promise topology-preserving ordinary scalar changes, while line 43 rejects insertion at a replacement boundary. R's crates/oa-core/src/source_project.rs:1360–1371 deliberately handles a syntax-kind change by calling erase_syntax and inserting new text at the original start; erase_syntax (:1101–1113) emits individual token deletions, leaving trivia. Thus an existing producer can issue a meaningful insertion/deletion pair sharing a boundary. Stable model IDs alone do not exclude this form of syntax change. Either initially allow only explicitly enumerated leaf/header edits whose producer shape is supported, rejecting other changes before formatting/planning, or normalize this ordinary-owned compound operation into a single witnessed replacement with the same comment-preservation result. Do not simply relax cross-producer overlap checks. Add a positive oracle for that normalization before broadening the advertised scalar set.

R's apply_edits (:679–718) also concatenates all equal-span replacements. The proposal correctly refuses to carry that rule into a mixed ownership boundary. Preserve defined same-position insertion ordering inside one ordinary producer; reject duplicate nonempty replacements from any producer unless a distinct, reviewed compound-operation rule justifies them. Run ordinary compatibility fixtures against the adapter when it becomes a production route, since a stricter validator can expose previously tolerated producer output. No evidence found here establishes that all ordinary duplicate spans are intentional or that they are all bugs.
P2 — make the batch/snapshot relationship and intended rename set constructor invariants. A's analysis_source/update.rs:15–19 binds a plan to an Index, and index.rs:122–128 binds the Index to numeric-keyed source strings. Neither object currently binds itself to a SourceBundle.entry, canonical-path table, checked ordinary Project, or parse-budget profile. Index::parse (index.rs:147–178) intentionally accepts an independently supplied ordinary Project for the private typed-only harness. Rust borrowing prevents dangling references; a shared lifetime does not prove that two borrowed objects came from the same parse session. The proposal's opaque parsed-bundle requirement is therefore essential, not documentation around otherwise sufficient lifetimes.

For production composition, use one checked parsed-bundle witness that owns or borrows the exact complete bundle, ordinary model, typed index, file table and inherited limits. Create the collector and its complete rename intent from that same witness plus the independently supplied target Project. Permit only the witnessed typed batch for that index, with all global and scoped changes derived by stable identities; do not independently accept several batches whose individually valid final-name maps disagree. Final equality must still catch omitted ordinary references. Keep the old typed-only constructor test/staging-only. Add a negative control pairing a valid batch with another parse session/file table having equal numeric keys/spellings but different ownership, and a positive byte-identical complete snapshot control. No public raw build(Lowered, spans) is needed.
P2 — turn the initial scope restriction into a precise diff gate and a bounded completion path. “Topology-preserving” is not enough to identify supported changes. Require a gate over complete old/new Projects: unchanged global IDs and categories, declaration/lexical ownership, typed bodies/types/subjects/override structure, parameter IDs/order and all unsupported fields, allowing only the specific global/scoped names and ordinary leaf changes implemented in the first slice. An unrelated model-vector reorder, body edit, repaired deletion, or reidentity must not get silently ignored or regenerated. Return a specific unsupported-operation result before output construction. Keep those cases visible as unfinished source8 functionality.

The accepted r2 contract, “Source pipeline and exact editing” points 5–8, additionally requires subtree edits and retained removed-node comments, scoped parameter changes, global reidentity, explicit definition categories, file ownership, and complete acceptance. The S2 plan's S3 responsibility paragraph adds repaired deletion, missing-ID insertion/reindexing and history. A passing rename compositor does not complete those obligations. Before public activation, either implement their source8 paths or retain a clearly private experimental entrypoint; keeping the existing source7 updater intact does not by itself provide these operations for a source8 model.

Resource and acceptance conclusions

The typed-growth/ordinary-shrink concern is real. A's Plan::renames calls validate at update.rs:89; validate calculates a typed-only aggregate output and applies index_bytes (:154–170). Consequently, export cannot simply call the existing constructor under the final combined-output ceiling. Accept the proposed split into ownership/completeness validation and standalone output preflight, while preserving standalone renames/apply behavior.

The composition path should retain the inherited profile rather than accept a more permissive ambient Budget. If callers can tighten limits, use the intersection; do not allow composition to widen the parsed session's ceilings. Existing private tests deliberately construct new Budgets for boundary probes (update/tests.rs, output_and_plan_limits_are_aggregate_and_do_not_reset_parse_work), which is not proof of production profile binding. Count aggregate records/owned replacement text before merged reservations, then compute the actual combined per-file and whole-bundle output lengths, including unchanged files, before allocating output. A shrink can compensate output growth, but cannot erase already incurred parse, name-storage, record, or numeric costs. Retaining old parse counters unchanged through export is correct; a complete candidate reparse gets its own complete bounded pass. An unchanged usage counter alone does not prove that no redundant parsing occurred, so keep the single-parser control flow inspectable.

The final acceptance boundary is correctly chosen: compile the actual emitted complete bundle, use normal full structural/static admission, compare the entire resulting Project to independent intent, and install nothing on failure. R already checks stale public fields (source_project.rs:790–802) and whole-Project equality (:1058–1071); preserve those guarantees. Equality is necessary but cannot prove source preservation or edit provenance: retain independently authored exact-output and unchanged-byte oracles too. When choosing the negative omission fixture, keep the damaged output parseable/admissible (for example, omit an unreferenced definition) so the test actually exercises whole-Project equality instead of only an earlier parser error.

Concrete next implementation boundaries

Private batch patch: only the typed update module and its focused tests need change initially. Export a borrowed, opaque, complete validated batch; separate composition planning from standalone final-size preflight; prove no whole-file output is constructed. Exercise same-name no-op, full swaps, missing/duplicate occurrence, stale source, and typed-growth/ordinary-shrink compensation with independently authored ordinary witnesses. This is a meaningful deliverable now and needs no public dispatch change.
Private compositor patch: implement complete snapshot/file-map binding, owned ordinary-operation normalization for the explicit supported set, aggregate checks and one ascending copy pass. Exercise all-file atomic rejection, Unicode boundaries, wrong session/map, duplicate nonempty spans, ordered insertion groups, adjacent ranges, exact cap and cap-plus-one. Handwritten ordinary witnesses certify this algorithm only, not production ordinary edit collection.
Real mixed parse prerequisite: compose one raw scanner/cursor and bundle budget with ordinary and typed declaration parsing; collect full cross-file globals/signatures, lower once, retain full-buffer offsets and same-parse index provenance. Stabilize missing global/parameter IDs into the returned documents and rebuild indexes before granting editing authority. Preserve ordinary signed-number/legacy behavior and complete/compact edit semantics. Do not build a substring/masked-source bridge or expose raw lowerer/span fabrication to make the compositor appear integrated.
Integrated narrow edit slice: derive both producers from that checked parse and complete target, enforce the precise diff gate, emit once, and verify the whole candidate. Add independently authored mixed CRLF/Unicode files, cross-file ordinary-to-typed and typed-to-ordinary renames, final namespace swaps, scalar changes on both sides of typed declarations, exact bundle output and whole-Project equality. Include no-op equality and stale unrelated-file/entry refusal.
Complete required editing and publication: extend the ownership protocol to typed subtree changes, scoped-parameter insertion/deletion/reorder, same-batch repaired deletion, reidentity, movement/new-file ownership and stabilization/migration; verify history and checked-save failure atomicity. The existing structural second pass (R:source_project/source_structure.rs:34–84) reparses updated text and cannot simply be declared part of the original-coordinate one-pass proof. Design and verify its composition explicitly. These are remaining delivery gates, not grounds to postpone the useful private batch patch.

The proposal is appropriately cautious about public activation and does not weaken the required model semantics. With the scope/provenance clarifications above, implementation of the private adapter should now provide more value than another general redesign exercise.

Immediate export-only acceptance boundary

Parent's narrower first implementation is accepted: typed opaque batch export and validation split only; no mixed source_project compositor, real mixed parser, or public activation in this patch. Steps 2–5 above describe dependent work, not additional scope for this patch.

The composition constructor must share final-namespace, qualified-target, complete-occurrence, original-source, token-spelling, order/overlap and active-budget checks with the existing standalone path. It may defer only the hypothetical typed-only final output-length check. It must still bound rename-map records, owned final-name bytes and edit records before allocation. Export revalidates exact supplied current numeric file keys and bytes and returns an opaque read-only view borrowing the Plan; it cannot return/install a candidate Project or grant ordinary-file ownership. Keep constructors and internal edit fields private. An empty batch is valid. This private batch remains tied to the existing typed-only index provenance; it must not claim the future complete-bundle/session guarantee.

Use one caller-supplied inherited Budget throughout the private test operation, with no fresh permissive budget inside the new constructor/export. This patch need not redesign Budget or Index to store a production parse-session profile. That stronger ownership is deferred with the real parsed-bundle witness and must be settled before production composition. Budget::active() remains mandatory, including a previously latched parse/numeric failure.

Minimum focused acceptance oracles:

The existing standalone Plan::renames and Plan::apply keep rejecting typed-only output exceeding the profile. Under that same profile, the composition constructor/export accepts the otherwise valid growing batch when its plan/name/record storage fits. Independently calculate an ordinary shrink sufficient to place a hypothetical complete result exactly at the cap, and explicitly label this as deferred-final-size arithmetic, not a compositor test or accepted output. This proves why only output preflight was split.
Source/name-memory/record bounds remain effective on composition construction: invalid alias, final namespace collision, unknown qualified target, over-limit renamed-name bytes and edit count fail. A shrinking hypothetical ordinary edit cannot authorize an over-limit plan. Exact-bound positives and plus-one negatives should isolate the intended bound.
Export rejects missing/changed/extra source files or altered unrelated bytes, a forged/wrong token location, duplicate occurrence and incomplete edit membership. Use module-private damage controls to exercise checks inaccessible to normal callers. Failure returns no batch and leaves input unchanged.
A simultaneous global/scoped swap exposes exactly the independently enumerated original file/span/old spelling/new spelling/qualified key tuples in deterministic order. Include repeated call labels and same UUIDs in distinct parameter/global scopes. Same-name/no-op input exports zero edits.
Construction/export leave parse usage and numeric work unchanged; no whole-file typed output is materialized or Plan::apply invoked along the export path. Verify this through the narrow implementation's control flow plus tests of exposed records, rather than treating unchanged counters alone as proof of allocation behavior.
Existing standalone rename, stale-index, namespace and output-limit regressions retain their original oracles. The new type exposes read-only accessors and no mutable record, arbitrary batch constructor, or caller-selected subset acceptance API. Exact complete model/bundle equality remains a later integration gate, not an assertion that this export patch can establish.

Passing these oracles is sufficient to deliver the private export patch. It is not sufficient to close S3/S4 or claim graph/text/source8 functionality. The next dependency is concrete complete parsed-bundle provenance and ordinary-operation normalization, followed by the compositor and actual mixed parser path.

16 KiB Raw Blame History Unescape Escape