Skip to content

feat(retrieval): pack-loop budget uses compressed cost when flag ON (#434) - #497

Merged
robotrocketscience merged 2 commits into
mainfrom
feat/issue-434-pack-loop-compression
May 8, 2026
Merged

feat(retrieval): pack-loop budget uses compressed cost when flag ON (#434)#497
robotrocketscience merged 2 commits into
mainfrom
feat/issue-434-pack-loop-compression

Conversation

@robotrocketscience

@robotrocketscience robotrocketscience commented May 8, 2026

Copy link
Copy Markdown
Owner

Phase 2 of #434 — pack-loop budget rewrite. Phase 1 (compress_for_retrieval module + flag wiring + RetrievalResult.compressed_beliefs) shipped at a6f1582. This PR closes the A2 wider-pack precondition: at fixed budget, ON admits more beliefs than OFF.

Change

retrieve_with_tiers() accepts use_type_aware_compression: bool | None. When ON, pack accounting for L2.5 / L1 / BFS uses compress_for_retrieval(b, locked=...).rendered_tokens instead of _belief_tokens(b). Locked beliefs always render verbatim, so locked accounting is unchanged.

retrieve_v2() threads its existing flag through. The post-pack compressed_beliefs field is unchanged.

Default-OFF byte-identity

_cost(b) collapses to _belief_tokens(b) when compress_on resolves False, so the OFF-path arithmetic is identical to pre-#434-phase-2. Covered by test_pack_byte_identical_when_flag_off and the unmodified existing 2864-test baseline (now 2897 / 43 skipped, full suite green).

ON widens

test_pack_widens_when_flag_on at budget=80 against fact + 5×transient: OFF packs 1 belief (fact at ~30 tokens), ON packs ≥ 2 (fact + ≥1 transient at ~10-token stub). Strict inequality.

Acceptance against spec

Refs

Closes A2-precondition for #434. Issue stays open as umbrella for the lab-side bench cut.

Summary by Sourcery

Update retrieval pack-loop budgeting to optionally use type-aware compression while preserving default behavior.

New Features:

  • Allow retrieval_with_tiers and retrieve_v2 to account for belief pack costs using type-aware compression when the corresponding flag is enabled.

Enhancements:

  • Ensure locked beliefs are always treated with verbatim cost accounting regardless of compression settings.
  • Preserve byte-identical retrieval output when type-aware compression is disabled, maintaining backward compatibility.

Documentation:

  • Mark type-aware compression as implemented behind a default-off flag and note the pack-loop budget rewrite in the feature spec.

Tests:

  • Add integration tests verifying wider packing under compression, byte-identical behavior when the flag is off, and unchanged handling of locked beliefs.

Phase 2 of #434. retrieve_with_tiers now accepts
use_type_aware_compression and accounts for L2.5/L1/BFS beliefs at
their compress_for_retrieval rendered_tokens during pack accounting,
not raw _belief_tokens(b). Locks always render verbatim per the
strategy table, so locked accounting is unchanged.

retrieve_v2 threads its existing flag through to retrieve_with_tiers
instead of compressing only post-pack. Result: at fixed budget, the
pack admits strictly more beliefs when ON, closing the A2 wider-pack
precondition cited in feature-type-aware-compression.md.

Default-OFF preserves byte-identical selection — covered by the
test_pack_byte_identical_when_flag_off invariant. The
test_pack_widens_when_flag_on test fires at budget=80 against a
mixed fact+transient corpus where ON admits 5 stub-cost transients
that OFF cannot fit at raw cost.
Phase 1 (compress_for_retrieval + flag wiring) shipped at a6f1582;
phase 2 (pack-loop budget consumes rendered_tokens) lands here.
Status header reflects: implementation done, default-OFF flag held,
lab bench A2 / A4 still pending before flag flip.
@sourcery-ai

sourcery-ai Bot commented May 8, 2026

Copy link
Copy Markdown

Reviewer's Guide

Implements phase 2 of type-aware compression for retrieval by making pack-loop budgeting optionally use compressed token costs while keeping behavior byte-identical when the flag is off, adds tests to verify widening behavior and invariants, and updates the feature spec docs to reflect the implementation status.

Sequence diagram for retrieval_v2 using type-aware compression in pack-loop budgeting

sequenceDiagram
    actor Client
    participant RetrievalAPI as retrieve_v2
    participant RetrievalCore as retrieve_with_tiers
    participant Store as BeliefStore
    participant Compressor as compress_for_retrieval

    Client->>RetrievalAPI: retrieve_v2(query, use_type_aware_compression)
    RetrievalAPI->>RetrievalCore: retrieve_with_tiers(..., use_type_aware_compression)

    activate RetrievalCore
    RetrievalCore->>RetrievalCore: compress_on = resolve_use_type_aware_compression(use_type_aware_compression)

    RetrievalCore->>Store: list_locked_beliefs()
    Store-->>RetrievalCore: locked_beliefs

    loop For each unlocked L25 belief
        alt compress_on is False
            RetrievalCore->>RetrievalCore: cost = _belief_tokens(b)
        else compress_on is True
            RetrievalCore->>Compressor: compress_for_retrieval(b, locked=False)
            Compressor-->>RetrievalCore: compressed_belief
            RetrievalCore->>RetrievalCore: cost = compressed_belief.rendered_tokens
        end
        RetrievalCore->>RetrievalCore: used += cost
    end

    loop For each L1 belief
        alt compress_on is False
            RetrievalCore->>RetrievalCore: cost = _belief_tokens(b)
        else compress_on is True
            RetrievalCore->>Compressor: compress_for_retrieval(b, locked=False)
            Compressor-->>RetrievalCore: compressed_belief
            RetrievalCore->>RetrievalCore: cost = compressed_belief.rendered_tokens
        end
        RetrievalCore->>RetrievalCore: if used + cost <= budget then pack belief
    end

    loop For each BFS hop belief
        alt compress_on is False
            RetrievalCore->>RetrievalCore: cost = _belief_tokens(b)
        else compress_on is True
            RetrievalCore->>Compressor: compress_for_retrieval(b, locked=False)
            Compressor-->>RetrievalCore: compressed_belief
            RetrievalCore->>RetrievalCore: cost = compressed_belief.rendered_tokens
        end
        RetrievalCore->>RetrievalCore: if used + cost <= budget then pack belief
    end

    RetrievalCore-->>RetrievalAPI: packed_beliefs
    deactivate RetrievalCore

    RetrievalAPI-->>Client: RetrievalResult(packed_beliefs, compressed_beliefs unchanged)
Loading

Class diagram for retrieval pack-loop with optional type-aware compression

classDiagram
    class RetrievalModule {
        +retrieve_with_tiers(store, query, entity_index_enabled, posterior_weight, use_bm25f_anchors, bm25f_cache, heat_kernel_enabled, eigenbasis_cache, use_type_aware_compression) tuple
        +retrieve_v2(store, query, include_locked, entity_index_enabled, posterior_weight, use_bm25f, bm25f_cache, use_type_aware_compression) tuple
        -_cost(belief) int
    }

    class Belief {
        +id str
        +lock_level int
    }

    class CompressedBelief {
        +rendered_tokens int
    }

    class BeliefStore {
        +list_locked_beliefs() list~Belief~
    }

    class CompressionModule {
        +compress_for_retrieval(belief, locked) CompressedBelief
    }

    RetrievalModule ..> Belief : uses
    RetrievalModule ..> BeliefStore : uses
    RetrievalModule ..> CompressionModule : calls
    RetrievalModule ..> CompressedBelief : uses
    CompressionModule ..> Belief : compresses
    BeliefStore o--> Belief : contains

    class LockLevels {
        <<enumeration>>
        LOCK_USER
    }

    Belief --> LockLevels : uses

    class FlagsResolver {
        +resolve_use_type_aware_compression(use_type_aware_compression) bool
    }

    RetrievalModule ..> FlagsResolver : resolves_flag

    %% _cost behavior
    class CostBehavior {
        +_cost(belief) int
        -compress_on bool
    }

    CostBehavior ..> Belief : input
    CostBehavior ..> CompressionModule : optional_compression
    CostBehavior ..> CompressedBelief : reads_rendered_tokens
    RetrievalModule ..> CostBehavior : delegates_cost_calculation
Loading

File-Level Changes

Change Details Files
Pack-loop budget now optionally uses type-aware compressed cost instead of raw belief token estimates, with behavior preserved when the flag is off.
  • Add use_type_aware_compression parameter to retrieve_with_tiers and thread it from retrieve_v2
  • Resolve the compression flag at the start of retrieve_with_tiers and introduce a local _cost helper that chooses between compressed rendered_tokens and _belief_tokens
  • Use the new _cost helper for L2.5, L1, and BFS pack-loop accounting while keeping locked belief cost accounting unchanged
  • Extend retrieve_v2 to pass through its existing use_type_aware_compression flag into retrieve_with_tiers
src/aelfrice/retrieval.py
Add integration tests to validate pack-loop behavior under the type-aware compression flag, including widening when enabled and byte-identical behavior when disabled.
  • Add helper _populate_pack_widening_store to construct a MemoryStore with one fact and multiple large transient beliefs
  • Add test_pack_widens_when_flag_on to assert that with a fixed budget the ON path returns strictly more beliefs than the OFF path
  • Add test_pack_byte_identical_when_flag_off to assert that explicit OFF and default OFF produce the same ordered belief IDs
  • Add test_pack_locked_unchanged_when_flag_on to assert that locked beliefs are unaffected by the flag and remain verbatim in compressed_beliefs
tests/test_compression_integration.py
Update type-aware compression feature spec to reflect current implementation status behind a default-OFF flag and remaining lab-side work.
  • Change spec status line to indicate implementation behind default-OFF flag, pack-loop budget rewrite landed, and pending lab benchmarks
docs/feature-type-aware-compression.md

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented May 8, 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 50 minutes and 5 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: 402887f5-732f-4d6e-8d76-858a231500b1

📥 Commits

Reviewing files that changed from the base of the PR and between 5465b3c and 25134b7.

📒 Files selected for processing (3)
  • docs/feature-type-aware-compression.md
  • src/aelfrice/retrieval.py
  • tests/test_compression_integration.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-434-pack-loop-compression

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.

@robotrocketscience robotrocketscience added author-Kulili PR coordination mutex attn:review Needs review (PR open, awaiting reviewer) labels May 8, 2026

@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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[claim:review:Gylf:2026-05-08T19:48:35Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

Reviewed at 25134b7. Two atomic signed commits (G/G), discretion clean, FF-ready (already on top of main). All blocking CI green. Local suite 2889 passed / 25 skipped — no regressions.

Substance. Closes the A2-precondition for #434 — at fixed budget, ON admits more beliefs than OFF (test_pack_widens_when_flag_on). The change is the right shape:

  • New _cost(b) closure inside retrieve_with_tiers that short-circuits to _belief_tokens(b) when compress_on is False — preserves the byte-identical-OFF guarantee algorithmically, not just behaviorally. Verified by test_pack_byte_identical_when_flag_off.
  • Three pack-loop accounting sites switched (l25 sum at :1466, l1 per-belief at :1471, bfs per-belief at :1497). All three were the ones the spec at docs/feature-type-aware-compression.md § "Where compression sits" called out.
  • Locked-belief verbatim accounting preserved: _cost passes locked=(b.lock_level == LOCK_USER) into compress_for_retrieval, which renders verbatim for L0 per the strategy table. Locks always cost the raw token count regardless of flag state. test_compression_strategy_dispatches_by_retention_class verifies the L1 belief lands at STRATEGY_VERBATIM.

Cost-recompute observation (non-blocking). _cost(b) invokes compress_for_retrieval(b, locked=...) per call site, so a belief that survives the L2.5 sum + the L1 loop pays for compression twice. With compress_for_retrieval being a pure function and the v2.0.0 candidate pool typically <50 beliefs, this is microseconds — not a Phase-2 blocker. A memo at the top of retrieve_with_tiers that caches cb per id would be a v2.x ergonomic improvement; the current shape stays simple and the cost is dominated by other lanes (heat kernel, bm25f).

Spec acceptance status (per PR body): A1 lab-side; A2 pack-loop precondition closed here; A3 carries forward; A4 / A5 wait on lab cut + #154. Honest framing — the umbrella stays open and doesn't promise more than landed.

Merging.

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[release:review:Gylf:2026-05-08T19:51:32Z]

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

Labels

attn:review Needs review (PR open, awaiting reviewer) author-Kulili PR coordination mutex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant