Repository navigation
Port 'LLM example retrieval' from machine.py - #642
claude[bot] wants to merge 1 commit into
Conversation
Add TF-IDF and coverage example retrievers under Translation.Llm. Co-authored-by: Eli C. Lowry <83078660+Enkidu93@users.noreply.github.com> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #642 +/- ##
==========================================
+ Coverage 75.33% 75.47% +0.14%
==========================================
Files 454 461 +7
Lines 38509 38809 +300
Branches 5335 5389 +54
==========================================
+ Hits 29010 29293 +283
- Misses 8364 8373 +9
- Partials 1135 1143 +8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // Strip all punctuation-only tokens. | ||
| return _tokenizer | ||
| .Tokenize(text.ToLowerInvariant()) | ||
| .Where(token => token.Any(char.IsLetterOrDigit)) |
There was a problem hiding this comment.
Minor: F1. Supplementary-plane scripts lose all their tokens. char.IsLetterOrDigit tests one UTF-16 unit, and a surrogate is never a letter, so an Adlam (U+1E900) corpus indexes nothing: TfidfIndex.Create returns null and Rank returns the first k examples for any query. machine.py ranks them, because str.isalnum tests code points. Use char.IsLetterOrDigit(token, i) over code points. Unverified: reasoned from the .NET contract, not run.
| protected IReadOnlyList<Example> ExampleList => _examples; | ||
|
|
||
| /// <summary> | ||
| /// Builds the retrieval index. Each most-derived constructor must call this once its own state is set up. |
There was a problem hiding this comment.
Minor: F2. A subclass of the public, unsealed TfidfExampleRetriever or CoverageExampleRetriever cannot follow this contract. Their constructors already call Fit(), so an overriding Fit runs before the subclass constructor has assigned its fields, and calling Fit() again as documented builds the index twice. Seal the two concrete retrievers, or fit lazily on the first Rank. Sealing later would break callers. Unverified: follows from C# constructor order, not run.
| if (k <= 0) | ||
| return Array.Empty<Example>(); | ||
| if (excludeIndex.HasValue) | ||
| scores[excludeIndex.Value] = double.NegativeInfinity; |
There was a problem hiding this comment.
FYI: F3. This writes into the array that ScoreExamples returned. A subclass that caches its scores then ranks the excluded example last on every later query. machine.py copies first (scores = scores.copy() in top_k_example_retriever.py). The built-in retrievers return a fresh array, so only subclasses are affected.
|
Verdict: approve with fixes. This is a faithful port of sillsdev/machine.py#412, and the TF-IDF and coverage logic match the Python apart from the points below. Most important: F1, Counts: 0 Critical, 2 Important (F1, F2), 1 Low (F3). Ran: Not verified: F1 and F2 are not reproduced. The sandbox refused a scratch build, so both rest on the .NET char and constructor-order contracts. The char_wb n-gram difference for astral characters is disclosed in the PR body and is not raised. Public API: new public types in Upstream in sillsdev/machine.py, not this PR: Findings: F1 new, unverified; F2 new, unverified; F3 new. Reviewed at 0f60c7b |
Quick summary
Callers can now pick few-shot translation examples with a TF-IDF or a coverage retriever, ported from machine.py. Both are new types in
SIL.Machine.Translation.Llm, with no new package dependency. Nothing existing is touched.Where to look
TfidfIndexreimplements scikit-learn'sTfidfVectorizerdefaults (raw counts, smoothed idf, L2 norm) -- pinned by the ranking tests inTfidfExampleRetrieverTests.CoverageExampleRetrieverports the anchor and greedy selection, with char_wb 2-5 grams -- pinned byCoverageExampleRetrieverTests, including the duplicate and exclude-index cases.Deliberately not included
pyproject.toml,uv.lockand thellmextra: Python packaging only.__init__.pyand the sklearn availability check: no C# counterpart.Validation
./local_check.sh --agent-strict--local_check: PASSEDIssue / porting context
Ports sillsdev/machine.py#412.
Decisions, and why
Fit()at the end of their own constructor. Python calls it from the base constructor, but in C# derived fields are not yet initialized at that point.argsortandargpartitionleave tie order unspecified, so results can differ from Python only among equal scores.Exampleis a class with value equality, because the library does not use C# 9 records atnetstandard2.0.Closes #641
This change is