61 lines
12 KiB
Markdown
61 lines
12 KiB
Markdown
# Independent critique of the next producer proposal
|
|
|
|
Read-only review against source `fa7a980418ef99d772e67c66c28dfa7b5181f875`, `NEXT-PRODUCER-HOOKS.md`, the accepted carried-phase contract, the rename-plan successor requirements, Project resource contract and execution domain audit. No source changes, Cargo, network or runtime execution.
|
|
|
|
## Recommendation
|
|
|
|
Do not make the proposed complete U/M/T producer ledger plus selected parser scratch the next single implementation milestone. The inventory is useful and identifies real timing gaps, but spans the shared scanner, ordinary parser/projection, typed cursor/declarations/expressions, traversal and resolver handoff. Calling that a small set of hooks understates its policy, lifetime and regression surface.
|
|
|
|
The smallest useful next rename implementation is **private emission and authenticated output ownership**, stopping before accepted-output authority. Reserve exact output ownership while the old plan is live, emit from original coordinates, independently authenticate actual bytes against the authorized edit stream, and establish a constructor-controlled output owner with exact policy and a documented old-owner drop. Preserve an explicit unchanged outcome for no-op intent. This is necessary work under every plausible future parser accounting decision; it makes actual rename progress without silently settling that decision.
|
|
|
|
Do not expose installation/save/history adoption or label this result accepted edited source. The proposed milestone should be explicitly approved as a private prerequisite, just as carried-phase accounting was. A complete accepted-output API still requires actual-byte parse/resolve/admission, strict whole-Project equality with reserved E scratch and a resolved temporal accounting contract.
|
|
|
|
## What the contracts actually require
|
|
|
|
Three categories should remain separate:
|
|
|
|
1. **Necessary new output ownership.** The rename-plan successor explicitly names F output-map entries, complete strings, canonical copied keys and entry; output overlaps the old B during emission. Old syntax/index/plan allocations must actually drop before their reservations disappear, or both owners remain charged. These requirements are unambiguous and cannot be deferred behind a generic parser exclusion.
|
|
2. **Existing retained inventory.** Project §3 requires before-allocation aggregate checks for in-scope representations. Existing retained syntax aliases, field keys, AST text and projected copies are counted today, but some only at resolution. The domain audit and carried contract explicitly preserve this timing limitation, without waiving the stronger obligation for final execution. Producer hooks for those exact existing families are a defensible way to close it; a proven conservative reservation is another. The successful phase prerequisite does not itself discharge it.
|
|
3. **Explicit temporaries versus inferred universal scratch.** New emitter containers and already named accounting/equality work must be admitted. The original successor does not separately list every inherited traversal/path/scanner/duplicate-set allocation. Project §3's broad wording and parser proposal's temporary wording deserve an explicit finite scope interpretation; they do not by themselves establish that all of those structures already belong to one exact slot inventory. Automatically adding all of them is a metric expansion, not merely repairing the timing of existing T. Automatically exempting all of them would also be unjustified.
|
|
|
|
Accordingly, broad parser scratch expansion is **not established as a prerequisite to implementing private emission**, and should not be imposed implicitly. It may become necessary for the final selected guarantee if parent explicitly includes those structures. That decision should identify each family and its logical unit, rather than using either “everything is already bounded” or “all parser allocation must now be metered.” Allocator capacity, model/serde-conversion heaps, raw validator Issues, BigInt limbs and native stack remain distinct exclusions/limits as documented.
|
|
|
|
## Concrete bounded next implementation
|
|
|
|
Use the existing opaque RenamePlan and its complete snapshot/edit validation. Add one private emission module beside planning; do not change shared parser producers in this milestone.
|
|
|
|
1. Revalidate complete original bundle authority and all physical edits before copying. Preserve canonical paths/file set/entry and both occurrence families. No caller-supplied partial file list or replacement model authority.
|
|
2. Repeat the existing checked per-file sizing walk, without a lengths map. Compute exact O: F entries and `sum(final source lengths) + sum(copied canonical key byte lengths) + entry byte length`, zero lower occurrences. The existing resolved ledger already includes retained R+M plan maps: reserve O against that live total before constructing any output buffer/key/map. Reserving all O up front is simple and sufficient for a once-built complete bundle whose logical live output grows monotonically toward O. Do not debit old storage for shrinkage.
|
|
3. Construct each final file once by appending original untouched slices and borrowed desired aliases in increasing original coordinates. Copy untouched files too when delivering the specified complete owned output. Charge every implementation-added temporary if one is introduced; the preferred walk requires only iterator/scalar state and the currently growing output String. Container capacity is not a new logical-byte charge.
|
|
4. While the original owner still lives, use a separate streaming comparison of the **actual** output against the original-slice/replacement sequence. Check entry, exact canonical file set and every byte, including untouched files. No second output buffer, lexer or output hash as a substitute. This catches admissible-but-wrong non-name damage and creates useful authenticated topology evidence for any later envelope option.
|
|
5. Capture exact immutable policy and E scalar while the old witness still authorizes them. Consume/drop old syntax, indexes, warnings and plan maps at a named transition; retain only the complete output owner, desired borrow, policy and documented scalar metadata. Install O through the sole private output transition, never a general `new(bundle, arbitrary_slots)` constructor. The stable owner may later lend bytes through the carried guard. Do not return a self-borrowing bundle-plus-syntax pair.
|
|
6. For no-op intent, return the original owner in an explicit unchanged branch without constructing/copying a complete output. No history/install behavior follows from this private result.
|
|
|
|
The output handle should be opaque and have no public extraction/adoption route. Test-only inspection can examine bytes and provenance. A later internal scoped verifier can borrow it and drop temporary syntax before returning owning acceptance. Failure drops partial output and consumes no install authority; the immutable original caller-owned source bytes and desired model remain unchanged.
|
|
|
|
This is a proposal requiring a small successor contract, not authorization to add a production constructor under the already-finished phase-only contract.
|
|
|
|
## Acceptance for that bounded milestone
|
|
|
|
Reuse the independent complete rich OA/SA, swaps, scoped repeated/reordered call labels, cross-file Unicode/CRLF and ordinary-category maps. Author exact expected bytes from the original fixture and literal replacement coordinates, without invoking the emitter as its own oracle. Include untouched files and the previously agreed combined grow/shrink sizing case. Verify original bytes/intent unchanged and no-op preserves ownership.
|
|
|
|
Hand-count emission cap/cap-minus-one at **old live B including R+M plus O**, ensuring the same profile first admits parsing, indexing and planning. Assert exact O after old ownership ends. Corrupt emitted touched/untouched bytes, a non-name byte, entry/key/file set and a replacement belonging to the wrong qualified target; streaming authentication must reject each. These are emitter/ownership tests, not substitute proof of actual output semantic admission. Parent should retain the later whole-Project equality requirement explicitly.
|
|
|
|
## Quoted-text bound and future hooks
|
|
|
|
The proposal's `decoded UTF-8 bytes D <= raw payload UTF-8 bytes P` is sound for valid JSON strings, including escapes and surrogate pairs. P must exclude exactly the two ASCII quotes. Reserving P before decode and releasing P-D is a straightforward conservative logical-text bound. It can reject escape-heavy source whose decoded retained text would fit; that rejection is a real selected policy change for the opt-in path, not exact minimal acceptance. An escaped/unescaped pair with identical decoded text should document this tradeoff. No second decoder or Unicode normalization claim is warranted.
|
|
|
|
The scanner already decodes once for validation, before the grammar decoder. Therefore grammar-only hooks cannot claim preadmission of that earlier temporary. If it enters the selected domain, its reservation must occur inside the scanner after token extent discovery and before validation, then release before the later grammar decode. A post-next callback is too late. Adding this hook changes a shared API/error-transport seam; it should have independent invalid-string and legacy precedence controls, not be hidden in a retained-text patch.
|
|
|
|
If parent instead chooses a parser producer milestone next, narrow it first to **retained syntax text T and its once-only transfer**. List every counted String producer and temporary UUID/declaration-key disposition, keep U/M phase checks explicitly unchanged, and do not claim complete before-growth execution acceptance. The most consequential tests are copied ordinary reference/key overlap, escaped strings at P versus D caps, temporary UUID release and no double charge in resolver. This is still a meaningful prerequisite, but should not silently grow into traversal and all scratch in the same change.
|
|
|
|
## Compatibility, maintainability and stack
|
|
|
|
The accepted carried route is opt-in even when C=0; legacy constructors must keep their original admission and diagnostic timing. Any new hooks need an explicit mode boundary, not numeric-zero detection. Avoid cloning Budget, resetting live Usage or rebuilding policy from defaults. Resolver reconciliation must verify/transfer admitted retained counts, not reserve them twice.
|
|
|
|
A ledger moving from parser to resolver introduces subtleties: U is monotone grammar work, value-end/token-end metadata contains deliberate conservative retention, and scratch is genuinely released. Treating all three as interchangeable live Vec sizes risks weaker caps or double counting. Keep a short authoritative ownership table and tests for those distinct rules; a universal allocation abstraction is unnecessary.
|
|
|
|
The recent stack regression demonstrates that even an inline policy field can enlarge many ancestor frames. It does not prove producer hooks are unsafe, nor that more hooks solve stack usage. Keep policy in existing Budget/syntax, retain the slim owner guard, use scoped borrowed accounting state, and avoid large inline callback/guard enums on recursive paths. Preserve the nine existing stack cases and the earlier abort evidence. Neither emission nor producer accounting earns a universal stack guarantee.
|
|
|
|
Unresolved before accepted execution: the exact selected pre-growth retained/scratch domain; actual-output parser/resolver temporal coverage; E-reserved strict whole-intent equality; owning accepted-result construction and install/history lifecycle. The bounded emission step deliberately leaves these visible rather than letting a larger parser refactor substitute for actual rename output.
|