Filter stock-solution records out of the deep-research ranking (closes #124)#128
Merged
Merged
Conversation
…#124) The prioritizer documented a hard filter "category == solutions". That filter could never fire: `CategoryEnum` has no `solutions` member, so no record can carry the value. The ~4,784 MediaDive stock-solution records live in `bacterial/` stamped `category: bacterial` — because that is the directory they sit in — and 4,772 of them were scored and ranked as candidate media, 31% of the committed report. Researching "Solution B" can only ever spend credits for nothing: a stock solution has no organism association by construction. `category` cannot express this. The domain axis does not apply to a stock solution at all — SL10_elements is neither bacterial nor archaeal — so the fix is structural detection, not a restamp. (A restamp would also need a schema change to add the enum value.) - scripts/record_kinds.py — `is_solution_record()`, keyed on the `term.id` prefix. Deliberately not a shape heuristic on `composition:`/`ingredients:`, which would also catch malformed *media* records and silently drop them from the ranking. - validate_strict.py already had this rule inline to route these records to SolutionRecipe; it now imports the shared one so the validator and the prioritizer cannot drift. - prioritize_deep_research_candidates.py applies it as a hard filter and the module docstring no longer describes a filter that does not exist. Ranking: 15496 -> 10711 entries, zero solutions remaining. Removing the filter fails the corpus-level test. Does NOT move or restamp any record — the data-model fix (solutions belong in `solutions/`, which currently holds only an index JSON) is left to #124's follow-up, since moving 4,784 files is a much larger change than closing the budget leak. Closes #124. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
First regeneration since the reports became reproducible (#126), and the first that is honestly reviewable: with the tracked manifest as the only research-state input, re-running produces byte-identical output (verified). Three corrections land at once: - #126: excludes 21 already-researched media from the tracked manifest rather than from whatever was in one machine's gitignored research/ dir. - #120: picks up `category` for the 73 recategorized archaeal records. - #124: drops 4,772 stock-solution records that were being ranked as media. 15496 -> 10711 ranked entries. The top-100 is all `bacterial/`. Known limitation, NOT addressed here: the ranking contains heavy name-level duplication — 2240 recipe_names appear more than once, covering 5366 entries, and the top-100 holds only 86 distinct names (thermus_medium alone appears 12 times as distinct TOGO records). A batch run over the top-100 would research the same medium repeatedly. Filed separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes deep-research prioritization by ensuring MediaDive stock-solution records (which cannot have organism associations) are structurally detected and excluded from ranking, using a shared classification rule to prevent drift between the strict validator and prioritizer.
Changes:
- Added a shared
record_kinds.is_solution_record()helper and reused it in bothvalidate_strict.pyandprioritize_deep_research_candidates.py. - Updated the prioritizer to hard-filter solution records before scoring.
- Added tests to pin the schema premise (no
CategoryEnum.solutions) and assert consumer parity; regenerated committed priority reports.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_record_kinds.py |
Adds unit + corpus-level tests for solution detection and consumer parity. |
scripts/record_kinds.py |
Introduces the shared stock-solution vs medium classification rule. |
scripts/validate_strict.py |
Refactors target-class routing to use the shared rule. |
scripts/prioritize_deep_research_candidates.py |
Filters stock-solution records structurally during ranking collection and updates documentation. |
data/import_tracking/reports/deep_research_priority.md |
Regenerated report reflecting filtered ranking size/content. |
data/import_tracking/reports/deep_research_priority_top100.json |
Regenerated top-100 JSON reflecting filtered ranking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+112
to
+133
| def test_prioritizer_ranks_no_solution_records(): | ||
| """End-to-end: the ranking must contain zero stock solutions. | ||
|
|
||
| Before the fix this was 4,772 of 15,496 entries. | ||
| """ | ||
| rk_mod = _load("record_kinds") | ||
| pdrc = _load("prioritize_deep_research_candidates") | ||
|
|
||
| entries = pdrc.collect_records(set()) | ||
| assert entries, "expected a non-empty ranking" | ||
|
|
||
| normalized = REPO_ROOT / "data" / "normalized_yaml" | ||
| offenders = [] | ||
| for entry in entries: | ||
| path = normalized / entry["file_path"] | ||
| doc = pdrc.load_yaml(path) | ||
| if rk_mod.is_solution_record(doc): | ||
| offenders.append(entry["file_path"]) | ||
| if len(offenders) >= 5: | ||
| break | ||
| assert not offenders, f"stock solutions leaked into the ranking: {offenders}" | ||
|
|
Comment on lines
+63
to
+67
| Solution filter (#124): this used to be documented as "category == solutions", | ||
| which never fired — `CategoryEnum` has no `solutions` member, so no record can | ||
| carry that value. The ~4,784 MediaDive stock-solution records live in | ||
| `bacterial/` stamped `category: bacterial`, and 4,772 of them were being scored | ||
| and ranked as candidate media (31% of the committed report). They are now |
| from record_kinds import is_solution_record # noqa: E402 -- shared medium/solution rule | ||
|
|
||
|
|
||
| def infer_target_class(instance: dict) -> str: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
The prioritizer documented a hard filter:
That filter could never fire.
CategoryEnumhas nosolutionsmember — the permissible values are bacterial / fungal / archaea / specialized / algae / imported. No record can carry the value the filter tests for.So the ~4,784 MediaDive stock-solution records, which live in
bacterial/stampedcategory: bacterialbecause that is the directory they sit in, were scored and ranked as candidate media: 4,772 entries, 31% of the committed report. Researching "Solution B" can only ever spend credits for nothing — a stock solution has no organism association by construction.Why structural detection, not a restamp
The issue suggested restamping to
category: solutionsor moving tosolutions/. Restamping needs a schema change first, and more fundamentally the domain axis does not apply to a stock solution at all —SL10_elementsis neither bacterial nor archaeal. So the kind is read off structure.validate_strict.pyalready had exactly this rule inline, to route these records toSolutionRecipeinstead of false-failing them againstMediaRecipe. I lifted it toscripts/record_kinds.pyand pointed both consumers at it, so the validator and the prioritizer cannot drift.Keyed on the
term.idprefix rather than a shape heuristic oncomposition:/ingredients:— a shape check would also catch malformed media records and silently drop them from the ranking, which is the opposite of what we want.Result
Second commit regenerates the committed reports — the first regeneration since #126 made them reproducible, and the first that is honestly reviewable. Re-running produces byte-identical output (verified). It folds in three corrections at once: #126 (manifest-sourced exclusions), #120 (
categoryfor the 73 recategorized archaeal records), #124 (solutions dropped).Not done here
No record is moved or restamped. Solutions arguably belong in
solutions/(which today holds only an index JSON), but relocating 4,784 files is a far larger change than closing the budget leak, and it would collide with the archaeal recategorization work. Worth its own PR.Found along the way
The ranking is 50% name-level duplicates — 2,240 names cover 5,366 entries, and the top-100 holds only 86 distinct media (
thermus_mediumappears 12 times as distinct TOGO records). The #117 skip-guard does not collapse these, since each has its own slug. Filed as #127; it was invisible before because the solution records dominated the list.Test plan
CategoryEnumhas nosolutionsvaluejust validate-strict --sample 300→ 0 errors after the refactorCloses #124.
🤖 Generated with Claude Code