Skip to content

feat(repo): repo compare flags replay tasks a guidance revision may have seen - #392

Open
antonwolfy wants to merge 15 commits into
intel:mainfrom
antonwolfy:feat/replay-leak-flags
Open

antonwolfy wants to merge 15 commits into
intel:mainfrom
antonwolfy:feat/replay-leak-flags

Conversation

@antonwolfy

@antonwolfy antonwolfy commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Part 2 of #290 ("repo compare flags replay tasks a guidance revision may have seen"). Part 1, the recording, merged as #359; this part waited on #332 (per-task values in dokimi/repos/compare_values.py) and #336 (repo compare --ledger). It follows the design in the maintainer's comment on #290: a commit counts against a task only when it touches a guidance file that applies to a file in the task's patch, by committer date, with no content similarity, reading public_at, the uncapped guidance_latest for the date flag and the capped guidance_commits for the name flag, and with no new option (the clean ids go to --task).

A guidance:rev:<sha> arm runs a revision of a repository's guidance against review_replay tasks built from PRs merged earlier. Guidance written or edited after a task's review can encode that review, and the arm then "catches" the thread because a rule was written from it. In a replay of a dpnp guidance revision, three of nine "after the cut" tasks had their PRs named by later guidance commits, and one late guidance commit came after every task's first review comment. On main, repo compare prints nothing about this: the cells record arm_rev.guidance_latest / guidance_commits and task.json records public_at, but the check was done by hand.

Here, on a batch of six replay tasks (guidance:none against guidance:rev:<sha>, root AGENTS.md committed 2026-03-01T11:30Z, dpnp/tests/AGENTS.md committed 2026-03-10, and a commit "Update test guidance for the fixes in #3103"), repo compare --baseline guidance:none now ends its comparison with:

leak check guidance:rev:ffff…ffff (main ffffffffffff): 6 checked, 2 by date, 1 by name, 1 unknown, 2 clean
  by name: review_replay_pr3103 (dddddddddddd)
  unknown, no public_at: review_replay_pr3310
  clean: --task review_replay_pr3260 --task review_replay_pr3300
task public_at patch_files result
pr3200 2026-03-01T20:00+09:00 (11:00Z) dpnp/linalg.py by date: root AGENTS.md committed 11:30Z
pr3250 2026-03-05 dpnp/tests/test_b.py by date: dpnp/tests/AGENTS.md committed 03-10
pr3260 2026-03-05 dpnp/fft.py clean: dpnp/tests/AGENTS.md does not apply
pr3103 2026-03-12 dpnp/tests/test_a.py by name: the commit subject names #3103
pr3300 2026-05-01 dpnp/x.py clean
pr3310 none dpnp/y.py unknown by date (no public_at), checked by name

The exit code is 0, as before; the deltas, intervals and p values are unchanged.

Changes

  • repo tasks replay (dokimi/repos/replay/build.py) writes patch_files into task.json: the sorted paths change.patch changes, taken from the same --no-renames diff the patch is built from, so a rename gives both its paths, and without the left_out_files.
  • New dokimi/repos/compare_leak.py. It checks every compared arm, baseline or treatment, whose cells' meta.json has arm_rev (or whose name starts with guidance:rev:), and patch: arms. An arm whose name says it has no history (guidance:none, guidance:raw and the other guidance parts, guidance:format=<fmt>, a skill-only arm) is never checked, even with an unreadable meta.json; that file makes only an arm of another name checked, with its replay tasks unknown. It reads the tasks whose cell task.json has public_at or source_pr, and --task narrows it.
    • By date: an applicable guidance file's guidance_latest commit was committed after public_at, compared as aware datetimes, so Z, +00:00 and other offsets all work. A nested AGENTS.md, CLAUDE.md, CLAUDE.local.md or GEMINI.md applies to tasks whose patch_files has a path under its directory; one in a .claude/ directory counts for that directory's parent. Every other guidance path applies to every task (the strict side). So does every file for a task without patch_files, whose entry says scoped: false and is named in the text.
    • By name: a guidance_commits subject names the task's PR as #N, as PR#N, PR #N, PR N or GH-N (any case), as owner/repo#N of its own source_url, or by its PR URL, none followed by a digit. #31030, PR#31030, dpnp#3103, other/repo#3103, …/#3103 and a URL whose host is glued to a longer name (notgithub.com/…, evil.github.com/…) do not name #3103; a URL without its scheme does. When guidance_commits_truncated is set, the JSON and the text both say so.
    • Unknown, with a reason: no public_at (by date only), a patch: arm, an arm_rev without guidance_latest, a meta.json / task.json of the wrong shape, or no task.json in any of the task's cells (the runner writes it only once the trace parsed, so a host error leaves a cell without it). With guidance_commits_truncated, a task no kept subject names is unknown by name when its public_at is missing or earlier than the oldest kept commit's committed (the list is newest first, so every cut commit is no later than that one), and clean otherwise. These never crash.
    • Pooled batches: when the cells of one task differ, a flag set by any cell counts, and the named commits are their union. A cell with no task.json whose arm_rev no checked cell of the task has makes the task unknown ("a cell with no task.json ran a guidance history no other cell read"), as that history was never compared with the task; one that ran the same arm_rev as a checked cell is covered by it.
    • clean: line: the --task options are the arm's clean tasks and the tasks it does not check (not_checked: a task.json with neither public_at nor source_pr), so the rerun leaves out only the flagged ones; a task any checked arm flags or reports unknown is left out.
  • dokimi/repos/compare.py only calls the new module: compare() adds summary["leak"] (one entry per checked arm, [] when none is checked), and format_compare appends its lines. compare() builds the per-arm cells once, for both leak_check and main's judged_cells. The file is 783 lines (cap 800).
  • Docs: contract §3 (arm_rev: what repo compare reads), §4.6 (patch_files, and how public_at is read), §9 (each flag, the scope rule and its strict side, unknown, the name rule, truncation, the JSON keys, the clean ids and the unchecked ones as --task). Also the reviewing guide's leakage-cut bullet, which now names this output instead of a hand check, and the architecture.md module map.
  • Changelog fragment (Added).

Closes #290

Risk

  • repo compare --json gains a top-level leak key, always present ([] when no compared arm is checked); each entry also lists the arm's not_checked tasks. Text output gains a leak check block only when a guidance:rev: or patch: arm has a replay-style task or a task with no task.json in any cell; other batches print the same as before.
  • Tasks built before this change have no patch_files. They are read strictly (every guidance file applies), so they may be flagged by date more often than a scoped read would flag them. Their entries say scoped: false, and the text names them. Rebuilding the tasks with repo tasks replay --force writes the field.
  • No delta, interval, p, Holm decision or exit code changes. repo compare --ledger calls compare() and gets the extra key, which it does not read.

Testing

On a Linux review host, at cb26557 (on main 37c0c21):

  • make ci-local: 30 passed, 0 failed, 7 skipped (external-catalog-smoke: needs docker; test-latest, docs deploy, pip-audit report: job conditions false on a PR; cli-surface "Package the result": unknown expression; conda-forge-deps: no micromamba; semgrep: not on PATH). The test run: 16326 passed, 11 skipped. Changed-line coverage 100% (206 lines measured). Not run here: the test legs on Python 3.11, 3.12 and 3.14.
  • New tests/test_repo_compare_leak.py (82 cases) covers:
    • the issue's two cases, flagged, with the other tasks clean and the comparisons identical to the same batch without arm_rev;
    • per-file scope: sub/AGENTS.md, sub/CLAUDE.md, sub/.claude/CLAUDE.md and sub/GEMINI.md flag a task patching sub/deep/x.py but not other2/y.py or subway/z.py, while a root AGENTS.md, .claude/rules/x.md or a non-instruction guidance file flags all three;
    • a task without patch_files read strictly;
    • committer date later with author date earlier is flagged, and the reverse is not;
    • the name forms in both directions, PR#3103, pr#3103, PR #3103, PR 3103 and GH-3103 named, PR#31030, dpnp#3103, SPR#3103 and other/PR#3103 not;
    • truncated history reported; with it, a task public before the oldest kept commit unknown by name, one public at that commit's date clean, one with no public_at unknown, and one a kept commit names flagged;
    • a patch: arm unknown; a guidance:rev: baseline checked while guidance:none / guidance:raw are not;
    • a guidance:none, guidance:raw, guidance:format=yaml, guidance:none+skill:lint or skill:lint baseline with one unreadable meta.json: not checked, and the guidance:rev: arm keeps its clean: line;
    • a batch mixing replay tasks with hand-written ones: the clean: line keeps them, and drops one that another batch's guidance:rev: arm flags;
    • a replay task with no task.json in any cell: unknown and off the clean: line, while a task with one in only some cells is checked by those when the others ran the same arm_rev, and is unknown when one of them, in another pooled batch, ran a later history;
    • a task with no public_at: unknown by date, still flagged by name;
    • --task narrowing; pooled cells;
    • twelve bad cell shapes, each unknown with its reason;
    • the text lines and the cap at ten.
  • tests/test_repo_replay.py: patch_files on PR 7, and a new PR whose patch renames src/r.py to src/q.py, which lists both paths and leaves out the nested AGENTS.md it adds.
  • Mutants: 31 breaks of the code each test covers, each run against its test in a copy of the commit. All 31 fail their test. Among them:
    • comparing dates as strings;
    • no per-file scope; scope by string prefix; .claude/ not read as its parent;
    • reading the author date; recording the earliest tripping commit;
    • each name-rule boundary, and a case-sensitive repo name;
    • truncation not recorded;
    • a patch: arm read as a revision; every arm or every task checked;
    • no public_at or no guidance_latest read as clean;
    • only the first cell of a task read; --task ignored;
    • each bad-shape guard dropped; the text conditions and the cap;
    • patch_files keeping left-out files, taken from a rename-detecting diff (only the new path), or not written.
      One mutant first survived (dropping / from the #N lookbehind), so the test now has a …/#3103 subject.
  • Review round at 3d9c9d4: 12 more mutants, all failing their test: the arm-name rule dropped, or missing skill-only arms, guidance:format= arms, or the guidance part of a +skill: name; the clean: line without the unchecked tasks, not_checked left empty, or the unchecked tasks not filtered by the other arms' flags; the PR or GH- prefix dropped, PR needing a # or taking no space, and the boundary applied only before #. The filter mutant first survived, so the mixed-batch test now has a task another batch's arm flags. The new cases also fail at 3d9c9d4 itself (12 of 79; PR #3103 already matched there).
  • Review round at 7381f9e: main merged (37c0c21), then two fixes. The repro of each point, on a batch built with the test helpers, before and after: with 200 kept commits, the oldest from 2026-03-10, the task public on 03-01 went from clean: to unknown, guidance_commits_truncated after its public_at; a replay task whose guidance:rev: cell has no task.json went from clean: --task review_replay_pr1 --task review_replay_pr3 to unknown, no cell has a task.json: review_replay_pr3 and clean: --task review_replay_pr1. 3 mutants, all failing their test: the truncation check dropped, its boundary made strict (< for <=), and the no-task.json state dropped. Both new tests also fail at the merge commit before the fixes.
  • Review round at 3598d88: the repro, two pooled batches built with the test helpers, where pr1's b1 cell has a task.json and a root AGENTS.md committed before its public_at, and its b2 cell has no task.json and a root AGENTS.md committed after it, with pr2 flagged by date in b2: before, 0 unknown, 1 clean and clean: --task review_replay_pr1; now 1 unknown, 0 clean and unknown, a cell with no task.json ran a guidance history no other cell read: review_replay_pr1, with no clean: line. 3 mutants, all failing their test: the new unknown state dropped, every cell without task.json made unknown whatever its history (fails the test of a task with one in only some cells), and every cell's history compared as equal. The new test also fails at 3598d88.
  • repo compare on the hand-built batch above: exit 0, with the output shown.

Revert

A plain revert. Tasks written meanwhile keep a patch_files key that nothing else reads; no schema version changes.

Checklist

  • Tests added/updated and pytest passes
  • ruff, mypy, and schema checks pass
  • Docs updated (or N/A) — see the docs-drift checklist in CONTRIBUTING.md
  • Self-reviewed per "Before opening a PR" in AGENTS.md
  • No ad-hoc run artifacts committed
  • New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

Part 2 of issue intel#290, after the recording in intel#359. A guidance:rev: arm
runs a revision of the guidance against replay tasks built from PRs
merged earlier; guidance written after a task's review can encode that
review, and nothing in a batch showed it.

repo tasks replay now writes patch_files into task.json: the sorted
paths change.patch changes, from the same --no-renames diff, so a
rename counts as both its paths.

repo compare checks every compared arm whose cells record arm_rev (new
module dokimi/repos/compare_leak.py), over its tasks with public_at or
source_pr:

- by date: a guidance file that applies to the task has its
  guidance_latest commit committed after public_at, compared as aware
  datetimes. A nested AGENTS.md, CLAUDE.md, CLAUDE.local.md or
  GEMINI.md applies to tasks patching below its directory; any other
  guidance path, and every file for a task without patch_files,
  applies to every task (the strict side, marked scoped: false).
- by name: a guidance_commits subject names the task's PR (#N,
  owner/repo#N of its own repository, or its PR URL; not #N0 or
  other/repo#N). A cut history (guidance_commits_truncated) is said.
- unknown, with the reason: no public_at (by date only), a patch: arm,
  a batch with no recorded history, a cell file of the wrong shape.

The --json summary gains leak (one entry per checked arm, [] when
none); the text prints the counts, the tasks named with their commit
short shas, and the clean tasks as --task options. No delta, interval,
p or exit code changes. Contract sections 3, 4.6 and 9 and the
reviewing guide describe the flags.

Closes intel#290

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
@antonwolfy
antonwolfy requested a review from a team as a code owner November 4, 2026 18:12
Copilot AI balanced review requested due to automatic review settings November 4, 2026 18:12
@antonwolfy antonwolfy self-assigned this Nov 4, 2026
@exai-bot

exai-bot commented Nov 4, 2026 •

Copy link
Copy Markdown

✅ OpenCodeReview: Review complete: 0 finding(s) across 9 selected item(s).

CI at `cb26557`: 4 still running, 14 passing, 2 skipped
Check Status
CLI interface diff still running
CodeQL (actions) still running
CodeQL (python) still running
Windows smoke test (py3.13) still running

This PR has reached its review limit, so only a re-request starts a review.

Review details
  • Commit reviewed: cb26557 against merge base 37c0c21
  • Coverage: 9 files in 1 groups
  • Engine: OpenCodeReview v1.13.5-exai.3.d7112371, us.openai.gpt-6.1-sol
  • Usage: 150k tokens (69k from cache), 40s
  • Tool calls: 6 (3 file_read, 2 code_search, 1 pr_threads)
  • Replay leak detection, comparison integration, tests, and documentation: 9 files

Comment thread dokimi/dokimi/repos/compare_leak.py Resolved

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

Leak detection still falsely flags unrelated URLs and repository references while missing startup-loaded rules and imported guidance.

4 open issues
What changed in this PR

4 new findings.

This PR adds replay-leakage reporting to repo compare, using recorded guidance history to flag tasks a guidance revision may have seen.

Changes:

  • Records replay patch paths for directory-scoped guidance checks.
  • Adds date/name flags, unknown reasons, and clean-task filters to comparison output without changing comparison statistics.
  • Adds regression tests and documents the checks and their limitations.
File Description
dokimi/​tests/​test_repo_replay.py Tests patch-path recording and rename handling.
dokimi/​tests/​test_repo_compare_leak.py Tests leakage flags, scope, malformed records, and output.
dokimi/​dokimi/​repos/​replay/​build.py Writes patch_files into replay tasks.
dokimi/​dokimi/​repos/​compare.py Integrates leakage checks into JSON and text output.
dokimi/​dokimi/​repos/​compare_leak.py Implements leakage classification and formatting.
dokimi/​docs/​reviewing.md Updates leakage-cut guidance to use comparison output.
dokimi/​docs/​repo-evaluation-contract.md Documents metadata, scope rules, and output fields.
dokimi/​docs/​architecture.md Adds the leakage-check module to the architecture map.
dokimi/​changelog.d/​20261104_120000_antonwolfy_replay-leak-flags.md Announces leakage reporting and replay patch metadata.

Review effort: Balanced

Comment on lines +168 to +169
if not applies(path, files):
continue

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.

Valid, not fixed here: guidance_latest records the last commit per guidance path, not the import graph, so which nested files a root file imports is not known to the check. Treating every scoped file as unknown would drop the scoping the check exists for. 58d9f0c states the gap in the module docstring and in contract §9 (@ imports are not followed; a nested file a root file imports counts as scoped and can be missed). Recording the imports at build time is a separate change.

Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
…e its host

The URL form had no leading boundary, so notgithub.com/<owner>/<repo>/pull/N
or evil.github.com/... flagged a clean task by name. The scheme stays
optional so a bare github.com/... URL still flags.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
@antonwolfy
antonwolfy requested a review from exai-bot November 4, 2026 18:23
@codecov-commenter

codecov-commenter commented Nov 4, 2026 •

Copy link
Copy Markdown

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

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.86%. Comparing base (040f86d) to head (cb26557).
⚠️ Report is 5 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #392      +/-   ##
==========================================
+ Coverage   94.78%   94.86%   +0.07%     
==========================================
  Files         354      357       +3     
  Lines       46588    47367     +779     
  Branches     7974     8143     +169     
==========================================
+ Hits        44159    44934     +775     
- Misses       1541     1545       +4     
  Partials      888      888              
Files with missing lines Coverage Δ
dokimi/dokimi/repos/compare.py 100.00% <100.00%> (ø)
dokimi/dokimi/repos/compare_leak.py 100.00% <100.00%> (ø)
dokimi/dokimi/repos/replay/build.py 98.18% <100.00%> (+0.01%) ⬆️

... and 5 files with indirect coverage changes

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

…epo name's - or . is not a PR reference

A file under .claude/rules/ is a rule whatever its basename, so it applies to every task, not only those patching its parent directory. #N glued to - or . is another repository's PR (other/repo-#N). The docs say that @ imports are not followed.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
…rce_url, and an unparsable public_at

The changed lines of compare_leak.py that codecov reported uncovered: the absent-file read, a task with no source_pr, a source_url that is no PR URL, and a public_at that is not a date.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>

@napetrov napetrov 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.

Verdict: one blocking thread (the clean: suggestion) and two nits; COMMENT review. CI red: none on head (0 failing Actions jobs).

Coverage: checked the diff, the leak batches (plain and leak, head and base), the custom-arm and big-source_pr probes, and the test run on head (test_repo_compare_leak.py and test_repo_replay.py, 116 passed) with probe-only shims for regex and scipy. Not looked at: the full pytest suite (not run), the 46 leak tests on the real packages.

Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
…readable meta.json and huge source_pr stay unknown

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
…er arm reports unknown

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
@antonwolfy

Copy link
Copy Markdown
Contributor Author

Review round 2 is addressed on 3d9c9d4 (CI green); the four threads are replied to and resolved.

  • clean: suggestion: a task is printed as a --task option under an arm only when no checked arm flags it (napetrov's blocking point, c92ffd1) and no checked arm reports it unknown (exai-bot, 3d9c9d4).
  • An arm whose meta.json is unreadable is checked, so its replay tasks are reported unknown with that reason instead of vanishing.
  • A source_pr of more than 18 digits is unknown (source_pr is not a PR number) and no longer crashes repo compare.

Each fix has a test that fails without it, and the bad-shape test covers the 5000-digit source_pr. make ci-local was run offline on c92ffd1 with no failures; the last commit was checked with its tests, ruff and mypy, and CI on the PR covers the full run.

@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 3d9c9d4. The check does what issue #290 asks and leaves every delta, interval and p as they were. What is left is in the clean: suggestion and in a few cases where a task reads as clean or as unknown when it should not; see the line comments.

What I ran, at 3d9c9d4:

  • make ci-local --offline: 0 failed, 19 passed, 13 skipped (network/docker steps); the test suite: 15,687 passed, 63 skipped.
  • A repro with the test file's batch helpers: a batch that mixes replay and hand-written tasks, a guidance:none cell with an unreadable meta.json, and names_pr(3103, ...) against the subjects PR#3103, pr#3103, GH-3103, PR 3103 and dpnp#3103.

Checked, no issues

  • Earlier review threads: the cross-arm clean: filter (flagged and unknown), the unreadable meta.json of a custom-named arm, and the over-long source_pr are fixed at compare_leak.py:242, :91 and :201, each with a test.
  • Scope and dates: .claude/rules/* applies everywhere whatever its name, .claude/CLAUDE.md counts for its parent, and dates are compared as aware datetimes, so +09:00 against Z is right.
  • patch_files: built from the same changed list as the patch minus left_out_files, with both paths of a rename. The new test covers it.
  • Docs and changelog: contract §9 and §4.6, reviewing.md and architecture.md match the code. The fragment is under Added and says what a user sees; the stored keys and schemas are unchanged.

Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
Comment thread dokimi/dokimi/repos/compare_leak.py Resolved

@napetrov napetrov 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.

Re-review at 3d9c9d4 (round 58d9f0c..3d9c9d4). Verdict: no new thread from me.

  • My blocking (clean: suggests a task another arm flags): fixed, flagged set across arms.
  • My nit (unreadable meta.json drops the arm from the check): fixed by the _kind fallback.
  • My nit (source_pr over 4300 digits crashes): fixed, bounded to 18 digits.
  • Round pass found one row: the _kind fallback turns a plain arm with one corrupt meta.json into a checked arm, whose healthy cells go unknown and then drop out of every arm's clean: line. Same as @vlad-perevezentsev's open thread on compare_leak.py (line 91); agree it needs a fix, not repeated here.

Coverage: round diff, plain + edge pass, no probes beyond the passes' own. CI green on head.

An unreadable meta.json made the leak check treat its arm as one with a
guidance history, so a guidance:none baseline with one corrupt cell was
checked: its healthy cells were reported unknown ("its meta.json has no
arm_rev object") and, being unknown in some arm, dropped out of the
guidance:rev arm's clean: line.

The arm's name now decides first: guidance:none, guidance:raw, the other
guidance parts, guidance:format=<fmt> and skill-only arms have no history
and are not checked, whatever their meta.json holds. The unreadable
meta.json still makes an arm of another name checked, its replay tasks
unknown. Settles the review thread on compare_leak.py:91.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
The clean: line listed only the replay tasks the check found clean, so
on a batch mixing replay and hand-written tasks the suggested --task
options also dropped every task the check does not read (no task.json,
or one with neither public_at nor source_pr), against contract section 9's
"to compare without the flagged tasks".

Each leak entry now lists those tasks as not_checked, and the clean:
line suggests them with the clean ones, leaving out only what some arm
flags or reports unknown. Contract section 9, the reviewing guide and
the changelog fragment say so. Settles the review thread on
compare_leak.py:261.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
The by-name check took only #<n> not preceded by a word character, so a
guidance commit titled "Add rule from PR#3103 review" (or pr#3103,
PR 3103, GH-3103) left the task built from PR 3103 clean, a miss on the
side the check is there to catch.

The number form now also takes PR, PR#, PR # or GH- right before the
number, in any case, under the same boundary: dpnp#3103, SPR#3103 and
other/PR#3103 still do not name it, nor does PR#31030. The module
docstring, contract section 9 and the changelog fragment list the forms.
Settles the review thread on compare_leak.py:221.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
@antonwolfy

Copy link
Copy Markdown
Contributor Author

(commented) Review at 3d9c9d4. The check does what issue #290 asks and leaves every delta, interval and p a

@vlad-perevezentsev (#392 (review)): The three line comments are fixed: the clean: line keeps the tasks the check does not read (714b283), an arm named guidance:none, guidance:raw, guidance:format= or skill-only is not checked even with an unreadable meta.json (83a0168), and PR#N, PR N and GH-N name the PR (27f45a5). Each has a test that fails at 3d9c9d4; the replies on the threads give the details.

@antonwolfy

Copy link
Copy Markdown
Contributor Author

(commented) Re-review at 3d9c9d4 (round 58d9f0c..3d9c9d4). Verdict: no new thread from me.

@napetrov (#392 (review)): The row you agreed with (a plain arm with one corrupt meta.json turned into a checked arm, its healthy cells going unknown and dropping out of every clean: line) is fixed in 83a0168: the arm's name decides first, so guidance:none, guidance:raw, guidance:format= and skill-only arms stay unchecked; the unreadable meta.json fallback now applies only to an arm of another name.

compare.py keeps both imports: main's compare_interval and this PR's own module.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>

@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 7381f9e. The three points of my last round are fixed, each with a test: the clean: line keeps the hand-written tasks (714b283), a plain arm with a broken meta.json stays unchecked (83a0168), and PR#N, PR N and GH-N name the PR (27f45a5). One gap is left in the first fix, about cells with no task.json; see the line comment.

What I ran, at 7381f9e:

  • make ci-local --offline: 0 failed, 19 passed, 13 skipped (network/docker steps); the test suite: 15,873 passed, 63 skipped.
  • A repro with the test file's batch helpers for each fix (a mixed batch, a guidance:none cell with {broken as meta.json, the PR/GH subjects together with APR 3103, dpnp#3103, PR#31030 and other/PR#3103), and a guidance:rev: cell with no task.json.

Checked, no issues

  • Merge of main: the compare.py changes sit next to #409's interval code without overlap; the leak entry and its text are unchanged by it.
  • Arm names: guidance:none+skill:x, skill:x and guidance:format=... are left out, while guidance:rev:...+skill:x and patch:... are still matched first.
  • Docs and changelog: contract §9, reviewing.md and the fragment describe not_checked and the new name forms.

Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
Brings in main up to 37c0c21, which conflicted in two files:

- dokimi/dokimi/repos/compare.py: main's intel#395 added compare_judged
  (judge:recall_judged), which builds the same per-arm cell map that
  leak_check reads. Both imports are kept; compare() now builds the map
  once as arm_cells and passes it to judged_cells and leak_check, and
  the summary keeps both "judged_cells" and "leak".
- dokimi/docs/architecture.md: the repo compare entry names both
  compare_judged.py and compare_leak.py.

docs/reference/cli-interface.json merged without conflict; a fresh
scripts/cli_surface.py dump matches it.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Review point: with guidance_commits_truncated, a replay task whose PR
none of the kept commits names was listed clean, and offered as a
--task option, though a cut commit after its public_at may name it.

guidance_commits is newest first by committer date, so every cut commit
is no later than the oldest kept one. _by_name now reports such a task
unknown ("guidance_commits_truncated after its public_at") when its
public_at is earlier than the oldest kept commit's committed date, and
("guidance_commits_truncated, no public_at date to bound it") when it
has no public_at; a task public at or after that commit was fully
checked and stays clean. Contract section 9, the module docstring and
the changelog fragment say so.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Review point: a replay task whose cells in a checked arm have no
task.json was put in not_checked, and so on the clean: line as a
--task option, though its leak was never read. The runner writes
task.json only once the trace parsed, so a host error before that
leaves a finished cell without it, whatever the task is.

_arm_entry now reports a task none of whose cells has a task.json as
unknown ("no cell has a task.json"); not_checked keeps only the tasks
whose task.json was read and is not replay-style, so a hand-written
task still stays on the clean: line. The test of that line gives its
hand-written tasks a task.json, as the runner writes one. Contract
section 9, the module docstring and the changelog fragment say so.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
@antonwolfy

Copy link
Copy Markdown
Contributor Author

(commented) Review at 7381f9e. The three points of my last round are fixed, each with a test: the clean:

@vlad-perevezentsev (#392 (review)): The gap from the line comment (cells with no task.json) is fixed in 3598d88, and the truncation point from the other thread in bff9c32; see those threads. main is merged in 1c7e9ba. The one overlap was in compare(), where #395's judged_cells builds the same per-arm cell map that leak_check reads. The map is now built once and passed to both, and the summary keeps both judged_cells and leak.

@antonwolfy
antonwolfy requested a review from exai-bot November 6, 2026 11:36
Comment thread dokimi/dokimi/repos/compare_leak.py Resolved
…known

Review point: with pooled batches, a task whose b1 cell has a task.json
and whose b2 cell has none was judged by b1's guidance history alone.
_check_cell skips a cell with no task.json, and the "no cell has a
task.json" rule did not apply since b1 has one, so b2's later root
AGENTS.md was never compared with the task's public_at and the task
was offered as a clean --task option.

_arm_entry now keeps the arm_rev each checked cell of a task read, and
a cell with no task.json whose arm_rev matches none of them makes the
task unknown ("a cell with no task.json ran a guidance history no
other cell read"). A spec-less cell that ran the same history as a
checked cell stays covered by it, as before. A flag another cell sets
still wins. The module docstring, contract section 9 and the changelog
fragment say so.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
@antonwolfy
antonwolfy requested a review from exai-bot November 6, 2026 11:55

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

LGTM

I checked cb26557. The two points of my last round are fixed, each with a test that fails on the previous head. A cut history now leaves a task public before the oldest kept commit unknown by name (bff9c32). A task with no task.json in any cell is unknown, not clean (3598d88), and so is a pooled task whose spec-less cell ran a history no other cell read (cb26557).

Merge: safe now: it merges cleanly into current main, 3 commits behind, and its tests pass on the merge. No open PR conflicts with it.

What I ran, at cb26557:

  • make ci-local --offline: 0 failed, 19 passed, 13 skipped (network/docker steps); the test suite: 16,267 passed, 70 skipped.
  • The round's three new tests on the previous head 7381f9e: all fail there.
  • A repro with the test file's batch helpers: a replay task whose one guidance:rev: cell has no task.json is now unknown, no cell has a task.json, and a hand-written task with a task.json stays on the clean: line.
  • test_repo_compare_leak.py, test_repo_replay.py and test_repo_compare.py on this branch merged with main: 256 passed.

Checked, no issues

  • The truncation bound: guidance_commits is sorted by committer date in guidance_history.py, so every cut commit is no later than the last one kept, and public_at equal to it is rightly clean.
  • Merge of #395: compare() builds the per-arm cell map once for judged_cells and leak_check, and judged_cells does not change it.
  • Docs and changelog: contract §9, the module docstring and the fragment describe the new unknown reasons and not_checked.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

repo compare: flag replay tasks a guidance revision may have seen

7 participants