Add HasPersonality projection; 0.1.1 - #12
Conversation
Downstream personality gates (modmath's NonCt marker) previously required the gating crate to name every backend type — an optional regular dep per backend, since the orphan rule forces the impl into whichever crate owns the trait. HasPersonality inverts the flow: the carrier declares its personality once (type P = P for typestate carriers, Nct for primitives), and consumers bound on HasPersonality<P = Nct> without naming any backend. The declaration is the carrier author's contract; it cannot be verified structurally, same trust model as any trait impl.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new ChangesHasPersonality API and docs refresh
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces the HasPersonality trait to project a carrier type's Personality at the type level, implementing it for primitive integer types with the Nct (non-constant-time) personality. The feedback recommends extending this implementation to standard library wrappers Wrapping<T> and Saturating<T>, and adding corresponding unit tests to verify their personality projection.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61d827852a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The HasPersonality doc named fixed-bigint's concrete FixedUInt type and justified the primitives' Nct projection with "arithmetic is variable-time on common hardware" — which is false for add/sub/bitwise/shifts (constant- time everywhere) and for multiply on mainstream cores. Primitives project Nct because it is the conservative default: a single hardware-backed impl, no CT variant selected, no CT contract — and division alone is genuinely operand-dependent on common CPUs.
cargo fmt wraps the widened root re-export and the impl_has_personality_nct! argument list; the CI gate runs fmt --check, so this unbreaks it.
a075b67 to
a392b40
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
continue.sh (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHard-coded session ID makes this script unusable for others.
The
--resumeUUID is tied to a specific Claude session and will go stale, so this script won't work for other contributors or future sessions. Consider parameterizing it (e.g., via an argument or environment variable) or excluding this personal dev-helper script from version control.♻️ Proposed fix
-claude --resume 221d0e5d-0638-4cf2-84ef-96a86d545cbb +#!/usr/bin/env bash +claude --resume "${CLAUDE_RESUME_ID:?Set CLAUDE_RESUME_ID}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@continue.sh` at line 1, The continue.sh script is tied to a single Claude session because it hard-codes the --resume UUID, so update the script to avoid embedding a personal session ID. Refactor the resume command in continue.sh to accept the session ID from an argument or environment variable, or remove the script from version control if it is only for local use, so others can use it safely without editing the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 179-180: Update the crate map entry for src/personality.rs to
include HasPersonality alongside Ct, Nct, and Personality so the public API list
matches the new projection trait. Make the change in the CLAUDE.md section that
summarizes the crate surface, and keep the description concise but explicit that
HasPersonality is part of the pure-core personality markers.
- Line 5: Update the version reference in CLAUDE.md to match the crate metadata:
the note that says Cargo.toml stays on 0.1.0 is now stale and should be changed
to 0.1.1. Keep the rest of the baseline/branch/CI notes intact and only refresh
the version mention so the docs stay aligned with the current release state.
In `@DESIGN.md`:
- Around line 167-168: The disposition table rows for Bounded and
Float/FloatCore/Real/Pow/MulAdd only have two cells, so the rendered table drops
the final column; update the table in DESIGN.md to use a consistent three-cell
layout for these entries, including an explicit empty third cell where needed,
so the row structure matches the rest of the table.
- Around line 71-82: The architecture diagram block in DESIGN.md is currently an
untyped fence, which triggers markdownlint and makes the intent less clear.
Update the fenced block around the L2/L1/L0 trait diagram to use a plain-text
fence (for example, a text-labeled code block) so it renders cleanly without
being treated as code; locate the block by the “L2 compat bundles / L1
std-mirror / L0 capability atoms” labels.
In `@MIGRATION.md`:
- Around line 229-240: The additive surface list in MIGRATION.md is missing the
new carrier projection atom. Update the “2.7 New things you can rely on” section
to include HasPersonality alongside the other atoms, and mention the intended P
= Nct / P = Ct usage pattern so downstreams can discover it next to RingOps,
FromStrRadix, and the other listed items.
---
Nitpick comments:
In `@continue.sh`:
- Line 1: The continue.sh script is tied to a single Claude session because it
hard-codes the --resume UUID, so update the script to avoid embedding a personal
session ID. Refactor the resume command in continue.sh to accept the session ID
from an argument or environment variable, or remove the script from version
control if it is only for local use, so others can use it safely without editing
the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f305392e-29d3-4bc9-906a-912ebc37138a
📒 Files selected for processing (12)
API_BREAKS.mdCLAUDE.mdCOVERAGE.mdDESIGN.mdHOW_TO_FIX.mdMIGRATION.mdNIGHTLY_PIN.mdPORTING_REPORT.mdREADME.stashed.patchcontinue.shsrc/lib.rssrc/personality.rs
✅ Files skipped from review due to trivial changes (3)
- API_BREAKS.md
- README.stashed.patch
- COVERAGE.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib.rs
- src/personality.rs
Adds the
HasPersonality { type P: Personality; }projection alongsidePersonality/Nct/Ct, with impls for the integer primitives (P = Nct).Motivation: a personality gate otherwise has to name every carrier type it admits, because the orphan rule forces the marker impl into whichever crate owns the marker trait. With a projection, a carrier declares its personality once at its own definition site, and a consumer bounds
T: HasPersonality<P = Nct>(variable-time paths) orP = Ct(constant-time-only paths) without enumerating carriers.Additive; version bumped to 0.1.1.
Summary by CodeRabbit
HasPersonalitytrait so types can expose their associatedPersonalityat the type level.Nctas the default personality.0.1.1.nightly >= 2026-06-19for const-enabled features.