feat(planner)!: decouple source and opening commitment bases - #355
feat(planner)!: decouple source and opening commitment bases#355quangvdao wants to merge 43 commits into
Conversation
Search inner and opening decomposition bases independently across dense, setup-prefix, and recursive commitments. Add exact i16 commitment support, wider SIS buckets, padded-prefix-aware selection, and dense nv26 schedule coverage.
Keep distinct next-witness classes so recursive feasibility remains exact, while dropping only dominated one-digit inner bases. Bound suffix memoization and catalog workers to prevent schedule regeneration from exhausting memory after the search-domain expansion.
PR SummaryHigh Risk Overview Precommit API cutover: Catalog, CI, and benches: Adds Supporting runtime/planner tweaks: Algebra exports cyclic/i16-tail NTT reconstruction helpers and modulus-aware CRT width checks. Fold-challenge production ladder carries precomputed support floor bits for faster entropy validation. Schedule table generation moves to Reviewed by Cursor Bugbot for commit 6f30be0. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Warning This PR has more than 500 changed lines and does not include a spec. Large features and architectural changes benefit from a short spec before implementation review. If this PR is a bug fix, small cleanup, or otherwise does not warrant a spec, feel free to ignore this message. |
Decouple raw inner commitment bases from opening bases while preserving exact one-hot and recursive balanced-digit sources. Add padded-prefix Pareto planning, profile-native precommits, canonical signed-digit support, bounded exact suffix search, atomic catalog publishing, and full SIS policy audit artifacts. Preserve the established complete-schedule selection objectives while refining recursive setup scoring to physical padded capacity.
github-actions
Bot
added
spec
Documentation blast radius (advisory)These regions may need doc/spec/book updates based on changed paths. Changed files in this PR: 185
|
CI test timing
Run summary
Wall time spans 2 parallel nextest slice shards. Slowest tests
Regressions vs mainNo per-test regressions above the threshold. New slow testsNo new tests ≥30s vs main baseline. |
Merge the latest main branch tensor-challenge removal while preserving independent inner-basis search, exact one-hot sizing, Pareto precommit planning, and the split suffix planner structure. Keep generated flat-family winners from PR #355, regenerate the new fp32 dense catalog, and wire its dense benchmark modes into profile CI validation.
Remove the stale all-schedules test import and regenerate standalone precommit profiles with the merged independent-basis planner so catalog coverage matches runtime lookup.
quangvdao
changed the title
Require the canonical proven-pruned profile and mandatory validation for production Rust artifacts. Correct certificate provenance and table digests, align the parallel exhaustive diagnostic profile with the quantum LGSA model, and split oversized estimator/type test modules.
Propagate the corrected SIS provenance digest into all generated catalog identities and precommitted descriptors. Recompute the four affected catalog key digests so validation remains fail closed without changing schedules or numeric SIS rows.
Keep generated_sis_table/policy_audit.csv as the only canonical production SIS certificate. Direct full comparison jobs to temporary output and align the book and estimator documentation with the proven-pruned shared-digest policy.
Add exact scalar witness sizing, reuse bounded checked compression plans, and reject non-contracting recursive split bodies before B/D construction. Preserve the exhaustive candidate domain and verify against the unpruned planner oracle.\n\nKnown limitation: fp128_onehot_recursive still does not finish catalog generation within the measured five-minute window, so this checkpoint does not resolve the stale recursive catalog blocker.
Use bounded second-chance eviction within the existing direct and prefixed suffix quotas. A 700,000-insertion census reduced recomputations from about 218,000 to 145,179 without changing candidate coverage.\n\nShare setup and payload frontier keys only in states that retain both projections, and use binary search over monotone SIS rank rows. The exact planner and unpruned oracle suite passes all 21 tests.
Keep recursive split enumeration exact through twelve reduced variables. For larger multi-candidate suffix states, retain both extremes and a five-point window around the analytic balance point. Ordinary single-winner direct search still checks the complete split domain. Move prefix-only rejection before child recursion, avoid retaining unused prefix frontiers, and compact present setup lengths with NonZeroUsize. Regenerate both recursive catalog families. This intentionally trades global optimality for large frontier states for a practical, explicit search policy. Full 14-family generation now completes in 36 seconds.
Reconnect the split artifact digest test and make the generated digest constants the single runtime source of truth. Refresh the q128 Inner/512 extension digest for the checked-in rows and regenerate every catalog identity. Also replace the compression planner's manual loop counter so all configured CI Clippy graphs pass.
PCS Profile Benchmark
12 of 12 profiles passed. Times are medians of Each sample verifies the same proof first with the configured multi-threaded pool and then with one thread. Both timings reuse the same verifier setup. Head and merge base binaries ran interleaved on the same runner. Each delta below compares the head with that merge base. Profiles measured
One-hot profiles generate deterministic witnesses with one Direct evaluates the public setup contribution during Stage 2. Recursive carries the same check through a Stage 3 setup-product sumcheck. Both modes execute the complete fold schedule and terminal verification. The chunked profiles Generated profiles may select different A, B, and D ring dimensions at different fold levels. The short profile names omit those dimensions. Each sample generates deterministic witnesses and opening points, prepares setup, commits, proves, serializes the proof, checks its size, prepares verifier setup, and verifies the claimed openings. It does not test malformed proofs. Phase time
Memory and setup size
Proof size
Protocol shape
Each delta compares the head with the merge base. Negative is smaller or faster. Terminal response components
The Detailed schedule and proof-size breakdowns by fold level are available in the uploaded |
Route centered relation quotients through the existing five-prime CRT prefix plus the 12289 exactness tail when one base term cannot fit. Prewarm only the affected A prefixes and keep scalar arithmetic as the final capacity fallback. Parallelize independent exact-i16 commitment blocks and restore the advertised setup-first objective for direct grouped schedule generation.
Lock the fp32 and fp64 dense nv26 schedules to their ordinary cached NTT domains and verify that catalog planning agrees with field-typed runtime tail selection across every modulus profile. Remove the stale profile label that described shipped D128 catalogs as runtime-DP schedules.
Rename the existing three-field dense benchmark leg instead of duplicating runner work. Show the resolved A/B/D fold schedule in compact reports and update the active profiling documentation to explain why fp128 uses D256 only for the root A role.
…is-padded-prefix # Conflicts: # crates/akita-prover/src/compute/cpu/ring_switch.rs
Review — PR #355
|
| # | Severity | Item |
|---|---|---|
| 1 | High | Relaxed runtime admission predicate leaves log_basis_inner locally unbounded in verifier-reachable descriptor validation |
| 2 | High | Precommit admission logic duplicated across akita-planner and akita-schedules, and the two copies already differ |
| 3 | High | Two independent implementations of the scalar witness length, coupled only by one narrow parity test |
| 4 | High | PR description does not match the branch (base, file count, diff size) |
| 5 | Medium | Verifier entropy fast-path table is not cross-checked against the generic path; its guard test can truncate silently |
| 6 | Medium | Three hand-rolled Pareto frontiers with the same subtle, easy-to-desync tie-break |
| 7 | Medium | derive_candidate_level_params / ..._split_frontier are ~90% duplicated |
| 8 | Medium | Unexplained magic constants in sis_role_cell, plus a duplicated role→domain mapping |
| 9 | Medium | No normative spec for the headline change |
| 10 | Medium | New unsafe surface: one missing SAFETY comment, two missing # Safety docs |
| 11 | Low | i16 commitment path has dispatch tests but no exactness test against a reference |
| 12 | Low | Nits: dead alias, inconsistent regen validation, opaque memo key, hot-loop asserts, CI-guard duplication, formatting |
1. High — Relaxed runtime admission predicate leaves log_basis_inner locally unbounded
crates/akita-schedules/src/group_batch.rs:81 relaxes the runtime precommit guard:
- if log_basis_open < group.layout.log_basis_inner
- || log_basis_open < group.layout.log_basis_outer
- {
+ if log_basis_open < group.layout.log_basis_outer {That relaxation is required by the feature — with the bases decoupled, log_basis_inner (now up
to 11) legitimately exceeds log_basis_open (capped at 6). The problem is what it silently removed:
that guard was the only place a caller-supplied descriptor's log_basis_inner was bounded relative
to anything at all.
CommittedGroupProfile::validate (schedule.rs:128-132)
checks only:
if self.log_basis_inner == 0 || self.num_digits_inner == 0 { ... }There is no upper bound and no cross-check that num_digits_inner is consistent with
log_basis_inner and the field width — num_digits_inner_for_bound is never called from
schedule.rs or group_batch.rs. The width checks in validate only constrain the product
num_positions_per_block * num_digits_inner, which an attacker controls jointly with
num_digits_inner.
This matters because validate_frozen_precommit is called directly on a proof-supplied
descriptor in the verifier:
// crates/akita-verifier/src/protocol/core/verify.rs:259-264
for group in claims.groups() {
let descriptor = group.commitment().profile();
descriptor
.validate_frozen_precommit(Cfg::decomposition().field_bits())
.map_err(|_| AkitaError::InvalidProof)?;and on the prover side at api/commitment.rs:251, where
decompose_fold_partitioned.rs:73-75
then does:
SignedDigitKernel::for_log_basis(params.log_basis)
.expect("decompose-fold parameters must use a validated signed-digit basis")— a panic if the basis exceeds 16.
Why this is High and not Blocking: log_basis_inner is included in
append_descriptor_bytes (schedule.rs:98), so it is bound
into the schedule lookup key. A fabricated value will miss the catalog and be rejected downstream.
So the system almost certainly fails closed today. But that argument is ordering-dependent —
it relies on nothing consuming log_basis_inner between validate_frozen_precommit and the catalog
match — and it is not written down anywhere. The PR's own security section claims
"Verifier-reachable paths ... reject malformed data through typed errors"; a local bound is what
makes that claim self-evident instead of requiring a whole-path argument.
Fix (two lines, no behavioural change for honest inputs): in CommittedGroupProfile::validate,
add
if SignedDigitKernel::for_log_basis(self.log_basis_inner).is_none() { /* InvalidSetup */ }
if self.num_digits_inner != akita_types::sis::num_digits_inner_for_bound(/* ... */) { /* InvalidSetup */ }and add a regression test that a descriptor with an out-of-range inner basis is rejected by
validate_frozen_precommit itself, not by a later lookup.
2. High — Precommit admission logic is duplicated across two crates, and the copies already differ
crates/akita-planner/src/planner.rs:26-239 and crates/akita-schedules/src/group_batch.rs:16-180
contain near-identical copies of the same five items:
| Item | Planner | Runtime |
|---|---|---|
struct PrecommittedGroupSeed |
planner.rs:26-31 |
group_batch.rs:16-22 |
freeze_precommitted_group_layout |
planner.rs:37-99 |
group_batch.rs:24-73 |
materialize_precommitted_group_for_open_basis |
planner.rs:105-180 |
group_batch.rs:75-127 |
multi_group_root_precommitted_group_seeds |
planner.rs:191-210 |
group_batch.rs:129-150 |
*_groups_for_open_basis |
planner.rs:212-239 |
group_batch.rs:152-180 |
The A-bound and B-bound admission checks are byte-identical including their error strings
("precommitted A bound does not cover the certified opening basis", "no precommitted B-role norm",
"certified opening basis must dominate the precommitted outer basis"). The legitimate difference
is exactly one field: the planner computes num_digits_fold from the honest fold policy, the
runtime reads it from the generated row.
The copies have already drifted. The planner version additionally validates
(planner.rs:56-62):
if num_digits_outer != layout.num_digits_outer
|| width_s != layout.inner_commit_matrix.input_width()
{ /* reject */ }The runtime copy performs neither check. Whether that gap is load-bearing depends on
CommittedGroupProfile::validate covering it — which is precisely the question finding 1 raises.
This is the failure mode duplicated validation always produces: you cannot answer "is the verifier
as strict as the planner?" by reading one file.
This PR did not create the duplication, but it edits both copies (it fixed the opening-basis guard
in group_batch.rs while planner.rs already had the new form), which is the moment to remove it.
Fix: hoist the shared predicate into akita-types as
fn admit_precommitted_group(layout, num_digits_fold, policy, ring_challenge_cfg, log_basis_open)
and have both callers use it. The planner passes its computed num_digits_fold, the runtime passes
the table's. That deletes ~100 duplicated lines and makes the planner/verifier strictness question
answerable by inspection.
3. High — Two independent implementations of the scalar witness length
WitnessLayout::try_scalar_live_coeff_len
(witness/scalar_len.rs) is a new fast path that
recomputes the same cursor walk as WitnessLayout::new
(witness.rs:384-600). The performance motivation is
legitimate (this is a planner inner loop), but the two paths derive the quotient-row block
differently:
// scalar_len.rs:82-86 — hardcoded row families
for (rows, ring_dim) in [
(a_rows, role_dims.d_a()),
(params.b_rows_len(), role_dims.d_b()),
(lp.open_commit_matrix.output_rank(), role_dims.d_d()),
] {// witness.rs:474-495 — derived
let relation_layout = relation_rhs_layout_for(lp, opening_batch)?;
let row_families = relation_layout.row_families()?;The fast path bakes in an assumption about the shape of row_families — the count, the order, and
the a_rows + 1 offset. If a row family is ever added, reordered, or given a different ring
dimension in relation_rhs_layout_for, the planner will silently size schedules against a stale
model. The only coupling is one parity test
(scalar_len.rs:218-248) over
2 param sets × 2 payload modes × {1,2,5} polys × {1,2,4} chunks — good coverage of the current
shape, no coverage of a changed shape.
The blast radius is bounded — materialize_candidate_schedule recomputes and rejects on
disagreement, so a divergence becomes a hard planner error rather than a bad schedule. That is why
this is High and not Blocking. But it turns a future one-line layout change into an opaque
"cached schedule cost disagrees with materialized estimate" failure.
Fix: have try_scalar_live_coeff_len call relation_rhs_layout_for(...).row_families() and sum
quotient_depth * row.ring_dim() over the non-compression prefix, exactly as new does. That is
the expensive part only if relation_rhs_layout_for allocates — if it does, extract a
row_family_widths() accessor both can share. Alternatively, add a debug_assert! in new that
the two agree, so the parity check runs on every planner call in debug rather than on 12 fixtures.
4. High — PR description does not match the branch
The body's "Diff metadata" block states:
- Base:
b0f2cadc564c8f8e4f2d2488b9ca9ed1f217b9db(main)- Commits: 30 · Files changed: 158 · Diff: 18,611 insertions, 13,540 deletions
The actual PR is:
- Base ref:
quang/pr-345-prover-optimizations(merge basec45a5445b), notmain - 36 commits · 179 files · +19,348 / −13,645
This is not pedantry for a PR this size. A reviewer opening the GitHub diff sees a stacked diff
against #345, while the body describes a diff against main; the two differ by the whole prover-
optimizations stack. The "Validation at d9c0c13fb" section also predates the last four commits
(c3c0195dd, 53c9c243d, 466a97cf0, 6e620cd0c), one of which (53c9c243d) resolved a merge
conflict in ring_switch.rs and two of which fixed the last Bugbot finding.
Fix: regenerate the metadata block against the real base, state the stacking relationship to
#345 explicitly, and re-anchor the validation section to 6e620cd0c.
5. Medium — Verifier entropy fast path is not cross-checked against the generic path
akita-challenges/src/config.rs:167-177 adds a
lookup-table short circuit to validate_min_entropy_for_ring_dim — a verifier-reachable security
floor check:
if let Some((_, _, _, support_floor_bits)) = PRODUCTION_FOLD_CHALLENGE_LADDER
.iter()
.find(|(d, pm1, pm2, _)| *d == ring_dim && *pm1 == self.count_pm1 && *pm2 == self.count_pm2)
{
return if required_bits <= *support_floor_bits { Ok(()) } else { Err(...) };
}
__dispatch_fold_challenge_ring_dim!(self, ring_dim, required_bits)The optimization is sound in direction (the tabulated value is floor(log2_support), so the fast
path is never weaker than the float computation). Two problems with the guard:
(a) the guard test can silently shrink. production_ladder_matches_proof_optimized_dims
(config.rs:222-226) zips two independent lists:
for (&d, &(_, _, _, support_floor_bits)) in PRODUCTION_FOLD_CHALLENGE_RING_DIMS
.iter()
.zip(PRODUCTION_FOLD_CHALLENGE_LADDER)zip truncates to the shorter list. If someone adds a dimension to one list and not the other, the
test keeps passing while covering fewer entries. For a table that gates a min-entropy floor, that is
the wrong failure mode. Add assert_eq!(PRODUCTION_FOLD_CHALLENGE_RING_DIMS.len(), PRODUCTION_FOLD_CHALLENGE_LADDER.len()).
(b) fast path and generic path are never compared. The test asserts
validate_min_entropy_for_ring_dim(d, floor).is_ok() and (d, floor + 1).is_err() — but both calls
now take the fast path, so the assertion is self-referential. Nothing verifies that the table agrees
with __dispatch_fold_challenge_ring_dim! for the same (ring_dim, required_bits). Add a loop
comparing the two over required_bits in 0..=floor+2 for every ladder row.
6. Medium — Three hand-rolled Pareto frontiers with the same desync-prone tie-break
Three separate implementations of "skip if dominated → retain non-dominated → push":
suffix_dp/prune.rs:29-49— 5-D coordsschedule_params.rs:466-489— 3-D coordssuffix_dp/state.rs:66-78— 2-D mixed score
All three carry the same subtle descriptor tie-break, expressed as a predicate in the any and its
hand-written logical negation in the retain. In prune.rs the two use different comparison
operators (<= in the skip check at line 36, < in the retain at line 46). I verified this is
correct — reaching retain with other == coords implies other.desc > descriptor, so the strict
< is right — but that correctness argument is three inferential steps deep and lives nowhere in
the code. Getting the negation wrong in a future edit produces a planner that silently drops the
optimum, with no test that would catch it.
Fix: one generic helper,
fn insert_pareto<C: Ord, T>(frontier: &mut Vec<(C, Vec<u8>, T)>, coords: C, descriptor: Vec<u8>, value: T),
with the dominance and tie-break defined once and unit-tested directly (including the tie case).
All three call sites collapse to one line each.
7. Medium — derive_candidate_level_params and ..._split_frontier are ~90% duplicated
candidate/recursive.rs:453-651
— ~200 lines where both functions do, in the same order: prepare_recursive_level_search →
delta_commit/delta_open → recursive_split_search_domain → loop splits → lower-bound prune →
recursive_level_base_candidate_for_split → loop search.setup_prefixes →
finalize_recursive_level_candidate → next_witness_len < current_witness_len.
They differ only in (a) which lower bound is applied (recursive_split_lower_bound against the
running best vs recursive_witness_body_lower_bound against current_witness_len), and (b) keep-best
vs keep-all. Both differences are one closure parameter.
Fix: one private fn walk_recursive_splits(..., mut visit: impl FnMut(...) -> ControlFlow<...>)
driving both, or express derive_candidate_level_params as _split_frontier(...).min_by_key(order_key)
if the extra pruning is not load-bearing for generation time (worth measuring — the commit log says
this loop is the hot spot).
8. Medium — Unexplained magic constants and a duplicated role→domain mapping in SIS coverage
akita-types/src/sis/coverage.rs:79-87:
Some(SisRoleCell {
role, modulus_profile, ring_dimension, coeff_linf_bound,
max_module_rank: 20,
required_max_width: 6_400_000_000_000,
})Two of six fields are hard-coded globals dressed as per-cell data, with no comment deriving either
value. 6.4e12 in particular is a security-relevant width ceiling with no stated provenance. Since
sis_role_cell is the exact function the PR promotes to "the canonical role-aware coverage source
of truth", these need named constants and a one-line derivation each — or removal from the struct
if they are genuinely global.
Separately, sis_role_cells() (lines 90-120) re-derives the role→dimensions and role→bounds mapping
that sis_role_cell already contains internally (lines 68-71, 99-110). Two match statements that
must stay in sync, in the enumeration function guarding the other one. Extract
fn role_dimensions(role, profile) and fn role_bounds(role).
9. Medium — No normative spec for the headline change
The body cites four "normative design records". None of them define the change in the title. Across
the live spec tree, inner_basis_range appears exactly once — as a struct field in
specs/schedule-catalog-ownership.md:520 — and
InnerBasisSource appears nowhere. The other reference is in the archived
specs/archive/2026-Q3/multi-group-batching-legacy.md. The CI advisory bot flagged this
("more than 500 changed lines and does not include a spec"); the spec label is present but the
spec content is not.
Undocumented normative decisions, each of which a future reader will have to reverse-engineer:
- the three-way
InnerBasisSourcetaxonomy and why recursive digits must retain the producing
fold's basis (the soundness argument for not re-decomposing); - why
PROOF_OPTIMIZED_INNER_LOG_BASIS_MAX = 11, and why Q32 is separately capped at 10
(proof_optimized.rs:30-45) — the doc comment
says "exhaustive sweeps select 10 or 11", which is an observation, not a bound; - the argument that the A-role ℓ∞ bound still dominates the extracted-witness norm now that the
source basis is decoupled from the response basis. (I traced the plumbing and it is present via
FoldWitnessNorms::bounded(log_basis_inner, d_a)→num_digits_fold, but plumbing exists is
not the same as the bound is argued.)
Today the "recursive digits keep their basis" invariant is documented only by a test
(tests/basis_envelope.rs:62-72) and a doc
comment. specs/large-digit-ntt-infrastructure.md covers the i16 kernel and pre-dates this PR;
it does not cover the planner policy.
Fix: a short spec section — the taxonomy, the no-redecomposition rule, the basis caps and their
derivation, and the norm argument. This does not need to be long; it needs to exist.
10. Medium — New unsafe surface: one missing SAFETY comment, two missing # Safety docs
The PR body claims new unsafe code "is isolated behind crate-private safe wrappers that validate
exact lengths and challenge positions first". That is true — I verified the bounds:
digit_plane: &[i16; D] and acc: &mut [i32; D] make lengths type-guaranteed, positions are
asserted < D, and every pointer offset in sparse_mul_acc_i16_{neon,avx} stays inside [0, D).
Accumulator overflow is also fine: production challenges are ±1/±2 only
(config.rs:53-58), giving a worst case of
512 × 2 × 32768 ≈ 3.4e7, far inside i32.
Three documentation gaps against the file's own conventions:
fused_quotients.rs:823-828callsfrom_centered_i32_pair_with_lut_uncheckedwith no SAFETY
comment, while the two other call sites (lines 369-379 and 681-695) both have one. The invariant
does hold there (base_lutis built fromcentered_rows_abs_bound(z_folded_rings, width)and
j ∈ start..end ⊆ 0..width) — it just isn't written down at the one site a reader will check.sparse_mul_acc_i16_neon(decompose_fold_neon.rs:64-90)
andsparse_mul_acc_i16_avxhave no# Safetyblock, while their i8 siblings 40 lines above do.
pub(crate)means clippy'smissing_safety_docwill not catch this.
11. Low — i16 commitment path has dispatch tests but no exactness test
The i16 path is genuinely production-reachable — I confirmed it against the shipped catalogs:
| Family | Root A log_basis |
Uses i16 kernel (≥ 9) |
|---|---|---|
fp128_dense |
3, 5, 6, 7, 8, 9, 10 | yes |
fp128_dense_precommitted |
8, 11 | yes |
fp64_d128_dense_precommitted |
6, 8, 10 | yes |
fp32_d128_dense_precommitted |
8 | no (i8) |
Its dedicated tests (exact_i16_tests.rs)
assert commit(3) == commit(11) on inputs of all +1 / all −1. Those inputs decompose to a single
digit 1 at both bases, so the equality is structurally guaranteed regardless of whether the i16
matvec is correct — the test verifies dispatch and cache-key selection, which it does well, but
proves nothing about arithmetic exactness.
ntt_cache/tests.rs does carry real exactness checks for mat_vec_i16 at bases 10 and 16 (including
i16::MIN/i16::MAX saturation and the capacity-rejection path), so the underlying kernel is
covered. What is missing is coverage of the composition — balanced_decompose_pow2_i16_into feeding
mat_vec_i16 through dense_commit_rows — over random coefficients at the reachable bases 9/10/11.
Fix: one test comparing dense_commit_rows at log_basis_inner ∈ {9, 10, 11} against a scalar
reference product over pseudorandom field coefficients.
12. Low — Nits (cleanliness / de-slop pass)
Grouped, in descending order of how much a future reader will care:
Dead and inconsistent code
generated_families.rs:205-229:
regen_fp128_onehotandregen_fp128_denseareregen::<Cfg>verbatim minus
planned.schedule.validate_structure()?. Two families therefore skip a structural validation the
other twelve perform, and theirALL_GENERATED_FAMILIESrows are hand-expanded instead of using
family_row!solely to inject those functions. Either delete both and use the macro, or document
why these two must skip validation.generated_families.rs:51:
const FP128_DENSE_KEYS: &[...] = FP128_D64_DENSE_KEYS;— an alias with exactly one user and no
second definition. Inline it.generated_families.rs:274-327:
fourTypeId::of::<Cfg>()comparison blocks, three of which have the identical condition
(== TypeId::of::<fp128::OneHot>(), lines 289, 302, and part of 274). Collapse to one block, or
better, replace the whole chain withfn extra_precommit_groups::<Cfg>() -> &'static [PolynomialGroupLayout].schedule_params.rs:415-418:
plan_standalone_precommitnarrowsdirect_policy.opening_basis_rangeto(min, min), but
log_basis_search_range_at_level(policy, 0)already collapses level 0 to(configured_min, configured_min)
on the very next line. Either the mutation is dead or it matters forvalidate_policy— say which.
Readability
suffix_dp/state.rs:88-97:
ScheduleMemoKeyis a bare 8-tuple, andinsertbranches onif key.3 == 0to mean "direct
suffix" (line 168). Make it a
named struct withfn is_direct(&self) -> bool.suffix_dp/frontier.rs:38:
candidate.suffix_folds.len() + 1 < 2issuffix_folds.is_empty(). The surrounding predicate also
restates the offloaded-edge check already inchild_choice
(suffix_dp.rs:114-122) against a
different length — worth one shared, named helper so the reader can see they are deliberately different.frontier.rs:79-102vs115-154:
could_improveandconsidereach build the same two score tuples inline. Extract
fn setup_score(m: CandidateMetrics)andfn payload_score(m: CandidateMetrics); the ordering is
then defined once.ntt_cache.rs:914:if bound == 1u64 << (i16::BITS - 1) { return Ok(bound); }
is a correct but unexplained early return (atlog_basis == 16everyi16is trivially in the
balanced range, so the scan is skippable). One comment.emit/render.rs:41-52:emit_mod_wiringwalks
specstwice with two independentseensets. One loop.
Performance
prune.rs:36,46:
best_params.canonical_descriptor_bytes()/other_params.canonical_descriptor_bytes()are
recomputed (each allocating aVec<u8>) on every pairwise comparison in an O(n²) loop, while the
incoming candidate's descriptor is correctly hoisted.plan_standalone_precommitalready caches
the descriptor in its frontier tuple — do the same here.poly_helpers/mod.rs:383-387, 447-451:
sparse_mul_acc/sparse_mul_acc_i16runassert_eq!(positions.len(), coeffs.len())plus a full
positions.iter().all(|p| p < D)scan on every call, though the challenge is constant for an
entire fold level and these are called once per digit plane per ring element. Validate once at
SparseChallengeconstruction; keepdebug_assert!here.
Tooling / CI guards
scripts/check_profile_ci_features.shnow maintains
four parallel dicts keyed by mode name (MODE_FEATURE,MODE_NUM_POLYS,MODE_NUM_VARS, and
the newfeature_symbols). Adding one bench mode is four edits in one file. Worse,
feature_symbolsduplicates the(schedule_feature, const_name)pairing that
ALL_GENERATED_FAMILIESalready owns in Rust — a guard that itself needs a guard. Consider a
gen_schedule_tables --dump-registrymode the script consumes, or at minimum collapse the four
dicts into one dict of records.emit/mod.rs:55-67:
schedule_generation_worker_countdefaults to.unwrap_or(2)behind an undocumented
AKITA_SCHEDULE_GEN_JOBSenv var. The2is a bare magic number and the reason (memory bound
after the search-domain expansion — clear from the commit log, not from the code) is not stated.
Name the constant and add the one-line rationale. Related:bounded_parallel_filter_map
(lines 69-114) hand-rolls a bounded work-stealing pool withAtomicUsize+thread::scope
although the workspace already has rayon behindcfg_iter!/cfg_fold_reduce!; if the point is
to bound concurrency rather than maximize it, say so, because otherwise this reads as
reinvention.width_table/boundary.rs:
certified_boundary_from_hintandmax_true_in_prefixare the same gallop-then-bisect over a
monotone predicate, differing only in start point. Neither documents the monotonicity
precondition — which is precisely the assumption whose violation caused themin_secure_rank
partition_pointbug fixed earlier in this same PR. Document it on both; consider making
max_true_in_prefixdelegate.gen_schedule_tables.rs:206-227:
resolved_output_pathpops..lexically before canonicalizing, so a symlinked ancestor
combined with..can defeat thestarts_withisolation check. Low severity for an offline dev
tool, but canonicalize the deepest existing ancestor first, then apply the remaining components.
Tests
tests/basis_envelope.rs: hardcodes<= 6in five
places (lines 70-78) rather than usingCfg::opening_basis_range().1, andErr(_) => continue
(line 23) means the test passes vacuously if no key resolves. Add acovered > 0assertion at the
end and derive the bound from the config.precommitted_commitment.rs:36-42:
dense_precommit_profile_uses_independent_basisweakened from three exact assertions
(log_basis_inner == 3,log_basis_outer == 3,num_digits_inner == 43) to one inequality
(log_basis_inner > log_basis_outer). Understandable since the value is now search-derived, but
dense_inner_basis.rsshows the golden-snapshot pattern works well here — consider pinning the
exact profile the same way.
Formatting
proof_optimized.rs:636-637: a stray
double blank line beforefn inner_basis_range()in the second macro arm, where the first arm has
one.rustfmtdoes not normalize macro bodies, so this will persist.
Route precommit admission, scalar sizing, Pareto insertion, recursive split traversal, and generated-family providers through their canonical owners. Harden descriptor validation, entropy coverage, SIS limits, output isolation, and exact i16 composition tests in response to review.
…s' into codex/inner-basis-padded-prefix # Conflicts: # crates/akita-pcs/Cargo.toml # crates/akita-pcs/examples/profile/modes.rs # crates/akita-prover/src/api/commitment.rs # crates/akita-prover/src/compute/cpu.rs # scripts/check_profile_ci_features.sh # scripts/profile_bench_report.py # scripts/tests/test_profile_bench_report.py
…s' into codex/inner-basis-padded-prefix
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Reviewed by Cursor Bugbot for commit 6d9611a. Configure here.
2 participants
Summary
This PR lets the planner choose the A matrix source basis independently from the response and opening basis. Dense field inputs can use a wider source basis. B, D, opening witnesses, and recursive signed digit witnesses keep the basis that matches their own representation.
It also completes the planner and runtime cutover around that change:
i16commitment kernels for bases 9 through 16 behind validated crate private entry points.Normative design records:
specs/heterogeneous-group-source-contracts.mdspecs/modular-planner-and-precommit-roles.mdspecs/mixed-ring-dimension-per-level.mdspecs/setup-offloading-planner.mdspecs/sis-quantum128-scalar-n-table.mdDiff metadata
ef88d0a3fonquang/pr-345-prover-optimizations6f30be0a5codex/inner-basis-padded-prefixMotivation
The old planner used one basis range for two different values:
The protocol does not require these bases to match. Dense coefficients often benefit from a larger A basis. Response and opening paths often benefit from a smaller basis. Recursive digits must also keep the signed representation chosen by the fold that produced them. Treating those digits as raw coefficients would decompose them again and change their meaning.
The larger search also exposed older ownership problems. Partial suffix keys could merge different folds. Recursive split pruning used different domains at different levels. Compression sizing had parallel cache and wrapper paths. Grouped generation planned the same request more than once. Several security and sizing checks were copied between planner and runtime code.
Change surface
i8commitment pathSignedDigitKernelselects exacti8ori16storagePrecommittedLevelParams::admitboundaryPlanner, catalog, and generation
InnerBasisSourcebelongs to the planner and distinguishes raw coefficients, unit one hot inputs, and signed digits produced by an earlier fold. The active heterogeneous source specification defines those three cases and forbids recursive redecomposition.RecursiveSplitSearchPolicyis part ofGeneratedScheduleCatalogIdentity.Exhaustivevisits every admissible split.BoundedBalancedExtremesV1is exhaustive through twelve reduced variables. Above that point it keeps both extremes and a radius two window around the balanced split. Root, adaptive levels, and the uniform D64 suffix all callrecursive_split_search_domain. The bounded policy only claims optimality inside that declared domain.Suffix memoization now uses the full canonical first fold descriptor. Terminal ties and complete schedule ties use canonical descriptor ordering. All Pareto frontiers call the same insertion primitive. Setup prefix ties compare the complete
LayoutCandidateScorebefore the descriptor, which preserves the planner objective even when the primary Pareto coordinates are equal.Generated families bind their profile and grouped key providers directly in the registry. The old
TypeIdrouter, repeated family literals, reverse lookup, and duplicate planning phase are gone.Shared admission and verifier safety
PrecommittedLevelParams::admitis the canonical admission boundary used by both planner construction and generated schedule replay. It checks the frozen profile, modulus profile, outer digit depth, B source norm, opening basis, and the A and B SIS bounds before it returns validated parameters.CommittedGroupProfile::validatenow rejects unsupported inner bases and digit depths that exceed the field decomposition limit. One hot profiles can still use their exact single digit representation.The new direct tests use canonical matrices and reach the intended rejection branches for:
Verifier reachable code remains planner free. Malformed descriptors return typed errors. The change does not add a verifier panic path.
Prover, compression, and NTT ownership
SignedDigitKernelis the only storage selector. Fold decomposition chooses one typed scratch buffer for each worker. No raw basis threshold remains outside the signed digit owner.The
i8andi16sparse multiplication helpers are crate private. They accept exact digit planes and validate challenge positions before calling architecture specific unsafe kernels. A deterministic composition test covers bases 9, 10, and 11. It compares full balanced decomposition plus the preparedi16matrix path with schoolbook ring multiplication.Compression chain derivation has one direct path. The thread local cache, explicit search cache, and cache wrapper graph are gone.
NttExecutionRequirements::from_commit_and_prove_scheduleowns the full root and recursive prewarm layout.SIS coverage and rank semantics
akita-types::sisowns the exact production role and compression cells. The estimator derives its infinity norm work from those cells. The Euclidean estimator keeps its separate domain.The required maximum width and module rank are named constants and are also used by the artifact digest tests.
min_secure_rankuses the first admissible generated entry because generated SIS rows are not monotone in rank.Publication and recovery
Catalog emission stages a batch and rolls it back if publication fails. This is not a claim of crash durable multi file atomicity.
Staged files and backups have separate cleanup paths. If restoration cannot finish, the original backup remains on disk and the returned error includes its recovery path. Cleanup does not delete an unrestored backup.
Breaking changes
This repository does not promise backward compatibility. This PR intentionally changes:
CommitmentConfigbasis policy;There are no compatibility wrappers or parallel legacy representations. The proof schema and transcript labels do not change. Setups and proofs tied to old generated catalog identities are not compatible with the new catalog.
Validation at
6f30be0a5The full validation set below passed on substantive head
8feefc3fa:i16composition, output path isolation, SIS estimator, config, and verifier regressions;cargo machete,typos, and documentation guardrails.At exact head
6f30be0a5, the lightweight W8R2 schedule regression and its profile-ci Clippy target pass, all 54 script tests pass through the stacked tip, and formatting and diff hygiene are clean.The independent Rust cryptography review and the thermo nuclear code quality review both approve this exact code batch with no remaining actionable findings.
All 44 GitHub checks pass at
6f30be0a5. Bugbot reports no unresolved finding.Specification status
The independent basis, signed digit, exact SIS coverage, shared admission, and catalog identity cutovers are implemented.
BoundedBalancedExtremesV1is an intentional catalog policy. It does not claim global split optimality outside its declared domain.Exhaustiveremains available for oracle and audited workloads.The old multi group design history is archived in
specs/archive/2026-Q3/multi-group-batching-legacy.md. The livespecs/multi-group-batching.mddescribes the implemented generated profile and final root contract.Reviewer map
Suggested reading order:
specs/heterogeneous-group-source-contracts.mdspecs/modular-planner-and-precommit-roles.mdspecs/mixed-ring-dimension-per-level.mdcrates/akita-types/src/layout/params/precommitted.rscrates/akita-schedules/src/runtime.rsandcatalog_identity.rscrates/akita-planner/src/policy.rs,planner.rs, andschedule_params/crates/akita-types/src/sis/coverage.rs,sis/ajtai_key.rs, andcompression/chain.rscrates/akita-prover/src/backend/poly_helpers/andcompute/requirements.rscrates/akita-planner/src/generated_families.rs,emit/, and the generated schedule tests