Skip to content

refactor(memory/v2): unify skills into concept-page activation pool#29619

Merged
siddseethepalli merged 1 commit into
mainfrom
do/unify-skills-into-concept-pool
May 5, 2026
Merged

refactor(memory/v2): unify skills into concept-page activation pool#29619
siddseethepalli merged 1 commit into
mainfrom
do/unify-skills-into-concept-pool

Conversation

@siddseethepalli
Copy link
Copy Markdown
Contributor

@siddseethepalli siddseethepalli commented May 5, 2026

Summary

  • Skills are indexed into the unified memory_v2_concept_pages Qdrant collection under skills/<id> and flow through simBatch / computeOwnActivation / selectInjections exactly like concept pages — same decay, same spread, same top-K, same everInjected dedup.
  • simSkillBatch, computeSkillActivation, selectSkillInjections, the memory_v2_skills collection, and the top_k_skills config are removed; top_k defaults to 25 to preserve the prior 20 concepts + 5 skills budget.
  • The activation-log telemetry shape collapses to a single concepts array — skill rows appear with the skills/ slug prefix, so the inspector compares the same scoring axis end-to-end.

Original prompt

it


Open in Devin Review

Skills no longer live in a separate Qdrant collection with a parallel
activation pipeline. They are indexed into `memory_v2_concept_pages`
under the slug prefix `skills/<id>` and flow through the same
`simBatch` / `computeOwnActivation` / `selectInjections` pipeline as
concept pages, with the same decay carry-over, top-K, and everInjected
deduplication.

The render path branches by slug prefix to keep the prompt format
intact: skill slugs surface under `### Skills You Can Use`. The
inspector activation log collapses concepts and skills into a single
ranked array — the comparison is now apples-to-apples on the same
scoring axis.

`simSkillBatch`, `computeSkillActivation`, `selectSkillInjections`,
the `memory_v2_skills` collection, and `top_k_skills` are gone.
`top_k` defaults to 25 to preserve the prior 20 + 5 prompt budget.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@siddseethepalli siddseethepalli merged commit 2f8ef73 into main May 5, 2026
@siddseethepalli siddseethepalli deleted the do/unify-skills-into-concept-pool branch May 5, 2026 07:26
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5cb2b544b1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread assistant/src/memory/v2/skill-store.ts
Comment thread assistant/src/memory/v2/injection.ts
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 6 additional findings.

Open in Devin Review

siddseethepalli added a commit that referenced this pull request May 5, 2026
* refactor(memory/v2): unify rerank pool by pre-rerank A_o and apply boost additively

Cross-encoder rerank now selects its top-K from the unified pre-rerank-A_o
pool instead of running per-channel on top-K-by-fused-sim. Boost is added to
A_o weighted by c_user / c_assistant rather than folded into sim_u / sim_a,
so a slug strong in both channels can no longer crowd out single-channel
hits via a doubled per-channel boost.

simBatch loses its useRerank/rerankBoost options and becomes pure
dense+sparse fusion. computeOwnActivation owns the rerank step end-to-end:
it computes pre-rerank A_o per candidate, picks top-K, runs the
cross-encoder once per channel against that unified set, and adds
c_user·α·r_norm_u + c_assistant·α·r_norm_a to the final A_o. The macOS
inspector now renders rerank Δ_u / Δ_a as standalone additive A_o rows.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(memory/v2): unify rerank pool by pre-rerank A_o and apply boost additively

Cross-encoder rerank now selects its top-K from the unified pre-rerank-A_o
pool instead of running per-channel on top-K-by-fused-sim. Boost is added to
A_o weighted by c_user / c_assistant rather than folded into sim_u / sim_a,
so a slug strong in both channels can no longer crowd out single-channel
hits via a doubled per-channel boost.

simBatch loses its useRerank/rerankBoost options and becomes pure
dense+sparse fusion. computeOwnActivation owns the rerank step end-to-end:
it computes pre-rerank A_o per candidate, picks top-K, runs the
cross-encoder once per channel against that unified set, and adds
c_user·α·r_norm_u + c_assistant·α·r_norm_a to the final A_o. The macOS
inspector now renders rerank Δ_u / Δ_a as standalone additive A_o rows.

Also drops stale `skills` references from the concept-frequency test —
unblock the type-check that breaks on main after #29619 unified skills
into the concept pool.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Vellum Assistant <assistant@vellum.ai>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
siddseethepalli added a commit that referenced this pull request May 9, 2026
…ed on cache miss (#30106)

Address codex P1 review feedback on #29619:

1. Tag skill-seeded Qdrant points with payload.kind="skill" and have
   pruneSlugsWithPrefixExcept filter by that kind. validateSlug permits
   user-authored concept pages slugged `skills/foo`, which previously
   collided with the skill namespace and were repeatedly deleted by every
   seed run.

2. Exclude skill slugs whose in-process cache entry is missing from
   everInjected. Otherwise a startup race (Qdrant row present before the
   skill cache is seeded) marks the slug injected even though
   renderInjectionBlock silently dropped it, and per-turn deltas never
   re-attempt the attach until compaction.

Co-authored-by: Vellum Assistant <assistant@vellum.ai>
@siddseethepalli
Copy link
Copy Markdown
Contributor Author

Addressed in #30106

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.

1 participant