docs(feature-type-aware-compression): spec memo for #434 - #450
Conversation
Reviewer's GuideAdds a new feature specification document for type-aware retrieval compression, defining a deterministic Sequence diagram for type-aware compression in retrieval pipelinesequenceDiagram
actor Operator
participant Client as retrieval_client
participant Retrieval as retrieval_module
participant Compressor as compression_module
participant Rebuilder as context_rebuilder
Operator->>Client: call_retrieval(use_type_aware_compression flag)
Client->>Retrieval: retrieve(query, use_type_aware_compression)
Retrieval->>Retrieval: resolve_use_type_aware_compression
Retrieval->>Retrieval: lane_fan_out_and_rank
alt use_type_aware_compression is true
loop for each belief in ranked_candidates
Retrieval->>Compressor: compress_for_retrieval(belief, locked)
Compressor-->>Retrieval: CompressedBelief
end
Retrieval->>Retrieval: pack_loop_over_compressed_beliefs
Retrieval->>Client: RetrievalResult(beliefs, compressed_beliefs)
else use_type_aware_compression is false
Retrieval->>Retrieval: pack_loop_over_raw_beliefs
Retrieval->>Client: RetrievalResult(beliefs)
end
opt context rebuilder enabled
Client->>Rebuilder: run_rebuilder(RetrievalResult)
Rebuilder->>Rebuilder: apply_token_budget_with_compressed_output
Rebuilder-->>Operator: continuation_fidelity_report
end
Class diagram for CompressedBelief and retrieval result shapeclassDiagram
class Belief {
+str id
+str content
+str retention_class
+str belief_type
+str source_kind
+str lock_state
}
class CompressedBelief {
+Belief belief
+str rendered
+int rendered_tokens
+str strategy
}
class RetrievalResult {
+list~Belief~ beliefs
+list~CompressedBelief~ compressed_beliefs
}
class CompressionModule {
+CompressedBelief compress_for_retrieval(Belief belief, bool locked)
}
class TokenEstimator {
+int _belief_tokens(Belief belief)
+int _estimate_tokens(str rendered)
}
Belief <.. CompressedBelief : wraps
RetrievalResult o-- Belief : beliefs
RetrievalResult o-- CompressedBelief : compressed_beliefs
CompressionModule ..> Belief : input
CompressionModule ..> CompressedBelief : output
CompressionModule ..> TokenEstimator : uses
TokenEstimator ..> Belief : measures
class ConfigFlagResolution {
+bool resolve_use_type_aware_compression(bool kwarg, bool env_flag, bool config_flag)
}
ConfigFlagResolution ..> RetrievalResult : controls_shape
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a comprehensive documentation specification for type-aware compression in retrieval packs, defining the API contract ( ChangesType-Aware Compression Specification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The contract currently guarantees
rendered_tokens ≤ _belief_tokens(belief), but the spec never states how this is enforced or validated for each strategy (especially headline truncation and stubs); consider adding a brief rationale per strategy so future implementations don’t accidentally violate this invariant during refactors. - It may be useful to clarify edge cases for
CompressedBelief.rendered, e.g., how empty-content beliefs or all-code-fence beliefs are handled and whetherrenderedis ever allowed to be an empty string, so downstream consumers can rely on a well-defined minimum shape.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The contract currently guarantees `rendered_tokens ≤ _belief_tokens(belief)`, but the spec never states how this is enforced or validated for each strategy (especially headline truncation and stubs); consider adding a brief rationale per strategy so future implementations don’t accidentally violate this invariant during refactors.
- It may be useful to clarify edge cases for `CompressedBelief.rendered`, e.g., how empty-content beliefs or all-code-fence beliefs are handled and whether `rendered` is ever allowed to be an empty string, so downstream consumers can rely on a well-defined minimum shape.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[claim:review:Toug:2026-05-05T19:43:44Z] |
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
f68f270 to
5c1ec42
Compare
|
[claim:review:Kulili:2026-05-05T19:45:32Z] |
|
[release:review:Kulili:2026-05-05T19:45:37Z] |
|
[claim:review:Kulili:2026-05-05T19:48:02Z] |
|
[release:review:Kulili:2026-05-05T19:48:07Z] |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
docs/feature-type-aware-compression.md (3)
162-164: 💤 Low valueOptional: Add language marker to fenced code block.
The fenced code block at lines 162-164 is missing a language specifier. While the content is a comparison expression rather than executable code, adding
```textor```pythonwould satisfy the markdownlint rule and improve consistency.🤖 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 `@docs/feature-type-aware-compression.md` around lines 162 - 164, The fenced code block containing the expression `recall@k(use_type_aware_compression=ON) > recall@k(use_type_aware_compression=OFF)` needs a language marker to satisfy markdownlint; edit the block around that expression (the triple-backtick fence enclosing the `recall@k(...)` line) and add a language specifier such as ```text (or ```python) immediately after the opening ``` so the block becomes ```text ... ```.
226-226: 💤 Low valueClarify the code-fence example.
The phrase "one
python ... ``` `` block" has ambiguous backtick escaping that makes it hard to parse. Consider rephrasing to:
- "A belief whose entire content is a single fenced code block (e.g.,
```python\n...\n```)"- "A belief consisting solely of a code fence with no surrounding prose"
🤖 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 `@docs/feature-type-aware-compression.md` at line 226, The sentence describing the code-fence example is ambiguous due to backtick escaping; update the phrasing in the "Headline-strategy on code-fence-only content" paragraph to a clearer form such as "A belief whose entire content is a single fenced code block (e.g., `` ```python\n...\n``` ``)" or "A belief consisting solely of a code fence with no surrounding prose" so the example is unambiguous and easy to parse; edit the sentence that currently reads 'one ``` ```python ... ``` `` block' to one of these clearer alternatives.
61-63: ⚡ Quick winSpecify edge cases for headline strategy.
Two edge cases are not fully specified:
Unbalanced code fences: Line 61 mentions splitting "outside a balanced code-fence span" but does not specify behavior when fences are unbalanced (e.g., opening
```without closing). Should the algorithm attempt to balance them, treat them as not-fences, or fail gracefully?No whitespace in first 240 chars: Line 62 says "hard-truncate at the last whitespace boundary ≤ 240" but does not specify behavior when there is no whitespace in the first 240 characters (e.g., a 250-char continuous URL or code identifier).
Consider adding a fallback rule: if no whitespace ≤ 240, either truncate at exactly 240 or render verbatim.
🤖 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 `@docs/feature-type-aware-compression.md` around lines 61 - 63, The headline strategy lacks rules for unbalanced code fences and for when no whitespace exists within MAX_HEADLINE_CHARS (MAX_HEADLINE_CHARS = 240); update the spec to (1) define that unbalanced triple-backtick spans are treated as non-fences (i.e., ignore opening-only or closing-only ``` and allow splitting within them) rather than trying to auto-balance or failing, and (2) add a deterministic fallback for the "last whitespace ≤ MAX_HEADLINE_CHARS" rule: if no whitespace exists in the first MAX_HEADLINE_CHARS, hard-truncate at exactly MAX_HEADLINE_CHARS and append an ellipsis (…) (or explicitly state to render verbatim if you prefer that behavior), and reference MAX_HEADLINE_CHARS and the “split on first `. ` or `.\n` outside a balanced code-fence span” rule so implementers can apply these two edge-case policies consistently.
🤖 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 `@docs/feature-type-aware-compression.md`:
- Around line 173-177: The test test_compress_deterministic is tautological
because it compares two identical calls; change it to call
compress_for_retrieval(b, locked=locked) multiple times (e.g., N=10) and assert
all .rendered results are identical, or otherwise restructure the Hypothesis
property to produce a list of results and assert len(set(results)) == 1; update
references to compress_for_retrieval and the .rendered attribute in the test to
implement this non-tautological determinism check.
- Line 40: Update the documentation to use the correct attribute and remove the
incorrect classification mechanism: change the description for `locked: bool` to
state it is True when `belief.lock_level == LOCK_USER` (not `belief.lock_state`)
and remove any mention of an "otherwise classified as L0" mechanism; also keep
the note that locks always render `"verbatim"` regardless of `retention_class`.
---
Nitpick comments:
In `@docs/feature-type-aware-compression.md`:
- Around line 162-164: The fenced code block containing the expression
`recall@k(use_type_aware_compression=ON) >
recall@k(use_type_aware_compression=OFF)` needs a language marker to satisfy
markdownlint; edit the block around that expression (the triple-backtick fence
enclosing the `recall@k(...)` line) and add a language specifier such as ```text
(or ```python) immediately after the opening ``` so the block becomes ```text
... ```.
- Line 226: The sentence describing the code-fence example is ambiguous due to
backtick escaping; update the phrasing in the "Headline-strategy on
code-fence-only content" paragraph to a clearer form such as "A belief whose
entire content is a single fenced code block (e.g., `` ```python\n...\n``` ``)"
or "A belief consisting solely of a code fence with no surrounding prose" so the
example is unambiguous and easy to parse; edit the sentence that currently reads
'one ``` ```python ... ``` `` block' to one of these clearer alternatives.
- Around line 61-63: The headline strategy lacks rules for unbalanced code
fences and for when no whitespace exists within MAX_HEADLINE_CHARS
(MAX_HEADLINE_CHARS = 240); update the spec to (1) define that unbalanced
triple-backtick spans are treated as non-fences (i.e., ignore opening-only or
closing-only ``` and allow splitting within them) rather than trying to
auto-balance or failing, and (2) add a deterministic fallback for the "last
whitespace ≤ MAX_HEADLINE_CHARS" rule: if no whitespace exists in the first
MAX_HEADLINE_CHARS, hard-truncate at exactly MAX_HEADLINE_CHARS and append an
ellipsis (…) (or explicitly state to render verbatim if you prefer that
behavior), and reference MAX_HEADLINE_CHARS and the “split on first `. ` or
`.\n` outside a balanced code-fence span” rule so implementers can apply these
two edge-case policies consistently.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90a2cae9-b2ee-4bcd-b1bc-831e9ada1459
📒 Files selected for processing (1)
docs/feature-type-aware-compression.md
| Inputs: | ||
|
|
||
| - `belief: Belief` — the belief as returned by the lane fan-out. | ||
| - `locked: bool` — `True` when `belief.lock_state == LOCK_USER` or the caller has otherwise classified the belief as L0. Locks always render `"verbatim"` regardless of `retention_class`. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find L0 classification logic and lock conditions in retrieval.py
# Search for L0 lane classification and lock_state checks
rg -nP -C5 '\bL0\b|lock_state|LOCK_USER' --type=py src/aelfrice/retrieval.pyRepository: robotrocketscience/aelfrice
Length of output: 7822
🏁 Script executed:
#!/bin/bash
# Read the feature-type-aware-compression.md file to understand the context around line 40
cat -n docs/feature-type-aware-compression.md | sed -n '35,50p'Repository: robotrocketscience/aelfrice
Length of output: 861
🏁 Script executed:
#!/bin/bash
# Search for how L0 classification is determined in the codebase
# Look for Belief class definition and lock-related attributes
rg -nP 'class Belief|lock_state|LOCK_USER|LOCK_NONE' --type=py src/ -A2Repository: robotrocketscience/aelfrice
Length of output: 11136
🏁 Script executed:
#!/bin/bash
# Check for any other mechanisms that classify beliefs as L0 or locked
# Search in models.py and context_rebuilder.py
fd -e py -path '*models.py' -o -path '*context_rebuilder.py' | xargs rg -l 'L0|lock_state' 2>/dev/null || echo "Files not found or no matches"Repository: robotrocketscience/aelfrice
Length of output: 2128
🏁 Script executed:
#!/bin/bash
# Verify the exact attribute name in Belief dataclass
grep -A 30 "^class Belief:" src/aelfrice/models.py | head -40Repository: robotrocketscience/aelfrice
Length of output: 1444
🏁 Script executed:
#!/bin/bash
# Confirm there are only these two lock levels
rg -n "LOCK_" src/aelfrice/models.py | grep "Final"Repository: robotrocketscience/aelfrice
Length of output: 213
Fix attribute name and remove non-existent classification mechanism.
Line 40 contains two errors: (1) the attribute is belief.lock_level, not belief.lock_state, and (2) there is no "otherwise classified as L0" mechanism. The codebase defines exactly two lock levels (LOCK_NONE and LOCK_USER), and L0 beliefs are determined solely by belief.lock_level == LOCK_USER. Rewrite as:
locked: bool — True when belief.lock_level == LOCK_USER. Locks always render "verbatim" regardless of retention_class.
🤖 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 `@docs/feature-type-aware-compression.md` at line 40, Update the documentation
to use the correct attribute and remove the incorrect classification mechanism:
change the description for `locked: bool` to state it is True when
`belief.lock_level == LOCK_USER` (not `belief.lock_state`) and remove any
mention of an "otherwise classified as L0" mechanism; also keep the note that
locks always render `"verbatim"` regardless of `retention_class`.
| @hypothesis.given(belief_strategy(), st.booleans()) | ||
| def test_compress_deterministic(b, locked): | ||
| assert compress_for_retrieval(b, locked=locked).rendered \ | ||
| == compress_for_retrieval(b, locked=locked).rendered | ||
| ``` |
There was a problem hiding this comment.
Fix tautological determinism property test.
The proposed property test assertion is tautological:
assert compress_for_retrieval(b, locked=locked).rendered \
== compress_for_retrieval(b, locked=locked).renderedThis calls the same pure function twice with identical inputs and compares the results, which will always pass. It does not meaningfully test determinism (byte-stability across processes, time, or environment).
Consider one of these alternatives:
-
Multiple calls: Assert that calling the function N times (e.g., 10) on the same input yields identical results:
results = [compress_for_retrieval(b, locked=locked).rendered for _ in range(10)] assert len(set(results)) == 1, "Non-deterministic output detected"
-
Explicit property: The test as written might be a placeholder. Clarify that the property test should verify that Hypothesis-generated random beliefs always compress deterministically.
The fixture-based regression test mentioned at line 179 is the real determinism gate; this property test should complement it by covering a broader input space.
🤖 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 `@docs/feature-type-aware-compression.md` around lines 173 - 177, The test
test_compress_deterministic is tautological because it compares two identical
calls; change it to call compress_for_retrieval(b, locked=locked) multiple times
(e.g., N=10) and assert all .rendered results are identical, or otherwise
restructure the Hypothesis property to produce a list of results and assert
len(set(results)) == 1; update references to compress_for_retrieval and the
.rendered attribute in the test to implement this non-tautological determinism
check.
Per-retention-class compression of retrieved beliefs at fixed
token_budget. Strategy table:
fact -> verbatim
snapshot -> headline (first sentence, 240-char truncation cap)
transient -> stub ("[stub: belief={id} class=transient]")
unknown -> verbatim (migration safety)
locked -> verbatim regardless of retention_class
Pure deterministic transform — no LLM. Bench-gates: strictly positive
recall@k uplift on a labeled compression_uplift fixture, and
continuation-fidelity uplift through the v1.4 context rebuilder
(#141).
Substrate prereqs all on main: retention_class column and per-source
defaults (#290, v1.6.0); pack loops at retrieval.py:1048-1085 +
:1197-1232; flag-resolution convention at retrieval.py:118-131.
5c1ec42 to
34ddfb8
Compare
|
[release:review:Toug:2026-05-05T19:51:23Z] |
2 participants
Spec memo for #434 — Type-aware compression. Closes the recovery-inventory line at
docs/ROADMAP.mdrow 162 (Type-aware compression | v2.0.0).What this PR is
Docs-only. New file at
docs/feature-type-aware-compression.mdthat converts the bare issue acceptance sketch into a buildable contract:compress_for_retrieval()signature, per-retention-class strategy table, pack-loop integration points, configuration plumbing, bench-gate, determinism gate, and out-of-scope list.No code, no schema, no flag wiring yet — those land in a follow-up implementation PR. This PR moves #434 from
needs-spectobench-gated.Strategy table (per retention class)
factsnapshottransient[stub: belief={id} class=transient])unknownLocks override retention class — same rule as L0-never-trimmed at
retrieval.py:950and the lock-bypasses-hibernation rule at #196.Determinism
Issue acceptance #3 makes byte-stable output a hard requirement. The compressor is pure: no LLM, no clock, no random, no store reads. A property test plus a fixture-pinned regression test cover it.
Substrate
All on
mainas of68dafc0:models.py—RETENTION_*constants,Belief.retention_class(Rebuild redesign: belief typing + aging policy #290, v1.6.0)retrieval.py:215—_belief_tokens()reused onrenderedretrieval.py:118-131— flag-resolution conventionretrieval.py:1048-1085+:1197-1232— pack loops to rewritecontext_rebuilder.py— A4 consumertests/corpus/v2_0/,tests/bench_gate/— corpus + harnessNo new dependencies. No schema changes.
Test plan
github/main— clean.G).Refs
docs/ROADMAP.mdrow 162Summary by Sourcery
Document the specification for a deterministic, type-aware retrieval compression feature, defining its contract, placement in the retrieval pipeline, and acceptance criteria.
New Features:
Enhancements:
Documentation:
Summary by CodeRabbit