Skip to content

feat(skillrep): add the SkillRep retrieval code - #38

Merged
ypflll merged 7 commits into
mainfrom
feat/skillrep-code
Oct 23, 2026
Merged

ypflll merged 7 commits into
mainfrom
feat/skillrep-code

Conversation

@Tian-yi-Sun

Copy link
Copy Markdown
Collaborator

Summary

Adds skillrep/, the code for SkillRep, a skill retriever built from five representation choices (encoder fine-tuning, query-side and document-side LLM expansion, background projection, and a BM25 lexical channel), plus the analyses that measure the request–skill gap through three per-request quantities (background, unsaid vocabulary, dilution).

  • skillrep/skillrep/: library (data formats, encoder, generation prompts, sparse BM25, background projection, fusion, pipeline, metrics, measures)
  • skillrep/scripts/: generate expansions, encode, run SkillRep, ablation
  • skillrep/analysis/: the analyses of the paper's Sections 6–7 and appendices
  • skillrep/train/: encoder fine-tuning (request synthesis, hard negatives, InfoNCE training)
  • skillrep/baselines/README.md: how each baseline was run

Code only; no datasets, generated texts or weights. The folder is self-contained (its own pyproject.toml, package skillrep) and is not picked up by the root package build (include = ["skillcorpus", "skillcorpus.*"]).

Checks

  • scripts/check_repo_assets.py, scripts/check_file_sizes.py, scripts/check_project_inventory.py: pass locally
  • All files compile; no non-ASCII characters, internal paths or credentials
  • On the authors' cached inputs, the release reproduces the paper's main results (UniPool: R@10 89.59 / MRR@10 76.46 / Hit@1 67.18)

🤖 Generated with Claude Code

Adds skillrep/, the code for SkillRep, a skill retriever that combines encoder
fine-tuning, query-side and document-side expansion, background projection and
a lexical channel, together with the analyses of the request-skill gap
(background, unsaid vocabulary, dilution). Code only; no data or weights.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Tian-yi-Sun
Tian-yi-Sun requested a review from ypflll as a code owner October 21, 2026 13:31

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocking: the gradient-accumulation loop must handle the final partial group at each epoch boundary.

I reviewed the full origin/main...HEAD diff and the surrounding repository guidance, public entry points, cross-file data/embedding contracts, training and analysis callers, packaging/repository checks, and recent history. I also checked backward compatibility and found the addition self-contained; no existing tests were weakened. The repository gates and current GitHub checks pass, as do git diff --check, static compilation, file-size, inventory, and asset checks. Runtime tests could not start in this checkout because its Python environment lacks both torch and pytest; importantly, the current CI only tests the root package and does not execute the new skillrep code.

I found one concrete training correctness issue inline. Once that is fixed and covered for a non-divisible loader length, I do not see another merge blocker.

Comment thread skillrep/train/train_encoder.py Outdated
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocking: the gradient-accumulation loop must handle the final partial group at each epoch boundary.

I re-read the full current diff and the 83b2c0b51924..6d604c24b903 delta, including repository guidance, affected callers/history, backward compatibility, and test changes. The new commit only updates paper-table references; those references are consistent, no tests were weakened, and it introduces no new finding. It does not modify train_encoder.py, so the existing open accumulation finding still applies unchanged and remains the sole blocker.

Static compilation, git diff --check, and the repository file-size, inventory, and asset checks pass. GitHub's core/package, plugin, release, and CodeQL checks pass; the repository-gates job is still pending. Local pytest runs could not start because this checkout's Python environment has no pytest installed.

Docstrings, printed headers and the READMEs now name each result by its
content rather than by table, section, figure or equation number, so they
stay valid when the paper is renumbered.

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

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocking: the gradient-accumulation loop must handle the final partial group at each epoch boundary.

I reviewed the full current diff and the 6d604c24b903..38dee97bac33 delta, including repository guidance, callers/history, backward compatibility, and whether tests were weakened. This commit only replaces paper-number citations with descriptive labels; the edits are internally consistent and introduce no new finding. The training code is unchanged, however, so my existing open finding remains the sole blocker.

git diff --check, static compilation, and the repository file-size, inventory, and asset checks pass. All current GitHub checks pass. Local pytest still cannot start because this checkout's Python environment lacks pytest.

The examples now load the fine-tuned encoder from the Hugging Face Hub
(EverMind-AI/skillrep-embedding-0.6b) and take the chat completions URL as a
placeholder.

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

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No blockers; suggestions only, and they are marked inline.

I reviewed the full current diff and the 38dee97bac33..fd730e605693 delta, including repository guidance, affected callers/history, backward compatibility, and test changes. The new documentation consistently links the released encoder, and the Hugging Face URL returns HTTP 200. No tests were weakened and I found no new defect.

The partial gradient-accumulation issue remains a named nonblocking follow-up. In this fourth round it does not satisfy the required no-working-way-out condition: operators can use --grad_accum_steps 1 or a loader length divisible by the accumulation factor. I posted a closing reply withdrawing the blocker. I also attempted to resolve that thread immediately, but GitHub rejected the GraphQL mutation with FORBIDDEN for this authenticated account, so the thread remains mechanically open despite the explicit withdrawal.

git diff --check, static compilation, repository checks, and all current GitHub checks pass. Local pytest could not start because this environment lacks pytest.

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

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No blockers; suggestions only, and they are marked inline.

I reviewed the full current diff and the fd730e605693..0ec809c566af delta, covering repository guidance, callers/history, backward compatibility, and test changes. This commit only removes the baseline-reproduction notes and their README references; I found no remaining references or new defect, and no tests were weakened. The previously named gradient-accumulation follow-up remains nonblocking.

git diff --check, static compilation, repository checks, and all current GitHub checks pass. Local pytest could not start because this environment lacks pytest. I re-attempted cleanup of the withdrawn thread, but GitHub again returned FORBIDDEN for the authenticated account.

ypflll
ypflll previously approved these changes Oct 22, 2026
- Treat the last micro-batch of each epoch as an accumulation boundary and
  divide by the actual group size, so a loader length that is not a multiple
  of --grad_accum_steps no longer drops the tail or rescales it.
- Count updates as ceil(len(loader) / grad_accum_steps) per epoch, so the
  cosine schedule ends at the last update, and stop all epochs at --max_steps.
- Pass --seed to DistributedSampler, so changing the seed changes the data
  order under torchrun.

Checked on a 10-triplet set with batch 1 and 4 accumulation steps: 3 updates
per epoch (4 + 4 + 2 micro-batches), with the learning rate reaching 0 at the
last update for 1 and 2 epochs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The probe kept only shared words below the vocabulary's median IDF, which
on a 111K-skill library excludes just the words found in one or two skills.
Delete every shared word instead, as the analysis describes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll self-requested a review October 23, 2026 03:00
@ypflll
ypflll merged commit f9f303f into main Oct 23, 2026
22 checks passed
@ypflll
ypflll deleted the feat/skillrep-code branch October 23, 2026 03:01
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.

3 participants