mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
docs(finding-feedback-loop): requirements approved, design generated
The design turns on an asymmetry the session measured: two of the three matcher bugs needed no judgement at all -- once the first was understood, the others were the same fix carried across. So the automation that pays is propagation and memory, and the model is confined to the one step that needs inference: naming what a group of counter-examples has in common. Two prohibitions are enforced by tests rather than by convention. No module under feedback/ may import a writer for semantic/facts, semantic/obligations or ruleset/. The boundary between evolving the QUESTION and authoring the FACTS is the whole design, and a comment saying so is not a boundary. And `evaluation_set(recipe, dismissals, *, split)` has no "all" -- no argument returns both splits, and no code path returns holdout rows during optimisation. The missing argument IS the design: the 2026-09-05 leak was possible because reading the holdout was merely discouraged, and five of eleven candidate shapes were taught by the pairs they then recovered. A dismissal is recorded AT a rung. "Not a defect at suspicion" must not silence the same site when the graph later entails it -- that is a promotion and deserves a fresh look, which is also what keeps a dismissal file from becoming a graveyard. Risk recorded rather than glossed: the evidence base is two repositories, one of which has never produced a true positive. The ledger and the dismissals are useful immediately; the evolution should not be trusted until contributor-scan 2.15 or 5.2 lands a checkout that can produce one. Phase: design, generated, not approved. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
This commit is contained in:
co-authored by
Claude Opus 5
parent
bccc89be52
commit
290ecff9de
@@ -0,0 +1,218 @@
|
||||
# Design Document: finding-feedback-loop
|
||||
|
||||
## Overview
|
||||
|
||||
Three of nine engine defects found in one session were the same bug in three matchers: a spec token matched as
|
||||
a substring rather than a word. Each was fixed locally and left no trace, and the third survived until an
|
||||
89k-line C library made it visible as 26 identical false positives. This feature gives a rejected finding
|
||||
somewhere to live, groups rejections by what they share, and lets a model propose the cause without deciding
|
||||
anything.
|
||||
|
||||
The design turns on one asymmetry: **two of those three bugs needed no judgement at all.** Once the first was
|
||||
understood, the other two were the same fix carried across. So the automation that pays is *propagation and
|
||||
memory*, and the model is confined to the one step that genuinely needs inference — naming what a group has in
|
||||
common.
|
||||
|
||||
### Goals
|
||||
|
||||
- A dismissal a developer commits beside their code, which the tool reads and never writes.
|
||||
- A ledger that turns every rejection into a runnable regression case.
|
||||
- Grouping by shared cause, so a recurring bug is visible as one bug.
|
||||
- Prompt evolution scored on a split that the holdout never leaks into.
|
||||
|
||||
### Non-Goals
|
||||
|
||||
- **Automated fact-table edits.** Not deferred — prohibited. See Boundary Commitments.
|
||||
- Suppressing findings silently. A dismissal makes a finding *labelled*, never invisible.
|
||||
- A general-purpose optimiser. The only surfaces evolved are two prompt constants.
|
||||
- Replacing the pair corpus. It is the holdout that keeps per-repository tuning honest.
|
||||
|
||||
## Boundary Commitments
|
||||
|
||||
### This Spec Owns
|
||||
|
||||
- `dismissals.toml` in a target repository: its schema, its reader, and the rule that nothing writes it.
|
||||
- `benchmarks/ledger/`: counter-examples, their grouping, and their regression cases.
|
||||
- The review queue a proposal lands in.
|
||||
- The evolution harness over `CANDIDATE_QUESTION` and `residual_question`.
|
||||
|
||||
### Out of Boundary
|
||||
|
||||
- `semantic/facts`, `semantic/obligations`, `ruleset/`, the declared policy file — **no automated writer, by
|
||||
any path, at any time.** This is the boundary the whole design exists to respect.
|
||||
- The three gates. They are the invariant an adoption is measured against, not a thing this spec changes.
|
||||
- `model/taint.py`, `dominance.py`, `config_value.py` — arbiters are fixed here; a proposal that would change
|
||||
one is a proposal for a human, not an edit.
|
||||
|
||||
### Allowed Dependencies
|
||||
|
||||
Standard library, `tomllib`, and the existing model/endpoint client. Core `dependencies = []` holds: an
|
||||
evolution run is a maintainer operation behind an extra, never part of `ousast scan`.
|
||||
|
||||
### Revalidation Triggers
|
||||
|
||||
- A second true-positive repository appears → the evidence base changes and every adoption number is restated.
|
||||
- The pair corpus splits change → every train/holdout claim is recomputed.
|
||||
- An arbiter changes → the ledger's regression cases are re-run before anything is adopted.
|
||||
|
||||
## Architecture
|
||||
|
||||
### Existing Architecture Analysis
|
||||
|
||||
Three pieces already exist and are reused rather than rebuilt:
|
||||
|
||||
- **`benchmarks/repos/*.toml`** declares known vulnerabilities per checkout, with `in_scope` stating whether a
|
||||
family can decide one at all. That is the positive half of an evaluation set, already committed.
|
||||
- **`pairs.py`** already carries a train/holdout split and the machinery that respects it. The leak of
|
||||
2026-09-05 was in the *lever*, not the splitter — `semantic/loo.evaluate_loo` was honest throughout.
|
||||
- **`redaction.py`** is applied to every prompt and persisted trajectory, and applies here unchanged.
|
||||
|
||||
### Boundary Map
|
||||
|
||||
```
|
||||
target repository this spec unchanged
|
||||
────────────────── ────────── ─────────
|
||||
.openultrasast/ ledger/ semantic/facts
|
||||
dismissals.toml ── read ─────────▶ entries + groups semantic/obligations
|
||||
(never written) regression cases ruleset/
|
||||
review queue the three gates
|
||||
benchmarks/repos/*.toml ── read ────▶ evaluation set model/*.py arbiters
|
||||
known vulnerabilities (train / holdout)
|
||||
│
|
||||
└── evolves ──▶ CANDIDATE_QUESTION
|
||||
residual_question
|
||||
```
|
||||
|
||||
### Key Decisions
|
||||
|
||||
**A dismissal is a label, not a suppression.** A suppressed finding disappears and its reason rots; a
|
||||
labelled one is reported as dismissed with its reason, and a dismissal that stops matching is reported too.
|
||||
This is what turns a suppression list into the negative half of an evaluation set — the thing that makes the
|
||||
rest of the feature possible.
|
||||
|
||||
**The tool never writes the dismissal file.** Same rule as the declared policy file, for the same reason: a
|
||||
file the tool edits is a file no reviewer trusts. It is committed by a developer and reviewed like code.
|
||||
|
||||
**Grouping is deterministic; only the hypothesis is not.** Entries are grouped by shared spec token, matcher
|
||||
clause, family and witness shape — all mechanical. The model is asked one question about a group it did not
|
||||
choose, and its answer goes to a queue. This keeps the inference confined to the step that needs it.
|
||||
|
||||
**Evolution targets the question, never the facts.** A prompt is how we ask; a fact table is what we claim.
|
||||
`CANDIDATE_QUESTION` and `residual_question` are already the LLM-facing surface and are the only mutable
|
||||
targets. Enforced by a test that fails if any writer touches the fact modules, not by convention.
|
||||
|
||||
**The holdout is never read.** Not "should not" — the evaluation-set API has no path to it during
|
||||
optimisation, and a test asserts the absence. The 2026-09-05 leak was possible because reading it was merely
|
||||
discouraged.
|
||||
|
||||
## File Structure Plan
|
||||
|
||||
### New
|
||||
|
||||
```
|
||||
src/openultrasast/feedback/
|
||||
├── dismissals.py # read a target repo's dismissals; never write
|
||||
├── ledger.py # counter-example entries, grouping, regression cases
|
||||
├── evaluate.py # per-repository evaluation set + train/holdout split
|
||||
└── evolve.py # reflective evolution over the two question constants
|
||||
|
||||
benchmarks/ledger/
|
||||
├── README.md # what an entry is and why it is committed evidence
|
||||
└── entries/*.toml # one counter-example per file
|
||||
```
|
||||
|
||||
### Changed
|
||||
|
||||
- `cli.py` — a `feedback` subcommand: `ledger`, `groups`, `propose`, `evolve`. Maintainer surface; `scan`
|
||||
gains only the dismissal read.
|
||||
- `reports.py` — a dismissed finding is rendered as dismissed with its reason.
|
||||
|
||||
### Unchanged and load-bearing
|
||||
|
||||
`semantic/facts`, `semantic/obligations`, `ruleset/`, the three gates, `model/` arbiters, `redaction`.
|
||||
|
||||
## Components and Interfaces
|
||||
|
||||
### feedback/dismissals.py
|
||||
|
||||
```python
|
||||
@dataclass(frozen=True)
|
||||
class Dismissal:
|
||||
site: str # path:line:function, the same shape a finding's id carries
|
||||
rung: str # the rung it was dismissed AT; a later stronger claim is not covered
|
||||
reason: str # required; a dismissal without one is rejected at load
|
||||
author: str
|
||||
added: str
|
||||
|
||||
def load_dismissals(repo: Path) -> tuple[Dismissal, ...]: ...
|
||||
def applied(findings, dismissals) -> tuple[list[Finding], list[Dismissal]]:
|
||||
"""Findings marked dismissed, and the dismissals that matched nothing."""
|
||||
```
|
||||
|
||||
Dismissing at a rung rather than outright is deliberate: "this is not a defect at `suspicion`" must not
|
||||
silence the same site when the graph later *entails* it. That is a promotion, and it deserves a fresh look.
|
||||
|
||||
### feedback/ledger.py
|
||||
|
||||
```python
|
||||
@dataclass(frozen=True)
|
||||
class CounterExample:
|
||||
site: str
|
||||
rung: str
|
||||
witness: str
|
||||
tokens: tuple[str, ...] # the spec tokens that produced it
|
||||
clause: str # which matcher clause fired
|
||||
family: str
|
||||
commit: str
|
||||
source: str # "dismissal" | "judge_contradiction" | "declared_contract"
|
||||
|
||||
def group(entries) -> tuple[Group, ...]:
|
||||
"""Deterministic grouping by shared token, clause, family, witness shape."""
|
||||
```
|
||||
|
||||
`tokens` and `clause` are what make the three matcher bugs one group: all three were a spec token matching by
|
||||
substring, and recording *which clause fired* is what a witness alone would not have told us.
|
||||
|
||||
### feedback/evaluate.py
|
||||
|
||||
```python
|
||||
def evaluation_set(recipe, dismissals, *, split: str) -> EvalSet:
|
||||
"""Positives from the recipe's known vulnerabilities, negatives from dismissals.
|
||||
|
||||
`split` is "train" or "holdout". There is no "all", and no argument that returns both.
|
||||
"""
|
||||
```
|
||||
|
||||
The missing "all" is the design. An API that cannot express *read everything* cannot leak the holdout by
|
||||
accident, which is exactly how the last one leaked.
|
||||
|
||||
### feedback/evolve.py
|
||||
|
||||
```python
|
||||
def propose(group, *, client, model) -> Proposal: ... # to a review queue, never to disk under src/
|
||||
def evolve(questions, evalset, *, rounds: int) -> Evolution: ...
|
||||
def adopt(evolution) -> AdoptionRecord | None:
|
||||
"""None unless: fewer dismissed sites affirmed, no known vulnerability lost, gates byte-identical."""
|
||||
```
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
- **The prohibition is a test, not a comment.** Assert no module under `feedback/` imports a writer for
|
||||
`semantic/facts`, `semantic/obligations` or `ruleset/`.
|
||||
- **The holdout is a test.** Assert `evaluation_set` has no code path returning holdout rows during
|
||||
optimisation.
|
||||
- **The model's role is a measurement.** Revert the three matcher fixes, run grouping and proposal, and record
|
||||
how many known causes it recovers. This is the acceptance test for Requirement 3.4 and it runs offline
|
||||
against committed ledger entries.
|
||||
- **A dismissal without a reason fails to load**, and a dismissal that matches nothing is reported.
|
||||
|
||||
## Risks
|
||||
|
||||
**The evidence base is two repositories, one of which has never produced a true positive.** An optimiser tuned
|
||||
on that is tuned on very little, and Requirement 7 exists to keep it labelled rather than to pretend
|
||||
otherwise. The honest sequencing is that this feature's *ledger and dismissals* are useful immediately, while
|
||||
its *evolution* should not be trusted until `contributor-scan` 2.15 or 5.2 adds a checkout that can produce a
|
||||
true positive.
|
||||
|
||||
**Per-repository tuning is overfitting with a nicer name.** The pair corpus is the only thing standing
|
||||
against it, which is why the gates are an adoption criterion rather than a report line.
|
||||
@@ -1,16 +1,16 @@
|
||||
{
|
||||
"feature_name": "finding-feedback-loop",
|
||||
"created_at": "2026-09-09T12:00:00Z",
|
||||
"updated_at": "2026-09-09T12:30:00Z",
|
||||
"updated_at": "2026-09-09T13:00:00Z",
|
||||
"language": "en",
|
||||
"phase": "requirements",
|
||||
"phase": "design",
|
||||
"approvals": {
|
||||
"requirements": {
|
||||
"generated": true,
|
||||
"approved": false
|
||||
"approved": true
|
||||
},
|
||||
"design": {
|
||||
"generated": false,
|
||||
"generated": true,
|
||||
"approved": false
|
||||
},
|
||||
"tasks": {
|
||||
@@ -22,5 +22,5 @@
|
||||
"depends_on": [
|
||||
"contributor-scan"
|
||||
],
|
||||
"approval_note": "2026-09-09 brief drafted. Scope lifted out of contributor-scan, where two task groups (a counter-example ledger and per-repository evolutionary prompting) had been appended directly to tasks.md without passing requirements or design -- a workflow error, and they are a separate feature rather than a refinement of scan integration. Prompted by the maintainer asking for GEPA/SIMBA-style evolution with declarative persistent per-repository state so developers can commit dismissals and evolve the harness. The design boundary is that an optimiser may evolve the QUESTION (judge and residual prompts) and never the FACTS (tables, rulesets, declared policy), because the closed-loop leak measured 2026-09-05 turned a -0.059 holdout Youden into +0.059 by teaching from the pairs it then recovered. Not approved; requirements not generated. ; 2026-09-09 requirements generated at the maintainer's direction ('approve the amendments and generate the requirements for finding-feedback-loop'). Seven requirements: a dismissal file the developer owns and the tool never writes; a counter-example ledger whose entries are runnable regression cases grouped by shared cause; a model that proposes a cause into a review queue and decides nothing, validated by reverting the three known matcher fixes; dismissals plus recipe vulnerabilities as one split evaluation set with the holdout unread; evolution scoped to the judge and residual questions with the fact tables off limits by any automated means; an adoption that reports what it traded and reverts itself when net-negative; and the loop inheriting the corpus blind spot, so its evidence base is repositories and it says how few of them there are. Not approved."
|
||||
"approval_note": "2026-09-09 brief drafted. Scope lifted out of contributor-scan, where two task groups (a counter-example ledger and per-repository evolutionary prompting) had been appended directly to tasks.md without passing requirements or design -- a workflow error, and they are a separate feature rather than a refinement of scan integration. Prompted by the maintainer asking for GEPA/SIMBA-style evolution with declarative persistent per-repository state so developers can commit dismissals and evolve the harness. The design boundary is that an optimiser may evolve the QUESTION (judge and residual prompts) and never the FACTS (tables, rulesets, declared policy), because the closed-loop leak measured 2026-09-05 turned a -0.059 holdout Youden into +0.059 by teaching from the pairs it then recovered. Not approved; requirements not generated. ; 2026-09-09 requirements generated at the maintainer's direction ('approve the amendments and generate the requirements for finding-feedback-loop'). Seven requirements: a dismissal file the developer owns and the tool never writes; a counter-example ledger whose entries are runnable regression cases grouped by shared cause; a model that proposes a cause into a review queue and decides nothing, validated by reverting the three known matcher fixes; dismissals plus recipe vulnerabilities as one split evaluation set with the holdout unread; evolution scoped to the judge and residual questions with the fact tables off limits by any automated means; an adoption that reports what it traded and reverts itself when net-negative; and the loop inheriting the corpus blind spot, so its evidence base is repositories and it says how few of them there are. Not approved. ; 2026-09-09 requirements APPROVED and design generated ('approve the requirements and run the design phase'). The design turns on one asymmetry: two of the three matcher bugs needed no judgement at all, only the fix carried across, so the automation that pays is propagation and memory and the model is confined to naming what a group shares. Two prohibitions are enforced by tests rather than convention -- no module under feedback/ may import a writer for the fact tables or rulesets, and evaluation_set has no code path returning holdout rows during optimisation (the missing 'all' argument IS the design, since the 2026-09-05 leak was possible because reading the holdout was merely discouraged). A dismissal is recorded AT a rung, so dismissing a suspicion does not silence the same site when the graph later entails it. Risk recorded: the evidence base is two repositories, one of which has never produced a true positive, so the ledger and dismissals are useful immediately while the evolution should not be trusted until contributor-scan 2.15 or 5.2 lands a checkout that can."
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user