Skip to content

KS78: Relative supersession demotion — multiplicative 0.40 (#11)#22

Merged
Liorrr merged 2 commits intomasterfrom
fix/ks78-supersession-demotion
Apr 10, 2026
Merged

KS78: Relative supersession demotion — multiplicative 0.40 (#11)#22
Liorrr merged 2 commits intomasterfrom
fix/ks78-supersession-demotion

Conversation

@Liorrr
Copy link
Copy Markdown
Contributor

@Liorrr Liorrr commented Apr 10, 2026

Summary

  • Changed supersession demotion from absolute (score -= 0.15) to multiplicative (score *= 0.6^count)
  • Default supersedes_demotion changed: 0.150.40 (retain 60% of score per supersession edge)
  • Combines direct Supersedes edges + parent supersession into one unified factor
  • With old math: 1.027 - 0.15 = 0.877 (sometimes still > new memory). With new math: 1.027 * 0.6 = 0.616 (always below)

Root cause

Absolute demotion -0.15 insufficient when old memory has >0.15 higher cosine similarity than the new memory. Old superseded facts would still rank above their replacements.

Changes

  • crates/shrimpk-core/src/config.rs: Updated default + doc comments (+3/-3)
  • crates/shrimpk-memory/src/echo.rs: Multiplicative scoring + test updates (+31/-25)

Test plan

  • supersession_multiplicative_demotion_closes_gap passes
  • supersession_demotion_no_op_without_superseded_child passes
  • cargo test -p shrimpk-memory — all non-ignored tests pass
  • cargo check --workspace — clean

Closes #11

🤖 Generated with Claude Code

- Change supersession demotion from absolute penalty to multiplicative
  factor: score *= (1 - factor)^count instead of score -= constant
- Default changed from 0.15 (absolute) to 0.40 (multiplicative, retains 60%)
- Combines direct Supersedes edges and parent supersession into one factor
- Fixes case where old memory with high cosine sim still outranked new
  memory despite being superseded (e.g., 0.85 - 0.15 = 0.70 > 0.65)
- Updated tests to verify multiplicative math

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 10, 2026

Greptile Summary

This PR replaces the absolute supersession demotion (score -= 0.15) with a multiplicative scheme (score *= (1 - factor)^count, factor = 0.40) so that a superseded memory can never float above its replacement regardless of the similarity gap. The change also unifies direct Supersedes edges and parent-level supersession into a single compounding factor, and updates the default from 0.15 to 0.40.

Key changes:

  • crates/shrimpk-core/src/config.rs: Default value updated to 0.40, doc comment revised to describe multiplicative semantics.
  • crates/shrimpk-memory/src/echo.rs: hebbian_boosts now carries (f64, u32) instead of f64; parent_demotions carries a u32 count; both are combined into total_superseded before applying retain.powi(total_superseded).
  • Tests renamed and rewritten to verify the new math; both pass.

One issue found: #[serde(default)] on supersedes_demotion resolves to f32::default() = 0.0, not 0.40. Users whose config files omit this field will silently get no demotion. A default_supersedes_demotion() helper (matching the pattern used by recency_weight and child_rescue_only) is needed to make the advertised default take effect during deserialization.

Confidence Score: 4/5

Safe to merge after adding a default_supersedes_demotion() serde helper; core scoring math and tests are correct.

The multiplicative demotion logic in echo.rs is correct — direction conventions, compounding math, and test coverage all check out. The single blocking issue is in config.rs: the #[serde(default)] annotation silently uses 0.0 instead of 0.40 for partial config files, meaning the new default won't take effect in the most common deployment scenario. The fix is a two-line addition (a named default function) identical to the pattern already used for recency_weight. Once that's applied the PR is ready.

crates/shrimpk-core/src/config.rs — serde default annotation needs #[serde(default = "default_supersedes_demotion")].

Important Files Changed

Filename Overview
crates/shrimpk-core/src/config.rs Default value updated from 0.15 to 0.40 and doc comment revised for multiplicative semantics, but #[serde(default)] still resolves to f32::default() = 0.0 for partial config files — defeating the new default for most real-world deployments.
crates/shrimpk-memory/src/echo.rs Cleanly converts absolute additive demotion to multiplicative retain.powi(count) form; unifies direct Supersedes edges and parent supersession into a single compound factor; logic, direction conventions, and tests are correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Top-K candidates\n(idx, cosine_score)"] --> B["Hebbian pass\ncompute boost + superseded_count\nper direct Supersedes edge"]
    A --> C["Parent demotion pass\ncheck children for Supersedes edges\nstore count=1 if any found"]
    B --> D["total_superseded =\ndirect_superseded_count\n+ parent_demotions.get(idx)"]
    C --> D
    D --> E{"total_superseded > 0?"}
    E -- Yes --> F["retain = 1.0 - supersedes_demotion\nfinal_score *= retain.powi(total_superseded)"]
    E -- No --> G["final_score unchanged"]
    F --> H["Continue scoring pipeline\n(child penalty, confidence, soft-invalidation)"]
    G --> H
Loading

Reviews (2): Last reviewed commit: "fix: declare superseded_count in both te..." | Re-trigger Greptile

The hebbian_boosts closure referenced `superseded_count` but only declared
`demotion: f64` — renamed variable + fixed Vec<f64> to Vec<(f64, u32)> to
match downstream destructuring at line 1507.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines 244 to 245
#[serde(default)]
pub supersedes_demotion: f32,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Serde default mismatch — 0.40 only takes effect when no config file exists

#[serde(default)] resolves to f32::default() = 0.0, not 0.40. The struct Default impl (line 481) correctly sets 0.40, but that only applies when EchoConfig::default() is called programmatically (i.e. no config file). Any user with a config file that omits supersedes_demotion will get 0.0 from serde — meaning no multiplicative demotion at all, silently defeating the entire purpose of this PR.

Compare how recency_weight is handled: it uses #[serde(default = "default_recency_weight")] backed by a named function. supersedes_demotion needs the same treatment.

fn default_supersedes_demotion() -> f32 {
    0.40
}

Then update the field annotation:

/// Supersession demotion factor (multiplicative). 0.40 = retain 60% of score.
/// Applied as `score *= (1 - factor)^count` for each supersession edge.
#[serde(default = "default_supersedes_demotion")]
pub supersedes_demotion: f32,

This aligns serde deserialization with the struct Default impl so partial config files get 0.40 instead of 0.0.

@Liorrr Liorrr merged commit eb0f7b2 into master Apr 10, 2026
7 checks passed
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.

Supersession demotion too weak: absolute penalty insufficient when old memory has higher raw cosine similarity

1 participant