docs: say which decay ships, and which is designed but unwired (#1218) - #1221
Conversation
There was a problem hiding this comment.
Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
Reviewer's GuideDocs were corrected to accurately describe which decay mechanisms are actually wired in production (ranking vs posterior), PHILOSOPHY/ARCHITECTURE/CHANGELOG were updated to clarify lock behaviour and unwired posterior decay, and a new test was added to assert the code fact that posterior decay remains unwired or is explicitly removed, ensuring future wiring/removal forces doc updates. Flow diagram for posterior-decay enforcement testflowchart TD
A[Run pytest] --> B[tests/test_docs_posterior_decay_1218.py]
B --> C[test_posterior_decay_still_has_no_production_caller]
B --> D[test_the_decay_surface_is_still_there_to_be_unwired]
C --> E{any ImportFrom aelfrice.scoring includes decay}
E -->|yes| F[Fail: update ARCHITECTURE.md and PHILOSOPHY.md for wired posterior decay]
E -->|no| G[Pass: posterior decay still has no production caller]
D --> H{scoring.decay / type_half_life / TYPE_HALF_LIFE_SECONDS still defined}
H -->|no| I[Fail: update ARCHITECTURE.md and PHILOSOPHY.md for removed posterior decay]
H -->|yes| J[Pass: decay surface still present but unwired]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[claim:review:Setr:2026-07-30T22:27:56Z] |
Review: approved. The code facts the prose rests on all check out.A docs PR is only as good as the claims underneath it, so I re-derived each No posterior decay ships. The correction to the issue's own replacement text is right, and load-bearing. Entity-persistence is genuinely the one that ships. AC3 is enforced, and I confirmed the enforcement is realThe claim that matters here is that the test fails in both directions of a
Baseline Two notes, neither blocking
Adding the fourth statement ( CI green, |
|
merge-train: blocked branch is not fast-forward on The |
|
[claim:review:Toug:2026-07-30T22:31:58Z] |
|
[release:review:Toug:2026-07-30T22:32:05Z] |
scoring.decay, type_half_life and TYPE_HALF_LIFE_SECONDS have no caller under src/, so nothing moves a stored (alpha, beta) toward the Jeffreys prior. Three statements described posterior decay as shipped, and PHILOSOPHY's lock story rested on the lock short-circuit inside that function — an exemption from a mechanism that does not run. Corrects the filed issue's own replacement text, which describes retrieval-time _apply_temporal_decay as live: it is reachable only via retrieve_v2 behind temporal_sort, which defaults False and is set nowhere in src/, while the production hooks call retrieve(). The live, default-on demotion is entity-persistence (#1096), which acts on ranking position — and it is ranking, not the posterior, in every case. A fourth statement not listed in the issue carried the same error and is corrected alongside.
Fails in both directions of #1162's pending disposition: wiring scoring.decay trips the no-caller assertion, deleting it trips a negative control. The failure messages name the two files to update, so the cross-link the issue asked for is enforced rather than written down. Parses src/ rather than grepping it — `decay` is a local variable for the ranking factor inside _apply_temporal_decay, which is exactly the conflation being undone. Deliberately does not assert the docs' wording: a text match on prose breaks on rephrasing, and reading docs/ would require adding docs/** to CI's code path filter, taxing every docs-only PR with the full matrix.
|
Rebased onto current Verified the rebase is reviewably inert: the Re-adding |
eb028fb to
64f698e
Compare
|
merge-train: merged 64f698e → |
|
[release:review:Setr:2026-07-30T22:40:44Z] |
Closes #1218.
Verified the premise before editing anything
scoring.decay,type_half_lifeandTYPE_HALF_LIFE_SECONDShave no callerunder
src/.retrieval.pyimportsposterior_mean,partial_bayesian_score,gamma_posterior_score,zeta_posterior_scoreandthe zeta constants — and nothing else from
scoring. The only other importersin
src/arecontext_rebuilder(posterior_mean) andwonder/strategies(
uncertainty_score). So nothing ever moves a stored(α, β)toward theJeffreys prior, and PHILOSOPHY's lock story rested on the lock short-circuit
inside that function — an exemption from a mechanism that does not run.
One correction to the issue's own replacement text
The issue says to describe retrieval-time
_apply_temporal_decayas live:That is too generous, and writing it would have replaced one inaccuracy with
another. Traced:
_apply_temporal_decayis called atretrieval.py:4139, insideretrieve_v2— notretrieve().temporal_sort, which isbool = Falseatretrieval.py:3915.temporal_sort=Trueappears nowhere insrc/outside that signature.retrieve().So it is not on the default path either. The honest "what ships" list is
shorter than the issue's:
scoring.decay(posterior → prior)_apply_temporal_decay(#473)retrieve_v2+temporal_sort=FalseEntity-persistence is the one that ships by default — verified reachable from
retrieve(), which passesuse_entity_persist_demote=None(resolver-driven)rather than hard-off, landing at
_entity_persist_penaltyin the L1 rerank.Every live mechanism acts on ranking position, never on the posterior,
which is the distinction the docs were collapsing.
A fourth statement, not in the issue
PHILOSOPHY.md:145— "Beliefs are still mutated for decay and feedback" —carries the same error and is corrected alongside. Feedback mutates beliefs;
decay does not.
AC3 is enforced, not written down
The third criterion asks that these statements be revisited if #1162 later
wires or deletes
scoring.decay. A cross-link alone is a hope. Sotests/test_docs_posterior_decay_1218.pypins the code fact the prose restson, and fails in both directions of that disposition — verified by
performing each:
decayto afrom aelfrice.scoring importinsrc/test_posterior_decay_still_has_no_production_callerfailsscoring.decayawaytest_the_decay_surface_is_still_there_to_be_unwiredfailsBoth failure messages name ARCHITECTURE.md and PHILOSOPHY.md as the files to
update.
It parses
src/rather than grepping:decayexists as a local variablefor the ranking factor inside
_apply_temporal_decay, so a text searchreports exactly the conflation this issue exists to undo. A first version using
ast.Namehad the same bug and failed onretrieval.py— onlyImportFrom aelfrice.scoringandscoring.<name>attribute access count.It deliberately does not assert the docs' wording. Prose gets rephrased,
and a text match says nothing about whether the claim is true. It also would
have cost more than it is worth: my first draft read
docs/, andtest_ci_path_filtercorrectly failed —The fix it suggests is
docs/**in the filter, which would run the full pytestmatrix on every docs-only PR. For a test asserting that an issue number appears
in a markdown file, that is a bad trade, so the test was dropped instead. The
enforcement that matters is the code-fact assertion, which needs no docs read.
Acceptance criteria
ranking-time decay from posterior decay.
restated in terms of the lock floor and
aelf lockoverwrite semantics,with the decay short-circuit explicitly marked as design intent not wired.
disposition goes.
Verification
src/change.Note for sequencing
The marker-edge row above is described as opt-in rather than default, which is
true on
maintoday and stays true after #1220 (#1207) lands — that PR wiresthe pass but its producer,
aelf doctor --detect-stale, remains opt-in. Nomerge-order dependency either way.
Summary by Sourcery
Clarify documentation around decay mechanisms to distinguish designed-but-unwired posterior decay from the ranking-time decay that actually ships, and add a test to enforce that the docs track the code’s wiring status.
Documentation:
Tests: