Skip to content

fix(rebuilder): honour use_type_aware_compression in pack accounting (#798) - #815

Merged
github-actions[bot] merged 2 commits into
mainfrom
feat/issue-798-rebuilder-compression-aware-pack
May 14, 2026
Merged

fix(rebuilder): honour use_type_aware_compression in pack accounting (#798)#815
github-actions[bot] merged 2 commits into
mainfrom
feat/issue-798-rebuilder-compression-aware-pack

Conversation

@robotrocketscience

Copy link
Copy Markdown
Owner

Summary

Closes #798. rebuild_v14 was re-packing retrieve()'s candidate set with verbatim token cost regardless of use_type_aware_compression, so any ON-arm extras retrieve() admitted at compressed cost got trimmed back to the OFF-arm count. The downstream A4 continuation-fidelity bench gate (#775 / PR #776) was therefore structurally vacuous — per-row fidelity delta = 0 by construction, regardless of corpus.

Operator decision on #798: Option A — thread compress_on into rebuilder pack accounting. Smallest diff; rebuild-block content stays verbatim; A4 gate becomes structurally measurable. Options B (rebuilder emits compressed_beliefs[i].rendered) and C (drop A4 from #769 acceptance) declined.

What ships

  1. src/aelfrice/context_rebuilder.py

    • New use_type_aware_compression: bool | None = None kwarg on rebuild_v14. Resolves once via resolve_use_type_aware_compression(...) at function entry; resolved boolean threaded to all sites.
    • retrieve() call inside rebuild_v14 now passes use_type_aware_compression=compress_on so retrieve's L1 / L2.5 pack is compression-aware on the rebuilder path.
    • _estimate_belief_tokens(b, *, compress_on=False) gains a keyword-only flag. When True, it returns compress_for_retrieval(b, locked=(b.lock_level == LOCK_USER)).rendered_tokens; else the existing verbatim (len + 3) // 4 math. Default-OFF preserves the byte-identical contract for the [v1.4] PreCompact hook + rebuild logic (augment mode) #139 / Rebuild redesign: eval harness — per-rebuild log + fixed-corpus precision #288 regression tests and for the legacy _retrieve_for_rebuild (v1.2.0a0 alpha) path, which keeps its default-OFF call.
    • All three live rebuild_v14 pack sites (L0 init at :414, session tier at :463, L1 / L2.5 tier at :513) pass compress_on=compress_on.
  2. tests/test_context_rebuilder.py — two new tests:

  3. CHANGELOG.md — entry under [Unreleased] / Fixed.

  4. docs/feature-type-aware-compression.md — §"vs. context rebuilder" rewritten (the old text described an Option-B-style RetrievalResult shape change that never shipped); §A4 gains a prerequisite note pointing at the pack-size property test.

Observable behaviour change

  • Default-OFF (env unset, no kwarg, no TOML override): byte-identical. The new test pins this.
  • Default-ON (AELFRICE_TYPE_AWARE_COMPRESSION=1 or explicit kwarg): rebuild block now contains strictly more beliefs at the same token_budget when the candidate set under verbatim cost would have trimmed. Each belief still renders verbatim.

Tests

uv run pytest tests/ --ignore=tests/bench_gate — 4164 passed, 33 skipped, 75 xfailed (pre-existing).

The bench-gate tests at tests/bench_gate/test_compression_a4_fidelity.py remain bench_gated (skip without AELFRICE_CORPUS_ROOT); now that the rebuilder honours the flag, a lab-side run against a populated corpus will produce nonzero per-row deltas instead of 0-by-construction.

What this unblocks

#769 (flip default use_type_aware_compression on). The A4 axis of that flip-default decision was load-bearing on a gate that could not measure compression; with this fix it can. The A2 axis was unaffected (#769 acceptance remains as-is for both bench runs).

Risk

  • #139 (byte-identical v1.x rebuild) and #288 (rebuild_log shape) regression tests both run green under default-OFF; the keyword-only compress_on parameter on _estimate_belief_tokens defaults to False.
  • The legacy _retrieve_for_rebuild (v1.2.0a0 alpha contract, frozen docstring) is intentionally not modified — its two _estimate_belief_tokens(b) call sites use the default compress_on=False.
  • Default-ON behaviour change: consumers reading the rebuild block under the flag now see a larger block (more belief entries) at the same budget. Content per entry unchanged.

rebuild_v14 was re-packing retrieve()'s candidate set with verbatim
token cost regardless of use_type_aware_compression, so the ON-arm
extras retrieve() admitted got trimmed back to the OFF-arm count.
Per-row A4 fidelity delta was 0 by construction.

Resolve the flag once at function entry, pass to retrieve() so its
pack is compression-aware, and thread compress_on into
_estimate_belief_tokens so the rebuilder's own L0 / session / L1
budget math matches. Default-OFF leaves _estimate_belief_tokens
byte-identical (compress_on=False); _retrieve_for_rebuild (legacy
v1.2.0a0 path, byte-identical alpha contract) stays untouched.

Tests assert (a) ON packs strictly more beliefs than OFF at a
budget that forces trim, and (b) default-OFF block equals the
explicit use_type_aware_compression=False block.
)

Record the rebuilder pack-accounting fix under [Unreleased]/Fixed and
replace the obsolete §"vs. context rebuilder" wording in the
type-aware compression spec (which described an Option-B-style
`RetrievalResult` shape change that never shipped) with the
Option-A-style flag-threading behavior that actually ships now.

Adds a prerequisite note to §A4 pointing at the pack-size property
test — the A4 bench gate stays vacuous until rebuild_v14's pack
accounting honors the flag, and that prerequisite is the #798 fix.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@robotrocketscience has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 7 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c4dffaf1-68cc-455f-88ff-b01ec70611ca

📥 Commits

Reviewing files that changed from the base of the PR and between ae2afd8 and d6f0deb.

⛔ Files ignored due to path filters (1)
  • CHANGELOG.md is excluded by !**/CHANGELOG.md
📒 Files selected for processing (3)
  • docs/feature-type-aware-compression.md
  • src/aelfrice/context_rebuilder.py
  • tests/test_context_rebuilder.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-798-rebuilder-compression-aware-pack

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.

❤️ Share

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

@github-actions

github-actions Bot commented May 14, 2026

Copy link
Copy Markdown

PR-size soft cap

This PR is over the advisory size threshold:

  • 140 changed lines (limit: 200)
  • 4 changed files (limit: 3)

Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated attn:merge-conflict cycles (see #602). When practical, split into smaller PRs that each touch a focused surface.

This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the size:override label and this comment will be removed on the next push.

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[claim:review:bagheera:2026-05-14T20:32:17Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

Review (sister-session, cannot --approve — same GitHub author):

  • rebuild_v14 resolves use_type_aware_compression once at entry and threads the boolean to retrieve() and all three _estimate_belief_tokens sites (L0 init, session tier, L1 / L2.5 tier).
  • _estimate_belief_tokens(b, *, compress_on=False) mirrors retrieval._cost: compress_for_retrieval(b, locked=(b.lock_level == LOCK_USER)).rendered_tokens when ON, verbatim cost otherwise. Locks render verbatim either way per the strategy table.
  • Default-OFF byte-identical contract pinned by test_rebuild_v14_compression_off_byte_identical_default (pops AELFRICE_TYPE_AWARE_COMPRESSION env first).
  • Pack-size delta pinned by test_rebuild_v14_pack_size_matches_compression_flag — guards against the verbatim-cost regression A4 bench gate is structurally vacuous — rebuilder uses verbatim token cost, ignoring compression flag #798 documents.
  • Legacy _retrieve_for_rebuild (v1.2.0a0 alpha contract) intentionally unmodified — both call sites keep default compress_on=False.
  • Both commits SSH-signed (G). Discretion grep on the diff vs main is clean.
  • A4 bench gate remains bench-gated (skip without AELFRICE_CORPUS_ROOT) — this PR unblocks measurability; lab-side run still required to score Flip default: use_type_aware_compression on by default — gated on A2 + A4 bench run #769's flip-default decision.

Labeling ready-to-merge.

@robotrocketscience robotrocketscience added ready-to-merge Trigger merge-train: FF main to this PR's head and removed attn:review Needs review (PR open, awaiting reviewer) labels May 14, 2026
@robotrocketscience

Copy link
Copy Markdown
Owner Author

[release:review:bagheera:2026-05-14T20:35:55Z]

@github-actions github-actions Bot removed the ready-to-merge Trigger merge-train: FF main to this PR's head label May 14, 2026
@github-actions
github-actions Bot merged commit d6f0deb into main May 14, 2026
32 of 34 checks passed
@github-actions

Copy link
Copy Markdown

merge-train: merged d6f0debmain via FF push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author-oppenheimer Author label for oppenheimer session

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A4 bench gate is structurally vacuous — rebuilder uses verbatim token cost, ignoring compression flag

1 participant