Skip to content

feat(eval): add eval redteam to probe grading checks with planted answers - #281

Open
napetrov wants to merge 52 commits into
intel:mainfrom
napetrov:feat/verifier-redteam
Open

napetrov wants to merge 52 commits into
intel:mainfrom
napetrov:feat/verifier-redteam

Conversation

@napetrov

@napetrov napetrov commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Part of #220.

eval redteam QUESTIONS.json checks that each question's grading block rejects the answers it should. It grades four synthetic answers per question:

  • canary: the right answer, built from one literal check (match, fuzzy, exact, choice, json). It must pass.
  • wrong: a planted wrong answer that carries no positive check value. It must fail.
  • empty: the empty answer. It must fail.
  • hedge: a non-committal sentence with no check value. It must fail.

A negate check 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 as negated_only.

Statuses: pass, blocked (an attack was accepted or a canary was rejected), inconclusive (a graded question was not measured, or nothing was graded). The stuffed answer was dropped as an attack (it could not be a violation for a positive block, and a negated block is unmeasured either way); stuffed_for stays as a grade_check self-check with unit tests. Exit 0 on pass, 1 on blocked or inconclusive, 2 on an unreadable question set or an unwritable --out. --out writes redteam.v1. Judge checks are unscored unless --judge is given, so the default run makes no model call. No score changes.

Evidence

  • Red then green: each new test failed before its fix and passes after. The regressions cover: a planted wrong answer that contains a check value (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 for match at the end with ignore_punctuation: false has no full stop; a run that probed nothing is inconclusive; an unwritable --out exits 2; the summary counts unprobed questions.
  • Fresh-context review (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 negated exact, json or location: exact check 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).
  • Local CI on 6e16c6b (make ci-local --offline): ruff, mypy (273 files), deptry, bandit, schema and registry checks, mkdocs and the CLI-surface job (which reports the added eval 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/scratch worktrees and on a clean intel/main checkout (earlier run), and passed in the shorter base worktree.
  • Negated mode: all fix on e881ab5: a negated match with mode: all and the default location: end got the stuffed answer Paris 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 negated mode: all, as the positive rule already skips mode: all. Red then green: the two new cases failed on 6e16c6b (2 failed, 41 passed) and pass on e881ab5 (43 passed). ruff check and mypy pass. A fresh-context claude -p review of 6e16c6bb..e881ab57 plus stuffed_for found 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.
  • The redteam tests: 112 cases at 92830e84 (tests/test_redteam.py), all passing.

Merge Danger

  • New command eval redteam and new persisted format redteam.v1 (schema, fixture, registry entry). Both are new in this PR.
  • Exit 1 on a blocked or inconclusive run. A CI job that runs eval redteam now fails when a grader accepts a wrong answer, and when a question set has nothing gradeable (for example, judge checks only, without --judge).
  • Known limits, each confirmed by a probe on the final code (the threshold-block pass and the negated-check false block listed in earlier rounds are fixed: c09f9b6, 3a4cf5b):
    • A regex that matches the hedge text is not skipped; only literal values are checked for a clash with the hedge.
    • A single negated check with threshold 0 or below used to be blocked by its stuffed probe (the grader passes everything); with the probe gone it is inconclusive, exit 1 either way.
    • A choice check with several letters ("AB"): the canary ANSWER: AB is rejected by the grader, so the run blocks. This is a grader issue in validate_grading, not in the red team.
    • Exit 1 when any graded question is not measured, even if others are probed. The table and the summary name the unmeasured questions.
    • With --judge, judge checks call the model. The wrong answer is a fixed sentinel string, not one written for each question.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the cli-interface-change Changes dokimi's user-facing CLI surface label Oct 29, 2026
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

CLI interface change detected

This 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 cli-interface-change label is on while the diff is non-empty.

@codecov-commenter

codecov-commenter commented Oct 29, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 98.97959% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.12%. Comparing base (b4a0a8c) to head (29a65fe).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
dokimi/dokimi/commands/answer_formats.py 0.00% 2 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
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     
Files with missing lines Coverage Δ
dokimi/dokimi/artifacts.py 95.12% <ø> (ø)
dokimi/dokimi/cli_routes.py 100.00% <ø> (ø)
dokimi/dokimi/commands/evaluate.py 90.00% <100.00%> (+0.06%) ⬆️
dokimi/dokimi/commands/redteam.py 100.00% <100.00%> (ø)
dokimi/dokimi/eval/judge.py 94.28% <100.00%> (+0.07%) ⬆️
dokimi/dokimi/eval/redteam.py 100.00% <100.00%> (ø)
dokimi/dokimi/eval/reference_checks.py 94.59% <100.00%> (+4.05%) ⬆️
dokimi/dokimi/runs/kinds.py 100.00% <ø> (ø)
dokimi/dokimi/commands/answer_formats.py 66.66% <0.00%> (+0.75%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vlad-perevezentsev vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 under match (any and end) and fuzzy is blocked on stuffed; {threshold: 0.5, Paris + negated London} is pass while the real grade() accepts the planted wrong and the empty answer; a negated-only block is pass; five judge questions without --judge plus one match question is pass with 5 of 6 unprobed.

Checked, no issues

  • Wiring: eval redteam is registered under eval, 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.v1 is new, listed in SCHEMA_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.

Comment thread dokimi/dokimi/eval/redteam.py Resolved
Comment thread dokimi/dokimi/eval/redteam.py Resolved
Comment thread dokimi/dokimi/eval/redteam.py Resolved
Comment thread dokimi/dokimi/eval/redteam.py Resolved
Comment thread dokimi/dokimi/eval/redteam.py Resolved
Comment thread dokimi/dokimi/eval/redteam.py Resolved
Comment thread dokimi/dokimi/commands/redteam.py Resolved
Comment thread dokimi/dokimi/eval/redteam.py Resolved
Comment thread dokimi/dokimi/eval/redteam.py Resolved

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 is pip-audit, which reports advisories on filelock in 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 redteam is 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: -o output validates against redteam.v1; the kind is in SCHEMA_VERSIONS; the fixture and the changelog fragment are present; mkdocs build --strict passes.

Comment thread dokimi/dokimi/eval/redteam.py Resolved
Comment thread dokimi/dokimi/eval/redteam.py Resolved
@vchamarthi

vchamarthi commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

🔍 OpenCodeReview found 2 issue(s) in this PR.

By severity: 1 high, 1 low

Severity Category Location Finding
high bug redteam.py:197-198 must-fix: a probe is recorded as accepted even when part of the block was never scored.
low bug redteam.py:34-36 nit: a deeply nested questions file makes json.loads in load_questions raise RecursionError.
Review details
  • Commit reviewed: c68a1f1 against merge base f0f95a2
  • Coverage: 12 files in 3 groups
  • Engine: OpenCodeReview v1.13.3, us.anthropic.claude-sonnet-5-5
  • Usage: 730k tokens (556k from cache), 1m18s
  • Tool calls: 34 (19 code_search, 14 file_read, 1 code_comment)
  • Redteam feature core: command, eval logic, schema, artifacts, tests, changelog: 6 files
  • Evaluate/judge integration with redteam and related test: 3 files
  • Answer formats docs and related command/reference check changes: 3 files

Comment thread dokimi/docs/answer-formats.md Resolved
Comment thread dokimi/docs/answer-formats.md Resolved
Comment thread dokimi/tests/test_redteam.py Resolved
Comment thread dokimi/tests/test_redteam.py Resolved
Comment thread dokimi/tests/test_redteam.py Resolved
@napetrov

Copy link
Copy Markdown
Contributor Author

The findings in this review are all confirmed on e881ab5 and fixed, each answered in its thread: aliases c1596ef, threshold 6b6aa2f, negated-only a296b5a, unprobed a296b5a, numeric wrong d96f87d, hedge d96f87d, judge fields 4d7cef6, shared helpers 70e66bd, -o line ef1ed29.

@napetrov

Copy link
Copy Markdown
Contributor Author

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 pip-audit failure it reports is on filelock in the lock files; this PR changes no lock file (git diff --name-only against main lists none).

@napetrov

Copy link
Copy Markdown
Contributor Author

The review has no body of its own; its five inline findings are answered in their threads: doc rows 89641b7 and 89641b7, command and empty-input tests 27828e4, judge tests 0918eea, artifact on blocked runs 1946f48.

@napetrov

Copy link
Copy Markdown
Contributor Author

Nothing to change here: the interface diff lists exactly what this PR adds, the command eval redteam with questions, --judge, --judge-model, --judge-provider and --output/-o. The judge fields of this round add no option.

@napetrov

Copy link
Copy Markdown
Contributor Author

Covered by the tests of 27828e4, 0918eea and 1946f48: the report is for e881ab5. Locally, test_redteam.py with test_judge_provider_default.py now runs 100% of the lines of dokimi/eval/redteam.py and dokimi/commands/redteam.py (--cov, term-missing); the --judge path, the exit-2 path and the artifact of a blocked run were not run before. The Codecov report will update on this push.

@napetrov

Copy link
Copy Markdown
Contributor Author

The 5 inline findings it counts are answered in their threads (see 89641b7, 89641b7, 27828e4, 0918eea, 1946f48).

Comment thread dokimi/dokimi/commands/redteam.py Outdated
Comment on lines +34 to +36
except (OSError, ValueError) as e:
print(f"Error: {e}", file=sys.stderr)
sys.exit(2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · low
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:

Suggested change
except (OSError, ValueError) as e:
print(f"Error: {e}", file=sys.stderr)
sys.exit(2)
except (OSError, ValueError, RecursionError) as e:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread dokimi/dokimi/eval/redteam.py Outdated
Comment on lines +197 to +198
g = grade(grading, question, answer, ask)
out[name] = Probe(name, g.passed, _detail(g))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · high
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:

Suggested change
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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.passed

plus tests for both blocks above (the first should be unscored, the second unprobed and so inconclusive).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) and fuzzy, and numeric -7731.0, are now pass. {threshold: 0.5, Paris + negated London} is now blocked on wrong, empty and hedge. A negated-only block and the "5 judge + 1 match" set are now inconclusive. A value one keeps its hedge probe.
  • [judge, negated Lyon] without --judge is blocked (a false block). {threshold: 0.5, [judge, match Paris]} without --judge is pass, but blocked with 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_id still validates every row before any is graded.
  • Shared helpers: check_values is a pure rename. judge_ask builds the same Ask that eval reference used. Nothing else imported _values, and the open PR #280 that edits reference_checks.py adds no new call.
  • Artifact, docs and changelog: judge_model and judge_provider are required and nullable in redteam.v1, which no release has written yet. The docs table and the fragment now describe inconclusive and the stuffed rules as coded.

@exai-bot

exai-bot commented Oct 30, 2026 •

Copy link
Copy Markdown

🔍 OpenCodeReview found 1 issue(s) in this PR.

By severity: 1 low

Severity Category Location Finding
low documentation answer-formats.md:121-122 Nit: The table does not name every unmeasured question.
Review details
  • Commit reviewed: 29a65fe against merge base b4a0a8c
  • Coverage: 18 files in 3 groups
  • Engine: OpenCodeReview v1.13.3, us.openai.gpt-6.1-sol
  • Usage: 421k tokens (232k from cache), 35s
  • Tool calls: 35 (31 file_read, 2 code_search, 1 code_comment, 1 file_read_diff)
  • Red-team evaluation, CLI, schema, and tests: 9 files
  • Answer-format verification and evaluation integration: 5 files
  • Red-team artifact and run discovery support: 4 files

if violations:
status = BLOCKED
else:
status = PASS if rows and not summary["unprobed"] else INCONCLUSIVE

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

documentation · low
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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@napetrov
napetrov requested a review from a team as a code owner October 30, 2026 05:29

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.py against tests/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 JSON contains stuffed answer is rejected for [1,2], [{"a":1},{"b":2}] and ["a","b"].
  • Artifact and schema: real -o artifacts for pass, blocked and inconclusive runs, with and without --judge, validate against redteam.v1; the kind is in SCHEMA_VERSIONS.
  • Refactors: check_values and judge_ask have no stale callers; eval reference builds the same Ask.
  • Exit codes: 0, 1 and 2 behave as documented, including an unwritable -o.
  • Commits: all 28 non-merge commits carry Signed-off-by.

Comment thread dokimi/dokimi/eval/redteam.py Resolved
Comment thread dokimi/dokimi/eval/redteam.py Resolved
Comment thread dokimi/dokimi/eval/redteam.py Resolved
Comment thread dokimi/dokimi/commands/redteam.py Resolved

@vlad-perevezentsev vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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; the typecheck / mypy job fails because of this PR: candidates = JSON_DECOYS fixes 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 give inconclusive. With a judge that says "yes" to everything, the second block is still blocked.
  • 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 pass and a false blocked. Without a threshold, a failed scored check still counts, which is right, since all checks must pass there.
  • Stuffing for json with contains: 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])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread dokimi/dokimi/eval/redteam.py Outdated
Comment on lines +133 to +134
if _whole_answer(check) and not _reads_first_json(check):
return " ".join(values)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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].

Comment thread dokimi/dokimi/eval/redteam.py Outdated
Comment on lines +142 to +143
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread dokimi/dokimi/eval/redteam.py Resolved

@vlad-perevezentsev vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 / mypy and the CLI surface check included.
  • Numeric cases through run(). The negated -.5 at end and +.5 at begin are blocked on stuffed. The positive -.5 canary is now -0.5 and passes, at end and at exact. 1e-15, 1e20 and 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.

Comment thread dokimi/dokimi/eval/redteam.py Outdated
Comment on lines +139 to +156
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

@napetrov

Copy link
Copy Markdown
Contributor Author

The negated -.5 false blocked is fixed in 0aa6c16. Your question on the positive stuffed probe is asked in its thread and waits for a decision.

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>
@napetrov
napetrov force-pushed the feat/verifier-redteam branch from 301849e to 0ad171b Compare October 31, 2026 19:24
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])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Comment thread dokimi/dokimi/eval/redteam.py Outdated

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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, on harness_profile.v1.json. This PR does not touch that schema: the newer main changed it after this branch's base.
  • Both @exai-bot repros through run(). {regex .+, match [-, certain] weight 0} gives pass with only empty run, while the regex alone is blocked on wrong and hedge. {exact, [a, b]} is blocked on its canary a.
  • The repros of the earlier rounds on this head give the same results.

Checked, no issues

  • Stuffed removal: stuffed is out of ATTACKS, the table, the rates, the redteam.v1 attack enum (the schema is new in this PR, so narrowing it breaks nothing), the fixture, the docs and the changelog.
  • CLI flags: --judge PROVIDER/MODEL and --out FILE follow the flag registry. A malformed --judge exits 2, the route table lists eval redteam as paid, and the CLI snapshot is current. Keeping -o out is consistent with --out being the canonical spelling.
  • Runs kinds: redteam.v1 is listed in runs/kinds.py, so runs list and runs show see the artifact.

Comment thread dokimi/dokimi/eval/redteam.py Outdated
Comment on lines +94 to +152
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 69e3449: stuffed_for, JSON_DECOYS, _reads_first_json and the permutations import left redteam.py; the helpers now live in tests/test_redteam.py only. Lint is clean (7441aaf also folds an import).

…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>
Copilot AI balanced review requested due to automatic review settings November 1, 2026 05:20
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Comment thread dokimi/dokimi/eval/redteam.py Outdated
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
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:

Suggested change
partial = bool(g.unjudged or g.unrun)
partial = any(c.weight > 0 for c in (*g.unjudged, *g.unrun))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.v1 artifact 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

Comment on lines +65 to +66
usable = [str(v) for v in values if squad_normalize(str(v))]
return usable[0] if usable else None

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread dokimi/dokimi/eval/redteam.py Outdated
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)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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}."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread dokimi/dokimi/eval/redteam.py Outdated
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))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Comment thread dokimi/dokimi/eval/redteam.py Outdated
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))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 29a65fe. attacks() now counts only checks that are positive, weight above 0, and not unsupported, so the block in the report is inconclusive with no violations.
Regression: test_an_unsupported_check_does_not_give_a_negated_block_the_rejection_probes (red on 004d6fe).

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 in commands/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.py against tests/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 > 0 filters in attacks, _accepts and 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 the b canary and passes.
  • Dropped stuffed attack: it is gone from ATTACKS, the table, the rates, the schema, the fixture and the docs; -o is not documented anywhere.
Bot points I checked that no longer apply at this head (11)
  • exai-bot's point on the numeric canary for -.5 does not apply: canary_for spells the parsed number in plain decimal (redteam.py:68).
  • exai-bot's point on the stuffed list b a for exact [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 -o not being registered does not apply: --out is the canonical flag, and the docs, changelog and tests name --out only.
  • 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: _accepts counts 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_exact reports a rejected canary, so the run is blocked.
  • 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: applies needs a non-negated check with weight above 0 (redteam.py:129).

Comment thread dokimi/dokimi/eval/redteam.py Outdated
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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: status is blocked, with violations on wrong, empty and hedge. The only counted check is the negated one, which accepts those answers by design.
  • Without the unsupported check the same block is inconclusive (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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 == [].

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Comment on lines +121 to +122
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

documentation · low
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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli-interface-change Changes dokimi's user-facing CLI surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants