Repository navigation
Conversation
`repo reach --format receipt` prints, and writes to effective_instructions.json in --out, the effective-instructions/v0 receipt (agentsmd/agents.md#213) of each probed scenario. It joins the probe's marker tokens with budget's start-up rules (Claude Code only): per source the path, sha256, scope, expected state, observed state, observation_method, the harness name and version, the displacements seen in the run, and the unknowns. followed is always unknown. The mapping and its limits are in docs/repo-evaluation-contract.md §7.1. The receipt is validated against schemas/effective_instructions.v1.json and registered in artifacts.SCHEMA_VERSIONS. The default output of repo reach is unchanged. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
A local source's tree label is its absolute path, and the receipt copied it verbatim: a shared effective-instructions receipt then carried the home path. An absolute or drive-letter or ~ label now keeps only its last directory; URLs and relative labels are unchanged. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…r method The receipt's observation is dokimi's marker self-report, not a captured prompt: observation_method is unknown (the thread vocabulary has no method that fits), and dokimi_method is marker_self_report on every source the probe recorded. Schema v1 is edited in place, unreleased. repo reach --from <reach.json> builds the matrix or the receipt from a saved probe without a harness run. The source is read to check the file's repository, harness and commit; a mismatch, a missing or malformed file, or a probe-only flag is a usage error (exit 2). The receipt is written beside the saved file. --model is required only when probing. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
github-actions
Bot
added
the
cli-interface-change
CLI interface change detectedThis PR changes dokimi's user-facing CLI surface (commands and/or options): ~ `dokimi repo reach` parameters changed:
+ added option `--format`
+ added option `--from`
~ changed option `--model`: required: True -> False
~ changed option `--out`: default: 'reach-out' -> None
~ `dokimi runs list` parameters changed:
~ changed option `--kind`: choices: ["'answers'", "'arms'", "'cli_routes'", "'eval'", "'explore_runs'", "'guidance_reach'", "'matrix_rollup'", "'novelty'", "'plan'", "'plugin_delta'", "'questions'", "'repo_budget'", "'repo_environment'", "'repo_readiness'", "'result'", "'subject_scorecard'", "'task_inspect'", "'task_runs'", "'tool_preference'"] -> ["'answers'", "'arms'", "'cli_routes'", "'effective_instructions'", "'eval'", "'explore_runs'", "'guidance_reach'", "'matrix_rollup'", "'novelty'", "'plan'", "'plugin_delta'", "'questions'", "'repo_budget'", "'repo_environment'", "'repo_readiness'", "'result'", "'subject_scorecard'", "'task_inspect'", "'task_runs'", "'tool_preference'"]Updated on every push; the |
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 13 selected item(s). Review details
|
| receipt from a saved probe without running the harness: the source is read | ||
| only to check that the file is for the same repository, harness and commit |
There was a problem hiding this comment.
Must-fix: saved conversion rereads the source for more than identity checks. _print_receipt reruns budget, and build_receipt hashes the current source text. For example, probe a local checkout, edit AGENTS.md without committing, then run --from ... --format receipt: the observations stay unchanged but the sha256 changes, and edits to imports can also change expectations. No content check rejects this. Qualify “same receipt” as requiring unchanged source contents and harness configuration, and document that hashes/expectations describe the source at conversion time. The changelog's “same receipt” claim needs the same qualification.
There was a problem hiding this comment.
Fixed in 7411bac: §7.1 now says the source is read again at conversion time, so a saved probe
converts to the same receipt only for unchanged source contents and harness configuration.
sha256 and the expectations describe the source as it is at conversion time.
| | `dokimi_method` | | `marker_self_report` for a source the probe recorded; null for a start-up file budget found and reach did not probe | | ||
| | `states.accessed` | the Read tool | `true` if a Read-tool read was seen, `false` with tools off, else `unknown` | | ||
| | `states.selected` | the class | `true` for loaded or fetched (the source entered the request, by the harness or a read), else `unknown` | | ||
| | `states.loaded` | the class | `true` for loaded, `false` for not_loaded, else `unknown`; a fetched file is not loaded | |
There was a problem hiding this comment.
Must-fix: a fetched source gets states.loaded: "unknown", not false. _states() only maps loaded and not_loaded to booleans, and test_repo_receipt.py explicitly expects "unknown" for fetched. Thus a saved probe containing a fetched record produces a receipt that contradicts this statement. Remove “a fetched file is not loaded” and explicitly document that fetched maps to unknown, so consumers do not interpret a tool read as proof that startup loading did not occur.
Suggestion:
| | `states.loaded` | the class | `true` for loaded, `false` for not_loaded, else `unknown`; a fetched file is not loaded | | |
| | `states.loaded` | the class | `true` for loaded, `false` for not_loaded, `unknown` for fetched, truncated or error | |
There was a problem hiding this comment.
Fixed in 7411bac: the table maps fetched to states.loaded: unknown, as _LOADED and
test_repo_receipt.py do. The sentence that a fetched file is not loaded is gone.
| `runtime` and `model` are the harness's own name and version, as reach records | ||
| them. `observation.method` is `unknown` and `observation.dokimi_method` is |
There was a problem hiding this comment.
Nit: model is not the harness's name/version. build_receipt() copies reach["model"], which is the string passed with --model (for example, claude-haiku-4-5). Only runtime contains the harness name and version. Describe these separately so consumers do not mistake the model field for runtime identity or a resolved model version.
There was a problem hiding this comment.
Fixed in 7411bac: runtime is the harness name and version. model is the --model string,
and the doc says it is not a resolved model version.
| - a read through a shell (`cat`, `rg`, `grep`), which the `Read` tool check | ||
| does not see; |
There was a problem hiding this comment.
Must-fix: this limitation is not true for all supported harnesses. Codex and Copilot normalize supported shell reads into read events; classify() uses those events for read_by_tool, and the receipt uses that flag for states.accessed. For example, a successful Codex cat AGENTS.md can produce accessed: true, despite this section saying shell reads are invisible. Update this bullet and the states.accessed table row to describe normalized read events and the harness-specific shell coverage already documented in §7.
There was a problem hiding this comment.
Fixed in 7411bac: shell reads are seen only where the harness reports read events (Codex and
Copilot normalise supported shell reads, §7). The states.accessed row says the same.
| - start-up imports and skills from budget, which reach does not probe: they | ||
| appear with `observed: unknown` and a run-level unknown saying so; |
There was a problem hiding this comment.
Must-fix: skills do not appear as sources in the receipt. _budget_when filters entries to guidance and import, excluding budget's skill listing and skill body entries. Also, the not-probed notice is in receipt-level unknowns, not each run's unknowns. A repository with a start-up skill listing therefore does not produce the entries promised here. Describe only the unprobed start-up guidance/import entries that are actually included, explicitly note that skills are omitted, and locate the limitation at receipt level.
| raise ReachError(f"--from reads a saved probe, so {', '.join(given)} cannot be used with it") | ||
| profile = load_profile(args.harness) | ||
| source = RepoSource.parse(args.source) | ||
| reach = load_reach(Path(args.from_file)) |
There was a problem hiding this comment.
Must-fix: schema validation here does not guarantee the fields consumed afterward exist. guidance_reach.v1 does not require salt or generated_at; removing either from an otherwise valid saved reach.json passes load_reach, but --from --format receipt raises an uncaught KeyError in build_receipt. Matrix mode likewise assumes optional summary.matrix and scenario unknown fields. Validate the selected output mode's required fields (or supply appropriate fallbacks) and return a diagnostic with exit 2 rather than a traceback. Add tests using schema-valid saved files with these optional fields omitted.
There was a problem hiding this comment.
Fixed in 7968811: --from checks what the chosen format needs (salt and generated_at for the
receipt, summary.matrix for the matrix) and exits 2 with a message. The matrix reads no optional
scenario field, so only summary.matrix is checked.
|
|
||
| def _shareable_tree(label: str) -> str: | ||
| """The tree label with a local directory cut to its last component, so no home path reaches a shared receipt.""" | ||
| name, at, sha = label.rpartition("@") |
There was a problem hiding this comment.
Must-fix: an @ in an unpinned local directory is treated as a commit separator. _shareable_tree('/home/alice@corp/src/toy') returns alice@corp/src/toy, exposing the username and intermediate directories in the shared receipt. RepoSource explicitly accepts existing directories containing @, and WorkTree preserves them in its label, so this input reaches the sanitizer. Use the separate repo SHA to distinguish pinned labels, rather than splitting every label at its final @; add POSIX and Windows cases with @ in parent directories.
There was a problem hiding this comment.
Fixed in 7968811: the revision is the label's own @<hex> suffix, and a local directory is cut to
its basename. /home/alice@corp/src/toy now gives toy. Cases added to
test_the_tree_label_keeps_no_local_directory.
| if probed and tree_sha and probed != tree_sha: | ||
| raise ReachError(f"reach.json was probed at {probed[:12]}, the source is at {tree_sha[:12]}") |
There was a problem hiding this comment.
Must-fix: this skips provenance checking whenever either SHA is absent. For example, probe /repo@<commit-A>, change the working tree, then run repo reach /repo --from reach.json --format receipt. WorkTree has no SHA, so the receipt keeps commit A and its old observations but build_receipt computes hashes and expectations from the changed files. Two unpinned runs have the same problem after editing guidance. Reject an incomparable source for snapshot-attributed receipts, or persist and verify source digests; if unverifiable local sources remain supported, explicitly mark their hashes/expectations as current and provenance as unknown. Add tests that change guidance between probing and regeneration.
There was a problem hiding this comment.
Option (a), c4d78e2: repo reach --from --format receipt now refuses (exit 2) unless the source is pinned and the saved probe was made at a commit.
The matrix is unchanged. Tests cover an edited working tree, a pinned probe read against a working tree, and an unpinned probe read against a pinned source.
Not covered: the harness profile file is unpinned, so its guidance and import depth still feed expected; §7.1 says so.
| d = _dir(target) | ||
| return run.workdir == d or run.workdir.startswith(d + "/") |
There was a problem hiding this comment.
Must-fix: lazy loading scope is determined by the guidance entry path, not necessarily its resolved target. For example, sub/CLAUDE.md -> ../shared.md produces a budget entry for sub/CLAUDE.md with when: lazy, which _budget_when remaps to shared.md. This code then reports expected=true at the repository root and expected=false for cwd=sub, reversing the actual expectation. Preserve the activating guidance paths/subtrees when canonicalizing aliases, and test a nested guidance symlink whose target lives outside its subtree.
There was a problem hiding this comment.
Fixed in 7968811: a lazy expectation is given only for a nested AGENTS/CLAUDE file that budget lists
under its own path. A link, an import or a root-level lazy file is expected: unknown, so the
reversal is gone. Test: test_a_lazy_guidance_is_expected_only_in_the_subtree_it_activates.
There was a problem hiding this comment.
🟡 Changes recommended
An unresolved critical provenance flaw allows rebuilt receipts to combine old probe observations with changed working-tree content that was never probed.
5 open issues
What changed in this PR
6 new findings: 5 must-fix and 1 lower-priority nit.
This PR adds effective-instructions receipts to repo reach, combining expected instruction loading with probe observations and integrating the output into artifact validation and run inspection.
Changes:
- Add
--format receiptand replay saved probes with--from. - Define and register the receipt schema, including source states and observation limits.
- Add receipt tests, artifact fixtures, CLI reference updates, and documentation.
| File | Description |
|---|---|
dokimi/tests/test_runs_show.py |
Add receipt counts to run-display expectations. |
dokimi/tests/test_runs_scan.py |
Add receipt metadata to run-scan expectations. |
dokimi/tests/test_repo_receipt.py |
Test receipt mappings, replay, validation, and CLI behavior. |
dokimi/tests/fixtures/artifacts/effective_instructions.v1.json |
Provide a representative receipt artifact. |
dokimi/dokimi/schemas/effective_instructions.v1.json |
Define the versioned receipt format. |
dokimi/dokimi/runs/kinds.py |
Register receipt fields for run inspection. |
dokimi/dokimi/repos/receipt.py |
Build receipts from reach observations and budget expectations. |
dokimi/dokimi/commands/repo.py |
Add receipt output and saved-probe replay. |
dokimi/dokimi/artifacts.py |
Register the new artifact schema. |
dokimi/docs/repo-evaluation-contract.md |
Document receipt semantics and limitations. |
dokimi/docs/reference/cli-interface.json |
Update the CLI interface snapshot. |
dokimi/changelog.d/20261031_132853_napetrov_effective-instructions-receipt.md |
Announce receipt output and replay support. |
Review effort: Balanced
| rules = harness["name"] == "claude-code" # budget's start-up rules are Claude Code's | ||
| when_of = _budget_when(reach, budget) if rules else {} | ||
| start_up = [p for p, when in when_of.items() if when in ("start", "off") and p not in _known(reach)] | ||
| digests = source_digests(tree, [p for p, _ in _probed(reach)] + start_up) |
There was a problem hiding this comment.
Option (a), c4d78e2: repo reach --from --format receipt now refuses (exit 2) unless the source is pinned and the saved probe was made at a commit.
The matrix is unchanged. Tests cover an edited working tree, a pinned probe read against a working tree, and an unpinned probe read against a pinned source.
Not covered: the harness profile file is unpinned, so its guidance and import depth still feed expected; §7.1 says so.
| raise ReachError(f"--from reads a saved probe, so {', '.join(given)} cannot be used with it") | ||
| profile = load_profile(args.harness) | ||
| source = RepoSource.parse(args.source) | ||
| reach = load_reach(Path(args.from_file)) |
There was a problem hiding this comment.
Same fix as the repo.py thread, in 7968811: the prerequisites of the selected format are checked
before rendering, and a missing one is a usage error (exit 2) with no traceback.
| name, at, sha = label.rpartition("@") | ||
| if not at: | ||
| name, sha = label, "" | ||
| if not (PureWindowsPath(name).drive or PurePosixPath(name).is_absolute() or name.startswith("~")): | ||
| return label | ||
| return (PurePosixPath(name.replace("\\", "/")).name or "tree") + at + sha |
There was a problem hiding this comment.
Same fix as the label thread, in 7968811: the basename of the whole local label is kept, with a real
trailing @<rev>. /home/alice@corp.com/toy gives toy.
| """budget's ``when`` of each guidance file and import, keyed by the file the path resolves to.""" | ||
| target_of = {alias: item["path"] for item in reach["files"] for alias in item.get("aliases", [])} | ||
| return {target_of.get(e["path"], e["path"]): e["when"] | ||
| for e in budget["entries"] if e["kind"] in ("guidance", "import")} |
| if when == "lazy": | ||
| d = _dir(target) | ||
| return run.workdir == d or run.workdir.startswith(d + "/") |
There was a problem hiding this comment.
Same fix as the lazy-scope thread, in 7968811: sub/CLAUDE.md -> ../AGENTS.md now gives unknown in
both runs, not a reversed expectation. The root AGENTS.md is unknown too.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #335 +/- ##
========================================
Coverage 94.05% 94.05%
========================================
Files 297 298 +1
Lines 38018 38257 +239
Branches 6424 6455 +31
========================================
+ Hits 35756 35982 +226
- Misses 1452 1462 +10
- Partials 810 813 +3
🚀 New features to boost your workflow:
|
…elds - The receipt's tree label keeps its pinned revision as the label's own hex suffix, and a local directory is cut to its basename, so an @ in a parent directory no longer reaches a shared receipt. - A lazy expectation is given only for a nested AGENTS/CLAUDE file that budget lists under its own path. A link or an import is unknown, since budget's flattened when does not say where it is activated. - A start-up skill listing is a source (observed unknown). Skill bodies, which load on demand, stay out of the receipt. - repo reach --from exits 2 with a message when the saved probe lacks salt or generated_at (receipt) or summary.matrix (matrix), instead of a traceback. Thread: PRRT_kwDOVBg7g86yWZRT Thread: PRRT_kwDOVBg7g86yWaSw Thread: PRRT_kwDOVBg7g86yWZRX Thread: PRRT_kwDOVBg7g86yWaS_ Thread: PRRT_kwDOVBg7g86yWZRQ Thread: PRRT_kwDOVBg7g86yWaSc Thread: PRRT_kwDOVBg7g86yWaS7 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
- A saved probe is converted against the source as it is now: the receipt is the same only for unchanged source contents and harness configuration. - states.loaded is unknown for a fetched source, not false. - model is the --model string, not the harness's name or version. - Shell reads are normalised only where the harness reports read events (Codex and Copilot, §7); the rest are not seen. - Start-up skill listings are included; skill bodies are not. The not-probed notice is a receipt-level unknown. Thread: PRRT_kwDOVBg7g86yWZRF Thread: PRRT_kwDOVBg7g86yWZRH Thread: PRRT_kwDOVBg7g86yWZRM Thread: PRRT_kwDOVBg7g86yWZRN Thread: PRRT_kwDOVBg7g86yWZRO Thread: PRRT_kwDOVBg7g86yWaS7 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| print(f"✗ {exc}", file=sys.stderr) | ||
| sys.exit(_tree_read_exit_code(exc)) | ||
| if args.format == "receipt": | ||
| _print_receipt(tree, profile, reach, Path(args.from_file).parent) |
There was a problem hiding this comment.
Must-fix: the fixed output name can overwrite the saved probe being read. For example, rename a valid reach.json to effective_instructions.json and pass it with --from --format receipt; _print_receipt replaces that same file with a different artifact, losing the probe records. The same happens if the output name is a symlink to reach.json, since save_artifact follows output symlinks. Reject input/output collisions (including resolved symlinks) before saving, and add regression coverage.
There was a problem hiding this comment.
Fixed in 5c2c6da. --from refuses a receipt path that would replace the saved probe it reads (usage error, exit 2), including renamed and symlinked paths. The live path refuses a receipt that resolves to reach.json. Test: test_from_never_writes_the_receipt_over_the_probe_it_reads (renamed and symlink cases).
| from dokimi.repos.receipt import build_receipt | ||
|
|
||
| receipt = build_receipt(payload, budget(tree, profile.guidance_names, profile.import_max_depth), tree) | ||
| written = save_artifact("effective_instructions", receipt, out_dir / "effective_instructions.json") |
There was a problem hiding this comment.
Must-fix: receipt writes are outside exception handling in both live and --from paths, and the CLI boundary does not catch OSError. A readable saved probe in a non-writable directory, or an existing directory named effective_instructions.json, therefore produces a traceback rather than a runtime-failure diagnostic. Catch output I/O failures around receipt generation/saving, print the error to stderr, and exit 1; cover both callers.
There was a problem hiding this comment.
Fixed in 5c2c6da. Receipt writes are guarded on both paths: an OSError prints cannot write ... and exits 1 (runtime failure), with no traceback. Tests: test_a_receipt_that_cannot_be_written_is_a_runtime_failure_not_a_traceback (--from) and test_a_live_receipt_that_cannot_be_written_is_a_runtime_failure.
| """The fields a saved probe must carry to be rendered as ``fmt``: the schema leaves them optional.""" | ||
| if fmt == "receipt": | ||
| return [key for key in ("salt", "generated_at") if key not in payload] | ||
| return [] if "matrix" in payload["summary"] else ["summary.matrix"] |
There was a problem hiding this comment.
Must-fix: the earlier optional-field fix is still incomplete for matrix rendering. Remove unknown from a scenario in an otherwise valid saved reach.json: the schema allows this, saved_gaps accepts it, but format_matrix still accesses s["unknown"] and raises an uncaught KeyError. Either render that optional field using .get() or include missing scenario fields in the prerequisite check. Add this case to the saved-probe tests.
There was a problem hiding this comment.
Fixed in 5c2c6da. format_matrix reads unknown with .get, so a probe saved without it renders. Test: test_a_matrix_of_a_probe_saved_without_unknown_tokens_renders. The other receipt-side reads of that key already use .get.
| if target not in run.own or not d or target.rsplit("/", 1)[-1] not in SUBTREE_NAMES: | ||
| return UNKNOWN | ||
| return run.workdir == d or run.workdir.startswith(d + "/") |
There was a problem hiding this comment.
Must-fix: the lazy-scope fix still gives reversed expectations for an imported nested guidance file. With root CLAUDE.md, sub/CLAUDE.md containing @../other/AGENTS.md, and a regular other/AGENTS.md, budget lists the latter as kind: guidance, when: lazy because of the import (its automatic AGENTS loading is displaced). It therefore belongs to run.own, and this branch reports expected=false for cwd=sub and true for cwd=other, although the importer activates it in sub. Being a non-alias guidance entry does not establish its activation scope. Preserve the importing directory, or return unknown for these import-only lazy AGENTS entries; add a test using the real budget output for this layout.
There was a problem hiding this comment.
Fixed in 5c2c6da. A guidance file that another guidance file @-imports is no longer expected at the top level; it is expected only where the importing file is read. Test: test_a_lazy_guidance_imported_by_a_nested_file_is_expected_only_where_that_file_is.
repo reach --from --format receipt rebuilt sha256 and expectations from the source as it is now. A working tree can change after the probe, and a probe of one has no commit to check against, so the receipt mixed old observations with current digests. Refuse it with exit 2 unless the source is pinned and the saved probe was made at a commit. The matrix is unchanged. Tests: unpinned and edited, pinned probe read against a working tree, and the pinned round trip. docs §7.1 and the changelog say the limit. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| different one is a usage error, exit 2). A receipt also needs a pinned source | ||
| (`<path>@<rev>`) and a saved probe made at a commit: a working tree, or a probe | ||
| of one (`repo.sha` null), is a usage error (exit 2), because `sha256` and the |
There was a problem hiding this comment.
Must-fix: the pinned-source requirement currently applies only to saved-probe conversion, not to live receipt generation. repo reach /path/to/repo --model m --format receipt reaches _print_receipt without calling check_receipt_provenance; test_cli_receipt_format_prints_and_writes_a_valid_receipt explicitly exercises this unpinned case successfully. Either enforce this requirement before a live probe starts or qualify this paragraph as applying only to --from. The changelog's “both must be at a commit” claim needs the same alignment.
There was a problem hiding this comment.
Fixed in 5c2c6da. §7.1 now applies the pinned-source rule to --from only. A live repo reach --format receipt builds the receipt from the source it probes, and the section says so.
| One receipt is written per reach scenario, with `run_id` `<salt>:<scenario id>`. | ||
| Each source carries: |
There was a problem hiding this comment.
Nit: clarify that these are entries in one aggregate artifact, not separate output files. build_receipt returns a single object containing a receipts array, and _print_receipt writes that object once to effective_instructions.json, even for multiple scenarios.
Suggestion:
| One receipt is written per reach scenario, with `run_id` `<salt>:<scenario id>`. | |
| Each source carries: | |
| One artifact is written to `effective_instructions.json`, with one entry in its | |
| `receipts` array per reach scenario and `run_id` `<salt>:<scenario id>`. | |
| Each source carries: |
There was a problem hiding this comment.
Fixed in 5c2c6da. §7.1 now says one aggregate artifact, effective_instructions.json, with one entry in its receipts array per reach scenario. The changelog fragment matches.
| |---|---|---| | ||
| | `path`, `alias_of` | reach `files`, `aliases` | a link is `adapter` scope and takes its target's tokens | | ||
| | `sha256` | the text dokimi reads (LF-normalised UTF-8) | null when the file is not readable as text | | ||
| | `scope` | path | `repo` at the root or in `.claude/`; `subtree` for a nested AGENTS.md, CLAUDE.md or CLAUDE.local.md; else `unknown` | |
There was a problem hiding this comment.
Nit: narrow the .claude/ scope rule to match _scope. For example, .claude/CLAUDE.local.md is emitted with scope: subtree, and a budget-only .claude/skills/example/SKILL.md has scope: unknown, not repo. Only .claude/CLAUDE.md and .claude/AGENTS.md receive the special repo-wide scope. Enumerate those two paths rather than implying every file under .claude/ is repo-scoped.
There was a problem hiding this comment.
Fixed in 5c2c6da. The scope row now gives repo only for a root-level file and for .claude/CLAUDE.md and .claude/AGENTS.md, and lists .claude/CLAUDE.local.md as subtree, matching _scope.
| | `observed` | reach class | `loaded` → loaded, `fetched` → fetched, `not_loaded` → not_loaded, `truncated` and `error` → unknown | | ||
| | `observation_method` | | `unknown` for every source: the thread vocabulary's method, since no runtime event, prompt capture or hook backs the probe | | ||
| | `dokimi_method` | | `marker_self_report` for a source the probe recorded; null for a start-up file budget found and reach did not probe | | ||
| | `states.accessed` | the harness's read events | `true` if the harness reported a read of the file (the Read tool; Codex and Copilot also report supported shell reads, §7), `false` with tools off, else `unknown` | |
There was a problem hiding this comment.
Nit: the false-with-tools-off rule applies only to probed sources. With root CLAUDE.md importing docs/style.md and the cwd=. scenario (tools: none), the budget-only docs/style.md entry has states.accessed: unknown: _budget_only calls _states(None, False, False) regardless of the scenario. Document this exception so consumers do not interpret the emitted unknown as inconsistent with the contract.
There was a problem hiding this comment.
Fixed in 5c2c6da. The false-with-tools-off rule for states.accessed is now limited to probed sources; the table row says so.
| does not give an order of entry. The displacement follows budget's rule as | ||
| written (`repos/budget.py`: every AGENTS.md is `off` while a root CLAUDE\*.md | ||
| exists, nested ones included), so nested AGENTS.md files appear too. If budget's |
There was a problem hiding this comment.
Must-fix: this overstates budget's displacement rule. For example, with root CLAUDE.md containing @AGENTS.md, budget marks AGENTS.md as start, not off; imports from nested CLAUDE files can similarly make it lazy. repos/budget.py also exempts AGENTS files aliased by CLAUDE guidance. The receipt correctly lists only files actually marked off, so document these exceptions rather than saying every AGENTS.md is off whenever root CLAUDE guidance exists.
There was a problem hiding this comment.
Fixed in 5c2c6da. §7.1 states budget's displacement rule as it is: an AGENTS.md is off while a root CLAUDE* exists, unless a root CLAUDE* aliases or @-imports it, or a nested CLAUDE* @-imports it. Nested AGENTS.md files can appear, and a root AGENTS.md is not always off.
- repo reach --from refuses a receipt path that would replace the saved probe (usage error; also for renamed or symlinked paths) - a live receipt that cannot be written is a runtime failure (exit 1), not a traceback; a receipt path that resolves to reach.json is refused - a probe saved without `unknown` tokens renders its matrix - a lazy guidance file imported by a nested file is expected only where that file is read - contract section 7.1 and the changelog: the pinned-source rule is for --from, collision and write-failure exit codes, one aggregate artifact Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| conversion time and a working tree can change after the probe. A live | ||
| `repo reach --format receipt` run is not held to this: it builds the receipt | ||
| from the source it probes. The matrix (without `--format receipt`) still |
There was a problem hiding this comment.
Must-fix: this claims a live receipt describes the source actually probed, but an unpinned WorkTree is reread after the harness finishes. For example, run repo reach /repo --model m --format receipt and edit /repo/CLAUDE.md while the harness is running: run_reach probes the earlier copy in its temporary workspace, whereas _print_receipt recomputes budget and source_digests from the edited original. The receipt therefore combines old observations with a new hash and potentially new expectations. Either capture the source used to seed the probe for receipt generation, or document this live-run limitation and require the working tree to remain unchanged throughout the run.
|
|
||
| def _receipt_would_overwrite(probe: Path, receipt: Path) -> bool: | ||
| """A receipt written at the probe's path, directly or through a link, would replace the probe it is built from.""" | ||
| return probe.resolve() == receipt.resolve() |
There was a problem hiding this comment.
Must-fix: the overwrite guard itself can raise an unhandled exception. For example, create out/effective_instructions.json -> effective_instructions.json, then run a live probe with --out out --format receipt. Path.resolve() raises RuntimeError on Python 3.11/3.12 (supported versions), or OSError on newer Python, before the guarded receipt write is reached. The live caller catches neither; the --from caller also misses RuntimeError. Handle resolution failures at both call sites so an invalid output link produces a diagnostic and controlled exit rather than a traceback. Add a looping-link test alongside the existing symlink and unwritable-output tests.
| """The fields a saved probe must carry to be rendered as ``fmt``: the schema leaves them optional.""" | ||
| if fmt == "receipt": | ||
| return [key for key in ("salt", "generated_at") if key not in payload] | ||
| return [] if "matrix" in payload["summary"] else ["summary.matrix"] |
There was a problem hiding this comment.
Must-fix: checking only that summary.matrix exists does not make a saved probe safe to render. In an otherwise valid reach.json, set summary.matrix["CLAUDE.md"] to null: guidance_reach.v1 allows it because matrix is only constrained to be an object, so load_reach and this check succeed, but format_matrix raises an uncaught AttributeError on row.get. An unknown cell value similarly raises KeyError in short[...]. Validate the nested rows and class values here and report a usage error, or rebuild the matrix from the schema-validated records. Add saved-file tests for both malformed cases.
| if not (PureWindowsPath(name).drive or PurePosixPath(name).is_absolute() or name.startswith("~")): | ||
| return label |
There was a problem hiding this comment.
Must-fix: local path redaction misses pinned sources supplied as relative paths. For example, repo reach ../alice/private/toy@HEAD ... --format receipt produces a GitTree label ../alice/private/toy@<sha> (open_source preserves source.location), and this branch returns that whole directory path in the shareable receipt. file:///home/alice/toy.git@<sha> also passes through unchanged even though it names a local directory. Distinguish remote URLs from local labels and reduce all local labels to their final component; add relative pinned-path and file-URL regression cases alongside the absolute-path tests.
_reach_from_saved and cmd_repo_reach were at complexity 11 over the cap of 10 with no baseline entry. The saved-file checks and the receipt guard move into helpers; no behaviour change. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at a84f717. The receipt mapping, schema, exit codes and --from guards hold up, and most of the earlier bot findings are fixed. Four things are worth fixing before merge: the path redaction still leaks, the live-run limit is undocumented, and two test gaps leave reported state values unpinned.
What I ran, on a Linux review host at a84f717:
make ci-local: 30 passed, 0 failed, 7 skipped.- Reproductions for the code findings, with the LLM transport mocked.
- A mutation run on
receipt.py: 28 mutants, 8 of which passtest_repo_receipt.py.
Checked, no issues
- Schema:
effective_instructions.v1.jsonmatchesbuild_receiptoutput;SCHEMA_VERSIONS,runs/kinds.pyand the CLI snapshot are updated;schema_compatis OK. - Exit codes: repository, harness, commit and pin mismatches exit 2; a failed write exits 1; the probe is never overwritten.
- Commits: all nine are signed off.
Bot points I checked that no longer apply at this head (21)
- exai-bot's point on conversion rereading the source is fixed: the contract now qualifies "same receipt" for unchanged contents and profile.
- exai-bot's point on
fetchedmapping toloaded: falsedoes not apply: the table now saysunknown, as_LOADEDdoes. - exai-bot's point on shell reads being invisible does not apply: the contract now limits it to harnesses without normalised read events.
- exai-bot's point on skills missing from receipts does not apply: start-up skill listings are sources and the not-probed notice is receipt-level.
- exai-bot's point on missing
salt/generated_atdoes not apply:--fromchecks them and exits 2 (saved_gaps). - exai-bot's point on
@in a local parent directory does not apply:/home/alice@corp/src/toynow givestoy. - exai-bot's point on skipped provenance checks does not apply:
--from --format receiptnow refuses unpinned sources and probes. - exai-bot's point on reversed lazy scope for links does not apply: links and imports are
unknown. - Copilot's point on rebuilding from an unverifiable working tree does not apply:
--fromrefuses it (exit 2). - Copilot's point on a missing
salt/generated_attraceback does not apply: it exits 2 with a message. - Copilot's point on
alice@corp.comin the label does not apply: the label is cut to its basename. - Copilot's point on dropped skill listings does not apply: listings are included and the notice is receipt-level.
- Copilot's point on reversed import scope does not apply: such files are
unknownor expected only where the importer is. - exai-bot's point on overwriting the probe does not apply:
--fromrefuses a receipt path that resolves to the probe. - exai-bot's point on unhandled write errors does not apply: an
OSErrorprintscannot writeand exits 1. - exai-bot's point on a missing scenario
unknowndoes not apply:format_matrixuses.get. - exai-bot's point on imported nested guidance does not apply: it is expected only where the importing file is read.
- exai-bot's point on the pinned-source rule for live runs does not apply: the contract now scopes it to
--from. - exai-bot's point on the displacement rule does not apply: the contract now states the exceptions.
- exai-bot's point on a looping receipt symlink does not apply at this head: on Python 3.13 the guard exits 1 with a "cannot write" message, and only a hand-built link reaches it.
- exai-bot's point on a null or unknown matrix cell does not apply as a blocker: only a hand-edited reach.json reaches it, and
format_matrixis unchanged from the base.
| The revision is the label's own hex suffix, so an ``@`` in a parent directory is part of the path.""" | ||
| match = _REV_SUFFIX.search(label) | ||
| name, rev = (label[: match.start()], match.group()) if match else (label, "") | ||
| if not (PureWindowsPath(name).drive or PurePosixPath(name).is_absolute() or name.startswith("~")): |
There was a problem hiding this comment.
_shareable_tree leaves a local path in the receipt for a pinned relative path and for a file:// source. Only absolute, drive-letter and ~ labels are cut, so these pass through whole:
_shareable_tree('../alice/private/toy@4ff761e2d943')returns../alice/private/toy@4ff761e2d943_shareable_tree('file:///home/alice/toy.git@4ff761e2d943')returnsfile:///home/alice/toy.git@4ff761e2d943
Both are labels source.py builds from source.location as given, and repo.tree goes into the shared receipt. The test at test_repo_receipt.py:170 covers only absolute paths and a plain https URL.
Suggestion: reduce every local label to its last component. Treat a label as remote only when it has a scheme other than file, or the git@host: form. Add the two cases above to the parametrised test.
| pinned source (`<path>@<rev>`) and a saved probe made at a commit: a working | ||
| tree, or a probe of one (`repo.sha` null), is a usage error (exit 2), because | ||
| `sha256` and the expectations are computed from the source as it is at | ||
| conversion time and a working tree can change after the probe. A live |
There was a problem hiding this comment.
"Builds the receipt from the source it probes" is not true for an unpinned live run. The probe seeds its workspace at the start, but _print_receipt runs after the harness exits and re-reads the original tree for sha256, and reruns budget for expected. Edit CLAUDE.md while the run is in flight: a loaded observation of the old text is paired with the new digest, and the receipt still validates. In a reproduction the receipt printed a digest of the edited text while the observation was of the original.
Suggestion: say that a live receipt on a working tree is only as stable as the tree for the length of the run, and add a receipt-level unknowns entry for it. Or hash the seeded workspace instead.
| def _states(cls: str | None, tools_off: bool, read_by_tool: bool) -> dict[str, Any]: | ||
| """accessed: a Read-tool read was seen (false with no tools at all); selected: the probe saw the source enter the | ||
| request, by the harness or by the agent's own read; loaded: at the harness's own load step, not through a read.""" | ||
| accessed: bool | str = True if read_by_tool else (False if tools_off else UNKNOWN) |
There was a problem hiding this comment.
No test pins states.accessed when tools are off or on. Mutation check: False if tools_off else UNKNOWN replaced by UNKNOWN, and replaced by False, both pass the full suite. The second claims "no read happened" for a tools-on run whose shell read went unseen, which the receipt's own unknowns says cannot be seen.
Suggestion: add a test with a tools: none scenario (expect accessed: false) and a tools-on scenario with read_by_tool false (expect unknown).
| d = _dir(target) | ||
| if target not in run.own or not d or target.rsplit("/", 1)[-1] not in SUBTREE_NAMES: | ||
| return UNKNOWN | ||
| return run.workdir == d or run.workdir.startswith(d + "/") |
There was a problem hiding this comment.
The lazy-scope and alias rules behind expected and scope are not pinned. Mutation check: each of these passes the full suite.
- dropping
or run.workdir.startswith(d + "/"), or loosening it torun.workdir.startswith(d)(a run insub2then getsexpected: trueforsub/AGENTS.md) - dropping
p not in aliasesin_own_guidance(receipt.py:154), so a nested link gets true/false instead ofunknown - dropping
or path in ROOT_NESTEDin_scope(receipt.py:235), so.claude/CLAUDE.mdis reported assubtree
Suggestion: add cases for a run in a deeper directory and in a sibling sub2, a nested alias, and .claude/CLAUDE.md scope.
Summary
repo reach --format receiptemits an effective-instructions receipt: per instruction source, what the harness was expected to load and what the probe observed, in theeffective-instructions/v0shape discussed in agentsmd/agents.md#213 (proposed by caioribeiroclw-pixel). New schema:dokimi/dokimi/schemas/effective_instructions.v1.json.Per source:
path,sha256,scope,expected(fromrepo budget's start-up chain),observed(loaded|fetched|not_loaded|unknown),observation_method,dokimi_method, andstates{accessed, selected, loaded, followed}. Per receipt: harness name and version, target,precedence_observed,unknowns.Mapping, and what the marker method can and cannot prove
observation_methodisunknownin the thread's vocabulary, anddokimi_method: marker_self_reportnames what was actually done.accessedis true when the harness reports a read of the file: the Read tool, and on Codex and Copilot the shell reads they normalise. Other shell reads (cat/rg) may be classed fetched or missed.followedis alwaysunknown: reach doesn't measure adherence.expectedexists for Claude Code only (budget's rules). Other harnesses getunknown. A nested AGENTS.md that budget marks off because a root CLAUDE.md exists is listed as displaced, followingrepos/budget.py.Details are in
docs/repo-evaluation-contract.md§7.1.Evidence
make ci-localon0fe06ccb:0 failed, 30 passed, 7 skipped, exit 0. Two earlier runs on the same head failed timing-sensitive tests in untouched files (host load ~300). Those didn't reproduce, and the reach/receipt tests pass with-p no:randomly.--froma saved probe equals the live one. Also covered: missing file, malformed JSON, wrong shape, repo/harness/commit mismatch (exit 2), probe-only flags refused with--from.repo.tree, was fixed in6260ae49. These passes ran in a single agent context, not fresh-context reviewers.Merge danger
effective_instructions.v1, registered inartifacts.SCHEMA_VERSIONS. Additive only, so the existingmatrixoutput is unchanged.reach.jsonstill records the absolute tree path (pre-existing); the receipt itself is cleaned.--from --format receiptinto a read-only directory gives a traceback, as the probe path already does.ce-reach-matrix, nightly agent × version matrix built from these receipts) is based on this one and opens after it merges.🤖 Generated with Claude Code