Skip to content

feat(repo): record whether the agent used what a patch: arm added (#41 D4) - #153

Merged
napetrov merged 6 commits into
intel:mainfrom
napetrov:feat/repo-arm-uptake
Oct 22, 2026
Merged

napetrov merged 6 commits into
intel:mainfrom
napetrov:feat/repo-arm-uptake

Conversation

@napetrov

Copy link
Copy Markdown
Contributor

Part of #41 (D4).

Summary

An arm that adds a tool can move a score through the guidance line that names the tool, with the tool never called. Each cell now records whether its agent used what its patch: arm added.

 prep (patch: arm)
   git apply; stage
+  added       = files the patch added  − guidance globs
+  mcp_servers = keys of an added/changed .mcp.json
 runner
   transcript → events
+  result.json.uptake = {files, mcp_servers, commands, reads, mcp_calls, used, measured}
 report
+  note: "arm's additions used in U/N cell(s)"
 compare
+  provenance.uptake[treatment] = {cells, used, unmeasured}
+  warning: "<arm>: the agent used none of what the arm added in N cell(s); a delta measures the patch's text, not the tool"

Counting rules (dokimi/repos/uptake.py):

  • command: the text names an added file as a whole path: tools/fmt, ./tools/fmt, sh tools/fmt, /eval/run/workspace/tools/fmt. It does not count cd tools && ./fmt, tools/fmt2 or mytools/fmt.
  • read: a read event of an added file.
  • mcp call: a tool named mcp__<server>__… where <server> is a server in the arm's .mcp.json, spelled the way Claude Code spells it (code.graph → code_graph).
  • measured: false when the cell left no trace to count from.

Arms with nothing to use (only guidance files, guidance:*) get no uptake key.

Evidence

  • Before: result.json of a patch: cell had no record of whether the added tool was touched. A compare of guidance:raw against patch:kit looked the same whether the agent ran tools/fmt or ignored it.
  • After: tests/test_repo_uptake.py, with no docker and no LLM:
    repo run --arm guidance:raw --arm patch:kit --reps 2     (fake harness, no tool use)
      report  → "arm's additions used in 0/2 cell(s)"
      compare → warning "patch:kit@…: the agent used none of what the arm added in 2 cell(s) …"
    repo run … with FAKE_TOOL_USES=[Bash ./tools/fmt, mcp__code_graph__callers]
      result.json.uptake == {commands: 1, mcp_calls: 1, used: true, measured: true, …}
      compare → uptake {cells: 2, used: 2}, no warning
    
    Parametrized match and no-match cases for the command pattern. Prep records added == [".mcp.json", "tools/fmt"], which excludes the patched sub/AGENTS.md and the new src/AGENTS.md.
  • make ci-local: everything passes except pip-audit, which reports a virtualenv advisory in the locks. That workflow runs only on PRs that change a lock, and this one changes none.

Merge Danger

Door: two-way

The new keys are additive: meta.json.arm_patch.added / .mcp_servers, result.json.uptake and compare provenance.uptake. Old run dirs have no uptake and report as before.

Blast Radius: small

Only patch: cells change. A compare that includes them can gain one warning line.

🤖 Generated with Claude Code

@codecov-commenter

codecov-commenter commented Oct 21, 2026 •

Copy link
Copy Markdown

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

Codecov Report

❌ Patch coverage is 96.03960% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.55%. Comparing base (ad1cbd0) to head (3117e66).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
dokimi/dokimi/repos/uptake.py 93.33% 4 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
+ Coverage   91.40%   91.55%   +0.15%     
==========================================
  Files         216      223       +7     
  Lines       26741    27465     +724     
  Branches     4377     4522     +145     
==========================================
+ Hits        24442    25146     +704     
- Misses       1635     1648      +13     
- Partials      664      671       +7     
Files with missing lines Coverage Δ
dokimi/dokimi/repos/compare.py 99.63% <100.00%> (+<0.01%) ⬆️
dokimi/dokimi/repos/report.py 96.37% <100.00%> (+1.97%) ⬆️
dokimi/dokimi/repos/runner.py 93.91% <100.00%> (+0.19%) ⬆️
dokimi/dokimi/repos/workspace.py 87.56% <100.00%> (+0.33%) ⬆️
dokimi/dokimi/repos/uptake.py 93.33% <93.33%> (ø)

... and 31 files with indirect coverage changes

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

@vchamarthi vchamarthi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not approving yet: 2 items to fix before merge.

Two ways the uptake count reports a use that is not a use of the arm's tool, which hides the new "used none" warning; both are in uptake.py/workspace.py and inline. The red self / repo validate is a pip --require-hashes failure on the dokimi @ git+... install, the same failure as on feat/repo-skill-arms, and passes on fix/repo-eval-hashed-install; it is not caused by this change.

CI at send time: 1 failing, 0 pending.

Comment thread dokimi/dokimi/repos/uptake.py Outdated
servers = json.loads((ws / MCP_CONFIG).read_text(encoding="utf-8")).get("mcpServers")
except (OSError, ValueError, AttributeError):
return []
return sorted(str(k) for k in servers) if isinstance(servers, dict) else []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should fix: When the patch changes an existing .mcp.json, mcp_servers records every server in the file, including the ones the base already had, so calls to those count as uptake of the arm; record only the servers the patch added (diff the mcpServers keys against the file at before).

_apply_patch passes files (added or changed paths) to mcp_servers(ws, files) at workspace.py:485, and mcp_servers (uptake.py:37-46) returns sorted(mcpServers) of the patched file with no comparison to the base. Scenario: base .mcp.json has {"mcpServers": {"github": ...}}, the patch adds "code.graph". arm_patch.mcp_servers becomes ["code.graph", "github"]; an agent that only calls mcp__github__* gets mcp_calls > 0, used: true, so repo compare prints no warning although the added server was never called. The guidance:raw baseline has the same github server, so this use is not something the arm added. The contract text ("the servers of a .mcp.json ... the patch added or changed") documents the same behavior; it should say servers the patch added. test_repo_uptake.py only covers a new .mcp.json.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 3117e66: the servers the base .mcp.json already declares are subtracted (read from the tree before the patch), so only added servers count. Test: test_a_mcp_json_the_patch_changed_names_only_the_servers_it_added. Contract text updated to say 'added'.

files = sorted(set(changed) | touched)
# -z output ends in a NUL, so the split has one trailing "" the pairing drops
new = {p for k, p in zip(status[::2], status[1::2], strict=False) if k == "A"}
new |= {p for p in touched - set(changed) if (ws / p).is_file()}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should fix: .mcp.json stays in arm_patch.added, so a Read of it or a command naming it (cat .mcp.json) counts as a use and suppresses the warning without one MCP call; drop MCP_CONFIG from added since its servers are tracked through mcp_calls.

workspace.py:483 filters added by guidance globs only, and test_repo_uptake.py asserts meta["arm_patch"]["added"] == [".mcp.json", "tools/fmt"] and result["uptake"]["files"] == [".mcp.json", "tools/fmt"]. In arm_uptake, wanted = set(files) (uptake.py:58) and a Read event of /eval/run/workspace/.mcp.json is normalized to .mcp.json by events._relative, so reads becomes 1 and used true (uptake.py:63). The command cat .mcp.json also matches _path_pattern (space before, end after). Reading the config is reading text, which is the case the warning exists to catch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 3117e66: .mcp.json is no longer in arm_patch.added (its servers are counted through mcp_calls). Test: test_prep_records_the_added_files_that_are_not_guidance_and_the_mcp_servers now asserts added == ['tools/fmt'].

kind = ev.get("kind")
if kind == "command" and named is not None and named.search(str(ev.get("text", ""))):
out["commands"] += 1
elif kind == "read" and ev.get("path") in wanted:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question: Is a Read of an added script (or cat tools/fmt, git diff tools/fmt) meant to count as uptake? For a tool file that is reading its text, the same signal the warning treats as 'not the tool'.

uptake.py:62-64 counts every read event whose path is an added file, and any command whose text names one as a whole word. The contract (repo-evaluation-contract.md:376-385) lists reads as uptake, so this may be intended for added documents; a split between files meant to be run and files meant to be read, or reporting commands/mcp_calls separately in the warning, would keep the two apart.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Intended: a Read of an added file counts as uptake, as the contract lists reads (repo-evaluation-contract.md). The signal is 'the agent engaged with what the arm added', not only 'executed it'. 3117e66 states this in the uptake docstring and the contract; I did not split reads from commands, since the warning is about an arm whose additions the agent ignored entirely.

Comment thread dokimi/dokimi/repos/report.py Outdated
noted = ("agent_timeouts", "agent_errors", "over_budget", "cost_unknown")
notes = [f"{r[k]} {k.replace('_', ' ')}" for k in noted if r[k]]
if "uptake" in r:
notes.append(f"arm's additions used in {r['uptake']['used']}/{r['uptake']['cells']} cell(s)")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: uptake_counts returns unmeasured, but format_report and format_compare print only used/cells, so a treatment whose traces all failed to parse shows 0/0 and no warning; append the unmeasured count when it is non-zero.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 3117e66: format_report and the compare 'uptake:' line append '(warning: N more cell(s) left no trace to measure)' when unmeasured is non-zero. Test: test_the_text_report_and_compare_show_the_cells_that_could_not_be_measured (covers the note helper both use).

@napetrov
napetrov force-pushed the feat/repo-arm-uptake branch from 4b6f71e to afdc62c Compare October 22, 2026 03:40
napetrov and others added 4 commits October 21, 2026 20:41
…tel#41 D4)

A patch: arm that adds a tool can move a score through the guidance line
that names it, with the tool never called. Prep now records the files the
patch added that are not guidance, and the servers of an added .mcp.json.
The runner counts commands that name those files, reads of them and calls
of the servers' tools into result.json.uptake. report shows used/cells per
arm. compare shows it per treatment and warns when a treatment used none
of what its arm added.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
A patch: arm can add .mcp.json as a symlink. mcp_servers followed it, so
prep read a host file outside the workspace into meta.json, and a link to
/dev/zero read without end. Resolve the path first and read it only when
it stays in the workspace and is a regular file; a link loop names none.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
arm_uptake counted command, read and tool events whatever their denied
flag, so a call a PreToolUse hook or permission_denials refused gave
used: true and hid the "used none" warning. Skip denied events.

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

No test failed when the runner's measured gate, report's measured filter,
compare's cells guard or --model filter, the A-only added rule, the
gitignored added file or the .mcp.json-in-arm-files guard was removed.
The fake harness gains a silent and an idle action for the gate.

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

xaleryb commented Oct 22, 2026

Copy link
Copy Markdown
Contributor
  • Ran: the full suite, ruff, mypy and bandit, plus symlinked-.mcp.json and hook-denial probes and a mutation pass over the new guards.
  • A patch: arm that adds .mcp.json as a symlink to a host file -> prep reads that file and records its server names, and a link to /dev/zero reads until memory runs out -> expected: nothing read through a link that leaves the workspace.
  • A tool call refused by a PreToolUse hook or listed in permission_denials -> used: true and the "used none" warning is dropped -> expected: denied calls not counted.
  • Pushed:
    • cb94f92: resolve .mcp.json and read it only as a regular file inside the workspace; in-tree links still work, and link loops are handled.
    • 179faec: skip denied events in arm_uptake; doc and fragment updated.
    • 416a6ed: tests pinning the measured gate, the cells guard, the --model filter and the rule for which files count as added (no test failed when any of these was removed).
  • Left for the author: the two open should-fix threads (uptake.py:46, workspace.py:483) and the question on uptake.py:63.

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review at 416a6ed. The design is sound and the counting rules in the description hold, but two open points from @vchamarthi still reproduce at this head, and I found a bare-file false positive and a test gap inline, plus a doc nit.

I agree with @vchamarthi on the base .mcp.json servers and on .mcp.json staying in added; both still reproduce here (a base {"mcpServers": {"base": {}}} patched to add new gives mcp_servers == ["base", "new"]). Their question on whether a read of an added tool counts as use, and the unmeasured display, wait for the author.

What I ran, on a Linux review host at 416a6ed:

  • make ci-local: 25 passed, 1 failed, 6 skipped. The failure is pip-audit, which the description says reports a virtualenv advisory in the locks; this PR changes no lock (fixed on main by #152).
  • 6738 tests passed, 1 skipped; changed-line coverage 96%.
  • Reproductions of the code findings, with no model or network involved.
  • A mutation run on the new tests: 117 mutants, 53 survive; the ones that matter are in the test-gap item below.

Checked, no issues

  • Counting rules: tools/fmt, ./tools/fmt, sh tools/fmt and /eval/run/workspace/tools/fmt count; cd tools && ./fmt, tools/fmt2, mytools/fmt and tools/fmt.bak do not.
  • MCP names: code.graph matches mcp__code_graph__* and not mcp__code.graph__*.
  • Patch parsing: the --name-status -z pairing holds for paths with spaces and newlines, deletions, mode changes and binary files; renames are an add plus a delete under --no-renames.
  • Compat: old run dirs and guidance-only arms get no uptake key; no schema under schemas/ names these files, so no version bump is needed.
  • Denied calls: the denied skip is pinned from both sides by the tests.

Comment thread dokimi/dokimi/repos/uptake.py Outdated
return None
paths = "|".join(re.escape(f) for f in sorted(files, key=len, reverse=True))
root = re.escape(workdir.rstrip("/") + "/")
return re.compile(rf"(?:^|(?<=[\s'\"`=;&|(<>])|(?<=\./)|(?<={root}))(?:{paths})(?=$|[\s'\"`;&|)<>:])")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A root-level added file counts as used when its name appears as any word of an unrelated command. The pattern matches the bare path at any word boundary, so a file without a / matches like a common word.

With files=["lint"], ["run"], ["build"] and workdir /eval/run/workspace, arm_uptake returns commands: 1, used: True for npm run lint, docker run img and git log -- build. The agent never touched the added file, and the "used none" warning is suppressed.

The contract says the command names the file "as a workspace path", which npm run lint does not.

Suggestion: for a file without a /, count it only when it is ./-prefixed, under the workdir, or the first word of the command.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 3117e66: a file with no '/' counts only as ./name, under the workspace path, first word of a command (or after ; && | ( ), or after an interpreter (sh/bash/python...). 'npm run lint', 'docker run img', 'git log -- build' no longer match. Tests: test_a_root_level_added_file_is_not_named_by_a_common_word_of_a_command, test_a_root_level_added_file_is_named_by_a_path_or_as_a_command.

out["reads"] += 1
elif kind == "tool" and prefixes and str(ev.get("name", "")).startswith(prefixes):
out["mcp_calls"] += 1
out["used"] = bool(out["commands"] or out["reads"] or out["mcp_calls"])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The tests do not pin that a read alone, or an MCP call alone, makes used true. Both existing positive cases combine reads=1 or commands=1 with mcp_calls=1.

Mutation check: dropping reads from the used expression, dropping mcp_calls from it, matching the server prefix without the trailing __ (mcp__code_graph2__x counting for code.graph), and gating uptake on only the files or only the servers all pass the full suite (61 passed in test_repo_uptake.py + test_repo_arms.py). Each is a regression that hides the "used none" warning or raises a false one.

Tests to add: arm_uptake([{"kind": "read", "path": "tools/fmt"}], ["tools/fmt"], [], WS)["used"] is true; the same with one mcp__code_graph__x tool event and servers ["code.graph"]; mcp__code_graph2__x gives mcp_calls == 0; and a run_cell case for a patch that adds only an MCP server.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in 3117e66: test_a_read_alone_or_an_mcp_call_alone_is_a_use (read alone, MCP call alone, and mcp__code_graph2__x gives mcp_calls == 0). I did not add a run_cell case for an MCP-only patch or a files-only/servers-only gating test.

Comment thread dokimi/docs/repo-evaluation-contract.md
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I pushed a commit per nit, up to 073963e, which fix the nit from my review:

  • 073963e: dokimi/docs/repo-evaluation-contract.md:377: the uptake paragraph now names files and mcp_servers, and describes the uptake entry ({cells, used, unmeasured}) and the uptake: ... line of repo compare.

make ci-local at 073963e: 25 passed, 1 failed, 6 skipped (the failure is pip-audit, as at the reviewed head).

Not approving yet. Still open: the two risks from my review; @vchamarthi on arm_patch.mcp_servers records every server of a changed .mcp.json, including the base's; calls to a base server count as uptake and hide the warning (reproduced at this head); @vchamarthi on .mcp.json stays in arm_patch.added when the patch adds it, so a Read or cat .mcp.json counts as use (reproduced at this head).

…vel files strictly

.mcp.json is no longer an added file, mcp_servers excludes servers the base
declared, a slash-less added file matches only as ./name, under the workspace
path, first in a command or after an interpreter, and the text report and
compare show cells that left no trace to measure.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

@vchamarthi vchamarthi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blockers. 1 non-blocking comment.

Both earlier majors are fixed: mcp_servers now drops servers declared in the pre-patch tree, and .mcp.json is out of added. The reads question is answered in the contract, and unmeasured cells now show in report and compare. One open question remains, on bare root-level names in commands; it does not block.

CI at send time: 0 failing, 0 pending.

parts.append(rf"(?:^|(?<=[\s'\"`=;&|(<>])|(?<=\./)|(?<={root}))(?:{paths}){end}")
if flat:
names = "|".join(re.escape(f) for f in flat)
parts.append(rf"(?:^\s*(?:\./)?|[;&|(]\s*(?:\./)?|(?:^|(?<=\s)){_INTERPRETERS}\s+|(?<=[\s'\"`=;&|(<>])\./|(?<={root}))"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question: Is lint x (and cd d && lint, asserted at test_repo_uptake.py:244) meant to count as running the workspace's lint? A bare command name resolves through PATH, so it does not run ./lint unless the workspace root is on PATH, and after cd d it cannot name the root file at all.

The flat-name branch of _path_pattern (uptake.py:109) accepts ^\s*(?:\./)? and [;&|(]\s*(?:\./)?, so the ./ is optional for a name first in a command. The docstring at uptake.py:17-18 says cd tools && ./fmt is not counted, which treats cd as changing what a path names; cd d && lint is counted. If the profile does not put the workspace on PATH, requiring ./ (or the workspace path, or an interpreter) for the first-word case keeps the count to runs of the arm's file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not meant to: a bare lint runs whatever PATH finds, and the profile does not put the workspace on PATH. Fixed in eeff916: a root-level file counts only as ./lint, under the workspace path, or after an interpreter (sh lint). lint x and cd d && lint moved to the no-match cases in test_repo_uptake.py; the docstring and repo-evaluation-contract.md say why. Not covered: cd d && ./lint still counts, since the matcher does not track cd (same limit as the nested-path rule).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#153 merged before this landed; the fix is in #186.

@napetrov
napetrov merged commit 8782e1c into intel:main Oct 22, 2026
28 checks passed
napetrov pushed a commit to antonwolfy/dokimi that referenced this pull request Oct 22, 2026
…e in intel#153

intel#184 already took main's growth since the intel#93 seed into the baseline with
a generic rationale. Name the PR behind each of those entries instead, and
add dokimi/repos/compare.py::_provenance_warnings, which intel#153 took over the
complexity cap after intel#184 and which turns main's Tests job red again.
napetrov pushed a commit to antonwolfy/dokimi that referenced this pull request Oct 22, 2026
…e in intel#153

intel#184 already took main's growth since the intel#93 seed into the baseline with
a generic rationale. Name the PR behind each of those entries instead, and
add dokimi/repos/compare.py::_provenance_warnings, which intel#153 took over the
complexity cap after intel#184 and which turns main's Tests job red again.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
napetrov pushed a commit that referenced this pull request Oct 23, 2026
…153 (#183)

#184 already took main's growth since the #93 seed into the baseline with
a generic rationale. Name the PR behind each of those entries instead, and
add dokimi/repos/compare.py::_provenance_warnings, which #153 took over the
complexity cap after #184 and which turns main's Tests job red again.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants