mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 11:49:28 +00:00
fix(learning-harness): the round-3 review findings, remediated RED-first
Eight of nine group-3 tasks were rejected. Each defect was hiding a number, and the numbers are why the loop was not learning: - The fixed side was judged against the *vulnerable* side's function ranges. 191 of 227 pairs with a resolvable labeled span (84%) have different spans on the two sides, so a leak was recorded as silence, always in the direction that flatters Youden. - Round zero measured every family with the generalist. `family:unknown` earns no credit against any real family, so every family but `unknown` baselined at a structural zero and every noise floor with it. - The cross-family sweep counted absolute failures instead of negative flips. On a corpus where most pairs fail at round zero that rejects every round for damage it did not do. Unscorable rows voted too. - K runs was declared and never threaded; `pairs --k-runs` was parsed and discarded, and its only test grepped `--help`. - The meta-agent proposer was a stub, so `learning round` could never propose anything. It now composes MetaAgent.evolve against a scratch copy of the family directory, and `--proposals FILE` runs a recorded sequence offline. - `refused_holdout` was a vocabulary word no code path could produce, and the covering test accepted either outcome. - Cost was metered once, after everything had been paid for; a stage that raised left no record at all. - The corrected lever number was published beside the earlier one rather than in place of it, and the test named for that property never checked. - Refusals were `continue` statements: `Refusal.degradation()` existed and nothing called it. A train-split identical twin was reported as `split:train`, which is untrue. Test repairs that each changed a number: two split tests measured an `x = 1` fixture that teaches nothing whatever the rule says; two CLI scoreboard tests asserted against all-zero output produced by a stub that answers twice and puts its finding outside the labeled function. 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
c2d9c4ad01
commit
1843ef9a02
+106
-10
@@ -19,6 +19,33 @@ CATALOG = (
|
||||
)
|
||||
|
||||
|
||||
def _detector(monkeypatch: pytest.MonkeyPatch, line: int = 6) -> None:
|
||||
"""A deterministic detector that reads the file it was given and answers differently on the two sides.
|
||||
|
||||
The shipped `scripted` stub answers twice and then goes quiet, and its finding sits at line 3, outside `run`.
|
||||
Both make every family number zero, and a scoreboard assertion against zeros passes with the detector missing
|
||||
entirely — which is what these tests used to assert."""
|
||||
from openultrasast.learning.endpoint import ChatEndpoint
|
||||
from openultrasast.tool_hunter import ChatResponse, ToolCall
|
||||
|
||||
class Detector:
|
||||
def __init__(self) -> None:
|
||||
self.turn = 0
|
||||
|
||||
def complete(self, *, model: str, messages, tools=None, **options): # type: ignore[no-untyped-def]
|
||||
del model, tools, options
|
||||
self.turn += 1
|
||||
if self.turn % 2 == 1:
|
||||
return ChatResponse(tool_calls=(ToolCall(id=f"c{self.turn}", name="grep_repo", arguments={"pattern": "system"}),))
|
||||
body = "\n".join(str(message.get("content") or "") for message in messages)
|
||||
if "os.system" not in body:
|
||||
return ChatResponse(content="[]") # the fixed side really is different, and the detector sees it
|
||||
return ChatResponse(content=json.dumps([{"path": "app.py", "line": line, "title": "t", "rationale": "r"}]))
|
||||
|
||||
endpoint = ChatEndpoint(provider="scripted", base_url="", thinking=False)
|
||||
monkeypatch.setattr("openultrasast.learning.endpoint.resolve_chat_endpoint", lambda *a, **k: (Detector(), endpoint))
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def workspace(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
|
||||
for name in ("train-a", "hold-a"):
|
||||
@@ -39,14 +66,25 @@ def test_classify_reports_the_corpus_and_writes_the_queue(workspace: Path, capsy
|
||||
assert "classified 2" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_score_reports_family_numbers_for_a_catalog(workspace: Path, capsys: pytest.CaptureFixture[str]) -> None:
|
||||
def test_score_reports_family_numbers_for_a_catalog(
|
||||
workspace: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""The scripted client detects the vulnerable side and is silent on the fix, so this is a real 2/2, not a zero.
|
||||
|
||||
An all-zero scoreboard would satisfy "the family key exists"; it is what a generalist detector scoring every
|
||||
family produced before round zero ran each family's own configuration."""
|
||||
_detector(monkeypatch)
|
||||
assert main(["learning", "score", "--catalog", str(workspace / "pairs.toml"), "--out", str(workspace / "learning"), "--json"]) == 0
|
||||
payload = json.loads(capsys.readouterr().out)
|
||||
assert "injection" in payload["families"]
|
||||
assert payload["families"]["injection"]["taxonomy_version"]
|
||||
block = payload["families"]["injection"]
|
||||
assert block["taxonomy_version"] and block["scorable"] == 2
|
||||
assert block["outcomes"] == {"pair_correct": 2} and block["recall"] == 1.0 and block["youden"] == 1.0
|
||||
|
||||
|
||||
def test_baseline_writes_the_noise_floor_and_publish_reads_it(workspace: Path, capsys: pytest.CaptureFixture[str]) -> None:
|
||||
def test_baseline_writes_the_noise_floor_and_publish_reads_it(
|
||||
workspace: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
_detector(monkeypatch)
|
||||
assert (
|
||||
main(
|
||||
[
|
||||
@@ -65,7 +103,12 @@ def test_baseline_writes_the_noise_floor_and_publish_reads_it(workspace: Path, c
|
||||
== 0
|
||||
)
|
||||
floors = json.loads((workspace / "learning" / "baseline" / "scripted" / "noise-floors.json").read_text())
|
||||
assert floors["k_runs"] == 3 and "injection" in floors["floors"]
|
||||
assert floors["k_runs"] == 3 and floors["floors"]["injection"]["pairs"] == 2
|
||||
report = json.loads((workspace / "learning" / "baseline" / "scripted" / "report.json").read_text())
|
||||
# Measured with the injection configuration, not the generalist: `family:unknown` earns no credit against any
|
||||
# real family, so a baseline built from one shared detector reports a structural zero here.
|
||||
assert report["metrics"]["injection"]["recall"] == 1.0
|
||||
assert report["per_slice"]["vibe-py/injection"]["scorable"] == 2
|
||||
capsys.readouterr()
|
||||
roadmap = workspace / "roadmap.md"
|
||||
roadmap.write_text("# Roadmap\n\n## Overview\n\ntext\n")
|
||||
@@ -116,10 +159,46 @@ def test_a_round_runs_from_the_command_line_and_journals_its_outcome(workspace:
|
||||
)
|
||||
rounds = [json.loads(line) for line in (workspace / "learning" / "journal.jsonl").read_text().splitlines() if line.strip()]
|
||||
assert len(rounds) == 1 and rounds[0]["family"] == "injection"
|
||||
assert rounds[0]["outcome"] in {"accepted", "rejected", "reverted_cost"}
|
||||
# Without the extra there is no meta-agent, so the honest outcome is exactly this one. Accepting any member of
|
||||
# the enum would have hidden that the round could never propose anything at all.
|
||||
assert (rounds[0]["outcome"], rounds[0]["reason"]) == ("rejected", "no_proposal")
|
||||
assert (workspace / "learning" / "rounds" / "1" / "attribution.json").is_file()
|
||||
|
||||
|
||||
def test_a_recorded_proposal_makes_the_round_do_something(workspace: Path) -> None:
|
||||
"""`--proposals` is what makes a round runnable at all without the optional extra."""
|
||||
for command in (
|
||||
["learning", "baseline", "--catalog", str(workspace / "pairs.toml"), "--out", str(workspace / "learning"), "--k-runs", "3"],
|
||||
):
|
||||
assert main([*command, "--model", "scripted"]) == 0
|
||||
proposals = workspace / "proposals.json"
|
||||
proposals.write_text(
|
||||
json.dumps([{"hypothesis": "ask about quoting", "lever": "checklist", "change": {"checklist.md": "- ask about quoting\n"}}])
|
||||
)
|
||||
assert (
|
||||
main(
|
||||
[
|
||||
"learning",
|
||||
"round",
|
||||
"--catalog",
|
||||
str(workspace / "pairs.toml"),
|
||||
"--out",
|
||||
str(workspace / "learning"),
|
||||
"--family",
|
||||
"injection",
|
||||
"--k-runs",
|
||||
"3",
|
||||
"--proposals",
|
||||
str(proposals),
|
||||
]
|
||||
)
|
||||
== 0
|
||||
)
|
||||
(record,) = [json.loads(line) for line in (workspace / "learning" / "journal.jsonl").read_text().splitlines() if line.strip()]
|
||||
assert record["reason"] != "no_proposal", "a recorded proposal reached the round"
|
||||
assert json.loads((workspace / "learning" / "rounds" / "1" / "proposal.json").read_text())["lever"] == "checklist"
|
||||
|
||||
|
||||
def test_a_missing_endpoint_is_recorded_and_the_rest_still_runs(workspace: Path, monkeypatch: pytest.MonkeyPatch, capsys) -> None: # type: ignore[no-untyped-def]
|
||||
for name in ("OPENULTRASAST_HUNTER_CLIENT", "DEEPSEEK_API_KEY", "OPENROUTER_API_KEY"):
|
||||
monkeypatch.delenv(name, raising=False)
|
||||
@@ -153,7 +232,24 @@ def test_every_subcommand_is_listed_in_help(capsys: pytest.CaptureFixture[str])
|
||||
assert name in out
|
||||
|
||||
|
||||
def test_the_pairs_command_takes_a_k_runs_flag(capsys: pytest.CaptureFixture[str]) -> None:
|
||||
with pytest.raises(SystemExit):
|
||||
main(["pairs", "--help"])
|
||||
assert "--k-runs" in capsys.readouterr().out
|
||||
def test_the_pairs_command_scores_k_runs_per_side(workspace: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Grepping `--help` for the flag passes with the capability entirely absent, which is what it used to be."""
|
||||
from openultrasast import pairs as pairs_module
|
||||
|
||||
calls: list[str] = []
|
||||
real = pairs_module.make_hunter_scan
|
||||
|
||||
def counting(client: object, model: str, **kwargs: object): # type: ignore[no-untyped-def]
|
||||
inner = real(client, model, **kwargs) # type: ignore[arg-type]
|
||||
|
||||
def scan(root: Path): # type: ignore[no-untyped-def]
|
||||
calls.append(root.name)
|
||||
return inner(root)
|
||||
|
||||
return scan
|
||||
|
||||
monkeypatch.setattr(pairs_module, "make_hunter_scan", counting)
|
||||
monkeypatch.setattr("openultrasast.cli.make_hunter_scan", counting)
|
||||
monkeypatch.setenv("OPENULTRASAST_HUNTER_MODEL", "scripted")
|
||||
assert main(["pairs", "--catalog", str(workspace / "pairs.toml"), "--hunter", "--hunter-model", "scripted", "--k-runs", "4"]) == 0
|
||||
assert calls.count("vuln") == 8 and calls.count("fixed") == 8 # two pairs, four runs per side
|
||||
|
||||
Reference in New Issue
Block a user