-
Notifications
You must be signed in to change notification settings - Fork 1
temporal + located companion channels through the kernel and the pyramid (issue #410) #463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 2 commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
ebafc10
phase 1 of issue #410
espg a4e123a
phase 2 of issue #410
espg e1703a5
fold review: scope the temporal channel's invariance to the cell leve…
espg 03dc7a2
fold review: correct the pairwise reducer's fold-law claim (issue #410)
espg 0c99c67
fold review: declare the both-channels merge overloads (issue #410)
espg 36812c3
fold review: refuse the reserved 0 word on the pass-through arms too …
espg fe05788
fold review: pin the pairwise temporal fold and the unexercised arms …
espg be1b640
fold review: tighten the empty-segment match to the message it means …
espg 1639c72
phase 3 of issue #410
espg c602d1f
fold review: drop the double-counted leap shift from the toc encode (…
espg e6e6221
fold review: exercise the pre-2017 epoch branch of the toc encode (is…
espg 480ee86
fold review: round the toc encode to the true nearest nanosecond (iss…
espg c3f0ab0
fold review: bound the toc encode at the grammar's real span ceiling …
espg f666ec7
fold review: cross-check the two clock declarations (issue #410)
espg ea9d329
fold review: gate the derived toc column on the field that materializ…
espg 41cc48c
fold review: reserve the toc word name against coordinates too (issue…
espg fb9217e
fold review: describe the spill temporal probe as what it is (issue #…
espg cb5d064
fold the per-observation clock into the semantic core (issue #410)
espg 667654c
fold review: length-check the located pair on the single-contributor …
espg 8d82f6f
fold review: declare the located fold's arity with overloads (issue #…
espg aa195cf
fold review: check the located declaration at retrofit and at fold ti…
espg 0905e50
fold review: pin the heterogeneous-order claim and the per-centroid c…
espg 3fb8a21
fold review: fold the located channel into the leaf column artifact (…
espg 0e2ccd7
fold review: give the gather and the merge one posture on a half-read…
espg 547b5a9
fold review: cover the staged sweep's located gather and merge paths …
espg 2df49a7
fold review: document the manifest location key in the spec grammar (…
espg 4211f74
Merge remote-tracking branch 'origin/main' into claude/410-temporal-k…
espg dd53639
reconcile the toc clock into the post-#420 semantic core (issue #410)
espg 40b6fc2
Merge remote-tracking branch 'origin/main' into claude/410-temporal-k…
espg 3010a3c
phase 4 of issue #410
espg e9436ea
Merge remote-tracking branch 'origin/main' into claude/410-temporal-k…
espg c471526
phase 4 template wiring after the #466 sync (issue #410)
espg db4ebb1
fold review: normalize fold_digests channel slot order through the ta…
espg 7e0a6a0
fold review: cover a both-channel field through every fold site (issu…
espg 0f153a9
fold review: pin the folded column's companion words by value (issue …
espg cf699e0
fold review: scope the waveform temporal claim to native resolution (…
espg 8cc8bb5
fold review: stop promising a temporal reader that does not exist (is…
espg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Large diffs are not rendered by default.
Oops, something went wrong.
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 from Claude (review)
nit — this describes the spill path as not carrying the temporal channel, but the single-block regime does carry it, correctly.
validate_spill_foldis a probe here, not a refusal:A
temporal:config therefore constructs aSpillAggregatorwith_mergeable = False, andchunk_outputsroutes to_chunk_outputs_exact→_aggregate_chunk_cells, which emits the full{field: {"locations": [...], "times": [...]}}mapping. The refusal only lands when a block actually closes (SpillOverflowError, carrying_fold_problems) — loudly, so nothing is silently lost; I checked this specifically because a non-mergeable config skipping the fold gate is the failure I most expected here, and it isn't one.So the accurate statement is stronger than the one written: single-block spill serves both channels exactly; a block close refuses, because the cross-block channel state is located-only. Worth fixing here and in the
_chunk_outputs_mergedcomment at line 1093 ("temporal:never reaches here (refused by validate_spill_fold)") — true of that method, but for the reason that it only runs when_mergeable, not because the config was rejected.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 from Claude
Folded in
fb9217e3. TheSpillAggregatordocstring now describes the mechanism:validate_spill_foldis a probe here, atemporal:config lands_mergeable = Falseand routes through_aggregate_chunk_cells, so single-block spill carriestimesbesidelocationsexactly; what refuses is a block close (SpillOverflowError, carrying_fold_problems) — loudly — because the cross-block channel state is located-only. The_chunk_outputs_mergedcomment is corrected the same way:temporal:never reaches there because that method only runs when_mergeable, not because the config was rejected.streaming.validate_spill_fold's own docstring already frames itself as a mergeability probe, so it stands as written.