parity nits: reference-citation corrections, missing asserts, dead state - #172
Merged
Conversation
The D-class remainder of the definitions.py conformance review (the behavioral fixes landed in #164; adjudication items are tracked as #165-#171). Nothing here changes observable behavior; the asserts added are reference-parity insurance on invariants the callers already guarantee. Code: * Store.nestingDepth deleted — dead since inception; the reference's nesting_depth asserts tick is never reentered from inside lift, which deliberately does not map to deltic's nested-driving architecture, and a dead field implies a checked invariant that isn't. * setMaxFlatResults renamed setMaxFlatResultsForTesting (the schedulerSeedForTesting convention for test-only knobs). * storeString writes begin-then-length, matching store_string (definitions.py:1613-1616); previously swapped (unobservable). * sync-start-call's onCallerResults asserts r !== null instead of masking a cancelled resolution as [] — the reference's sync canon_lower path can only deliver RETURNED results (a sync-lowered subtask has no cancel channel). * unpackCallbackResult and streamCopy gain the reference's range/ element asserts (packed < 2^32; result < 2^4; no char element). Comments: * Thread.storage records the i64-above-2^53 precision limit (issue #12 will revisit). * setSubtaskPendingEvent's !resolveDelivered() guard cross-linked to unwindLenders (the reference delivers unguarded). * Stale definitions.py citations corrected across streams.ts, stream_builtins.ts, subtask.ts, intrinsics/context.ts, async_builtins.ts, intrinsics/mod.ts, handles.ts — each verified against the pinned submodule copy rather than taken from the review (two of the review's own corrections were off and the original citations kept; two others needed different lines than the review suggested). Gates: just gates (full pass; conformance 1257/0, no expectation changes; sched-seeds clean). No published-surface change; lockstep version untouched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
parity nits: reference-citation corrections, missing asserts, dead state
The D-class remainder of the definitions.py conformance review — the behavioral fixes landed in #164, and the adjudication items are tracked as #165–#171. Nothing here changes observable behavior; the added asserts are reference-parity insurance on invariants callers already guarantee.
Code
Store.nestingDepthdeleted (scheduler.ts): dead since inception. The reference'snesting_depthassertstickis never reentered from insidelift; deltic's nested-driving architecture (the host-stream pump inside open brackets) is deliberate, so the guard doesn't map — and a dead field implies a checked invariant that isn't.setMaxFlatResults→setMaxFlatResultsForTesting(flatten.ts), per theschedulerSeedForTestingconvention for test-only knobs.storeStringwrite order (strings.ts): begin pointer then tagged length, matchingstore_string(definitions.py:1613-1616). Previously swapped; unobservable (no trap can intervene), fixed for parity.sync-start-callresult invariant (fact_calls.ts):onCallerResults'sr ?? []masked an arity invariant — the reference's synccanon_lowerpath can only deliver RETURNED results (a sync-lowered subtask has no handle, hence no cancel channel). Nowassert_(r !== null).unpackCallbackResultrange (packed < 2**32),streamCopy's no-char-element and 4-bit packed-result asserts — all unreachable by construction, kept in step with the reference.Comments
Thread.storagerecords the i64-above-2^53 precision limit (revisited at issue memory64 + shared-everything threads: deferred feature tracking #12's closure).setSubtaskPendingEvent's!resolveDelivered()guard cross-linked tounwindLenders(the reference'ssubtask_eventdelivers unguarded and would assert on double delivery).definitions.pycitations corrected across streams.ts, stream_builtins.ts, subtask.ts, intrinsics/context.ts, async_builtins.ts, intrinsics/mod.ts (Subtask.release_lenders— a name that no longer exists — →deliver_resolve), and handles.ts. Each correction was re-verified against the pinned submodule copy rather than taken from the review: two of the review's own suggested corrections turned out to be wrong (the originals were already right and were kept), and two needed different lines than the review proposed.Gates
just gates— full pass. Conformance 1416 commands: 1257 passed, 0 failed, no expectation changes;sched-seedsclean (516/0 under both seeds). No published-surface change; lockstep version untouched per AGENTS.md.