Skip to content

facet: per-axis LCP reads the single register — the fold was done at mint - #1241

Merged
AdaWorldAPI merged 4 commits into
mainfrom
claude/c64-6502-falsifier-shztkk
Sep 16, 2026
Merged

AdaWorldAPI merged 4 commits into
mainfrom
claude/c64-6502-falsifier-shztkk

Conversation

@AdaWorldAPI

@AdaWorldAPI AdaWorldAPI commented Sep 16, 2026

Copy link
Copy Markdown
Owner

What

shared_prefix_tiles already reads the whole facet as one register (u128 xor + trailing_zeros/16); hi_distance/lo_distance (shared6) still gathered six strided bytes per axis into a chain and walked them in a loop.

Operator correction on the first cut (which re-folded the gathered chain into a u64): fold the {0}{1} -f logic once — and the u128 facet already holds both axes by position, so an axis prefix is the whole-facet xor masked to that axis's tier bytes (HI_BYTES = 5,7,…,15; LO_BYTES = 4,6,…,14), then trailing_zeros/16 past the classid. No gather, no per-call re-fold. const fn, no new deps.

Falsifier compares against the loop it replaced at every divergence tier on both axes plus the identical case; disable-run (swap HI_BYTES/LO_BYTES) fails at hi flip at tier 0.

Board: E-FORMAT-SLOT-FOLD-IS-THE-SAME-OP-AS-THE-VL-DESCENT-1 (EPIPHANIES prepend, same PR).

Reality check — before/after, 64K random facet pairs, release, best of 7

arm ns/pair
hi+lo distance, byte loop (before) 12.5–14.0
hi+lo distance, gathered chain → u64 fold (first cut) 8.3–9.3
hi+lo distance, masked single-register readout (shipped) 5.8 (2.9 per axis)
prefix_distance (u128, whole facet) 2.6–3.1

0 mismatches over 65 536 pairs at every step. Both axes together now cost what one whole-facet readout costs — the compare is the 1–4-cycle op and nothing is gathered in front of it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DCfrD5y19cvFc4AoyydXYv

Summary by CodeRabbit

  • Performance

    • Improved facet distance calculations, reducing measured latency from 12.5 ns to 5.8 ns.
    • Optimized handling of short vector tails, with measured speedups of up to 8× in applicable scenarios.
  • Bug Fixes

    • Improved consistency when comparing identical facets and facets that differ at individual hierarchy levels.
  • Documentation

    • Added documentation covering the updated facet processing approach and performance findings.

shared_prefix_tiles already reads the whole facet as one register
(u128 xor + trailing_zeros/16, one vpxor+tzcnt); the per-axis
hi_distance/lo_distance thirty lines above it still walked six bytes
in a loop. Same fold, per axis: the six tier bytes are formatted by
position into a LE u64 ("{0}…{5}" -f chain, tier 0 lowest), xor,
trailing_zeros/8, clamp 6 on xor == 0.

Falsifier compares the fold against the loop it replaced at every
divergence tier on both axes plus the identical case; disable-verified
red by reversing the fold's byte order (fails at "hi t=0").

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCfrD5y19cvFc4AoyydXYv
@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: 8d3d820c-5656-43f8-86df-63ba3dd28563

📥 Commits

Reviewing files that changed from the base of the PR and between e054dcf and 45c5199.

📒 Files selected for processing (2)
  • .claude/board/EPIPHANIES.md
  • crates/lance-graph-contract/src/facet.rs

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.


📝 Walkthrough

Walkthrough

FacetCascade now computes axis distances with masked u128 XOR values and trailing-zero counts. Tests compare the folded results with the former loop across identical facets and tier divergences. Documentation records benchmark results and follow-up gaps.

Changes

Facet prefix folding

Layer / File(s) Summary
Packed facet fold and validation
crates/lance-graph-contract/src/facet.rs, .claude/board/EPIPHANIES.md
FacetCascade uses masked XOR folding for hi_distance and lo_distance. Tests cover identical facets and each tier divergence against loop-based results. The notes record benchmark measurements and follow-up gaps.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to 45c51

The facet distance optimization preserves axis prefix calculations, with no actionable merge risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 1 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: per-axis LCP reads the pre-folded single-register facet value.
Full details: Docstring Coverage

Explanation

Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 1 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.

Comment @coderabbitai help to get the list of available commands.

@cursor

cursor Bot commented Sep 16, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_c76396a6-84a2-402b-aea1-3661b180a419)

@AdaWorldAPI
AdaWorldAPI marked this pull request as ready for review September 16, 2026 19:21
…eady done at mint

The u128 facet stores every tier as hi:lo by position, so the per-axis
prefix is the whole-facet xor masked to that axis's tier bytes, then
trailing_zeros/16 past the classid — no hi_chain/lo_chain gather, no
per-call re-fold. Replaces fold6 from the previous commit. Same tests;
disable-verified red by swapping HI_BYTES/LO_BYTES.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCfrD5y19cvFc4AoyydXYv
…out numbers

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCfrD5y19cvFc4AoyydXYv
@AdaWorldAPI AdaWorldAPI changed the title facet: fold the per-axis LCP (shared6) into one u64 xor+tzcnt facet: per-axis LCP reads the single register — the fold was done at mint Sep 16, 2026
@AdaWorldAPI
AdaWorldAPI merged commit d5d3f7a into main Sep 16, 2026
9 checks passed
AdaWorldAPI pushed a commit that referenced this pull request Sep 16, 2026
Hygiene-only PR — generates no further entry (termination clause).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCfrD5y19cvFc4AoyydXYv
AdaWorldAPI added a commit that referenced this pull request Sep 16, 2026
board: record PR #1241 post-merge (arc entry + LATEST_STATE)
AdaWorldAPI pushed a commit that referenced this pull request Sep 16, 2026
…the Locked formula (Codex on #1243)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCfrD5y19cvFc4AoyydXYv
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants