Repository navigation
Aggregation: concatenate fold inputs without promotion; pair_sum is Int-only - #32
Merged
Merged
Conversation
…(Codex P2 x2)
- oplus_fold_append_holds / aggregation_as_fold_holds joined their inputs
with vcat, which promotes element types (an Int next to a Float64 is
converted, and can round). With an exact non-promoting max this reported a
false counterexample on a lawful monoid. They now use _concat, which
keeps each element as given.
- pair_sum and pair_sum_is_fold accepted any Integer tuple, but sum_monoid's
unit is an Int, so Int8 inputs overflowed in the direct sum but not in the
fold and pair_sum_is_fold returned false. The example models N as
non-negative Int, so both methods are restricted to Tuple{Int,Int}.
Both reproduced before the change. Adds 5 assertions (297 -> 302).
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MF5GeeBBDF6sUzr6pZTjWV
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 17 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hyperpolymath
marked this pull request as ready for review
October 20, 2026 13:28
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
hyperpolymath
added a commit
that referenced
this pull request
Oct 20, 2026
## Summary Prepares v0.4.0. No code change. - `Project.toml`: 0.3.0 → 0.4.0. - `CITATION.cff`: adds `version: 0.4.0` and `date-released: 2026-10-20`. - README and the source header no longer call `EchoAggregation` "unreleased". ## Why 0.4.0 and not 0.3.0 `Project.toml` already said 0.3.0, but the tags stop at `v0.2.0`: **v0.3.0 was never tagged**. Its content (the Tier-3 audience spine, 258 assertions) ships in v0.4.0 together with `EchoAggregation`, which is a new module. The README status paragraph says so. ## Readiness - `julia --project=. -e 'using Pkg; Pkg.test()'`: **302 pass, 0 fail**. - `main` at the #31 merge: Governance green; the Secret Scanner run was cancelled only because #32 landed a minute later. - This repository has no Julia CI job, so the test count is a local result, as it was for v0.1.0 and v0.2.0. - Stale and untouched: `.machine_readable/contractiles/Justfile` and `.machine_readable/6a2/PLAYBOOK.a2ml` still carry `0.1.0`. ## After merge The tag is the owner's to push (signed, annotated, admin bypass of the tag rule): ``` git tag -s -a v0.4.0 -m "EchoTypes.jl v0.4.0 — adds EchoAggregation" <merge-commit-sha> git push origin v0.4.0 ``` --- _Generated by [Claude Code](https://claude.ai/code/session_01MF5GeeBBDF6sUzr6pZTjWV)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
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
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.
Summary
Follow-up to #31, which merged at 13:24:58Z on
9ef30be, about 30 seconds before Codex finished reviewing that commit. Codex raised two P2 findings on it. Both reproduce, so this PR fixes them.Changes
oplus_fold_append_holdsandaggregation_as_fold_holdsjoined their inputs withvcat, which converts element types. AnIntnext to aFloat64is converted and can round. With an exact, non-promoting max (Monoid(0, (a, b) -> a > b ? a : b)) on[2^53 + 1]and[Float64(2^53)], both checks returnedfalseon a lawful monoid. They now use_concat, which keeps each element as given.pair_sumaccepted anyIntegertuple.sum_monoid's unit is anInt, soInt8inputs overflowed in the direct sum (100 + 100 = -56) but not in the fold (200), andpair_sum_is_fold((Int8(100), Int8(100)))returnedfalse. The example models ℕ as non-negativeInt, sopair_sumandpair_sum_is_foldare now restricted toTuple{Int,Int}; other integer types raise aMethodErrorinstead of silently disagreeing.Testing
julia --project=. -e 'using Pkg; Pkg.test()'on Julia 1.10.4: 302 pass, 0 fail (was 297). Both findings were reproduced before the change; the new assertions cover the promotion case for both checks and theInt8rejection. The README test count is updated to match.Notes
dc1dd40, which I pushed to the oldfeat/echo-aggregation-mirrorbranch after Mirror EchoAggregation: monoid fold, homomorphism law, no-canonical-disaggregation #31 had merged. That branch now exists on the remote again (it holds the merged history plusdc1dd40) and can be deleted.Generated by Claude Code