Skip to content

Comments

fix: skill search ranking - use overview for embedding and fix visited set filtering#228

Merged
MaojiaSheng merged 1 commit intovolcengine:mainfrom
ZaynJarvis:fix/skill-embedding-ranking
Feb 20, 2026
Merged

fix: skill search ranking - use overview for embedding and fix visited set filtering#228
MaojiaSheng merged 1 commit intovolcengine:mainfrom
ZaynJarvis:fix/skill-embedding-ranking

Conversation

@ZaynJarvis
Copy link
Collaborator

@ZaynJarvis ZaynJarvis commented Feb 20, 2026

Problem

After add-skill, ov search returned incorrect/incomplete skill rankings:

Root Causes

1. Poor embedding text for skills (skill_processor.py)

What: The vectorization text for skills was set to just the short description field from SKILL.md frontmatter — typically one sentence. The LLM-generated overview (a rich multi-paragraph summary) was generated after the vectorize text was set and never used for embedding.

Compare with resources: semantic_processor.py correctly uses overview text for directory vectorization and full content/summary for files. Skills were the only context type embedding with just the short abstract.

Fix: Generate overview first, then use it as the vectorization text — aligning skills with how resources handle directory vectorization.

2. Visited set drops the most relevant results (hierarchical_retriever.py)

What: The _recursive_search method conflated two concepts: "visited for directory traversal" and "collected as a search result." This caused the highest-scoring results to be systematically dropped.

Detailed flow (before fix):

The retriever works in stages:

  1. Global vector search finds top-3 non-leaf entries across the entire collection. For a query like "adding memory" against skills, this returns e.g. adding-memory (0.5), searching-context (0.3), openviking (0.2).

  2. Merge starting points combines these with root URIs. Priority queue becomes: [(adding-memory, 0.5), (searching-context, 0.3), (openviking, 0.2), (viking://agent/skills, 0.0)]

  3. Recursive search pops highest-score first:

    • Pops adding-memory (0.5) → adds to visited set → searches for children with parent_uri=adding-memory0 results (skills have no children)
    • Pops searching-context (0.3) → adds to visited → 0 children
    • Pops openviking (0.2) → adds to visited → 0 children
    • Pops viking://agent/skills (0.0) → searches children → finds all 4 skills
    • Old code: if passes_threshold(score) and uri not in visited — three skills are in visited, so they are silently skipped
    • Only adding-resource (not in global top-3) gets collected

When it happens: Any time global search returns URIs that are also children of a root/parent directory. This affects all context types (skills, resources, memories) since they share HierarchicalRetriever. Ironically, the most relevant results (highest global scores → processed earliest → visited first) are the ones most likely to be dropped.

Fix: Separate "visited for traversal" from "collected as result." The visited set now only prevents re-entering directories for child search. Results that pass the score threshold are always collected, regardless of visited status.

Testing

Before fix:

ov search "adding memory" -u viking://agent/skills  →  searching-context (wrong, only 1 result)
ov search "search context" -u viking://agent/skills  →  adding-memory (wrong, only 1 result)
ov search "RAG semantic search" -u viking://agent/skills  →  adding-memory (wrong, only 1 result)

After fix:

ov search "adding memory" -u viking://agent/skills
  → adding-memory #1 (0.508) ✓
  → adding-resource #2 (0.241)
  → openviking #3 (0.222)
  → searching-context #4 (0.185)

ov search "search context" -u viking://agent/skills
  → searching-context #1 (0.500) ✓
  → openviking #2 (0.323)
  → adding-resource #3 (0.190)
  → adding-memory #4 (0.163)

ov search "RAG semantic search" -u viking://agent/skills
  → openviking #1 (0.372) ✓
  → searching-context #2 (0.343)
  → adding-resource #3 (0.259)
  → adding-memory #4 (0.141)

All 4 skills appear with correct ranking order.

@ZaynJarvis ZaynJarvis changed the title fix: skill search ranking - use overview for embedding and fix visited set filtering fix: hierarchical retriever visited set drops most relevant search results Feb 20, 2026
…d set filtering

1. skill_processor.py: Use LLM-generated overview for vectorization
   instead of short abstract, aligning with how resources handle
   directory vectorization in semantic_processor.py.

2. hierarchical_retriever.py: Separate 'visited for traversal' from
   'collected as result'. The visited set previously dropped the most
   relevant results - global search found them first, marked visited,
   then parent directory search skipped them as children.
@ZaynJarvis ZaynJarvis force-pushed the fix/skill-embedding-ranking branch from ee4532c to b7fe917 Compare February 20, 2026 04:06
@ZaynJarvis ZaynJarvis changed the title fix: hierarchical retriever visited set drops most relevant search results fix: skill search ranking - use overview for embedding and fix visited set filtering Feb 20, 2026
@MaojiaSheng
Copy link
Collaborator

lgtm

@MaojiaSheng MaojiaSheng merged commit 3f65a32 into volcengine:main Feb 20, 2026
4 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Feb 20, 2026
ZaynJarvis added a commit to ZaynJarvis/OpenViking that referenced this pull request Feb 20, 2026
…verview

Reverts the skill_processor embedding change from volcengine#228 while keeping
the retriever fix. Skills should embed using the frontmatter description
(abstract), not the LLM-generated overview.
MaojiaSheng pushed a commit that referenced this pull request Feb 20, 2026
…verview (#229)

Reverts the skill_processor embedding change from #228 while keeping
the retriever fix. Skills should embed using the frontmatter description
(abstract), not the LLM-generated overview.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants