Repository navigation
Conversation
github-actions
Bot
added
the
cli-interface-change
CLI interface change detectedThis PR changes dokimi's user-facing CLI surface (commands and/or options): + added command `dokimi context prefilter`
+ argument `guidance`
+ argument `sessions`
+ option `--help/-h`
+ option `--in-use-rate`
+ option `--json`
+ option `--min-sessions`
+ option `--rare-max`
+ option `--window`Updated on every push; the |
|
🔍 OpenCodeReview found 4 issue(s) in this PR. By severity: 3 medium, 1 low
Review details
|
| - **Corrected.** Within `--window` events after the trigger, the user's next | ||
| prompt pushes back (`no`, `don't`, `instead`, `forgot`, `revert`, ...) and |
There was a problem hiding this comment.
Nit: corrections are not limited to the user's next prompt. _scan() checks every event in the following window with any(_corrects(...)). For example, a changelog trigger followed by “thanks” and then “no, fix the changelog” within 10 events counts as corrected despite the next prompt not pushing back. Describe “a subsequent user prompt within the window” here so readers can interpret the correction counts accurately.
Suggestion:
| - **Corrected.** Within `--window` events after the trigger, the user's next | |
| prompt pushes back (`no`, `don't`, `instead`, `forgot`, `revert`, ...) and | |
| - **Corrected.** Within `--window` events after the trigger, a subsequent user | |
| prompt pushes back (`no`, `don't`, `instead`, `forgot`, `revert`, ...) and |
| | --- | --- | --- | | ||
| | `candidate_for_ablation` | Triggered in fewer than `--in-use-rate` of the sessions, or never. Ranked lowest trigger rate first. | yes | | ||
| | `in_use` | Triggered in at least `--in-use-rate` of the sessions. | no | | ||
| | `rare_not_ranked` | Triggered in at most `--rare-max` sessions. Few sessions are too little evidence to call a rule unused. | no | |
There was a problem hiding this comment.
Must-fix: the status conditions overlap, but the implementation explicitly treats zero triggers as a candidate before checking rarity. For example, six sessions with no onboarding keywords produce candidate_for_ablation with rank 1 under the defaults (also asserted by test_rare_and_protected_rules_are_listed_but_never_ranked), although this row says zero is rare and unranked. Likewise, two triggers in six sessions remain rare_not_ranked despite exceeding the default 20% in-use threshold. Document precedence (protected → insufficient corpus → zero-trigger candidate → positive rare count → in-use threshold → candidate), and clarify that rarity means 1 through --rare-max, so readers can interpret the actual rankings.
Suggestion:
| | `rare_not_ranked` | Triggered in at most `--rare-max` sessions. Few sessions are too little evidence to call a rule unused. | no | | |
| | `rare_not_ranked` | Triggered in 1 through `--rare-max` sessions; takes precedence over the in-use threshold. Zero-trigger rules are candidates unless protected or the corpus is insufficient. | no | |
There was a problem hiding this comment.
Fixed in 6b519d2. The doc now states the precedence the code applies in status_of:
kept_protected, insufficient_corpus, zero-trigger candidate, rare_not_ranked, in_use, then candidate.
A rule that never triggers is a candidate, and --rare-max takes 0 or more.
| for session in sessions: | ||
| for problem in session.errors: | ||
| print(f"warning: {session.name}: {problem}", file=sys.stderr) | ||
| loaded.extend(sessions) |
There was a problem hiding this comment.
Must-fix: repeated inputs and overlapping globs count the same session multiple times. For example, passing s.jsonl five times satisfies the default --min-sessions 5 with only one independent session, enabling rankings and inflating episodes. Repeating a Goose store similarly duplicates all its sessions. Deduplicate expanded sources by canonical path before reading them, and add regressions for repeated names, path aliases and overlapping globs.
There was a problem hiding this comment.
| pre_p.add_argument("--rare-max", type=int, default=3, | ||
| help="A rule triggered in at most this many sessions is rare: listed, not ranked (default 3)") | ||
| pre_p.add_argument("--in-use-rate", type=float, default=0.2, |
There was a problem hiding this comment.
Must-fix: these threshold types accept invalid domains without any later validation. --rare-max -1 disables the rarity safeguard, and --in-use-rate nan makes every in-use comparison false, ranking frequently triggered rules as candidates; --json also emits a non-standard NaN value. Reject negative rarity limits and require a finite rate in [0, 1] with argparse validators. Cover negative, out-of-range, NaN and infinity inputs.
There was a problem hiding this comment.
| raise SessionError(f"{path}: not a Goose sessions store: {exc}") from exc | ||
| finally: | ||
| conn.close() | ||
| kept = [s for s in sessions if s.events] # a session of unreadable messages only is no session |
There was a problem hiding this comment.
Must-fix: dropping eventless sessions also drops their parse diagnostics. In the existing unreadable-only-session test, s1 contains content_json = 'not json', but its recorded error never reaches _read_sessions(), which only prints errors from returned sessions. With a valid s0 beside it, the command silently reports a reduced corpus; the same malformed message would produce a warning if its session also contained one valid event. Keep unreadable-only sessions out of the corpus count, but propagate their errors separately so corruption is still reported. Extend the CLI test to check stderr for this mixed-store case.
There was a problem hiding this comment.
|
|
||
| def _at_path(key: str, text: str) -> bool: | ||
| """Whether ``key`` occurs in ``text`` from a path-segment start, so ``a.md`` does not match ``data.md``.""" | ||
| name = key.removeprefix("./") |
There was a problem hiding this comment.
Must-fix: path matching does not normalize Windows separators. For guidance - Update docs/guide.md before release. and a triggered Edit event whose file_path is C:\work\docs\guide.md, this returns false and reports acted=0 even though the named file was touched. _is_path also misclassifies a backslash-only directory such as docs\ as a command. Normalize separators in both path classification and matching, and add a Windows-path fixture.
| return (any(_names(k, event.command) for k in claim.commands) | ||
| or any(_at_path(k, p) for k in claim.paths for p in event.paths)) |
There was a problem hiding this comment.
Must-fix: path-based actions performed through Bash are never counted. For guidance - Update changelog.d/ for each release., a prompt update the release followed by a Bash call with command="printf 'entry' > changelog.d/x.md" reports acted=0. Both session readers put this command in event.command but leave event.paths empty, so the named directory is ignored even though it was touched. Match path keys against command text as well as explicit path fields (using the same lexical boundary checks), and add a regression test for shell-based file changes.
There was a problem hiding this comment.
| around = events[max(0, i - window): i + window + 1] | ||
| after = events[i + 1: i + window + 1] | ||
| acted += int(claim.has_keys and any(_acts(claim, e) for e in around)) | ||
| corrected += int(any(_corrects(claim, e) for e in after)) |
There was a problem hiding this comment.
Must-fix: this checks every later prompt in the window, not the user's next prompt as specified. With the CI rule, prompts push the branch, thanks, continue, and no, the branch must not go before ci produce a correction on the first episode despite its next prompt containing no pushback. This can also inflate user_overrides. Select the first prompt in after and evaluate only that prompt; add a regression with an intervening non-corrective prompt.
|
|
||
| def _rank_key(row: dict[str, Any]) -> tuple[float, float]: | ||
| acted = row["acted_rate"] | ||
| return (row["trigger_rate"] or 0.0, 0.5 if acted is None else acted) |
There was a problem hiding this comment.
Must-fix: ranking uses rates already rounded to three decimals by _ratio, so distinct trigger counts can tie and the acted-rate tiebreaker can reverse the promised lowest-trigger-rate order. With 3,000 sessions, rare_max=0, and two candidate rules triggered in 2 and 3 sessions, both trigger rates become 0.001; if the first has acted_rate=1 and the second has acted_rate=0, the more-triggered rule ranks first. Sort by the exact sessions_triggered count (the denominator is shared), retaining rounding only for output; use unrounded acted ratios for the secondary key as well.
There was a problem hiding this comment.
| from typing import Any | ||
|
|
||
| # Input keys whose value is file content, not the agent's intent: left out of an event's text. | ||
| _CONTENT_KEYS = frozenset({"old_string", "new_string", "content", "new_source", "edits"}) |
There was a problem hiding this comment.
Must-fix: the shared body filter does not cover Goose's text-editor fields file_text, old_str, and new_str. For example, a Goose tool call with {"path":"/w/README.md","file_text":"Keep the changelog fragment current"} retains the entire file body in Event.text. _triggers() then counts that copied text as a changelog episode, potentially changing a rule from an ablation candidate to in_use even though the session only copied documentation. Exclude the Goose edit/write body fields as well and add a Goose-reader regression test containing rule keywords in those fields.
Suggestion:
| _CONTENT_KEYS = frozenset({"old_string", "new_string", "content", "new_source", "edits"}) | |
| _CONTENT_KEYS = frozenset({"old_string", "new_string", "content", "new_source", "edits", | |
| "file_text", "old_str", "new_str"}) |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved session deduplication and rule-matching defects distort corpus counts and trigger/action rates, making rule recommendations unreliable.
8 open issues
What changed in this PR
8 new findings.
Adds deterministic context prefilter analysis to dokimi’s context tooling, using session logs to identify guidance rules worth testing through ablation—not to recommend removing them.
Changes:
- Reads Claude Code JSONL logs and Goose SQLite stores into prompt and tool events.
- Measures rule triggers, actions and corrections, then assigns statuses and ranks ablation candidates.
- Adds CLI registration, synthetic tests, documentation and updated reference metadata.
| File | Description |
|---|---|
dokimi/tests/test_context_prefilter.py |
Tests parsing, matching, ranking and CLI behavior. |
dokimi/tests/test_context_goose.py |
Tests Goose stores and mixed-format inputs. |
dokimi/tests/data/cli_flags_allowlist.txt |
Allows the three new threshold flags. |
dokimi/mkdocs.yml |
Adds the prefilter guide to navigation. |
dokimi/dokimi/context/session_log.py |
Extracts events from Claude Code logs. |
dokimi/dokimi/context/prefilter.py |
Matches guidance rules and builds reports. |
dokimi/dokimi/context/goose_log.py |
Reads Goose session stores in read-only mode. |
dokimi/dokimi/commands/context.py |
Registers and implements the new subcommand. |
dokimi/dokimi/cli_routes.py |
Registers prefilter as a free command. |
dokimi/docs/session-prefilter.md |
Documents usage, measures, statuses and limitations. |
dokimi/docs/reference/cli-interface.json |
Records the new command’s interface. |
dokimi/docs/README.md |
Links the prefilter guide. |
dokimi/docs/architecture.md |
Describes the session-analysis modules. |
dokimi/changelog.d/20261031_120000_napetrov_session-prefilter.md |
Announces the new command. |
dokimi/benchmarks/spec.v1.yaml |
Raises the document allowance for the added guide. |
Review effort: Balanced
| loaded, failed = [], False | ||
| for name in names: | ||
| matches = [name] if os.path.exists(name) else sorted(glob.glob(os.path.expanduser(name), recursive=True)) | ||
| for path in matches or [name]: | ||
| try: | ||
| source = Path(path) | ||
| sessions = read_goose_store(source) if is_goose_store(source) else [read_session(source)] |
| pre_p.add_argument("--rare-max", type=int, default=3, | ||
| help="A rule triggered in at most this many sessions is rare: listed, not ranked (default 3)") | ||
| pre_p.add_argument("--in-use-rate", type=float, default=0.2, | ||
| help="A rule triggered in at least this share of sessions is in use, not a candidate (default 0.2)") |
| raise SessionError(f"{path}: not a Goose sessions store: {exc}") from exc | ||
| finally: | ||
| conn.close() | ||
| kept = [s for s in sessions if s.events] # a session of unreadable messages only is no session |
| def _events_of(role: str, content: Any, user_visible: bool) -> list[Event]: | ||
| if not isinstance(content, list): | ||
| return [] | ||
| events = [] | ||
| for block in content: | ||
| if not isinstance(block, dict): | ||
| continue | ||
| if role == "user" and user_visible and block.get("type") == "text": | ||
| text = block.get("text") | ||
| words = prompt_text(text) if isinstance(text, str) else "" | ||
| if words: | ||
| events.append(Event("prompt", words)) | ||
| elif role == "assistant" and block.get("type") == "toolRequest": | ||
| call = block.get("toolCall") | ||
| value = call.get("value") if isinstance(call, dict) else None | ||
| if isinstance(value, dict): | ||
| events.append(tool_event(value.get("arguments"))) | ||
| return events |
| _RULE_KINDS = frozenset({"sentence", "list_item", "table_row"}) | ||
| _BACKTICK = re.compile(r"`([^`\n]+)`") | ||
| _WORD = re.compile(r"[a-z][a-z0-9_-]{2,}") | ||
| _PATH = re.compile(r"/|\.[A-Za-z0-9]{1,6}$") |
| keywords = _stems(_BACKTICK.sub(" ", span.text)) | ||
| if not keywords: | ||
| continue |
There was a problem hiding this comment.
|
|
||
| def _at_path(key: str, text: str) -> bool: | ||
| """Whether ``key`` occurs in ``text`` from a path-segment start, so ``a.md`` does not match ``data.md``.""" | ||
| name = key.removeprefix("./") |
| def _acts(claim: Claim, event: Event) -> bool: | ||
| if event.kind != "tool": | ||
| return False | ||
| return (any(_names(k, event.command) for k in claim.commands) |
There was a problem hiding this comment.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #334 +/- ##
==========================================
+ Coverage 94.29% 94.30% +0.01%
==========================================
Files 308 311 +3
Lines 39322 39800 +478
Branches 6651 6719 +68
==========================================
+ Hits 37079 37535 +456
- Misses 1430 1442 +12
- Partials 813 823 +10
🚀 New features to boost your workflow:
|
`context prefilter GUIDANCE SESSIONS...` splits an AGENTS.md or CLAUDE.md into rules (novelty span segmentation) and reports per rule how many sessions brought up its situation (trigger), whether the agent ran the command or touched the path it names (acted), and whether the user pushed back afterwards (corrected). Deterministic, no model call. Output is a ranked list of candidates for ablation, not a verdict. Rules with protection tags, rules triggered in few sessions, and rules in use are listed but not ranked, so a rare safety rule is never ranked as waste. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…word stems A Claude Code line nested too deeply for json.loads raises RecursionError, which the reader did not catch, so one line aborted the whole session. It now counts as a line that is not JSON. The keyword stems of an event are cached, since every claim is matched against the same event texts. The design note states that the JSON report is printed, not stored, so it has no schema under dokimi/schemas/. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
… a segment start str.splitlines() breaks a JSON line at U+2028, which Claude Code can write unescaped inside a prompt, so that prompt was lost and reported as two bad lines. Lines are now split on newline only. A path named in backticks matched any path that contained it, so a rule naming a.md counted an edit of data.md as acting on it. A name now has to start at a path segment. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Adds the prefilter route to the cost table, three allowlist rows for its threshold spellings (no registry concept yet), regenerates the CLI snapshot, raises the docs budget by one for the design note, and narrows an Any-typed command read so mypy passes. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Goose keeps each session's messages in SQLite (sessions.db under the Goose data directory). Each session in the store is now read as a session: text blocks of user messages are prompts, unless their metadata marks them not user-visible, and toolRequest blocks give the tool arguments. The layout is taken from the block/goose source; the tests use synthetic stores only, and the store is opened read-only. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
It was kept and counted in the sessions denominator, lowering every trigger rate. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
`a.md` matched `a.mdx` and `a.md.bak`; a name now ends where it ends. A directory (`docs/`) still matches the entries under it. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…s, validate thresholds - A file named twice (repeat, overlapping glob, symlink) is read once. - A Goose session of unreadable messages only is not counted, but its diagnostics still reach the command line. - The text blocks of one Goose user message are one prompt. - Goose's editor bodies (file_text, old_str, new_str) are left out of event text. - --rare-max takes 0 or more; --in-use-rate takes a number from 0 to 1. - cli-interface snapshot regenerated for the new argument types. Thread: PRRT_kwDOVBg7g86yWV7R Thread: PRRT_kwDOVBg7g86yWWKv Thread: PRRT_kwDOVBg7g86yWV7U Thread: PRRT_kwDOVBg7g86yWWK0 Thread: PRRT_kwDOVBg7g86yWV7V Thread: PRRT_kwDOVBg7g86yWWK3 Thread: PRRT_kwDOVBg7g86yWWK4 Thread: PRRT_kwDOVBg7g86yWV7b Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
- A rule with no prose keywords (only named commands, paths or a protection tag) is kept, and triggers on the event that names one of them. - Windows separators are normalized for path matching and classification; a backslash directory is a path. - A shell command that writes into a named directory counts as acting. - A command counts as run only at the start of a shell segment, after runner words (uv run, sudo, env assignments); echo, grep arguments and comments do not. - A correction is read from the user's next prompt only. - Candidates rank by exact triggered-session count, then the lowest acted share, so rounding cannot reverse the order. Thread: PRRT_kwDOVBg7g86yWV7W Thread: PRRT_kwDOVBg7g86yWWLE Thread: PRRT_kwDOVBg7g86yWV7X Thread: PRRT_kwDOVBg7g86yWWLH Thread: PRRT_kwDOVBg7g86yWWK_ Thread: PRRT_kwDOVBg7g86yWV7Y Thread: PRRT_kwDOVBg7g86yWWLQ Thread: PRRT_kwDOVBg7g86yWV7Z Thread: PRRT_kwDOVBg7g86yWV7a Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Each review finding gets a test that fails on the previous code: the keyless rules, Windows paths, shell-written directories, command invocation versus mention, the next-prompt correction, rank ordering, source de-duplication, threshold domains, Goose text blocks and diagnostics, and editor bodies. Thread: PRRT_kwDOVBg7g86yWV7R Thread: PRRT_kwDOVBg7g86yWWKv Thread: PRRT_kwDOVBg7g86yWV7U Thread: PRRT_kwDOVBg7g86yWWK0 Thread: PRRT_kwDOVBg7g86yWV7V Thread: PRRT_kwDOVBg7g86yWWK3 Thread: PRRT_kwDOVBg7g86yWWK4 Thread: PRRT_kwDOVBg7g86yWV7b Thread: PRRT_kwDOVBg7g86yWV7W Thread: PRRT_kwDOVBg7g86yWWLE Thread: PRRT_kwDOVBg7g86yWV7X Thread: PRRT_kwDOVBg7g86yWWLH Thread: PRRT_kwDOVBg7g86yWWK_ Thread: PRRT_kwDOVBg7g86yWV7Y Thread: PRRT_kwDOVBg7g86yWWLQ Thread: PRRT_kwDOVBg7g86yWV7Z Thread: PRRT_kwDOVBg7g86yWV7a Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The next-prompt correction, the shell-segment rule for acting, keyless rules, the status precedence and the rare range now match the code. Thread: PRRT_kwDOVBg7g86yWV7C Thread: PRRT_kwDOVBg7g86yWV7N Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
… quotes and comments - A rule with only named commands or paths triggers on a prompt that names one, not on the tool call that runs it (which would make it always acted). - Command matching ignores quoted arguments and comments, splits on & and parentheses, normalises spaces, and accepts timeout N as a runner word. Thread: PRRT_kwDOVBg7g86yWWLQ Thread: PRRT_kwDOVBg7g86yWV7W Thread: PRRT_kwDOVBg7g86yWWLE Thread: PRRT_kwDOVBg7g86yWV7Y Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
f4d6a05 to
27e620e
Compare
| words = " ".join(key.split()) | ||
| if not words: | ||
| return False | ||
| run = re.compile(rf"{_RUNNER}{re.escape(words)}(?![\w-])") |
There was a problem hiding this comment.
Must-fix: the end boundary accepts punctuation that can be part of a shell token. For example, _invokes('make ci-local', 'make ci-local.debug') returns true even though make runs a different target; _invokes('pytest', 'pytest.py') likewise counts a different executable. This inflates acted counts and can change candidate ordering. Require the final matched command/argument token to end (while still allowing additional whitespace-separated arguments), and extend the longer-command regression to dotted and slash-suffixed tokens.
| if not words: | ||
| return False | ||
| run = re.compile(rf"{_RUNNER}{re.escape(words)}(?![\w-])") | ||
| bare = _COMMENT.sub("", _QUOTED.sub("''", command)) |
There was a problem hiding this comment.
Must-fix: removing every quoted token also removes real command arguments. For a claim naming pytest tests/test_x.py, _invokes(..., 'pytest "tests/test_x.py"') returns false, although this runs exactly the named command. Commands with quoted arguments in the guidance itself also cannot match. Preserve and normalize quoted argument values while distinguishing executable positions from argument mentions; add a regression covering quoted test-file arguments alongside the existing echo/grep cases.
There was a problem hiding this comment.
Fixed in a8d3b2b: quotes are removed from the command and the key, and the quoted separators and # are kept as text.
pytest "tests/test_x.py" -q now runs the named pytest tests/test_x.py (test: quoted argument of the named command).
| if event.kind != "tool": | ||
| return False | ||
| return (any(_invokes(k, event.command) for k in claim.commands) | ||
| or _path_named(claim, (*event.paths, event.command))) |
There was a problem hiding this comment.
Must-fix: searching the entire shell command counts path mentions as file activity. For - Keep changelog.d/ current., a prompt of keep the changelog current followed by Bash echo 'changelog.d/x.md' produces acted: 1, although the command never accesses that directory. Shell comments have the same problem. Both log readers preserve the raw command here, so this affects Claude Code and Goose reports and can change candidate ordering. Distinguish shell path operands/redirection targets from literal text and comments, and add negative regressions alongside the existing shell-redirection test.
…ount; printed paths do not A command that runs make ci-local.debug does not run make ci-local. A quoted argument of the named command is kept (pytest "tests/x.py"). A path that echo or printf prints, or a comment names, is not an action; a redirection target and the operands of other commands are. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| `` `changelog.d/` ``). A command counts when it runs the named command at the start of | ||
| a shell segment (after `uv run`, `sudo` or environment assignments), so an `echo`, a | ||
| `grep` argument or a `#` comment that names it does not. A path counts when a tool call | ||
| names it, a shell command included; a Windows path matches with backslashes too. |
There was a problem hiding this comment.
Must-fix: this definition says any shell path mention counts, but _shell_paths() excludes echo/printf arguments and shell comments. For the rule Keep changelog.d/ current, the existing test with echo 'changelog.d/x.md' expects acted == 0, despite the command naming the path. Document these exclusions so users can interpret the reported counts correctly; distinguish a printed mention from a redirection target such as printf 'entry' > changelog.d/x.md, which does count.
Suggestion:
| names it, a shell command included; a Windows path matches with backslashes too. | |
| + names it in a path input or shell command, excluding shell comments and | |
| + echo/printf arguments (their redirection targets still count). A Windows path | |
| + matches with backslashes too. |
There was a problem hiding this comment.
Fixed in 67c65ee (docs). The Acted paragraph now lists the exclusions: shell comments, heredoc bodies, echo/printf arguments (redirect targets still count), quoted sentences, and a > inside quotes.
The Windows-backslash sentence is kept. The Limits section notes the one-word quoted argument case.
| segments that are not printing commands. Comments are gone and printed text is left out.""" | ||
| parts: list[str] = [] | ||
| for seg in _segments(command): | ||
| parts.extend(_REDIRECT.findall(seg)) |
There was a problem hiding this comment.
Must-fix: the earlier path-mention fix remains incomplete for quoted redirection text. For - Keep changelog.d/ current., a trigger prompt keep the changelog current followed by Bash echo '> changelog.d/x.md' produces acted: 1, although it only prints text. _segments() removes the quotes before this regex runs, so the quoted > becomes indistinguishable from a real redirection. This also affects printf and can change candidate tie-breaking. Extract redirections while quote context is still available, and add a regression test alongside the existing printed-path tests (retaining the real printf 'entry' > changelog.d/x.md case).
There was a problem hiding this comment.
Fixed in 67c65ee. The redirect search no longer sees > after unquoting: quoted separators, #, < and > are masked inside quotes, so echo '> changelog.d/x.md' gives acted 0.
Regression tests sit next to the printed-path tests, including printf '%s' '>>changelog.d/x'. A real redirect after quoted text still counts.
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at a8d3b2b. The earlier review rounds are fixed and CI is green, but the shell matching still counts things the agent never did, and the new tests leave the status thresholds unpinned. The inline comments are the points to fix before merge.
What I ran, on a Linux review host at a8d3b2b:
make ci-local: 30 passed, 0 failed, 7 skipped (the skips are jobs that do not run on a PR or need tools absent here).- Reproductions for the code findings, with no model call involved.
- A mutation run on the two new test files: 54 mutants, 28 pass the full suite, 5 of those equivalent.
- All 13 commits carry
Signed-off-by.
Checked, no issues
- CLI: exit 1 for a missing, unreadable, directory, corrupt or non-Goose input, exit 2 for
--window 0,--in-use-rate nanand--rare-max -1; a repeated file counts once. - Docs: flags, defaults, statuses and their precedence match
prefilter.py;cli-interface.jsonequals a fresh dump; the page is in themkdocs.ymlnav; the changelog fragment follows the format. - Tests: they write nothing outside
tmp_path.
Bot points I checked that no longer apply at this head (21)
- Copilot's and exai-bot's point on the status table does not apply: the doc now states the status precedence the code applies.
- exai-bot's point on repeated inputs does not apply: sessions are de-duplicated by real path and a repeated file counts once.
- exai-bot's point on threshold values does not apply:
--rare-maxand--in-use-rateare validated at parse time and-1andnanexit 2. - exai-bot's point on dropped Goose diagnostics does not apply: an unreadable-only session is kept out of the corpus and its diagnostics reach the CLI.
- exai-bot's point on rules with only backticked names does not apply: such rules and protected rules are kept.
- exai-bot's point on Windows separators does not apply: rule and event paths are normalized before matching.
- exai-bot's point on Bash path actions does not apply: shell redirections and operands are matched against the named path.
- exai-bot's point on corrections does not apply: only the first prompt after the trigger is read.
- exai-bot's point on rounded ranking does not apply: candidates sort on the exact triggered-session count.
- exai-bot's point on Goose editor bodies does not apply:
file_text,old_strandnew_strare excluded from event text. - Copilot's point on repeated inputs does not apply: the real-path de-duplication covers it.
- Copilot's point on threshold values does not apply: the parse-time validators cover it.
- Copilot's point on Goose diagnostics does not apply: they now reach the CLI output.
- Copilot's point on Goose text blocks does not apply: the blocks of one message are joined into one prompt event.
- Copilot's point on backslash directories does not apply: a key with a backslash is classified as a path.
- Copilot's point on command-only rules does not apply: they stay in the report.
- Copilot's point on Windows separators does not apply: paths are normalized on both sides.
- Copilot's point on echo and grep mentions does not apply: a command counts only at the start of a shell segment.
- exai-bot's point on
make ci-local.debugdoes not apply: a command key now ends at its last word. - exai-bot's point on quoted arguments does not apply:
pytest "tests/test_x.py"now matches the named command. - exai-bot's point on echoed path text does not apply: echo and printf arguments and comments are no longer path mentions.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
| if isinstance(rec, dict): | ||
| session.events.extend(_events_of(rec)) |
There was a problem hiding this comment.
Must-fix: diagnose invalid JSON shapes instead of silently dropping them. For example, parse_session() given a valid user prompt followed by a line containing [] returns no errors, so the CLI gives no indication that part of the input was unreadable. The same issue exists in goose_log.py: object-valued content_json is silently discarded by _events_of() rather than reported. This can hide incompatible or damaged logs while producing rankings from incomplete evidence. Validate the top-level record/content shapes, record a line/message diagnostic through add_error(), and add mixed-valid/invalid-shape tests for both readers. Continue skipping recognized non-event records such as summaries and tool results.
There was a problem hiding this comment.
Fixed in 67c65ee. parse_session reports a line that is JSON but not an object ([], 42) as not a JSON object, and a prompt or tool record whose message is not an object or whose content is neither text nor a list.
Goose reports a message whose content_json is not a list of blocks, by message id. Tool results and summaries are still skipped without a diagnostic.
Mixed valid and invalid tests cover both readers.
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to b024307, which fix the two nits from my review:
7af97be:dokimi/docs/session-prefilter.md:43: the path rule now says shell comments and echo/printf arguments do not count, while their redirection targets still do.b024307:dokimi/dokimi/commands/context.py:166: the command prints a warning to stderr when the guidance file yields no rules.
make ci-local at b024307: 29 passed, 1 failed (host timing flake, passes alone), 7 skipped.
Not approving yet. Still open: the three bugs and the two risks from my review.
…lter; report bad session shapes - A `>` inside quotes is text, not a redirection, so printed or quoted `> path` no longer counts as acting on the path. - A heredoc body is data: its lines are dropped before the command is split. - A bare file name in backticks (Makefile, Dockerfile, LICENSE) matches an edit of that file. - A quoted argument with a space is a sentence, not a path: a commit message that names a file does not act on it. A one-word quoted argument still counts. - Boundary tests for min_sessions, rare_max, in_use_rate, per-session episodes, the window and the rank tie-break. - A line that is JSON but not an object, a record with a non-object message or bad content, and a Goose message whose content is not a list are reported with add_error and skipped. Summaries and tool results are still skipped. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| _WORD = re.compile(r"[a-z][a-z0-9_-]{2,}") | ||
| _PATH = re.compile(r"[/\\]|\.[A-Za-z0-9]{1,6}$") | ||
| # a shell command's runner words before the command it runs (``uv run pytest``, ``sudo make``, ``A=1 make``) | ||
| _RUNNER = r"(?:(?:[A-Za-z_]\w*=\S*|sudo|env|time|nohup|exec|npx|uv\s+run|poetry\s+run|timeout\s+\d+)\s+)*" |
There was a problem hiding this comment.
Must-fix: runner options prevent real invocations from being counted. For the existing CI guidance and a push the branch now prompt followed by sudo -n make ci-local, _invokes('make ci-local', ...) returns false, although sudo executes that command. The same omission affects env -u PYTHONPATH make ci-local and timeout --signal=TERM 600 make ci-local. This understates acted counts and can change candidate tie-breaking. Parse supported runners' options (including options with arguments) before matching the invoked command, and add regression tests for these forms.
There was a problem hiding this comment.
Fixed in aafa091: runner options are read with the shell walker, so sudo -n, env -u X, timeout --signal=TERM 600 and uv run --frozen still count the command as run.
Regression tests added for each form (red before the fix).
| _PATH = re.compile(r"[/\\]|\.[A-Za-z0-9]{1,6}$") | ||
| # a shell command's runner words before the command it runs (``uv run pytest``, ``sudo make``, ``A=1 make``) | ||
| _RUNNER = r"(?:(?:[A-Za-z_]\w*=\S*|sudo|env|time|nohup|exec|npx|uv\s+run|poetry\s+run|timeout\s+\d+)\s+)*" | ||
| _QUOTED = re.compile(r"'[^']*'|\"[^\"]*\"") # a quoted argument is text, not a command |
There was a problem hiding this comment.
Must-fix: this pattern treats an escaped double quote as the end of a quoted argument. For a trigger followed by the shell command echo "say \" ; make ci-local ; \"", the shell only prints text, but _segments() exposes make ci-local as a separate segment and counts it as acted. The same quote pairing is used by _hide_sentence, so escaped quotes can also expose quoted path mentions to _shell_paths. Handle shell escapes when identifying quoted regions, and add regressions for both command and path matching.
There was a problem hiding this comment.
Fixed in aafa091: a backslash escapes the next character, also inside double quotes, on both the command and the path side.
echo "say \" ; make ci-local ; \"" is text, not a run. Tests added (red before the fix).
| if hide_spaced: | ||
| text = _QUOTED.sub(_hide_sentence, text) # the same pairing of quotes as _mask | ||
| masked = _COMMENT.sub("", _QUOTED.sub(_mask, text)) | ||
| return [_unquote(seg) for seg in _SEGMENT.split(masked)] |
There was a problem hiding this comment.
Must-fix: an unquoted backslash-newline is a shell line continuation, not a command separator. With a trigger followed by echo \\ (one literal backslash), a newline, and make ci-local, the shell prints make ci-local; this split instead creates a segment starting with that command and reports acted=1. Conversely, splitting make \\ followed by a newline and ci-local misses a real invocation. Process shell line continuations before segment splitting, while preserving literal newlines in single-quoted arguments, and test both cases.
There was a problem hiding this comment.
Fixed in aafa091: an unquoted backslash-newline joins the line before splitting; a newline inside single quotes is kept.
echo \ + newline + make ci-local is text; make \ + newline + ci-local is a run. Tests added for both (red before the fix).
| words = " ".join(_unquote(key).split()) | ||
| if not words: | ||
| return False | ||
| run = re.compile(rf"{_RUNNER}{re.escape(words)}(?=\s|$)") |
There was a problem hiding this comment.
Must-fix: shell redirection operators also terminate command words. For guidance - Run \pytest` when testing changes.and events containing the prompttesting changesfollowed bypytest>results.txt, this pattern rejects the executed command because >is neither whitespace nor the end of the segment, soactedis incorrectly 0. The same happens withpytest<input.txtand named multiword commands such asmake ci-local>results.txt`. Recognize unquoted redirection operators as token boundaries (without accepting longer command names), and add regression tests for redirects without surrounding spaces.
There was a problem hiding this comment.
Fixed in aafa091: a redirection sign right after the command ends the command word, so pytest>results.txt, pytest<input.txt and make ci-local>results.txt count as runs. Longer command names still do not match. Tests added (red before the fix).
…and line continuations A command counts as run when its program words start with the claim's command, after the wrappers the shell walker knows (sudo -n, env -u X, timeout --signal=TERM 600, uv run, A=1). A redirection right after it (make ci-local>log) still runs it. A backslash escapes the next character, also inside double quotes, and an unquoted backslash-newline joins the line, so a quoted or escaped separator is text and a split line is one command. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| matches = [name] if os.path.exists(name) else sorted(glob.glob(os.path.expanduser(name), recursive=True)) | ||
| for path in matches or [name]: |
There was a problem hiding this comment.
Must-fix: expand ~ before checking whether the input is a literal file. For example, with both $HOME/logs/session[1].jsonl and $HOME/logs/session1.jsonl present, passing '~/logs/session[1].jsonl' fails the unexpanded existence check and glob expansion silently loads session1.jsonl instead. If only the intended file exists, it is rejected as unreadable. This can change the corpus and rankings. Check existence on the expanded path, use it for the fallback too, and add a home-relative literal-path regression test.
Suggestion:
| matches = [name] if os.path.exists(name) else sorted(glob.glob(os.path.expanduser(name), recursive=True)) | |
| for path in matches or [name]: | |
| expanded = os.path.expanduser(name) | |
| matches = [expanded] if os.path.exists(expanded) else sorted(glob.glob(expanded, recursive=True)) | |
| for path in matches or [expanded]: |
There was a problem hiding this comment.
Fixed in 58f7428: ~ is expanded before the literal-file check, and the glob fallback uses the expanded name too, so ~/logs/session[1].jsonl reads that file. Tests: test_a_glob_path_with_brackets_under_home_is_read_as_the_literal_file and test_a_home_relative_literal_path_is_read_when_only_that_file_exists (HOME monkeypatched).
| def _masked_segments(command: str, *, hide_spaced: bool = False) -> list[str]: | ||
| """The command's segments with their quotes removed, still masked. A heredoc body is dropped; with | ||
| ``hide_spaced`` a quoted argument with a space in it is replaced by a placeholder.""" | ||
| text = _CONTINUATION.sub(_join_line, _HEREDOC.sub(r"\1", command or "")) |
There was a problem hiding this comment.
Must-fix: heredoc removal runs before quote handling, so quoted << text can erase actual commands. For a triggered rule naming changelog.d/, the valid command echo "<<true"\ncat changelog.d/x.md\ntrue makes this regex discard the cat line as a heredoc body, reporting acted=0 instead of 1 and potentially changing candidate rankings. The same quote-unaware removal already exists in shell.split_commands, now used by _invokes, so replacing cat with make ci-local also hides a real invocation. Recognize heredoc operators only outside quotes/escaped text, preferably through shared parsing, and add regressions for both path and command actions alongside the real-heredoc tests.
There was a problem hiding this comment.
Fixed in 58f7428: a << inside quotes no longer opens a heredoc. Bodies are dropped line by line and their quotes are not paired with the rest of the line, so echo "<<true" then cat changelog.d/x.md then true counts the cat. Tests: the quoted-<< cases in test_context_prefilter.py (path and command actions).
| (``sudo -n``, ``env -u X``, ``timeout 600``, ``uv run``, ``A=1``), so an ``echo``, a ``grep`` argument or a | ||
| ``#`` comment that names it does not count. The key ends where its last word ends: ``make ci-local`` does | ||
| not run ``make ci-local.debug``, while a redirection after it (``make ci-local>log``) still runs it.""" | ||
| words = _unquote(key).split() |
There was a problem hiding this comment.
Must-fix: wrappers are stripped from the executed command but not from the guidance key. For guidance - Run python -m pytest when testing changes. and a testing changes prompt followed by the exact command python -m pytest, the actual tokens become ['pytest'] while the key remains ['python', '-m', 'pytest'], so acted is incorrectly zero. The same mismatch affects keys such as uv run pytest. Normalize both sides consistently and test wrapped commands named explicitly in guidance.
There was a problem hiding this comment.
Fixed in 58f7428: the key is read with the same wrapper stripping as the command (python -m pytest, uv run pytest, sudo make install, timeout 300 make test); the stripped key is kept when it is non-empty. Test: test_a_wrapper_in_the_guidance_key_matches_the_same_wrapped_command (4 cases).
| words = _unquote(key).split() | ||
| if not words: | ||
| return False | ||
| return any(shell.strip_wrappers(shell.tokens(seg))[:len(words)] == words for seg in shell.segments(command)) |
There was a problem hiding this comment.
Must-fix: this checks only the first token sequence returned by shell.segments, which does not split pipelines or remove compound-command syntax. After a push the branch now prompt, true | make ci-local, (make ci-local), and if true; then make ci-local; fi all run the named command but report acted=0. Tokenize each pipeline stage and remove compound syntax (using shell.clean) before matching; add regression tests for these forms.
There was a problem hiding this comment.
Fixed in 58f7428: commands are split at |, ( and ) outside quotes, and then/do/( are cleaned off each stage with shell.clean. So true | make ci-local, (make ci-local), if ...; then make ci-local; fi and for ...; do make ci-local; done count. Test: test_a_command_run_as_a_pipeline_stage_or_inside_compound_syntax_is_an_action.
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Not approving. The earlier threads are fixed, but the last commit makes _invokes miss commands it used to count, and two more shell and input cases still give a wrong acted or a wrong corpus. Seven points: four bugs and two risks inline or below, one nit.
What I ran, on a Linux review host at aafa091:
make ci-local: 30 passed, 0 failed, 7 skipped (jobs that do not run on a PR or need tools absent here); 12,086 tests, changed-line coverage gate passes.- Reproductions for the code findings, with no model call involved.
- A mutation run on the two new test files: 51 mutants, 12 pass the full suite (the ones that matter are in point 2 below and the inline test-gap comment).
- All 17 commits carry
Signed-off-by.
Findings that cannot be anchored inline:
- Several mutants on the shell paths survive the full suite, and each decides
acted. With ruledocs/guide.md: dropping<<-handling in_HEREDOCturnscat > out.txt <<-EOFwith a tab-indented body naming the file fromactedFalse to True; dropping the quoted-delimiter case does the same for<<"EOF"; dropping the top-level\\[\s\S]alternative in_QUOTEDturnsecho \; cat docs/guide.mdfrom False to True; letting the redirect target admit<turnsecho hi >out.txt<docs/guide.mdfrom False to True. For a bare-name rule,key in pin_file_namedcountsMakefile.bakas an edit ofMakefile, and dropping_slashesthere stopsC:\r\Makefilefrom counting. Droppingstrip_wrappersin_printsmakessudo echo docs/guide.mdact. Add one case each next to the existing heredoc and printed-path tests.
Checked, no issues
- Threads: the quoted
>cases, the heredoc body, bare names (Makefileyes,Makefile.bakno), the quoted sentence ingit commit -mand the status boundaries all behave as the replies say; the one-word quoted argument stays as the maintainer decided. - CLI: a missing, unreadable, directory or non-UTF-8 input exits 1 with a message and no traceback; malformed records are reported as warnings.
- Docs: flags, defaults, statuses and precedence match the code;
cli-interface.jsonequals a fresh dump; the page is in themkdocs.ymlnav.
Bot points I checked that no longer apply at this head (28)
- exai-bot's point on the status table does not apply: the doc now states the status precedence the code applies.
- exai-bot's point on repeated inputs does not apply: sessions are de-duplicated by real path and a repeated file counts once.
- exai-bot's point on threshold values does not apply:
--rare-maxand--in-use-rateare validated at parse time. - exai-bot's point on dropped Goose diagnostics does not apply: an unreadable-only session is kept out of the corpus and its diagnostics reach the CLI.
- exai-bot's point on rules with only backticked names does not apply: such rules and protected rules are kept.
- exai-bot's point on Windows separators does not apply: rule and event paths are normalized before matching.
- exai-bot's point on Bash path actions does not apply: shell redirections and operands are matched against the named path.
- exai-bot's point on corrections does not apply: only the first prompt after the trigger is read.
- exai-bot's point on rounded ranking does not apply: candidates sort on the exact triggered-session count.
- exai-bot's point on Goose editor bodies does not apply:
file_text,old_strandnew_strare excluded from event text. - Copilot's point on repeated inputs does not apply: the real-path de-duplication covers it.
- Copilot's point on threshold values does not apply: the parse-time validators cover it.
- Copilot's point on Goose diagnostics does not apply: they reach the CLI output.
- Copilot's point on Goose text blocks does not apply: the blocks of one message are joined into one prompt event.
- Copilot's point on backslash directories does not apply: a key with a backslash is classified as a path.
- Copilot's point on command-only rules does not apply: they stay in the report.
- Copilot's point on Windows separators does not apply: paths are normalized on both sides.
- Copilot's point on echo and grep mentions does not apply: a command counts only as the program of a shell segment.
- exai-bot's point on
make ci-local.debugdoes not apply: a command key ends at its last word. - exai-bot's point on quoted arguments does not apply:
pytest "tests/test_x.py"matches the named command. - exai-bot's point on echoed path text does not apply: echo and printf arguments and comments are not path mentions.
- exai-bot's point on the path rule in the doc does not apply: the doc lists the exclusions.
- exai-bot's point on a quoted
>does not apply:echo '> changelog.d/x.md'gives acted 0. - exai-bot's point on silent JSON shapes does not apply: both readers report a bad shape through a diagnostic.
- exai-bot's point on runner options does not apply:
sudo -n make ci-localcounts as a run. - exai-bot's point on escaped double quotes does not apply:
echo "say \" ; make ci-local ; \""is text. - exai-bot's point on backslash-newline does not apply: an unquoted line continuation joins the line before splitting.
- exai-bot's point on redirections without spaces does not apply:
pytest>results.txtcounts as a run.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
|
|
||
| def _is_path(key: str) -> bool: | ||
| """A bare file name or path; a key with a space is a command line that names a file.""" | ||
| return " " not in key and bool(_PATH.search(key)) |
There was a problem hiding this comment.
Must-fix: this also classifies filenames containing spaces as commands, preventing explicit file-tool actions from matching. For guidance - Update docs/my guide.md when behaviour changes., a prompt update behaviour changes followed by an Edit with file_path='/repo/docs/my guide.md' reports acted=0: _invokes has no command to match, and _file_named compares the basename my guide.md with the entire key. The documented exclusion of quoted shell sentences does not apply to an explicit path input. Preserve spaced filenames as path keys (without misclassifying command lines), and add an Edit/Write regression test.
There was a problem hiding this comment.
Fixed in 58f7428: a key whose first word is a directory and whose last word has an extension (docs/my guide.md) is a path, so an Edit of /repo/docs/my guide.md counts. A command with a file argument (pytest "tests/my test.py") stays a command. Test: test_a_path_key_with_a_space_in_the_file_name_is_a_path_and_an_edit_of_it_is_an_action.
| (``sudo -n``, ``env -u X``, ``timeout 600``, ``uv run``, ``A=1``), so an ``echo``, a ``grep`` argument or a | ||
| ``#`` comment that names it does not count. The key ends where its last word ends: ``make ci-local`` does | ||
| not run ``make ci-local.debug``, while a redirection after it (``make ci-local>log``) still runs it.""" | ||
| words = _unquote(key).split() |
There was a problem hiding this comment.
Must-fix: removing quotes and then splitting the guidance key loses shell argument boundaries, whereas shell.tokens(seg) preserves them. For guidance - Run pytest "tests/my test.py" when testing changes., a testing changes prompt followed by the identical command reports acted=0: the key contains separate tests/my and test.py tokens, but the executed command contains one tests/my test.py token. Tokenize command keys with the same shell-aware tokenizer used for executed commands, and add a regression test for a quoted argument containing spaces.
There was a problem hiding this comment.
Fixed in 58f7428: command keys are read with the shell tokenizer, as the executed command is, so pytest "tests/my test.py" is two words on both sides. Test: test_a_key_with_a_quoted_argument_with_a_space_matches_the_same_argument.
| if event.kind != "tool": | ||
| return False | ||
| return (any(_invokes(k, event.command) for k in claim.commands) | ||
| or any(_file_named(k, event.paths) for k in claim.commands) |
There was a problem hiding this comment.
Must-fix: bare filenames are classified as command keys, but their file matching only examines explicit tool path inputs. For guidance - Edit Makefile carefully., a prompt edit the file carefully followed by Bash cat Makefile or sed -i 's/old/new/' Makefile reports acted=0, whereas an Edit tool targeting the same file reports acted=1. Include shell file references when matching bare filename keys (while retaining the exclusions for printed text/comments), and add a regression test for shell access to Makefile.
There was a problem hiding this comment.
Fixed in 58f7428: a capitalised bare file name key (Makefile, LICENSE) also matches a shell command that names it as an argument (cat Makefile, sed -i ... Makefile). A lowercase bare word stays a command, so grep pytest does not count, and echo Makefile does not either. Tests: test_shell_access_to_a_bare_file_name_key_is_an_action, test_a_printed_bare_file_name_key_is_not_an_action, test_a_lowercase_command_key_named_as_an_argument_is_not_an_action.
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to 39260e4, which fix the nit from my review:
39260e4:dokimi/docs/session-prefilter.md:47: added the sentence that a quoted path containing a space is read as a sentence and does not count.
make ci-local at 39260e4: 30 passed, 0 failed, 7 skipped.
Not approving yet. Still open: the four bugs and the two risks from my review.
…redoc signs; ~ globs - a wrapper in a guidance key (`python -m pytest`, `uv run pytest`) matches the same wrapped command - a command in a pipeline stage or compound syntax (`true | make ci-local`, `(make ci-local)`, `then`/`do`) counts - a quoted `<<` no longer erases the commands after it: heredoc bodies are read line by line - a quoted argument with a space is one word on both sides of the key match - a path key with a space in its name (`docs/my guide.md`) is a path - a capitalised bare file name (`Makefile`) matches a shell command that names it as an argument - `~` is expanded before the literal-file check in `context prefilter`, so `session[1].jsonl` is not a glob - a test for an action before the trigger inside the window Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| _CONTINUATION = re.compile(r"\\\n|\\[\s\S]|'[^']*'|\"(?:\\[\s\S]|[^\"\\])*\"") | ||
| # a heredoc header is ``<<WORD`` (or ``<<'WORD'``) outside quotes and not a here-string (``<<<``) | ||
| _HEREDOC_OPEN = re.compile(r"(?<!<)<<(?!<)") | ||
| _HEREDOC_WORD = re.compile(r"-?[ \t]*(['\"]?)(\w+)\1") |
There was a problem hiding this comment.
Must-fix: shell heredoc delimiters can contain punctuation, but \w+ either truncates them or rejects them. After a push the branch now prompt, cat <<EOF-TEXT\nnotes\nEOF-TEXT\nmake ci-local reports acted=0 because the reader waits for EOF and drops the real command. Conversely, cat <<'EOF-TEXT'\nmake ci-local\nEOF-TEXT reports acted=1 because the quoted delimiter is not recognized and its body is treated as executable code. Parse the entire shell delimiter word (with quote removal), and add tests for both quoted and unquoted punctuation-containing delimiters.
There was a problem hiding this comment.
Fixed in 8497905. A heredoc WORD is now any shell word, quoted or not (EOF-TEXT, 'EOF.md'), and the body ends on that WORD.
Pinned: cat <<EOF-TEXT ... EOF-TEXT then make ci-local acts; the quoted <<'EOF-TEXT' body does not.
|
|
||
| def _heredoc_words(line: str) -> list[str]: | ||
| """The words of the heredocs a line opens: a ``<<`` inside quotes is text and opens none.""" | ||
| masked = _QUOTED.sub(lambda m: "_" * len(m.group(0)), line) # same length: positions line up |
There was a problem hiding this comment.
Must-fix: heredoc detection scans shell comments before comments are removed. With a trigger followed by # <<EOF\nmake ci-local, _heredoc_words queues EOF and _without_heredoc_bodies drops the executable command, reporting acted=0 even though the shell runs it. An inline comment such as true # <<EOF has the same problem. Exclude unquoted comments before detecting heredoc headers, and add regressions for both forms; this affects both command matching and shell-path matching.
There was a problem hiding this comment.
Fixed in 8497905. Comments are blanked before heredoc headers are read, so # <<EOF and true # <<EOF open none.
Both forms are in the heredoc parametrized test; command and path matching share the same body removal.
| words = shell.strip_wrappers(text.split()) | ||
| return bool(words) and words[0] in ("echo", "printf") |
There was a problem hiding this comment.
Must-fix: printing-command detection does not remove compound-command syntax. For guidance - Keep changelog.d/ current., a matching prompt followed by if true; then echo changelog.d/x.md; fi reports acted=1: the segment starts with then, so _prints misses the echo and treats its printed argument as file access. do echo ... and else printf ... have the same issue. Strip compound syntax before checking the program (as _words_of already does), while retaining redirection targets, and test these cases.
There was a problem hiding this comment.
Fixed in 8497905. _prints reads the program after the compound keywords (shell.clean): if/then, for/do and else printf print their argument and touch no file.
A redirection target of such a command still counts. Pinned by a parametrized test.
| return False | ||
| shell_text = _shell_paths(event.command) | ||
| return (any(_invokes(k, event.command) for k in claim.commands) | ||
| or any(_file_named(k, event.paths) for k in claim.commands) |
There was a problem hiding this comment.
Must-fix: this file-name fallback also applies to lowercase command keys. For - Run pytest before pushing a branch., a push the branch now prompt followed by an Edit call with file_path="/repo/pytest" produces acted=1, although no pytest command ran. This contradicts the distinction between capitalised bare filenames and lowercase commands. Gate this fallback with _is_bare_file(k) and add a regression test for a lowercase command name appearing in an explicit tool path.
Suggestion:
| or any(_file_named(k, event.paths) for k in claim.commands) | |
| or any(_file_named(k, event.paths) for k in claim.commands if _is_bare_file(k)) |
There was a problem hiding this comment.
Already fixed before this push: 2683108 gates the file-name fallback with _is_bare_file(k), so a lowercase key (pytest) never matches an Edit of /repo/pytest.
Pinned by test_an_edit_of_a_file_named_like_a_lowercase_command_key_is_not_an_action, which passes on 8497905; this thread is outdated against the current head.
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Not approving yet. The five earlier threads are fixed, but the new heredoc reader and the print check still give a wrong acted on shell forms, and three test gaps let such changes through. Six points to fix and one nit, all inline.
What I ran, on a Linux review host at 58f7428:
make ci-local: 30 passed, 0 failed, 7 skipped (jobs that do not run on a PR or need tools absent here); 12,111 tests, the changed-line coverage gate passes.- Reproductions for the code findings, with no model call involved.
- A mutation run on the delta in
prefilter.pyandcommands/context.py: 23 mutants, the ones that decide a result and still pass the full suite are in the inline comments. - All 19 commits carry
Signed-off-by.
Checked, no issues
- Threads: a wrapper in the key (
python -m pytest,uv run pytest,sudo make install,timeout 300 make test), a command in a pipeline stage, parentheses,ifandfor, a quoted<<before a real command, a bracketed~path, and an action before the trigger all behave as the replies say. - Keys: a spaced path key (
docs/my guide.md) is a path and a command line with a file argument stays a command; a key made only of wrappers does not become empty. - Docs and repo rules: the Acted paragraph matches the code; defaults match
commands/context.py; the page is in themkdocs.ymlnav; the changelog fragment follows the format; nothing generated is committed; the new tests write only intotmp_path.
Bot points I checked that no longer apply at this head (35)
- exai-bot's point on the status table does not apply: the doc states the status precedence the code applies.
- exai-bot's point on repeated inputs does not apply: sessions are de-duplicated by real path and a repeated file counts once.
- exai-bot's point on threshold values does not apply:
--rare-maxand--in-use-rateare validated at parse time, and-1andnanexit 2. - exai-bot's point on dropped Goose diagnostics does not apply: an unreadable-only session stays out of the corpus and its diagnostics reach the CLI.
- exai-bot's point on rules with only backticked names does not apply: such rules and protected rules are kept.
- exai-bot's point on Windows separators in path matching does not apply: rule and event paths are normalized before matching.
- exai-bot's point on Bash path actions does not apply: redirection targets and operands in a shell command are matched against the named path.
- exai-bot's point on corrections does not apply: only the first prompt after the trigger is read.
- exai-bot's point on rounded ranking does not apply: candidates sort on the exact triggered-session count.
- exai-bot's point on Goose editor bodies does not apply:
file_text,old_strandnew_strare excluded from event text. - Copilot's point on repeated inputs does not apply: the real-path de-duplication covers it.
- Copilot's point on threshold values does not apply: the parse-time validators cover it.
- Copilot's point on Goose diagnostics does not apply: they reach the CLI output.
- Copilot's point on Goose text blocks does not apply: the blocks of one message are joined into one prompt event.
- Copilot's point on backslash directories does not apply: a key with a backslash is classified as a path.
- Copilot's point on command-only rules does not apply: they stay in the report.
- Copilot's point on Windows separators does not apply: paths are normalized on both sides.
- Copilot's point on echo and grep mentions does not apply: a command counts only as the program of a pipeline stage.
- exai-bot's point on
make ci-local.debugdoes not apply: a command key ends at its last word. - exai-bot's point on quoted arguments does not apply:
pytest "tests/test_x.py"matches the named command. - exai-bot's point on echoed path text does not apply: echo and printf arguments and comments are not path mentions.
- exai-bot's point on the path rule in the doc does not apply: the doc lists the exclusions.
- exai-bot's point on a quoted
>does not apply:echo '> changelog.d/x.md'gives acted 0. - exai-bot's point on silent JSON shapes does not apply: both readers report a bad shape through a diagnostic.
- exai-bot's point on runner options does not apply:
sudo -n make ci-localcounts as a run. - exai-bot's point on escaped double quotes does not apply:
echo "say \" ; make ci-local ; \""is text. - exai-bot's point on backslash-newline does not apply: an unquoted line continuation joins the line before splitting.
- exai-bot's point on redirections without spaces does not apply:
pytest>results.txtcounts as a run. - exai-bot's point on
~and a literal file does not apply:~is expanded before the existence check and the glob fallback. - exai-bot's point on a quoted
<<does not apply:echo "<<true"opens no heredoc, and thecatline after it counts. - exai-bot's point on wrappers in the guidance key does not apply:
python -m pytest,uv run pytest,sudo make installandtimeout 300 make testmatch the same wrapped command. - exai-bot's point on pipelines and compound syntax in
_invokesdoes not apply:true | make ci-local,(make ci-local),if ...; then make ci-local; fiandfor ...; do make ci-local; donecount as runs. - exai-bot's point on file names with spaces does not apply: a key like
docs/my guide.mdis a path and an Edit of it counts. - exai-bot's point on quoted key arguments does not apply: a key
pytest "tests/my test.py"is tokenized like the command. - exai-bot's point on bare file names does not apply:
cat Makefileandsed -i ... Makefilecount for aMakefilekey.
| _CONTINUATION = re.compile(r"\\\n|\\[\s\S]|'[^']*'|\"(?:\\[\s\S]|[^\"\\])*\"") | ||
| # a heredoc header is ``<<WORD`` (or ``<<'WORD'``) outside quotes and not a here-string (``<<<``) | ||
| _HEREDOC_OPEN = re.compile(r"(?<!<)<<(?!<)") | ||
| _HEREDOC_WORD = re.compile(r"-?[ \t]*(['\"]?)(\w+)\1") |
There was a problem hiding this comment.
I agree with exai-bot here; the repro is below.
A heredoc delimiter that is not \w+ is cut short, so the body is read as commands or the real command is dropped.
_HEREDOC_WORD takes (\w+), so <<EOF-TEXT is read as the word EOF. For the rule - Run `make ci-local` before pushing. _acts gives:
cat <<EOF-TEXT\nnotes\nEOF-TEXT\nmake ci-local: False. No line isEOF, so every later line is skipped as body and the real run is lost. The same withEOF.md. WithEOFas the word it is True.cat <<'EOF-TEXT'\nmake ci-local\nEOF-TEXT: True. The body is not skipped, so a line of data counts as a run. With'EOF'it is False.echo $((1<<2))\nmake ci-local: False. The shift is read as a heredoc with the word2.
A rule that was followed is ranked as if it was not, and the reverse.
Suggestion: read the whole delimiter word, with quotes removed ('([^']+)', "([^"]+)" or a run of characters other than whitespace and ;&|<>()), and do not open a heredoc for a << inside $((...)). Add the two delimiter cases above next to the existing heredoc tests.
There was a problem hiding this comment.
Fixed in 8497905, same change as the delimiter thread: the WORD is read whole with quotes removed, so <<EOF-TEXT is that word.
$((1<<2)) opens no heredoc (_ARITHMETIC is blanked first). Both cases are in the heredoc test.
| return "" if match.group(0) == "\\\n" else match.group(0) | ||
|
|
||
|
|
||
| def _heredoc_words(line: str) -> list[str]: |
There was a problem hiding this comment.
I agree with exai-bot here; the repro is below.
A << inside a comment opens a heredoc, and the real command after it is dropped.
_heredoc_words masks quotes but not comments, and _without_heredoc_bodies runs before _COMMENT is removed. For - Run `make ci-local` before pushing. _acts gives False for both:
# <<EOF\nmake ci-localtrue # <<EOF\nmake ci-local
The control true\nmake ci-local gives True. The same text hides a path from _shell_paths. A shell comment that mentions a heredoc is common in a script a session writes, and the rule is then ranked as not followed.
Suggestion: after the quote masking in _heredoc_words, blank out an unquoted comment with spaces of the same length (re.sub(r"(?:^|(?<=\s))#.*$", lambda m: " " * len(m.group(0)), masked)) so positions still line up, and add both cases above to the heredoc tests.
There was a problem hiding this comment.
Fixed in 8497905. _heredoc_words blanks an unquoted comment before it looks for a <<, so # <<EOF and true # <<EOF run make ci-local.
Tests added next to the heredoc cases.
| return any(shell.strip_wrappers(_words_of(seg))[:len(words)] == words for seg in _command_segments(command)) | ||
|
|
||
|
|
||
| def _prints(text: str) -> bool: |
There was a problem hiding this comment.
I agree with exai-bot here; the repro is below.
A printed path inside then, do or else counts as touching the file.
_prints takes the first word of the segment, which is then in then echo changelog.d/x.md, so the segment is kept as a path mention. _invokes already cleans compound syntax with shell.clean; _prints does not. For - Keep `changelog.d/` current. _acts gives True for:
if true; then echo changelog.d/x.md; fifor i in 1; do echo changelog.d/x.md; done
Plain echo changelog.d/x.md gives False. Nothing was written, but acted rises and the rule moves down the candidate ranking.
Suggestion: words = shell.strip_wrappers(shell.clean(text).split()), check that else printf ... is covered too, and add the three forms to the printed-path tests.
There was a problem hiding this comment.
Fixed in 8497905. _prints uses shell.clean, so then, do and else are not the program: if true; then echo ..., for ...; do echo ... and else printf ... no longer touch the file.
A positive control (then cat changelog.d/x.md acts) pins the keyword handling.
| pending: list[str] = [] # the WORDs of the bodies still to skip, in order | ||
| for line in command.split("\n"): | ||
| if pending: | ||
| if line.strip() == pending[0]: |
There was a problem hiding this comment.
No test pins how a heredoc ends, and <<- with an indented terminator is the case that breaks.
Mutation check: line.strip() == pending[0] to line == pending[0] passes all 139 tests in test_context_prefilter.py and test_context_goose.py. With the mutant cat <<-EOF\n\tbody\n\tEOF\nmake ci-local gives _acts False; on the real code it is True. An indented terminator after a plain <<EOF behaves the same. The body then swallows the next command and acted is wrong.
Suggestion: add a test with a <<-EOF heredoc whose terminator is tab-indented and a command on the next line.
There was a problem hiding this comment.
Fixed in 8497905 (tests and the close rule). A plain << closes only on a WORD alone, as bash does, so an indented EOF no longer ends its body; <<- still closes on a tab-indented WORD.
Both cases are pinned in the heredoc test.
| continue | ||
| episodes += 1 | ||
| around = events[max(0, i - window): i + window + 1] | ||
| after = events[i + 1: i + window + 1] |
There was a problem hiding this comment.
The last event of the correction window and the episode skip are not pinned by any test.
My earlier review named the window bounds in _scan; the new tests cover the action side only. Mutation check, each passes all 139 tests:
after = events[i + 1: i + window](line 337). With window 3 and[trigger prompt, tool, tool, "no, the branch must not go before ci"],_scanreturns(1, 0, 1)on the real code and(1, 0, 0)on the mutant. A correction exactlywindowevents after the trigger is lost, andcorrectedanduser_overridesdrop.i += windowinstead ofi += window + 1(line 341). With window 3 and a second trigger prompt right after the window,_scanreturns(1, 0, 0)on the real code and(2, 0, 0)on the mutant.episodesand the trigger rate rise.
Suggestion: one case for each: a correction exactly window events after the trigger and one at window + 1; two triggers window and window + 1 apart, asserting episodes.
There was a problem hiding this comment.
Fixed in 8497905 (tests). Pinned: a correction exactly window events after the trigger counts and window + 1 does not; triggers window apart are one episode, window + 1 apart two.
I checked locally that changing either bound fails a test.
| loaded, failed, seen = [], False, set() | ||
| for name in names: | ||
| expanded = os.path.expanduser(name) # a literal file is checked after ~ is expanded, before any glob | ||
| matches = [expanded] if os.path.exists(expanded) else sorted(glob.glob(expanded, recursive=True)) |
There was a problem hiding this comment.
No test globs under ~, so the expansion in glob.glob can be dropped without a failing test.
Mutation check: glob.glob(expanded, recursive=True) to glob.glob(name, recursive=True) passes all 139 tests. The two new ~ tests use a literal file that exists, which never reaches the glob. With the mutant context prefilter AGENTS.md '~/.claude/projects/x/*.jsonl', the usage the --help text gives, finds no file and exits 1 with cannot read.
Suggestion: add a test with HOME set to tmp_path, two files under ~/logs/, and the input ~/logs/*.jsonl, asserting both sessions load.
There was a problem hiding this comment.
Fixed in 8497905 (test). test_a_home_glob_reads_every_match sets HOME, writes two files under ~/logs/ and reads ~/logs/*.jsonl: both are loaded.
I checked locally that dropping the expansion in glob.glob fails it.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
| # a heredoc header is ``<<WORD`` (or ``<<'WORD'``) outside quotes and not a here-string (``<<<``) | ||
| _HEREDOC_OPEN = re.compile(r"(?<!<)<<(?!<)") | ||
| _HEREDOC_WORD = re.compile(r"-?[ \t]*(['\"]?)(\w+)\1") | ||
| _COMMENT = re.compile(r"(?m)(?:^|\s)#.*$") |
There was a problem hiding this comment.
Must-fix: shell comments can start immediately after a control operator, not only after whitespace. With a matching prompt and true;# ignored; make ci-local, the shell never runs make ci-local, but this regex leaves the comment intact and _command_segments splits its second semicolon into a command, reporting acted=1. _shell_paths likewise counts paths in true;# changelog.d/x.md. Recognize comment starts at shell token boundaries before splitting, and add regressions for both command and path matching.
There was a problem hiding this comment.
Fixed in 8497905. A # starts a comment after a control operator too (true;# ...), so the make ci-local after its second ; is not a command.
Pinned for the command case.
| def _unquote(text: str) -> str: | ||
| return text.replace('"', "").replace("'", "") |
There was a problem hiding this comment.
Must-fix: removing every quote also removes literal filename characters inside shell quotes. For guidance - Update docs/oreilly.md when behaviour changes., a matching prompt followed by cat "docs/o'reilly.md" reports acted=1 even though a different file was read. Conversely, guidance naming docs/o'reilly.md reports acted=0 for that command. Decode syntactic quoting without deleting quoted/escaped literal characters, and add tests for both the exact filename and the near-match.
There was a problem hiding this comment.
Fixed in 8497905. Quotes are removed as quoting, not by deleting every quote: cat "docs/o'reilly.md" no longer matches docs/oreilly.md, and matches docs/o'reilly.md.
Both cases are pinned.
| words = _key_words(key) | ||
| if not words: | ||
| return False | ||
| return any(shell.strip_wrappers(_words_of(seg))[:len(words)] == words for seg in _command_segments(command)) |
There was a problem hiding this comment.
Must-fix: function definitions are counted as command executions. After a matching prompt, check_ci() { make ci-local; } reports acted=1 even though the shell only defines the function and never runs its body. _command_segments splits the parentheses and _words_of removes the opening brace, exposing the body as an executed command; _shell_paths similarly counts file accesses inside uncalled functions. Exclude definition bodies from action matching (or conservatively treat definitions as unsupported) and test an uncalled function containing the named command/path.
There was a problem hiding this comment.
Fixed in 8497905. A function definition's body is dropped before matching, so check_ci() { make ci-local; } acts 0, and so does a path in an uncalled body.
Known limit, now in session-prefilter.md: a call of the function is not followed.
|
|
||
|
|
||
| # a redirection target is written | ||
| _REDIRECT = re.compile(r">>?\s*([^\s;&|<>]+)") |
There was a problem hiding this comment.
Must-fix: input-redirection targets are lost for printing commands. For guidance - Update docs/guide.md when behaviour changes., a matching prompt followed by printf 'done\n' < docs/guide.md reports acted=0: this regex only collects output targets, and _prints then discards the entire segment. The shell opens the named file for input, so this is a path access, not a printed mention. Extract input as well as output redirection targets (while excluding heredoc delimiters and descriptor duplication), and add a regression test.
There was a problem hiding this comment.
Fixed in 8497905. < now reads its target: printf 'done\n' < docs/guide.md touches docs/guide.md. Heredoc << and descriptors (<&, >&2) are not targets.
Pinned by a test.
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to 2683108, which fix the nit from my review:
2683108:dokimi/dokimi/context/prefilter.py:299: the file-name match now applies only to capitalised bare file names, and a test covers an Edit of/repo/pytestand/repo/make.
make ci-local at 2683108: 30 passed, 0 failed, 7 skipped.
GitHub CI has not run on these commits: the branch conflicts with main, and Actions runs no workflow on a conflicting PR. CI will check them once main is merged in.
Not approving yet. Still open: the two bugs and the four risks from my review.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> # Conflicts: # dokimi/tests/data/cli_flags_allowlist.txt
…way the shell does - A heredoc WORD is any shell word: punctuation or quotes. <<- closes on a tab-indented WORD; << only on a WORD alone. - A << in a comment or in $((...)) opens no heredoc; a # starts a comment after a control operator too. - Compound syntax (then, do, else) is not the program of a printing command. - A < reads its file; > and >> write it, as before. - Quotes are removed, not the text: a quoted apostrophe or escape keeps its literal character. - A shell function body counts only as defined, not run; a call of it is not followed (documented). - Pins: an edit at the window edge and one more, episodes window and window+1 apart, a home glob. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| argument with a space in it is text, so `git commit -m "update docs/guide.md"` does not act on | ||
| the file, while `cat "docs/guide.md"` does. A quoted path that itself contains a space is read as a | ||
| sentence and does not count. A `>` inside quotes is text, not a redirection. A `<` reads its file, and a | ||
| shell function's body counts only as defined, not run: a call of the function is not followed. |
There was a problem hiding this comment.
Nit: “counts only as defined” suggests that defining a function contributes to acted. The implementation removes function definitions before command/path matching, and the tests assert acted == 0 for both check_ci() { make ci-local; } and an uncalled body naming changelog.d/x.md. State explicitly that function bodies are ignored and calls are not followed.
Suggestion:
| shell function's body counts only as defined, not run: a call of the function is not followed. | |
| shell function's definition does not count as acting: its body is ignored, and calls of the function are not followed. |
| # ``$((1<<2))`` is arithmetic, not a heredoc | ||
| _ARITHMETIC = re.compile(r"\$\(\([\s\S]*?\)\)") | ||
| # a shell function definition: its body is defined, not run (a call of it is not followed) | ||
| _FUNCTION = re.compile(r"(?s)\b[A-Za-z_][\w-]*\s*\(\s*\)\s*\{.*?\}") |
There was a problem hiding this comment.
Must-fix: this non-greedy match ends at the first }, including one inside quotes or a nested brace group. For example, after a matching prompt, check_ci() { echo '}'; make ci-local; } leaves '; make ci-local; } for command matching, so make ci-local reports acted=1 although the function was only defined. The same leak affects path matching, e.g. a subsequent cat changelog.d/x.md inside that body. Strip bodies with quote-aware, balanced-brace scanning and add regressions for quoted closing braces and nested groups; the existing tests cover only flat bodies.
| """The simple commands of a shell line, quotes kept: split at separators and pipes outside quotes, with | ||
| heredoc bodies, line continuations and comments left out. Read here, not by ``shell.segments``, because that | ||
| walker cuts a heredoc at a ``<<`` inside quotes and would hide the command after it.""" | ||
| masked = _COMMENT.sub("", _QUOTED.sub(_mask, _executable(command))) |
There was a problem hiding this comment.
Must-fix: quote masking runs before comment removal, so an apostrophe in a comment can pair with a quote on a later line and mask the intervening newlines. For example, # don't skip CI\nmake ci-local\n# user's request is valid shell that runs make ci-local, but the masked text becomes one comment and _invokes returns false (acted=0 after a matching prompt). _masked_segments has the same ordering and can similarly lose real path accesses. Ignore quotes inside comments while scanning, and add regression tests for command and path matching after comments containing apostrophes.
| words = _key_words(key) | ||
| if not words: | ||
| return False | ||
| return any(shell.strip_wrappers(_words_of(seg))[:len(words)] == words for seg in _command_segments(command)) |
There was a problem hiding this comment.
Must-fix: redirections before the program or between its arguments prevent a real invocation from matching. For example, after the prompt push the branch now, a Bash call with >results.txt make ci-local (or make >results.txt ci-local) reports acted=0 for the make ci-local rule, although both commands run that target. shell.tokens retains the redirection operator and target, and strip_wrappers does not remove them, so the prefix comparison fails. Remove redirection operators and their operands from the invocation words before comparing, and add tests for leading and interleaved redirections.
Summary
context prefilterreads an agent's session logs and reports, per rule of an AGENTS.md or CLAUDE.md, whether the situation the rule describes came up, whether the agent acted on it, and whether the user pushed back. The output is a ranked list of candidates for ablation, not a verdict. Rare and protected rules are listed, never ranked. Output never says a rule should be cut.Statuses:
candidate_for_ablation(ranked),in_use,rare_not_ranked,kept_protected(negation, version gate, ordering or threshold tag),insufficient_corpus.Session logs are read as follows:
~/.claude/projects/<project>/. Injected records (isMeta), subagent briefs (isSidechain) and tool results are not prompts.sessions.db(~/.local/share/goose/sessions/on Linux), opened read-only. Layout taken from block/goose source (session_manager.rs,message.rs,tool_result_serde.rs):messages.session_id,content_jsonas a list of blocks,toolRequest.toolCall.value.arguments, andmetadata_json.userVisible. Related: rp-why: Configuration Effectiveness (CE) - Phase 1 block/agent-skills#52 (rp-why CE reads Goose logs). That PR queriesrun_idand readstoolCall.name; the source sayssession_idandtoolCall.value.name, and this reader follows the source.Output: a table on stdout, or
--jsonon stdout. The JSON is printed, not stored, so no schema underdokimi/schemas/;format_versionmarks its shape. Design note:docs/session-prefilter.md. Related: agentsmd/agents.md#213.Other changes that come with the command:
spec.v1.yaml: golden manifestmax_docs37 → 38 (one more doc).ce-replaymakes the same bump; whichever merges second rebases.cli_routes.py,cli_flags_allowlist.txt(--min-sessions,--rare-max,--in-use-rate: no registry concept yet), regenerateddocs/reference/cli-interface.json.Evidence
dokimi context prefilterunknown; Goose stores not readable by any command.test_context_prefilter,test_context_goose,test_cli_flags,test_cli_routes,test_docs_code_refs,test_docs_links,test_import_architecture) → 169 passed onf4d6a05b. Tests use synthetic JSONL and synthetic SQLite stores only; no real transcript or real Goose store was read.make ci-localon26ad2407(run before the last commit): 29 pass, 7 skip, oneFAILrow,ci.yml:test / Run tests. The failures are unrelated timing-sensitive tests, run on a host under load ~300 (shared with other sessions' benchmarks):test_repo_environment.py::test_unbalanced_quote_in_hook_completes_quickly,test_repo_environment.py::test_unclosed_heredocs_are_linear,test_reference_checks.py::test_the_regex_limit_is_shared_by_every_pattern_of_one_check,test_reference_checks.py::test_a_regex_that_backtracks_past_the_timeout_is_unscored[True],test_audit_test_quality.py::test_no_new_test_that_cannot_fail. Across three full runs a different subset failed each time. Each of the five passes when run alone onf4d6a05b. The full ci-local was not re-run onf4d6a05b. GitHub CI is the check of record.Merge Danger
Door: two-way. A new read-only subcommand. It writes no file, stores no output and changes no existing command. The Goose store is opened
mode=ro.Blast Radius: low. The new subcommand, and the golden manifest and CLI snapshot regenerated with it.
Known limits and follow-ups (not in this PR):
26ad2407: one non-blocking finding fixed (an unreadable-only Goose session was counted in the session total). Pass 2 on the last commit: a file name no longer matches a longer name or its backup (a.mdvsa.mdx,a.md.bak), with tests. No blocking finding open.🤖 Generated with Claude Code