docs(learning-harness): tick group 1 and record review round 1

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
This commit is contained in:
Marius Ciepluch
2026-09-06 09:40:45 +02:00
co-authored by Claude Opus 5
parent 17fc18c785
commit e0a28eb813
+11 -6
View File
@@ -1,29 +1,29 @@
# Implementation Plan
- [ ] 1. Foundation: taxonomy, endpoint, corpus fields, grammar
- [ ] 1.1 Closed family taxonomy with a verifier per family
- [x] 1. Foundation: taxonomy, endpoint, corpus fields, grammar
- [x] 1.1 Closed family taxonomy with a verifier per family
- Ten families exactly as decided, each with its CWE set, mechanism set, verifier kind (canary, static, none) and description; `unknown` always present with no verifier; a version string.
- Loader rejects an unknown field, an eleventh family, a missing verifier kind or a CWE claimed by two families, naming the offender; relation lookup answers same, parent–child, lateral or fabricated for any two ids.
- Observable: the taxonomy loads from the ruleset directory, a fixture with a fabricated family fails by name, and every mechanism id in the mechanism vocabulary resolves to exactly one family.
- _Requirements: 1.1, 1.2, 1.3, 1.4_
- [ ] 1.2 Independent chat endpoint with the DeepSeek adapter
- [x] 1.2 Independent chat endpoint with the DeepSeek adapter
- Endpoint resolution order: explicit override, scripted client modes, DeepSeek key and base URL, then OpenRouter; the embedding client keeps reading only its own variables; `[models]` gains the hunter and judge defaults and the endpoint keys, `[learning]` gains its section.
- Adapter disables thinking and sends temperature zero for detector and classifier calls, keeps the provider's reasoning field on assistant messages so a caller can replay it, supports JSON-object mode with one retry on empty content and optional log-probabilities, and reports normalized usage and cost from the provider's cache-hit fields.
- Observable: with both provider keys set the hunter client targets the DeepSeek base URL without a version suffix while the embedding client still targets OpenRouter; a scripted client is chosen under the existing environment flag; a recorded request shows thinking disabled and the reasoning field retained on the reply.
- _Requirements: 6.6, 11.4_
- [ ] 1.3 Additive corpus fields and hygiene at load
- [x] 1.3 Additive corpus fields and hygiene at load
- Expected rows accept an opaque family id (validated later by the classifier task, so the corpus module never imports the learning package); a pair carries a computed unscorable reason (identical twin by normalized body comparison with headers stripped, computed only when the catalog does not already declare a known limit) and is never dropped; duplicates across pairs are reported before any split; a repository-and-date split helper assigns train or holdout to rows that lack a split, keeps declared splits, and reports rows from one repository that straddle the split; pair signals take a split filter.
- Observable: the five identical vibe-py twins load with the identical-twin reason and the existing catalogs load with unchanged row counts; the straddle report on the current catalogs is produced and committed.
- _Requirements: 4.1, 4.2, 4.3, 4.4_
- [ ] 1.4 TypeScript rows parse under the extra
- [x] 1.4 TypeScript rows parse under the extra
- The semantic extra gains the TypeScript grammar and the parser maps TypeScript and TSX to it; the extra-free suite still skips them with the existing marker.
- Observable: every vendored TypeScript pair parses on both sides under the extra, and the parser reports no unsupported-language result for TypeScript or TSX.
- _Requirements: 10.1_
- [ ] 1.5 Curated hunter tools and hunter-loop parameters
- [x] 1.5 Curated hunter tools and hunter-loop parameters
- Four new repository tools beside the existing three: resolve a definition by symbol, list entry points, list flows for a function, list obligations for a function, each clamped to the scanned repository and returning identifiers and line spans only; the hunter loop gains prompt, context files, tool set, character limit and tag parameters with today's behavior as defaults and replays the provider's reasoning field across tool turns.
- Observable: the existing hunter tests pass unchanged with default parameters; each new tool answers on the shared handler fixture and refuses a path outside the repository; a recorded tool-turn conversation shows the reasoning field replayed.
- _Requirements: 6.4, 6.5_
@@ -182,6 +182,11 @@
- Task 1.3 (2026-09-06, RED first: 7 failing tests in `tests/test_pair_hygiene.py`; then green): `ExpectedFinding.family` (opaque; the classifier validates it), `PairCase.unscorable`, `PairCase.split_declared`, `PairCase.fix_date` (the generator fills it in 4.3), `duplicate_groups`, `split_by_repository` with `SplitReport`, and a split filter on `build_pair_signals`. Measured and committed as `benchmarks/measurements/2026-09-06-corpus-hygiene.json`: **19 repositories straddle the train/holdout split** (a learning path can be taught by one file of a repository and evaluated on another) and **10 groups of pairs share a vulnerable excerpt byte for byte**, mostly the same fix harvested twice at different granularity. Only three vendored rows (the `local` slice) declare no split.
- Task 1.4 (2026-09-06, RED first: 2 failing tests in `tests/test_typescript_grammar.py`; then green): the semantic extra gains `tree-sitter-typescript`; `grammar_for` maps a language to (module, attribute) because the wheel ships two grammars, and `parse_with_cst` picks the JSX-aware one for a `.tsx` path while the parsed file keeps the language it was given. Eighteen agent-vfc rows plus the TypeScript absence rows now parse on both sides.
- Task 1.5 (2026-09-06, RED first: 5 of 6 tests in `tests/test_hunter_curated_tools.py` failing; then green): `hunter_tools` gains `read_definition`, `entry_points`, `flows`, `obligations`; `run_tool_hunter` gains `system_prompt`, `user_prompt`, `context_files`, `tools`, `max_chars` and `tags`, all defaulting to today's behaviour, and replays the provider's reasoning field on the next turn. Deviations: `read_definition` returns the definition text as well as its span, and includes preceding decorator lines, because a resolver that returns only line numbers forces another `read_file` and a handler's decorators are exactly what the obligations work showed to matter; the three list tools return identifiers and spans only. No caching: a stale view of a tree a round may have rewritten is worse than a reparse.
- Review round 1 (2026-09-06) APPROVED 1.1, 1.2, 1.4, 1.5; REJECTED 1.3. The reviewer ran every check in a detached worktree at the group-1 tip and confirmed RED-first by running each new test file against the pre-feature commit (37 failures there, all passing after), the three gate outputs byte-identical, and `pairs`/`benchmark` free of any learning import.
- **1.3 rejection, remediated RED-first**: `_body_digest` stripped every leading comment line, which in C and C++ swallows `#include` and `#define`, and in any language swallows an ordinary leading comment. Two files differing only in a buffer size on a `#define` hashed equal and were marked `identical_twin`, removing a real pair from every denominator. Measured before the fix: twelve file sides across six vendored pairs had directives inside the stripped prefix, including the Juliet stack-overflow pair whose whole point is that constant. The strip is now anchored to the grammar `harvest.provenance_header` writes: a leading comment counts as header only when it carries one of the eleven provenance keys followed by a colon, and the scan stops at the first line of code. Four regression tests pin both directions plus a corpus-wide assertion that no vendored excerpt loses a directive. The five twins, ten duplicate groups and nineteen straddling repositories are unchanged, so the committed hygiene artifact still holds.
- Suggestions applied: a mechanism claimed by two families now fails loud by name; the obligations tool's path-escape refusal and the endpoint's degrade-to-None on a provider error are now asserted; the TypeScript parse assertion is exact instead of a 25% allowance, which surfaced one real corpus defect (`lensops-kubernetesmanagement-fe4910` was harvested as a truncated destructuring parameter list and is not valid TypeScript on its own; it belongs to the 4.1 re-harvest).
- **Two findings escalated to the maintainer, both blocking task 3.8 rather than group 1.** (a) `untrusted_destination` contradicts the approved requirement sentence, which says "server-side request forgery": the family id is the key every published number carries, so `requirements.md` Req 1.1 and the brief's Decision 1 must be amended to the widened name, or the family renamed back and CWE-601's 34 rows re-homed. (b) The shipped taxonomy is flat, so `parent_child` is unreachable in production and the "partial credit for a parent-child relation" in Req 2.3 and 3.1 is inert at version 1; either declare real parents or record the inertness beside every published agreement number.
- Carry-forward: the nineteen straddling repositories cross the split by declaration, so `split_by_repository` cannot fix them and task 3.1's teacher rule inherits the leak until a maintainer re-declares those splits. The `preprocess_repository` cost of the three list tools should be measured in 2.5 before round zero.
- Task 2.1 (2026-09-06, RED first: 8 failing tests in `tests/test_learning_classify.py`; then green): `learning/classify.py` with three tiers, multi-label answers in taxonomy order, and `validate_family_labels` owning the taxonomy check the corpus loader deliberately does not do.
- Task 2.2 (2026-09-06, RED first: 5 failing tests in `tests/test_learning_classifier_report.py`; then green): `measure_classifier` and `write_review_queue`. Bug caught by the RED test: the classifier read the declared label it was being scored against, making agreement trivially 1.0; `classify_pair(..., use_declared=False)` is what the measurement uses. Measured on the 277 vendored pairs: tier one places 275 without a model call, 2 land in `unknown`, and the 176 memory-safety rows resolve to one family. `labeled` is 0 because no row carries a maintainer family label yet, and the report says so instead of inventing a score.
- Task 2.3 (2026-09-06, RED first: 8 failing tests in `tests/test_learning_scoring.py`; then green): `learning/scoring.py` `score_pair_family`. Detection is a family-tagged finding inside the labeled function; text is never read. Findings of other families or outside the function are counted separately and never called leaks; a fabricated family id is counted as noise. K runs, majority outcome, disagreeing runs recorded as flips.