Skip to content

feat(*): expose the RRF damping constant and rebalance skill source fusion - #290

Open
ypflll wants to merge 2 commits into
mainfrom
refactor/rrf_k_configurable
Open

feat(*): expose the RRF damping constant and rebalance skill source fusion#290
ypflll wants to merge 2 commits into
mainfrom
refactor/rrf_k_configurable

Conversation

@ypflll

@ypflll ypflll commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

RRF_K was a module constant at 60 -- the value classic RRF uses for TREC-scale
runs of ~1000 hits. Skill sources return ~10 hits (top_k=5 x over_fetch_factor=2),
and at that list length 60 flattens the entire rank ladder to a 15% score spread:

rank score at rrf_k=60 vs rank 1
1 1/61 = 0.01639 100.0%
5 1/65 = 0.01538 93.8%
10 1/70 = 0.01429 87.1%

That 15% ladder is narrower than the 17.6% gap between the old Local (1.0) and
Hub (0.85) weights, so weight decided the order outright and a source's internal
rank stopped mattering. The concrete consequence with the shipped settings:
Local's rank-10 hit (1.0/70 = 0.014286) still outscored Hub's rank-1 hit
(0.85/61 = 0.013934), and over-fetch caps each source at 10 hits, so a run with
disjoint results returned Local's own top-5 verbatim. Neither EverOS nor Hub
could place a hit unless the same skill also surfaced from Local. That
contradicts the weighted blend the weights docstring describes, and it silently
disabled the EverOS source whose whole purpose is surfacing auto-evolved skills
Local does not have.

Two knobs move here, and they do different jobs:

  • rrf_k 60 -> 10 removes the tiering. It restores the ladder to an 82%
    spread, so each source's top hit competes on rank. This is the load-bearing
    change: at rrf_k=10 with the old weights untouched the sources already
    interleave (Local#1, Local#2, EverOS#1, Hub#1, Local#3).
  • weights.local 1.0 -> 0.96 fine-tunes slot order. It narrows the
    source-weight gap from 17.6% to 12.9%, which moves EverOS#1 ahead of Local#2.
    It does not remove a tier.

Resulting order with two hits per source, asserted in
test_shipped_defaults_interleave_the_three_sources:

before (rrf_k=60, local=1.0):   Local#1  Local#2   Local#3  Local#4  Local#5
after  (rrf_k=10, local=0.96):  Local#1  EverOS#1  Local#2  Hub#1    EverOS#2

Lowering rrf_k also brings the cross-source agreement bonus back toward
proportionate, though it does not fully fix it. At rrf_k=60 a pair of rank-10
hits from EverOS+Hub scored (0.9+0.85)/70 = 0.025 against Local rank-1 at
1.0/61 = 0.0164, a 1.52x margin -- two weak agreements beat one strong single
hit. At rrf_k=10 the same pair scores 1.75/20 = 0.0875 against Local rank-1 at
0.96/11 = 0.08727, so the margin collapses to 1.003x. The pair still edges
ahead; it no longer dominates. Sizing that bonus deliberately is left as
follow-up work.

No retrieval-quality data backs 10 over 60. The change is motivated by the
strict-tiering behaviour above contradicting the stated design intent, not by a
measured improvement. Reviewers who prefer the old ordering can restore it with
{"skillForge": {"router": {"rrfK": 60, "weights": {"local": 1.0}}}} -- the
point of the change is that the value is reachable at all.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

uv run pytest tests/ --ignore=tests/integration
  -> 2 failed, 5896 passed, 45 skipped
     Both failures reproduce unchanged on origin/main:
       tests/test_cli_theme.py::test_bold_accent_renders_styled_not_bare
       tests/test_read_file_image.py::test_an_attachment_that_cannot_be_read_costs_a_note_not_the_turn
     (origin/main baseline for the same command: 4 failed, 5888 passed -- the two
      extra are tests/test_provider_rates.py, order-dependent, green in isolation.)

uv run ruff check raven/ tests/          -> All checks passed!
uv run ruff format --check raven/ tests/ -> 814 files already formatted

Every new test was mutation-checked, so none of them pass vacuously:

Mutation Test that reddens
drop rrf_k=self._rrf_k in router.py test_rrf_k_reaches_the_fusion
drop rrf_k=... in factory.py test_rrf_k_forwarded_from_config, test_rrf_k_defaults_to_config_default
weights.local back to 1.0 test_shipped_defaults_interleave_the_three_sources
rrf_k default back to 60 test_shipped_defaults_interleave_the_three_sources

Without the first two a dropped hop would silently fall back to the module
default, leaving the config looking effective while doing nothing.

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

Risk

This changes default behaviour, not just the availability of a knob. Any
deployment running more than one skill source gets a different ordering after
this lands. Single-source deployments are unaffected: with one source RRF is
order-preserving, so neither rrf_k nor the weights change anything.

Installs that have already onboarded get only half of it.
init_extension_block_defaults writes router.weights with setdefault, so an
existing config.json keeps local: 1.0 permanently and has no rrfK key at
all. Those installs pick up rrf_k=10 from the code default on top of the old
weights, landing on Local#1, Local#2, EverOS#1, Hub#1, Local#3 rather than the
after-table order. Seeding rrfK during onboarding is left as follow-up
(over_fetch_factor, dedup_by and top_k are unseeded too, so this matches
existing practice).

rrf_merge_weighted and SkillForgeRouter both accept rrf_k=None meaning "use
the module default", so existing library callers are untouched. No consumer reads
rrf_score as an absolute value, so the ~5.5x magnitude shift crosses no
threshold.

Rollback is config-only: set skillForge.router.rrfK back to 60 and
weights.local back to 1.0. No code redeploy required.

docs/architecture/skill_hub_retrieval.svg still renders the formula with the
old constant. Left untouched because repository policy excludes SVG assets from
commits; it needs regenerating separately.

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Related Issues

N/A

RRF_K was a module constant at 60, the value classic RRF uses for
TREC-scale runs of ~1000 hits. Skill sources return ~10 hits, where 60
flattens the whole rank ladder to a 15% score spread -- narrower than
the 1.0/0.85 gap between source weights. Weight then decides the order
outright and a source's internal rank stops mattering: with the shipped
settings no EverOS or Hub hit could ever reach the output unless the
same skill also surfaced from Local, which contradicts the weighted
blend the config docstring describes.

Expose it as skill_forge.router.rrf_k and lower the default to 10. That
restores the ladder to an 82% spread, so each source's top hit competes
on rank, and it keeps the cross-source agreement bonus proportionate:
at 60 two rank-10 hits outscored a single rank-1 hit, at 10 they do not.
Local's weight tightens to 0.96 so the three sources interleave rather
than tier.

No retrieval-quality data backs 10 over 60. The change is motivated by
the strict-tiering behaviour above, not by a measured improvement.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
@ypflll
ypflll requested review from 0xKT August 11, 2026 06:19
@0xKT

0xKT commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Reviewed 3585376 read-only; tests were run in an isolated worktree, this repo's
tree was not touched.

No blocking findings: no correctness defect, all 9 checks green, and both
library entry points still accept rrf_k=None meaning "module default", so
existing callers are untouched. Everything below is a doc/wording fix or a
follow-up.

Tiers: [before-merge] fix in this PR - [describe] wording only, but must land
before the squash - [follow-up] separate PR - [nit] take or leave.

Verified sound, so nobody redoes it: the strict-tiering premise holds (at k=60,
Local rank-10 = 1/70 = 0.014286 beats Hub rank-1 = 0.85/61 = 0.013934, and
over-fetch 10 >= output 5, so a disjoint run really does return Local's own
top-5); the 15% / 82% / 17.6% figures are self-consistent under the
(top-bottom)/bottom reading; the after-table arithmetic checks out
(.087273 / .081818 / .08 / .077273 / .075); "only the ratios matter" and
"single source is order-preserving" are both true; no consumer in raven/,
ui-tui/ or bridge/ reads rrf_score as an absolute value, so the ~5.5x magnitude
jump breaks no threshold; and both mutation claims reproduce exactly as
described (dropping rrf_k=self._rrf_k reddens test_rrf_k_reaches_the_fusion,
dropping the factory argument reddens both factory tests: 3 failed, 39 passed).

R1 [before-merge] Stale formula in the file this PR is about
File: raven/memory_engine/skill_forge/fusion.py:11
Problem: the module docstring still says with k = 60 (the long-standing RRF constant), 32 lines above the new RRF_K: int = 10. The same file now
also uses k in two senses -- the docstring formula's k is the damping
constant, the function parameter k is the output cap -- which is the exact
confusion the new rrf_k arg doc warns about.
Fix: rename the formula variable and restate the default, e.g.
rrf_score(d) = sum_i w_i / (rrf_k + rank_i(d)), with rrf_k defaulting to
10 and overridable via skillForge.router.rrfK.
Verify: read the docstring end to end; no test needed.

R2 [describe] The description's numbers invert under the weights this PR ships
File: commit body + PR description
Problem: "at 60 two rank-10 hits (0.025) outscored a single rank-1 hit
(0.0164), at 10 they no longer do" is false for the pair it cites. 0.025 is
(0.9+0.85)/70, i.e. EverOS+Hub. At rrf_k=10 that pair is 1.75/20 = 0.0875,
which still beats Local rank-1 at 0.96/11 = 0.0872727. The claim only holds
against the old local=1.0 (1/11 = 0.0909) -- and it is this PR's own drop of
local to 0.96 that falsifies it. The conclusion survives, the assertion does
not: the margin collapses from 1.53x to ~1.003x rather than flipping.
Two smaller overstatements in the same paragraph:
- "Raising the source weights instead would have fixed the interleaving ...
so the constant is the right knob" -- lowering local 1.0 -> 0.96 is the
weight knob (the gap narrows 17.6% -> 12.9%), and at that new gap even
k=60 no longer strictly tiers (0.96/70 = 0.013714 < 0.85/61 = 0.013934).
Both knobs were turned; the paragraph argues for one.
- "local tightens to 0.96 so the three sources interleave rather than tier"
-- rrf_k=10 with local=1.0 already interleaves (L1, L2, E1, H1, L3). The
0.96 reorders slots 2-3; it does not remove a tier.
Fix: restate as a margin collapse, and call the weight tweak a fine-tune of
slot order rather than the tier fix. Code needs no change. Timing matters
because the squash body is taken from this description, so this is the
permanent record of the rationale.
Verify: recompute 1.75/20 vs 0.96/11.

R3 [describe] + [follow-up] Existing installs get only half the change
File: raven/config/update.py:311 (init_extension_block_defaults) vs the Risk
section
Problem: onboarding writes weights into ~/.raven/config.json with
setdefault, so any install that has already onboarded keeps local=1.0
permanently. Those deployments get rrf_k=10 on top of the old weights, i.e.
L1, L2, E1, H1, L3 -- not the after-table ordering. Checked against a real
config: weights = {"everos": 0.9, "hub": 0.85, "local": 1.0}, no rrfK key. So
"Rollback: set weights.local back to 1.0" is a no-op for them; they were never
moved off 1.0.
Fix [describe]: say in Risk that already-onboarded installs keep
weights.local=1.0 and only the rrf_k half applies to them.
Fix [follow-up, optional]: seed the new field too --
router.setdefault("rrfK", router_defaults.rrf_k) plus the matching
assertion in tests/test_config_update.py. Leaving it unseeded is defensible
(over_fetch_factor / dedup_by / top_k are unseeded as well), but the new
weights docstring tells the reader to "read them together with rrf_k" while
config.json shows only one of the two.
Verify: jq '.skillForge.router' ~/.raven/config.json on any install that
has run onboard.

R4 [before-merge] Canonical docs still carry the old weight
File: CONTEXT.md:417 -- "weight 0.85, below Local 1.0 and Everos 0.9"
Fix: 0.96. The same stale value sits in
docs/architecture/skill_hub_retrieval.svg:87 ("score = sum w_i / (60 +
rank_i)") -- your call whether that asset gets touched.
Verify: git grep -n "Local 1.0" CONTEXT.md returns nothing.

R5 [follow-up] The headline behaviour claim has no test
File: tests/test_skill_router_fusion.py
Problem: test_rrf_k_override_steepens_rank_ladder uses hand-written 1.0/0.85
weights, so the after-table ordering under the shipped defaults
(0.96/0.9/0.85 + rrf_k=10) -- the one behavioural claim this PR makes -- lives
only in prose. Changing a weight does redden test_skill_router_defaults, but
that is a value assertion: updating it is a mechanical one-liner that gives no
signal the interleave moved. For the record the interleave holds for local in
(0.927, 0.982), so 0.96 is mid-band, not fragile -- this is about making the
claim executable, not about brittleness.
Fix: one test that builds SkillForgeRouterConfig(), feeds two hits per source,
and asserts [Local#1, EverOS#1, Local#2, Hub#1, EverOS#2].
Verify: the new test fails if weights.local is raised to 1.0.

N1 [nit] The commit type is refactor while the Risk section states this
changes default behaviour. Versions here are bumped by hand so semver is
unaffected, but a reader scanning release notes for "why did my skill ordering
change" will not look under refactor -- fix or feat fits better, and the diff
spans memory_engine / config / context_engine so (*) is available as the
scope.

The module docstring in fusion.py still stated the formula with k = 60
and reused the name k for the damping constant, which now collides with
the output-cap argument of the same name. CONTEXT.md still described
Local's source weight as 1.0.

Also assert the ordering the shipped config produces instead of only
describing it: the interleave held solely in prose, and the value
assertion in test_skill_router_defaults gives no signal that slot order
moved.

docs/architecture/skill_hub_retrieval.svg carries the same stale
formula. Left untouched: repository policy excludes SVG assets from
commits.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
@ypflll ypflll changed the title refactor(memory_engine): make the RRF damping constant configurable feat(*): expose the RRF damping constant and rebalance skill source fusion Aug 11, 2026
@ypflll ypflll closed this Aug 11, 2026
@ypflll

ypflll commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

All six addressed.

R1 fusion.py module docstring now writes the formula with rrf_k and flags
the collision with the output-cap argument of the same name.
R2 Confirmed -- the claim was false under this PR's own local=0.96:
pair@10 = 1.75/20 = 0.0875 still edges Local#1 at 0.96/11 = 0.08727.
Restated as a margin collapse 1.52x -> 1.003x, and the two knobs are
now split: rrf_k removes the tiering, weights.local only reorders
slots 2-3.
R3 Documented in Risk. Seeding rrfK during onboarding left as follow-up.
R4 CONTEXT.md updated to 0.96. The SVG is left stale on purpose --
repository policy excludes SVG assets from commits.
R5 Added test_shipped_defaults_interleave_the_three_sources. Mutation-
checked: reddens if weights.local goes back to 1.0 or rrf_k back to 60.
N1 Retitled to feat(*).

Thanks for recomputing the numbers -- R2 was a real error in the permanent
record, since the squash body comes from the description.

@ypflll ypflll reopened this Aug 11, 2026
@0xKT

0xKT commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Close-out: all six addressed. Re-verified independently on 2cc24c3 rather than
taking the summary on trust.

  • R1 / R4 read back: the fusion.py module docstring now writes the formula with
    rrf_k and flags the collision with the output-cap k; CONTEXT.md reads
    0.96.
  • R5: test_shipped_defaults_interleave_the_three_sources pins the shipped
    config, and both mutations really do change the order it asserts --
    weights.local back to 1.0 gives Local#1, Local#2, EverOS#1, Hub#1, EverOS#2,
    and rrf_k back to 60 gives Local#1, Local#2, EverOS#1, EverOS#2, Hub#1.
  • R2 / R3: every number in the rewritten description recomputes -- 1.52x
    (0.025 / 0.016393), 1.003x (0.0875 / 0.087273), the weight gap 17.6% -> 12.9%,
    the old-weights interleave Local#1, Local#2, EverOS#1, Hub#1, Local#3, and the
    ~5.5x magnitude shift.
  • Verification section reproduced locally: ruff format --check raven/ tests/
    -> 814 files already formatted. The description is pure ASCII (0 matches for
    [^\x00-\x7F]), so the squash body is clean, and the prospective squash header
    feat(*): ... (#290) is 81 chars against commitlint's limit of 100.
  • N1: title and Type box now agree, and (*) is right for a diff spanning
    memory_engine / config / context_engine.

The two follow-ups logged in the description are not blocking: seeding rrfK
during onboarding, and sizing the cross-source agreement bonus deliberately --
at the shipped defaults a rank-10 pair still edges a single rank-1 hit by 0.3%.

@0xKT 0xKT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved on 2cc24c3 -- see the close-out comment above. Not merging; leaving the squash to the maintainer.

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