Repository navigation
Conversation
CLI interface change detectedThis PR changes dokimi's user-facing CLI surface (commands and/or options): + added command `dokimi eval redteam`
+ argument `questions`
+ option `--help/-h`
+ option `--judge`
+ option `--out`
~ `dokimi runs list` parameters changed:
~ changed option `--kind`: choices: ["'answers'", "'arms'", "'cli_routes'", "'eval'", "'explore_runs'", "'guidance_reach'", "'matrix_rollup'", "'novelty'", "'plan'", "'plugin_delta'", "'questions'", "'repo_budget'", "'repo_environment'", "'repo_readiness'", "'result'", "'subject_scorecard'", "'task_inspect'", "'task_runs'", "'tool_preference'"] -> ["'answers'", "'arms'", "'cli_routes'", "'eval'", "'explore_runs'", "'guidance_reach'", "'matrix_rollup'", "'novelty'", "'plan'", "'plugin_delta'", "'questions'", "'redteam'", "'repo_budget'", "'repo_environment'", "'repo_readiness'", "'result'", "'subject_scorecard'", "'task_inspect'", "'task_runs'", "'tool_preference'"]Updated on every push; the |
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #281 +/- ##
==========================================
+ Coverage 94.05% 94.12% +0.07%
==========================================
Files 297 299 +2
Lines 38018 38207 +189
Branches 6424 6453 +29
==========================================
+ Hits 35756 35964 +208
+ Misses 1452 1441 -11
+ Partials 810 802 -8
🚀 New features to boost your workflow:
|
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at e881ab5. The idea is useful and the plumbing is right, but the verdict is not yet trustworthy both ways: the stuffed probe blocks every match or fuzzy check with several aliases on a correct grader, and a threshold block whose negated check carries a wrong answer over the line still gets pass. Details are in the line comments.
What I ran, at e881ab5:
make ci-local --offline: 9,686 passed, 0 failed, 57 skipped; every CI row passed or was skipped for the network.run()on small grading blocks: an alias pair undermatch(anyandend) andfuzzyisblockedonstuffed;{threshold: 0.5, Paris + negated London}ispasswhile the realgrade()accepts the planted wrong and the empty answer; a negated-only block ispass; five judge questions without--judgeplus one match question ispasswith 5 of 6 unprobed.
Checked, no issues
- Wiring:
eval redteamis registered undereval, added to the judge-provider coverage test, and makes no model call without--judge; exit codes 1 (blocked, inconclusive) and 2 (bad input, unwritable-o) are tested. - Schema and changelog:
redteam.v1is new, listed inSCHEMA_VERSIONS, with a fixture; the fragment is under Added and says what the command does. No existing score, default or stored key changes. - New files: all carry the SPDX header.
antonwolfy
left a comment
There was a problem hiding this comment.
Review at e881ab5. The wiring, exit codes and the redteam.v1 schema hold up, but the verdict is not yet trustworthy: it blocks correct graders and passes weak ones. Two further problems are inline.
I agree with @vlad-perevezentsev's four points on the verdict (alias stuffing, threshold with a negated check, negated-only and unprobed runs reporting pass, wrong_for and numeric: true); I reproduced each. New evidence: {match, values: ["Paris", "Lyon"]} is blocked on stuffed, and {match, values: ["-7731.0"], numeric: true} is blocked on wrong.
What I ran, on a Linux review host at e881ab5:
make ci-local: 27 passed, 1 failed, 7 skipped; 9,739 tests passed, 4 skipped; changed-line coverage passed. The failure ispip-audit, which reports advisories onfilelockin the lock files; this PR does not touch them.- Reproductions for the code findings, with the LLM transport mocked.
- A 107-mutant run on
tests/test_redteam.py: 37 mutants pass the full suite; exit codes and the status verdict are pinned, the canary text and the clash logic are not. - Commits: all six carry
Signed-off-by.
Checked, no issues
- Wiring:
eval redteamis registered, a default run makes no model call, exit 2 on a missing, unreadable or non-JSON question file and on an unwritable-o. - Schema:
-ooutput validates againstredteam.v1; the kind is inSCHEMA_VERSIONS; the fixture and the changelog fragment are present;mkdocs build --strictpasses.
|
🔍 OpenCodeReview found 2 issue(s) in this PR. By severity: 1 high, 1 low
Review details
|
|
The verdict findings it agrees with are fixed (see the replies in the vlad-perevezentsev threads) and its two own threads are answered: repeated ids 1b64508, canary tests f108f53. The |
|
Nothing to change here: the interface diff lists exactly what this PR adds, the command |
|
Covered by the tests of 27828e4, 0918eea and 1946f48: the report is for e881ab5. Locally, |
| except (OSError, ValueError) as e: | ||
| print(f"Error: {e}", file=sys.stderr) | ||
| sys.exit(2) |
There was a problem hiding this comment.
nit: a deeply nested questions file makes json.loads in load_questions raise RecursionError. It is not caught here, so the command exits with a traceback and code 1, which CI reads as a blocked run. Other commands (arms, catalog) catch RecursionError alongside OSError and ValueError for this reason.
Suggestion:
| except (OSError, ValueError) as e: | |
| print(f"Error: {e}", file=sys.stderr) | |
| sys.exit(2) | |
| except (OSError, ValueError, RecursionError) as e: |
There was a problem hiding this comment.
Fixed in 98ccb96. eval redteam now catches RecursionError next to OSError and ValueError when it reads the question set, so a set nested too deep to parse exits 2 with Error: ... on stderr instead of a traceback and exit 1. arms and catalog already catch it the same way.
Reproduced on the PR head with a 100,000-deep [[...]] file: the command died in json.loads with a RecursionError traceback and exit code 1. The new test test_command_exits_2_on_a_question_set_nested_too_deep_to_read fails on the old code and passes with the change.
| g = grade(grading, question, answer, ask) | ||
| out[name] = Probe(name, g.passed, _detail(g)) |
There was a problem hiding this comment.
must-fix: a probe is recorded as accepted even when part of the block was never scored. Grade.passed ignores checks with no score (counted drops them). Take a block of a judge check plus a negated literal check, for example [{judge: X}, {match: Lyon, negate: true}], in the default run with no --judge. The wrong, empty and hedge answers are graded on the negated check alone, which they pass by design. passed comes back True, so each is reported as a violation: the run is blocked and exits 1 for a block that is not gameable. The same happens when a positive regex times out. Mark the probe unscored when any check was unjudged or unrun and the grade says pass.
Suggestion:
| g = grade(grading, question, answer, ask) | |
| out[name] = Probe(name, g.passed, _detail(g)) | |
| g = grade(grading, question, answer, ask) | |
| # A pass built from only part of the block (judge checks without a judge, a timed-out regex) is not a measurement. | |
| passed = None if g.passed and (g.unjudged or g.unrun) else g.passed | |
| out[name] = Probe(name, passed, _detail(g)) |
There was a problem hiding this comment.
Blocking: Confirmed, and it has a second side that the suggested fix does not cover. I missed this in my first round.
The false block: [{judge}, {match: Lyon, negate: true}] without --judge is blocked on wrong, empty and hedge, because only the negated check is counted. This came in with 6b6aa2f, which now grades the whole block; before it the judge-only part gave None.
The false pass: {threshold: 0.5, checks: [{judge: names Paris}, {match: Paris}]} without --judge returns pass, exit 0, with the question counted as probed: the judge is dropped from the score, so wrong, empty and hedge score 0 and fail. With a judge that says yes to everything, the real grade() passes the planted wrong answer (0.5 >= 0.5), and run(ask=...) is blocked on all three. So the default run reads an unmeasured judge as a rejection.
Without a threshold a failed counted check is final (every check must pass), so only a pass is unsafe there. With a threshold neither result holds while a check has no score.
Suggestion:
g = grade(grading, question, answer, ask)
partial = g.unjudged or g.unrun
passed = None if partial and (g.passed or g.threshold is not None) else g.passedplus tests for both blocks above (the first should be unscored, the second unprobed and so inconclusive).
There was a problem hiding this comment.
Fixed in c09f9b6, and it holds at 3a4cf5b. A probe is unscored when a check in its block was unjudged or unrun and the grade is a pass, or when the block has a threshold.
Both repros now give the right result: [judge, negated match Lyon] without --judge is inconclusive with no violation, and {threshold: 0.5, judge Paris, match Paris} with a judge that accepts every answer is blocked on wrong, empty and hedge.
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at c68a1f1. Every point of my first round is fixed and I re-ran each repro. One problem is left, and it goes both ways: when part of a block has no score (a judge check without --judge, a timed-out regex), the probe is still read as a measurement. I added the false-pass side to @vchamarthi's thread.
What I ran, at c68a1f1:
make ci-local --offline: 10,553 passed, 0 failed, 57 skipped; every CI row passed or was skipped for the network.- The round-one repros: alias pairs under
match(any,begin,end) andfuzzy, andnumeric-7731.0, are nowpass.{threshold: 0.5, Paris + negated London}is nowblockedon wrong, empty and hedge. A negated-only block and the "5 judge + 1 match" set are nowinconclusive. A valueonekeeps its hedge probe. [judge, negated Lyon]without--judgeisblocked(a false block).{threshold: 0.5, [judge, match Paris]}without--judgeispass, butblockedwith a judge that says yes to everything.
Checked, no issues
- Stuffed answers: decoys are picked with the check's own
grade_check, so the decoys never hold an accepted value. Checks that read a value anywhere in the answer get no probe. - Repeated ids: each row is graded with its own block.
grading_by_idstill validates every row before any is graded. - Shared helpers:
check_valuesis a pure rename.judge_askbuilds the sameAskthateval referenceused. Nothing else imported_values, and the open PR #280 that editsreference_checks.pyadds no new call. - Artifact, docs and changelog:
judge_modelandjudge_providerare required and nullable inredteam.v1, which no release has written yet. The docs table and the fragment now describeinconclusiveand the stuffed rules as coded.
|
🔍 OpenCodeReview found 1 issue(s) in this PR. By severity: 1 low
Review details
|
| if violations: | ||
| status = BLOCKED | ||
| else: | ||
| status = PASS if rows and not summary["unprobed"] else INCONCLUSIVE |
There was a problem hiding this comment.
This code returns inconclusive (exit 1) when any graded question is unmeasured. The PR description says the opposite in two places: inconclusive (no question was probed) and the known limit "Exit 0 when any one question is probed, even if others are unprobed". The module docstring, schema and changelog match the code. Update the PR description, since the repo guide asks that exit codes be described accurately. A mixed set, such as one negated-only block among normal ones, fails CI.
There was a problem hiding this comment.
Right: the code is the intended behaviour (exit 1 when any graded question is unmeasured), and the PR body was wrong in two places ("inconclusive (no question was probed)" and "Exit 0 when any one question is probed"). I am correcting the body to match the code.
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 04c4541. The earlier verdict problems are fixed: a partial block (judge without --judge, timed-out regex) is now unscored, and I re-ran the alias, threshold, negated-only and unprobed cases from the earlier rounds. mypy fails at redteam.py:140, which I would fix before merge, and two test gaps are left on lines that decide pass against inconclusive and blocked. One nit inline.
What I ran, on a Linux review host at 04c4541:
make ci-local: 28 passed, 1 failed (mypy), 7 skipped; 10,716 tests passed, 4 skipped; changed-line coverage 99%.- Reproductions of grading blocks through
run(), with the LLM transport mocked. - A 46-mutant run on
eval/redteam.pyagainsttests/test_redteam.py: 38 killed, 8 pass the full suite (90 passed each time). Two of them decide a verdict (inline).
Checked, no issues
- Canary and stuffed answers: each canary kind is accepted by
grade(); the JSONcontainsstuffed answer is rejected for[1,2],[{"a":1},{"b":2}]and["a","b"]. - Artifact and schema: real
-oartifacts for pass, blocked and inconclusive runs, with and without--judge, validate againstredteam.v1; the kind is inSCHEMA_VERSIONS. - Refactors:
check_valuesandjudge_askhave no stale callers;eval referencebuilds the sameAsk. - Exit codes: 0, 1 and 2 behave as documented, including an unwritable
-o. - Commits: all 28 non-merge commits carry
Signed-off-by.
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at 04c4541. The fix for a partly scored block is right: a probe is now unscored when any check in the block could not be scored. Only the mypy failure blocks the merge; it is a one-line fix.
I agree with all four of @antonwolfy's new points (the mypy failure at redteam.py:140, the untested decoy filter and >= 2 threshold, the untested any/all in _unmeasured, the docstring on exit 1) and with @exai-bot's point on the PR description.
What I ran, at 04c4541:
make ci-local --offline: 10,663 passed, 0 failed, 57 skipped; thetypecheck / mypyjob fails because of this PR:candidates = JSON_DECOYSfixes the type as a tuple of two strings, and line 140 assigns a tuple of four.- Without
--judge, the blocks[judge, ¬Lyon]and{0.5, judge + Paris}now giveinconclusive. With a judge that says "yes" to everything, the second block is stillblocked. - The repros from the earlier rounds give the same results on 04c4541; no regressions.
Checked, no issues
- Partly scored blocks: both directions are covered, a false
passand a falseblocked. Without a threshold, a failed scored check still counts, which is right, since all checks must pass there. - Stuffing for
jsonwithcontains: the check reads the first JSON in the reply, and the rejected decoy always comes first. - Docs, schema and changelog: they match the code.
| mode, location = str(check.get("mode", "any")), str(check.get("location", "end")) | ||
| if not values or (len(values) > 1 and mode == "all"): | ||
| return None | ||
| text = str(values[0]) |
There was a problem hiding this comment.
Must-fix: a numeric target's raw spelling is not always a valid numeric canary. For run([{"id": "1", "question": "Q?", "grading": {"checks": [{"type": "match", "value": "-.5", "numeric": true}]}}]), the canary is The answer is -.5.. The grader parses the target as -0.5 but its answer-number regex extracts positive 5, so this run is blocked solely for rejecting the generated canary even though -0.5 passes the correct grader. Canonicalize numeric targets into an answer spelling the numeric matcher recognizes before constructing the canary (for begin/end/any locations), and add a regression for this case.
There was a problem hiding this comment.
Confirmed: the number regex reads the 5 of -.5 as a positive 5, so a correct grader rejected the canary The answer is -.5..
Fixed in 3a4cf5b: a numeric match canary now spells the parsed number in plain decimal (-0.5). Regression tests cover -.5 and +.5 at end, begin, any and exact, both as the canary and as a run.
| if _whole_answer(check) and not _reads_first_json(check): | ||
| return " ".join(values) |
There was a problem hiding this comment.
Must-fix: the bare list is not necessarily different from every accepted value after normalization. For {"checks": [{"type": "exact", "values": ["b", "a"]}]}, the canary b passes, but stuffing produces b a; squad_normalize removes the article a, so the correct grader accepts it as b. This incorrectly reports a stuffed violation and returns blocked/exit 1. Check the synthesized list with grade_check and skip it when the positive check accepts it, rather than assuming concatenation cannot equal a target. Add this case as a regression.
There was a problem hiding this comment.
Confirmed: b a normalizes to b (the article goes), so the exact check accepts the stuffed answer and the run was blocked.
Fixed in 3a4cf5b: a bare list that the check itself accepts is no stuffed probe any more, so it is skipped. Regression test for [b, a].
| decoys = [w for w in candidates if not grade_check(check, "", w, None).passed] | ||
| return f"{decoys[0]} {' '.join(values)} {decoys[1]}" if len(decoys) >= 2 else None |
There was a problem hiding this comment.
Must-fix: rejecting each decoy in isolation does not ensure the assembled answer is rejected. For {"checks": [{"type": "match", "values": ["Paris QZX-BOGUS-REPLY-4096", "Paris"]}]}, both selected decoys fail individually, but the stuffed answer ends in Paris QZX-BOGUS-REPLY-4096 (the last listed value plus the trailing decoy). The correct end-match grader therefore accepts it and this command incorrectly reports blocked. The same context problem occurs with leading-space values and strip: false. Validate the complete candidate with grade_check before treating it as a rejection probe, trying other decoy pairs or skipping when none is clean; add a regression for a value spanning the list/decoy boundary.
There was a problem hiding this comment.
Confirmed: each decoy was rejected alone, but Paris QZX-BOGUS-REPLY-4096 ends the stuffed answer and is itself a value, so the correct grader accepted it.
Fixed in 3a4cf5b: the stuffed answer is built from the first decoy pair whose complete answer the check rejects (checked with the grader, not assumed). When no pair is clean, no stuffed probe is built. Regression tests cover the case with strip true and false.
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at 3a4cf5b. Everything from the earlier rounds is fixed, and mypy is green now. One false blocked is left, from a negated numeric value. I also have one question about whether the positive stuffed probe can ever fire.
Agreed with @exai-bot: the negated -.5 / +.5 stuffed answer is a false blocked. I added a general fix in its thread.
What I ran, at 3a4cf5b:
make ci-local --offline: 10,745 passed, 0 failed, 57 skipped. Every row passed or was skipped for the network,typecheck / mypyand the CLI surface check included.- Numeric cases through
run(). The negated-.5atendand+.5atbeginareblockedonstuffed. The positive-.5canary is now-0.5and passes, atendand atexact.1e-15,1e20and a long decimal also pass. - The repros of the earlier rounds give the same results.
Checked, no issues
- Stuffed self-checks: a bare list that the check accepts (
b a) is skipped. Decoy pairs are tried until the whole answer is rejected, so a value across the list/decoy boundary no longer blocks. - Earlier review threads: the decoy filter, the two-decoy fallback and the any-probe rule now have tests, and the docstring names the unmeasured case.
| bare = " ".join(values) | ||
| # The list is no attack where the grader's own rules accept it (``b a`` is ``b`` once the article goes). | ||
| return None if grade_check(check, "", bare, None).passed else bare | ||
| candidates: tuple[str, ...] | ||
| if _reads_first_json(check): | ||
| candidates = JSON_DECOYS | ||
| elif check.get("type") != "match" or check.get("numeric") or str(check.get("location", "end")) == "any": | ||
| return None | ||
| else: | ||
| candidates = WRONG_CANDIDATES | ||
| decoys = [w for w in candidates if not grade_check(check, "", w, None).passed] | ||
| # Each decoy is rejected alone, but the whole answer can still end (or begin) with a value: the first | ||
| # decoy pair whose complete answer the check rejects is used, and none is built when no pair is. | ||
| for before, after in permutations(decoys, 2): | ||
| stuffed = f"{before} {' '.join(values)} {after}" | ||
| if not grade_check(check, "", stuffed, None).passed: | ||
| return stuffed | ||
| return None |
There was a problem hiding this comment.
After 3a4cf5b the positive stuffed probe can never be a violation: it is built only when grade_check rejects it, and then graded by the same check.
A block with one check gives the same result from grade() as from grade_check. The only exception is a threshold of 0, and there wrong already blocks the run. So stuffed N scored 0 accepted in the table reads as a measured protection, but it comes from how the probe is built, not from the block. Since the first round, every stuffed violation from a correct grader was a false blocked.
Suggestion: drop the positive stuffed probe, or keep it out of the table and the rates (it is a self-test of grade_check, not of the block). If keyword stuffing should be measured, it is the location: any / fuzzy case (a right value listed among wrong ones), reported as a warning, which this PR skips by design.
There was a problem hiding this comment.
Waiting on a decision before I change this: drop the positive stuffed probe from the attack table and the rates, or keep it out of them and keep only the grade_check self-test? Context: you are right that for a single-check block grade() and grade_check() agree, so stuffed 0 accepted measures nothing. My recommendation: drop it from the table and the rates, and keep the self-test as a unit test. The thread stays open until then.
There was a problem hiding this comment.
Done in d542245 (tests in 301849e). The positive stuffed probe is no longer probed or rated: probe() skips it for a block with a positive check, so it is out of the table, the rates and the artifact. It stays as the grade_check self-test (stuffed_for), and the tests assert it directly. The negated stuffed probe is unchanged: it is the only probe of a block with only negated checks, which would otherwise be inconclusive. The docs and the changelog say so. Leaving the thread open for you to confirm.
There was a problem hiding this comment.
Removed stuffed from both branches, as recommended: it is out of the attack table, the rates, the artifact, the docs and the changelog. stuffed_for stays as a grade_check self-check, covered by unit tests only.
Fixed in 81fcc17. One verdict changes: a single negated check with threshold 0 or below was blocked by its stuffed probe and is now inconclusive (exit 1 in both cases).
|
The negated |
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
make cli-snapshot after the merge of intel/main: the snapshot on main does not list the redteam subcommand this branch adds. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…decoy pair that completes a value
- A numeric match canary spells the number the grader parsed: the number regex reads the
"5" of "-.5" with no sign, so a correct grader rejected the canary.
- A bare list the grader accepts as a value ("b a" is "b" once the article goes) is no
stuffed attack; the probe is skipped.
- A stuffed list between two decoys is built from the first decoy pair whose whole answer
the check rejects, so a value no longer ends the answer ("Paris QZX-BOGUS-REPLY-4096").
Regression tests for each: 12 new cases in tests/test_redteam.py.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> # Conflicts: # dokimi/dokimi/artifacts.py
`--judge PROVIDER/MODEL` replaces the `--judge` switch with `--judge-model` and `--judge-provider`, which are banned spellings in the flag registry; `--out FILE` replaces `-o/--output`. A malformed `--judge` is a usage error (exit 2). The route table lists `eval redteam` as a paid leaf, and the CLI snapshot is regenerated. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…not stuffed The numeric matcher reads "5" out of "-.5", so a negated check on "-.5" accepts the stuffed answer as it should, and the run reported a false `blocked`. The negated stuffed answer is now built only where the same check without `negate` accepts it. Regressions: "-.5" at end and "+.5" at begin build no probe, and "7731" is still stuffed. Thread: PRRT_kwDOVBg7g86yKAAM Thread: PRR_kwDOVBg7g88AAAABVPIvTg Thread: IC_kwDOVBg7g88AAAABhDPe6A Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
eval redteam is now in the route table as a leaf, so the snapshot lists it. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The command registers only --out, and the flag registry bans -o. Thread: PRRT_kwDOVBg7g86yMEzK Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The runs scan and show tests check every schema version has a row. redteam.v1 was added by this PR without one, so the Tests and Windows smoke jobs failed. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
A positive check's stuffed answer is built only when that check rejects it, and the same check then grades it, so it could never be a violation and its rate measured nothing. Keep it as a grade_check self-test; probe and rate only the negated one, which is the only probe of a block that has nothing else. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
301849e to
0ad171b
Compare
A positive block's stuffed answer is built only when grade_check rejects it,
and its one check then grades it, so it can never be a violation. A negated
block's stuffed answer was its only probe, and the block stays unmeasured
("only negated checks") either way. The one status it could change is a
single negated check with threshold 0 or below: blocked becomes inconclusive,
both exit 1.
A grader that ignores location (match Paris, Lyon at end) passed the run
with the stuffed row at scored 0, accepted 0.
Remove stuffed from ATTACKS, the attack table, the rates, the artifact schema
and fixture, the docs and the changelog. stuffed_for stays as a grade_check
self-check that the unit tests assert directly.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| mode, location = str(check.get("mode", "any")), str(check.get("location", "end")) | ||
| if not values or (len(values) > 1 and mode == "all"): | ||
| return None | ||
| text = str(values[0]) |
There was a problem hiding this comment.
Must-fix: choosing the first alias unconditionally can falsely block a correct grader. For run([{"id": "1", "question": "Q?", "grading": {"checks": [{"type": "exact", "values": ["a", "b"]}]}}]), validation succeeds, but the canary is a. The exact matcher drops that article during SQuAD normalization and explicitly rejects empty normalized targets, so redteam reports a rejected-canary violation even though b is an accepted answer. Choose a usable alias when constructing the canary (or leave it unbuilt if none is usable), and add a regression for this alias order; the existing ["b", "a"] test does not cover it.
There was a problem hiding this comment.
Fixed in 69e3449. An exact canary now takes the first alias that normalizes to a non-empty string; if none does, no canary is built. A correct grader with article-only aliases is no longer blamed for its canary. Pinned by test_an_exact_canary_skips_an_alias_that_normalizes_to_nothing and test_a_correct_grader_with_an_article_alias_is_not_blamed_for_its_canary (both red before the fix).
|
|
||
| def _accepts(grading: Mapping[str, Any], text: str) -> bool: | ||
| """Whether a non-negated literal check of the block accepts the text on its own, by the grader's rules.""" | ||
| return any(grade_check(c, "", text, None).passed for c in _positive_checks(grading) if c.get("type") in _LITERAL) |
There was a problem hiding this comment.
Must-fix: ignore zero-weight checks when deciding whether an attack clashes with an accepted value. For a block with checks: [{"type": "regex", "value": ".+"}, {"type": "match", "values": ["-", "certain"], "location": "any", "ignore_punctuation": false, "weight": 0}], all planted candidates match - and the hedge matches certain, so both probes are skipped. Only the empty probe runs and fails, causing run() to report pass, even though the only counted check accepts every nonempty answer. Grade.counted explicitly excludes weight-zero checks. Apply that same constraint here and add a regression asserting this block is blocked.
There was a problem hiding this comment.
Fixed in 69e3449. The clash test for an attack now counts only checks with weight > 0, so a zero-weight regex no longer stops a wrong answer from being probed. Pinned by test_a_zero_weight_check_does_not_stop_a_wrong_answer_from_being_probed (red before the fix).
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at 81fcc17. Both points of my last round are settled: the negated -.5 false block is fixed, and then the stuffed probe was dropped as I suggested. The open points left are @exai-bot's two threads below.
Agreed with @exai-bot: a zero-weight check that accepts every planted answer silences wrong and hedge, so a block that accepts anything gets pass; the canary built from an article alias (exact [a, b]) is rejected and blocks a correct grader.
What I ran, at 81fcc17:
make ci-local --offline: 11,365 passed, 0 failed, 58 skipped. One row fails,schema_compat vs the PR base, onharness_profile.v1.json. This PR does not touch that schema: the newermainchanged it after this branch's base.- Both @exai-bot repros through
run().{regex .+, match [-, certain] weight 0}givespasswith onlyemptyrun, while the regex alone isblockedon wrong and hedge.{exact, [a, b]}isblockedon its canarya. - The repros of the earlier rounds on this head give the same results.
Checked, no issues
- Stuffed removal:
stuffedis out ofATTACKS, the table, the rates, theredteam.v1attack enum (the schema is new in this PR, so narrowing it breaks nothing), the fixture, the docs and the changelog. - CLI flags:
--judge PROVIDER/MODELand--out FILEfollow the flag registry. A malformed--judgeexits 2, the route table listseval redteamas paid, and the CLI snapshot is current. Keeping-oout is consistent with--outbeing the canonical spelling. - Runs kinds:
redteam.v1is listed inruns/kinds.py, soruns listandruns showsee the artifact.
| def stuffed_for(grading: Mapping[str, Any]) -> Optional[str]: | ||
| """The block's one literal check's values, listed with no sentence around them; None when not built. | ||
|
|
||
| Not an attack: no probe uses it. It is a self-check of ``grade_check`` that the unit tests assert. | ||
|
|
||
| A positive check needs two or more values and must accept any one of them (mode ``any``): a block | ||
| that needs every value, or several checks, is satisfied by the stuffed answer by design, so | ||
| accepting it is not a violation. A single positive value listed alone is the right answer itself, | ||
| not stuffing. The list is an attack only where the check does not read it: | ||
|
|
||
| - a check on the whole answer (``exact``, ``json`` without ``contains``, ``match`` at ``exact``): the bare list; | ||
| - a ``match`` at ``begin`` or ``end``: the list between two decoys the check rejects, so no accepted | ||
| value is at the place the check reads; | ||
| - a ``json`` check with ``contains`` reads the first JSON in the text, so the list goes after two JSON | ||
| decoys the check rejects: a bare list would start with a listed value and be accepted; | ||
| - ``match`` at ``any``, a numeric ``match`` and ``fuzzy`` find a listed value wherever it is, so the | ||
| grader is right to accept it and no probe is built. | ||
|
|
||
| A negated check is stuffed with its one or more values: the stuffed answer carries the forbidden | ||
| value, so it must fail, unless the check compares the whole answer (see ``_whole_answer``), which the | ||
| listed values never equal, or is ``mode: all``, which the stuffed answer can pass by its location | ||
| (the skip is kept for every such check, for simplicity), and the list is built only where the same | ||
| check without ``negate`` accepts it, so a value the grader reads as another one is no attack. | ||
| A choice check is left out: its answer is a bare letter. | ||
| """ | ||
| checks = grading.get("checks", []) | ||
| if len(checks) != 1 or checks[0].get("type") not in _LITERAL - {"choice"}: | ||
| return None | ||
| check = checks[0] | ||
| values = [str(v) for v in check_values(check)] | ||
| mode = str(check.get("mode", "any")) | ||
| if check.get("negate"): | ||
| bare = " ".join(values) | ||
| built = bool(values) and not _whole_answer(check) and mode != "all" | ||
| # Built only when the grader reads the value out of the list, as the positive check does. | ||
| carries = grade_check({**check, "negate": False}, "", bare, None).passed | ||
| return bare if built and carries else None | ||
| if len(values) < 2 or mode == "all": | ||
| return None | ||
| if _whole_answer(check) and not _reads_first_json(check): | ||
| bare = " ".join(values) | ||
| # The list is no attack where the grader's own rules accept it (``b a`` is ``b`` once the article goes). | ||
| return None if grade_check(check, "", bare, None).passed else bare | ||
| candidates: tuple[str, ...] | ||
| if _reads_first_json(check): | ||
| candidates = JSON_DECOYS | ||
| elif check.get("type") != "match" or check.get("numeric") or str(check.get("location", "end")) == "any": | ||
| return None | ||
| else: | ||
| candidates = WRONG_CANDIDATES | ||
| decoys = [w for w in candidates if not grade_check(check, "", w, None).passed] | ||
| # Each decoy is rejected alone, but the whole answer can still end (or begin) with a value: the first | ||
| # decoy pair whose complete answer the check rejects is used, and none is built when no pair is. | ||
| for before, after in permutations(decoys, 2): | ||
| stuffed = f"{before} {' '.join(values)} {after}" | ||
| if not grade_check(check, "", stuffed, None).passed: | ||
| return stuffed | ||
| return None | ||
|
|
There was a problem hiding this comment.
Nit: stuffed_for and its helpers (JSON_DECOYS, _reads_first_json, the permutations import) no longer have a caller in the package after 81fcc17; only tests/test_redteam.py uses them. About 60 lines of shipped code, with a long docstring about an attack that is gone, now only test themselves. Suggestion: delete them with their tests, or move the self-check into the test file if it is worth keeping.
…and dead helpers - An exact canary now takes the first alias that normalizes to a non-empty string; if none does, no canary is built, so a correct grader with article-only aliases is not blamed (Thread: PRRT_kwDOVBg7g86yT82e). - The clash test for an attack ignores checks with weight <= 0, so a zero-weight regex no longer stops a wrong answer from being probed (Thread: PRRT_kwDOVBg7g86yT82f). - stuffed_for, its JSON decoys and the permutations import had no caller in the package; they move into the tests (Thread: PRRT_kwDOVBg7g86yVQqQ). - Typing imports for the test helpers (ruff F821). Thread: PRRT_kwDOVBg7g86yT82e Thread: PRRT_kwDOVBg7g86yT82f Thread: PRRT_kwDOVBg7g86yVQqQ Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| g = grade(grading, question, answer, ask) | ||
| # A check left unrun (no judge, or a regex past its timeout) makes a pass on the rest no measurement; | ||
| # with a threshold it is no result either, so the score is taken over the counted checks alone. | ||
| partial = bool(g.unjudged or g.unrun) |
There was a problem hiding this comment.
Must-fix: exclude zero-weight checks when deciding whether a probe is partial. For a block with threshold: 0.5 and checks: [{"type": "regex", "value": ".*"}, {"type": "judge", "criterion": "names Paris", "weight": 0}], running without a judge returns inconclusive with all three attacks unscored instead of blocked. Grade.counted excludes the judge regardless of its verdict, so the counted regex definitively accepts every attack; the missing zero-weight verdict cannot affect that result. Filter g.unjudged and g.unrun by positive weight here, and add a regression for this configuration.
Suggestion:
| partial = bool(g.unjudged or g.unrun) | |
| partial = any(c.weight > 0 for c in (*g.unjudged, *g.unrun)) |
There was a problem hiding this comment.
Fixed in 004d6fe. A check makes a probe partial only when its weight is above 0, so a zero-weight judge with no judge model no longer leaves wrong and hedge unscored.
Regression tests: test_a_zero_weight_judge_without_a_judge_does_not_leave_the_wrong_answer_unscored and test_a_zero_weight_judge_under_a_threshold_does_not_make_a_blocked_run_inconclusive.
There was a problem hiding this comment.
🟡 Changes recommended
Critical probe-handling defects can falsely certify unusable graders, while zero-weight checks and incorrectly normalized fuzzy canaries cause additional erroneous gating outcomes.
4 open issues
What changed in this PR
4 new findings.
This PR adds eval redteam, a measurement gate for #220 that probes question grading blocks before they are trusted for evaluation or optimization.
Changes:
- Adds synthetic canary, wrong, empty and hedge probes, with optional model judging and explicit result statuses.
- Introduces the
redteam.v1artifact and integrates it with run discovery and display. - Adds regression tests, CLI documentation and release notes.
| File | Description |
|---|---|
dokimi/tests/test_runs_show.py |
Verifies redteam artifact display counts. |
dokimi/tests/test_runs_scan.py |
Verifies redteam artifact discovery. |
dokimi/tests/test_redteam.py |
Tests probes, reporting, judging and CLI behavior. |
dokimi/tests/test_cli_routes.py |
Includes the new command route. |
dokimi/tests/fixtures/artifacts/redteam.v1.json |
Provides a representative blocked-run artifact. |
dokimi/dokimi/schemas/redteam.v1.json |
Defines the persisted report format. |
dokimi/dokimi/runs/kinds.py |
Registers redteam metadata and counts. |
dokimi/dokimi/eval/reference_checks.py |
Exposes the shared check-values helper. |
dokimi/dokimi/eval/redteam.py |
Implements probes, statuses and reporting. |
dokimi/dokimi/eval/judge.py |
Extracts a reusable judge callback. |
dokimi/dokimi/commands/redteam.py |
Adds command arguments, output and exit handling. |
dokimi/dokimi/commands/evaluate.py |
Registers eval redteam. |
dokimi/dokimi/commands/answer_formats.py |
Reuses the shared judge callback. |
dokimi/dokimi/cli_routes.py |
Declares the new CLI route. |
dokimi/dokimi/artifacts.py |
Registers redteam.v1. |
dokimi/docs/reference/cli-interface.json |
Records the updated CLI interface. |
dokimi/docs/answer-formats.md |
Documents probes, statuses and judging. |
dokimi/changelog.d/20261028_120100_napetrov_feat-verifier-redteam.md |
Announces the command and artifact. |
Review effort: Balanced
| usable = [str(v) for v in values if squad_normalize(str(v))] | ||
| return usable[0] if usable else None |
There was a problem hiding this comment.
Fixed in 004d6fe. An exact check whose values all normalize to nothing now gets a rejected canary, so the run is blocked and not certified. Usable aliases are still skipped as before.
Test: test_an_exact_check_whose_values_all_normalize_to_nothing_is_a_rejected_canary_not_a_pass.
| g = grade(grading, question, answer, ask) | ||
| # A check left unrun (no judge, or a regex past its timeout) makes a pass on the rest no measurement; | ||
| # with a threshold it is no result either, so the score is taken over the counted checks alone. | ||
| partial = bool(g.unjudged or g.unrun) |
There was a problem hiding this comment.
Fixed in 004d6fe. Zero-weight entries no longer decide whether a probe is partial, for judge checks and for regex checks past their timeout.
Tests: test_a_zero_weight_judge_without_a_judge_does_not_leave_the_wrong_answer_unscored and test_a_zero_weight_regex_past_its_timeout_does_not_leave_the_wrong_answer_unscored.
| return f"{text} is the answer." | ||
| if kind == "match" and location == "end" and not check.get("ignore_punctuation", True): | ||
| return f"The answer is {text}" # the full stop would be the raw end of the answer, not the value | ||
| return f"The answer is {text}." |
There was a problem hiding this comment.
Fixed in 004d6fe. A fuzzy canary is the bare value, so it normalizes the way the answer does.
Test: test_a_fuzzy_canary_is_the_bare_value_so_the_grader_accepts_it.
| asked with the grader's own rules, so a numeric match that reads a number out of the planted answer, | ||
| or a short value the check does not find in the hedge, are judged as the grader judges them). | ||
| """ | ||
| applies = bool(_positive_checks(grading)) |
There was a problem hiding this comment.
Fixed in 004d6fe. Wrong, empty and hedge need a non-negated check with weight above 0. A block with only a zero-weight exact and a negated match is inconclusive, not blocked.
Test: test_a_zero_weight_positive_check_does_not_give_a_negated_block_the_wrong_answer_probe.
…unusable exact is a canary violation A zero-weight judge or regex left unrun no longer makes a wrong answer unscored. A block with no positive-weight non-negated check gets no wrong, empty or hedge probe. A fuzzy canary is the bare value; an exact check whose values all normalize to nothing is reported as a rejected canary instead of passing. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| reads a number out of the planted answer, or a short value the check does not find in the hedge, are | ||
| judged as the grader judges them). | ||
| """ | ||
| applies = any(float(c.get("weight", 1.0)) > 0 for c in _positive_checks(grading)) |
There was a problem hiding this comment.
Must-fix: unsupported checks are never counted by Grade, but this condition treats them as positive checks. For run([{"id": "1", "grading": {"checks": [{"type": "unsupported", "native": "code assertion"}, {"type": "match", "value": "Paris", "negate": true}]}}]), wrong, empty and hedge all become violations and the run is blocked. The only counted check is negated and accepts those answers by design, so this should be inconclusive, just like the covered zero-weight-positive case. Exclude unsupported checks when deciding whether rejection probes apply, and add a regression for this combination.
There was a problem hiding this comment.
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 004d6fe. All four of my earlier threads are fixed, and mypy is green now. Two things are left before merge, both inline: an unsupported check still turns a negated block into a false blocked, and the new unusable_exact guard for negated checks is not pinned by a test.
What I ran, on a Linux review host at 004d6fe:
make ci-local: 30 passed, 0 failed, 7 skipped; 12,097 tests passed, 59 skipped; changed-line coverage 98% (the two uncovered lines are incommands/answer_formats.py, a refactor line this PR only touches).- Reproductions for the code findings through
run(), with no model call. - A 32-mutant run on
eval/redteam.pyagainsttests/test_redteam.py: 27 killed, 5 pass the full suite. One of them decides a verdict (inline). - Commits: all non-merge commits carry
Signed-off-by.
Checked, no issues
- Zero-weight handling: the
weight > 0filters inattacks,_acceptsand the partial-probe rule each have a test that fails when the filter is loosened to>= 0, dropped, or applied to the wrong side. - Canary and exact aliases: a fuzzy canary is the bare value; an exact check with article-only aliases gives a rejected canary and
blocked, and[a, b]builds thebcanary and passes. - Dropped
stuffedattack: it is gone fromATTACKS, the table, the rates, the schema, the fixture and the docs;-ois not documented anywhere.
Bot points I checked that no longer apply at this head (11)
- exai-bot's point on the numeric canary for
-.5does not apply:canary_forspells the parsed number in plain decimal (redteam.py:68). - exai-bot's point on the stuffed list
b aforexact [b, a]does not apply: the stuffed attack and its builder are gone from the package. - exai-bot's point on a decoy pair that does not reject the whole stuffed answer does not apply: no stuffed answer is built any more.
- exai-bot's point on
-onot being registered does not apply:--outis the canonical flag, and the docs, changelog and tests name--outonly. - exai-bot's point on choosing the first exact alias does not apply: the canary takes the first alias that normalizes to a non-empty string (
redteam.py:65). - exai-bot's point on zero-weight checks silencing the wrong and hedge probes does not apply:
_acceptscounts only checks with weight above 0 (redteam.py:109). - exai-bot's point on a zero-weight judge making a probe partial does not apply: the partial rule counts only unrun checks with weight above 0 (
redteam.py:165). - Copilot's point on an exact check whose aliases all normalize to nothing passing does not apply:
unusable_exactreports a rejected canary, so the run isblocked. - Copilot's point on a zero-weight judge or regex leaving wrong and hedge unscored does not apply: zero-weight entries no longer make a probe partial.
- Copilot's point on the fuzzy canary
The answer is a.does not apply: a fuzzy canary is the bare value (redteam.py:77). - Copilot's point on a zero-weight positive check enabling the probes for a negated block does not apply:
appliesneeds a non-negated check with weight above 0 (redteam.py:129).
| reads a number out of the planted answer, or a short value the check does not find in the hedge, are | ||
| judged as the grader judges them). | ||
| """ | ||
| applies = any(float(c.get("weight", 1.0)) > 0 for c in _positive_checks(grading)) |
There was a problem hiding this comment.
An unsupported check counts as a positive check here, so a negated block with one gets a false blocked.
Grade never counts an unsupported check, but applies at redteam.py:129 only looks at negate and weight. Input: run([{"id": "1", "question": "Q?", "grading": {"checks": [{"type": "unsupported", "native": "code assertion"}, {"type": "match", "value": "Paris", "negate": true}]}}]).
- Observed:
statusisblocked, with violations onwrong,emptyandhedge. The only counted check is the negated one, which accepts those answers by design. - Without the
unsupportedcheck the same block isinconclusive(negated_only: 1). The earlier fix for zero-weight positive checks covers the same case in the other form.
exai-bot raised the same point.
Suggestion: count only checks the grader counts: applies = any(float(c.get("weight", 1.0)) > 0 and c.get("type") != "unsupported" for c in _positive_checks(grading)). Add a regression for this block expecting inconclusive and no violations.
There was a problem hiding this comment.
Fixed in 29a65fe, same change as the exai-bot thread: applies excludes unsupported. The block gives inconclusive with no violations, pinned by test_an_unsupported_check_does_not_give_a_negated_block_the_rejection_probes.
| def unusable_exact(grading: Mapping[str, Any]) -> bool: | ||
| """Whether the block is one counted exact check whose values all normalize to nothing, so it rejects every answer.""" | ||
| checks = grading.get("checks", []) | ||
| if len(checks) != 1 or checks[0].get("type") != "exact" or checks[0].get("negate"): |
There was a problem hiding this comment.
No test pins the negate guard of unusable_exact, and it decides blocked against inconclusive.
Mutation check: dropping or checks[0].get("negate") from the condition at redteam.py:99 passes the full test_redteam.py. With the mutation, {"checks": [{"type": "exact", "value": "the", "negate": true}]} gets an injected rejected canary and the run is blocked (violation: canary, passed: False). At the head it is inconclusive with no violations. A negated check builds no canary, so a negated exact check with an article-only value must not be reported as one.
Suggestion: add a test with this block and assert status == "inconclusive" and violations == [].
There was a problem hiding this comment.
Added in 29a65fe: test_a_negated_exact_check_of_an_article_is_inconclusive_not_given_a_canary pins the negate guard in unusable_exact. Mutant check: dropping or checks[0].get("negate") fails this test (run is blocked, canary violation), so the guard is covered.
A negated block with an unsupported check got wrong, empty and hedge probes, because applies counted the unsupported check as positive. Grade never counts it, so the block is inconclusive, as without it. Adds regression tests for that block and for the negate guard in unusable_exact (a negated article-only exact gets no canary). Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| and it has no probe at all. The table names every question that was not | ||
| measured and counts it as `unprobed`; only a run that measured every graded |
There was a problem hiding this comment.
Nit: The table does not name every unmeasured question. format_table() displays only the first 20 and appends “and N more”; for a set of 21 judge-only questions without --judge, the last question's ID appears only in the artifact. Say that the table lists up to 20 unmeasured questions and that --out FILE preserves the complete list.


Summary
Part of #220.
eval redteam QUESTIONS.jsonchecks that each question'sgradingblock rejects the answers it should. It grades four synthetic answers per question:match,fuzzy,exact,choice,json). It must pass.A
negatecheck does not switch the other attacks off: wrong, empty and hedge grade the block's non-negated checks. A block of only negated checks gets none of the three and is reported asnegated_only.Statuses:
pass,blocked(an attack was accepted or a canary was rejected),inconclusive(a graded question was not measured, or nothing was graded). Thestuffedanswer was dropped as an attack (it could not be a violation for a positive block, and a negated block is unmeasured either way);stuffed_forstays as agrade_checkself-check with unit tests. Exit 0 on pass, 1 on blocked or inconclusive, 2 on an unreadable question set or an unwritable--out.--outwritesredteam.v1. Judge checks are unscored unless--judgeis given, so the default run makes no model call. No score changes.Evidence
match 31,match 1,fuzzy on) no longer blocks a correct grader; negated checks no longer silence the attacks on a positive check; a single negated value builds its self-check; a negated whole-answer check builds none; a check value in the hedge skips the hedge probe; the canary formatchat the end withignore_punctuation: falsehas no full stop; a run that probed nothing is inconclusive; an unwritable--outexits 2; the summary counts unprobed questions.FRESH-REVIEW-A2, diff e2147b1...04f2142; its quoted line numbers match 04f2142, not 71cc8e6) found three blockers, all fixed in 6e16c6b: a wrong answer that contained a check value blocked correct graders; stuffing a negatedexact,jsonorlocation: exactcheck blocked correct graders; a block with one negated value was never probed. In-session review found the negated-check silencing (fixed in 04f2142) and the hedge clash (fixed in 068abc8).make ci-local --offline): ruff, mypy (273 files), deptry, bandit, schema and registry checks, mkdocs and the CLI-surface job (which reports the addedeval redteam) pass. Pytest: 9735 passed, 5 skipped, 1 failed:test_container_agent.py::test_probe_reports_a_writable_docker_socket(AF_UNIX path too long). That test depends on the path length of the worktree: it failed in these/localdisk/napetrov/scratchworktrees and on a clean intel/main checkout (earlier run), and passed in the shorter base worktree.mode: allfix on e881ab5: a negatedmatchwithmode: alland the defaultlocation: endgot the stuffed answerParis Lyon. That answer ends with one value only, so the all-match is false and the negation passes it: the real grader accepted it and a correct grader was blocked. The stuffed probe is now skipped for negatedmode: all, as the positive rule already skipsmode: all. Red then green: the two new cases failed on 6e16c6b (2 failed, 41 passed) and pass on e881ab5 (43 passed).ruff checkandmypypass. A fresh-contextclaude -preview of6e16c6bb..e881ab57plusstuffed_forfound no blocker; it confirmed the grader rejects every stuffed answer that is still built, and its two minor points (the skip is wider than the one failing case; the docstrings overstated) are addressed.tests/test_redteam.py), all passing.Merge Danger
eval redteamand new persisted formatredteam.v1(schema, fixture, registry entry). Both are new in this PR.eval redteamnow fails when a grader accepts a wrong answer, and when a question set has nothing gradeable (for example, judge checks only, without--judge).threshold0 or below used to be blocked by its stuffed probe (the grader passes everything); with the probe gone it isinconclusive, exit 1 either way."AB"): the canaryANSWER: ABis rejected by the grader, so the run blocks. This is a grader issue invalidate_grading, not in the red team.--judge, judge checks call the model. The wrong answer is a fixed sentinel string, not one written for each question.🤖 Generated with Claude Code