[napetrov]

Summary

docker-claude arm
  arm_argv()                      # the arm's own argv builder
  run_docker() / run_host()       # claude -p ... against RecordingServer (no model call)
  normalize_request()             # blank dates, ids, versions, host, work dir
  diff_requests() / leaks_against()
    -> tests/fixtures/fidelity/claude-code-2.1.318/{bare,skill}.json

• dokimi/harnesses/fidelity.py: record, normalize, compare, capture one arm.
• scripts/refresh_fidelity_golden.py: retake the golden (clean container, pinned version).
• tests/test_fidelity_golden.py: normalization, operator-extras filter, golden pins, live audit, sealed argv flags, and the arm-vs-golden check (skips without the pinned binary, fails with DOKIMI_FIDELITY_REQUIRED=1).
• CI job fidelity (after static) installs @anthropic-ai/claude-code@2.1.318 and runs the test.
• License-check exclusion for the fixture skill (its body is what the client reads).

Refs #354.

Evidence

• Before: no check that a docker-claude arm sends what Claude Code sends.
• After: pytest tests/test_fidelity_golden.py gives 8 passed with the native claude 2.1.318 and with the npm build @anthropic-ai/claude-code@2.1.318 (DOKIMI_FIDELITY_REQUIRED=1). ruff check clean on the new Python files. Local skywalking-eyes header check flags no fidelity file.
• CI: fidelity job, first run pending.

Merge Danger

Door: two-way. A CI job, a test and a golden; revert removes the check.

Refreshing the golden is a deliberate step: tests/fixtures/fidelity/README.md says what to review in the diff.

Blast Radius: CI. One new job per PR (npm install of a pinned CLI, one short local run, no model calls). A failure blocks merge until the golden or the arm is fixed.

[exai-bot]

OpenCodeReview: Commit e00782f was not reviewed. This PR has reached its review limit (6 reviews).

CI at `e00782f`: 9 still running, 7 passing, 2 skipped

Check	Status
CLI interface diff	still running
CodeQL (actions)	still running
CodeQL (python)	still running
Dependency review	still running
Fidelity golden (claude-code 2.1.318)	still running
Package install smoke test (py3.11)	still running
Tests (py3.11)	still running
Tests (py3.14)	still running
Windows smoke test (py3.13)	still running

To review this commit: re-request review from exai-bot in the Reviewers list.

<details><summary>Last review (5323b53): no findings</summary>

Open threads from the bot: 0

</details>

[Copilot]

🟡 Changes recommended

Fidelity comparisons remain unreliable because excluded skill descriptions and billing-version suffixes survive filtering and normalization, while both runners omit the solver’s skill-disabling environment settings.

3 open issues

• Normalize the complete billing-version value, including suffixes · New
• Filter complete skill entries, including multiline descriptions · New
• Use agent_env() in both fidelity runners · New

What changed in this PR

3 new must-fix findings.

This PR adds a Claude Code request-fidelity check for dokimi’s docker-claude arm, supporting #354 by comparing recorded requests with pinned golden captures without calling a model.

Changes:

• Adds request recording, normalization, comparison, and golden-refresh tooling.
• Introduces bare and skill-enabled fixtures with fidelity and payload-audit tests.
• Adds a CI job using pinned Claude Code and exempts the fixture skill from license headers.

File	Description
dokimi/​tests/​test_fidelity_golden.py	Tests normalization, payload auditing, sealed flags, and golden comparisons.
dokimi/​tests/​fixtures/​fidelity/​skill/​sum-fast/​SKILL.md	Provides the fixed skill used during captures.
dokimi/​tests/​fixtures/​fidelity/​README.md	Documents the fidelity contract and refresh procedure.
dokimi/​tests/​fixtures/​fidelity/​claude-code-2.1.318/​skill.json	Stores the skill-enabled request baseline.
dokimi/​tests/​fixtures/​fidelity/​claude-code-2.1.318/​bare.json	Stores the no-skill request baseline.
dokimi/​scripts/​refresh_fidelity_golden.py	Captures and writes refreshed golden requests.
dokimi/​dokimi/​harnesses/​fidelity.py	Implements recording, normalization, runners, and comparisons.
dokimi/​changelog.d/​20261104_130000_napetrov_fidelity-golden.md	Announces the fidelity check and tooling.
.github/​workflows/​ci.yml	Adds the pinned-client fidelity job.
.github/​.licenserc.yaml	Exempts the measured fixture skill from header insertion.

_{Review effort: Balanced}

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 95.72954% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.79%. Comparing base (94ad4db) to head (e00782f).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
dokimi/dokimi/harnesses/fidelity.py	95.72%	7 Missing and 5 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #400      +/-   ##
==========================================
+ Coverage   94.76%   94.79%   +0.02%     
==========================================
  Files         352      355       +3     
  Lines       46407    46869     +462     
  Branches     7949     8013      +64     
==========================================
+ Hits        43978    44429     +451     
- Misses       1539     1548       +9     
- Partials      890      892       +2     

Files with missing lines	Coverage Δ
dokimi/dokimi/harnesses/fidelity.py	95.72% <95.72%> (ø)

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[napetrov]

Fixed in 83c816d: same finding as the thread above (test argv forwarding), same fix.

[napetrov]

Already fixed in 83c816d (napetrov): all three findings are on head, tested.

No claim here: the review body is the bot's run marker, so there is nothing to answer in it.

No claim here: the review body is the bot's run marker, so there is nothing to answer in it.

No claim here: the review body is the bot's run marker, so there is nothing to answer in it.

No claim here: the review body is the bot's run marker. Its two findings are the threads above.

Not a blocker: codecov.yml makes the patch status informational. The two new branches of this PR have tests.

[napetrov]

Re-signed for DCO (sign-off only, tree unchanged). Old → new:

• ded1628 → fcabab0
• 9f696b4 → f4af9f1
• e5778b8 → 574b5f2

[antonwolfy]

Review at 574b5f2. The recorder, the runners and the fixes for the earlier bot threads hold up: the three fixes I re-checked (continuation lines, text after an excluded-only listing, a client that fails after posting) are pinned by tests. The problems are in what the normalizer hides and in what its tests do not pin. Four things are worth fixing before merge, and one is a nit.

What I ran, on a Linux review host at 574b5f2:

• make ci-local with Docker: 35 passed, 2 failed, 7 skipped; 15,734 tests, 1 failed. The failed test step is a timing test (test_unbalanced_quote_in_hook_completes_quickly, 1.35 s against a 1 s limit) in a file this PR does not touch. The failed Install the pinned Claude Code step is the ci_local.py finding below. The changed-line coverage gate and the fidelity golden step (24 passed) pass.
• tests/test_fidelity_golden.py with the pinned claude 2.1.318: 24 passed.
• Reproductions for the code findings, offline.
• A mutation run on fidelity.py: 77 mutants, 35 of which pass the 22 tests that run without the claude binary. The ones that decide a verdict are in the inline comments.

Fidelity step in make ci-local runs a global npm install. ci_local.py skips only pip install lines (PIP_INSTALL) and has no entry for the new Install the pinned Claude Code step (.github/workflows/ci.yml:406). The step ran npm install -g @anthropic-ai/claude-code@2.1.318 here and failed with EACCES ... mkdir '/usr/local/lib/node_modules'. On a machine where the prefix is writable it replaces the contributor's global claude. Suggestion: in ci_local.py, treat the step like the pip ones (skip it and use a claude already on PATH, or add ("fidelity", "Install the pinned Claude Code") to NEEDS_NETWORK).

[antonwolfy]

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

• 5323b53: .github/workflows/ci.yml:393: added timeout-minutes: 15 after runs-on: in the fidelity job.

make ci-local at 5323b53: 36 passed, 1 failed, 7 skipped.

Not approving yet. Still open: the two bugs and the two risks from my review.

[napetrov]

Fixed in efcae57, b331d32, 0a22ac8 and 84b2024, one commit each.

Open risks map to efcae57, b331d32, 0a22ac8, 84b2024. Ready to re-review.

[vlad-perevezentsev]

Review at 84b2024. The recorder, the runners and the fixes for the earlier threads hold up. Two things need a change before merge: the skill-listing filter hides a broken seal from the golden, and make ci-local now fails for anyone without claude 2.1.318. The rest is in the line comments.

What I ran, at 84b2024:

• make ci-local --offline: 19 passed, 1 failed, 14 skipped. The failed row is fidelity / Run the fidelity golden: this host has claude 2.1.303, and with DOKIMI_FIDELITY_REQUIRED=1 both golden cases fail (see the ci_local.py comment). The other 28 tests of test_fidelity_golden.py pass.
• Offline repros on the committed goldens: two bundled skills added to the skill listing, and another model in the bare request, both normalize to the golden with an empty diff.
• external-catalog-smoke (docker-oracle, docker-probe) with Docker: both pass, every isolation probe closed.

Checked, no issues

• Earlier threads: the continuation-line, closing-tag, exact-name, billing-suffix, raw anthropic-* header, failed-client and residue fixes are on this head, each with a test.
• Arm argv: arm_argv calls DockerSolverHarness._claude_args, so the capture uses the flags the arm sends; the fake client checks the prompt, the plugin dir and the sealed skill switches.
• ci-local: the global npm install -g is skipped, and test_host_install_entries_match_real_steps catches a renamed step.
• Changelog and licenses: the fragment is under Added and says how to refresh; new files have the SPDX header, and the fixture skill is exempt for a stated reason.

[napetrov]

Both blockers fixed in ddd0dd0; goldens retaken, ci-local green.

[xaleryb]

• Ran with the pinned @anthropic-ai/claude-code@2.1.318 (npm) on the host and in debian:trixie-slim, and on main merged with the head: the full suite, ruff, mypy, bandit and the benchmark gate.
• ci_local.py with DOKIMI_FIDELITY_REQUIRED=1 exported (README step 2) and claude 2.1.303 on PATH: the NOTICE says the variable is unset, then both golden cases fail and the row FAILs. Expected: they skip. The cause is del step_env[var], while the step env is merged over os.environ.
• Clean capture: the stock client sends the <total_tokens> and commit-attribution reminders, and normalize_request always stripped them. An attribution setting carrying expected_skill judge then gave diff 0 and leaks 0. Expected: a diff and a leak.
• Pushed:
  • 5d1ae60: sets the variable to 0 instead of deleting it; the tests export it.
  • e00782f: strips the reminders only with DOKIMI_FIDELITY_OPERATOR_HOST=1. The goldens are retaken with the pinned client; the only change is the two reminder blocks per case. The README is reworded.

[vlad-perevezentsev]

LGTM

I checked e00782f. All seven points from my first round are fixed, each with a test: the skill listing and the client's reminders are compared as sent, so a broken seal is a diff, and make ci-local no longer fails with another claude on PATH.

Merge: safe now: it merges cleanly into current main, 4 commits behind, and its tests, the growth ratchet and schema_compat pass on the merge. No open PR conflicts with it once it is in.

What I ran, at e00782f:

• make ci-local --offline: 19 passed, 1 failed, 14 skipped. The failed row is schema_compat vs the PR base: main added run_id / dokimi_version to the schemas in #387 after this branch's base, and this PR changes no schema. The fidelity row passes with claude 2.1.303 on PATH: the NOTICE sets DOKIMI_FIDELITY_REQUIRED=0, and the two golden cases skip.
• My earlier repros on the new goldens: two extra skills in the listing give a diff in both cases; a request for another model is refused by check_model; claude-haiku-4-5-20251001 is kept and named <MODEL_NAME>. keeps its period.
• On the merge with main: test_fidelity_golden.py, test_ci_local.py, test_refresh_fidelity_golden.py, test_growth_ratchet.py, test_schema_compat.py: 208 passed, 2 skipped.
• I could not run the pinned 2.1.318 client here (no npm); @xaleryb ran it on the host and in debian:trixie-slim.

Checked, no issues

• Operator-host mode: DOKIMI_FIDELITY_OPERATOR_HOST=1 filters the capture and, through operator_view, the golden the same way; CI never sets it, and the refresh script cannot write a --host capture into the golden directory.
• ci-local: the pin variable is set to 0, not deleted, so a value exported in the shell does not come back; the version comes from the job's own npm pin, and a renamed step fails test_pinned_claude_entries_match_real_steps.

[antonwolfy]

Review at e00782f. The changes since my last review hold up: the skill listing and the client's reminders are now compared as sent, the model is checked in capture_arm, the memory path matches the arm's, and --host needs --out. Three risks are left, all in the operator-host path and the refresh script. I'd fix them before merge.

What I ran, on a Linux review host at e00782f:

• make ci-local with Docker: 35 passed, 1 failed, 8 skipped; 15,708 tests passed. The changed-line coverage gate and the fidelity golden step pass. The failed Run tests step is test_unbalanced_quote_in_hook_completes_quickly (2.4 s against a 1 s limit) plus two timeouts in test_audit_test_quality.py, all in files this PR does not touch.
• Reproductions for the code findings, offline.
• A mutation run on fidelity.py and refresh_fidelity_golden.py: 30 mutants, 8 of which pass the full fidelity test files. The ones that decide a verdict are in the inline comments.

Checked, no issues

• Operator-host view: operator_view(golden) equals the operator-host normalization of a raw request with extra skills, a continuation line, a trailing bullet and the two reminders put back.
• Model id: only whole ids are blanked (claude-haiku-4-5-20251001 is kept); residue flags a model field that is not <MODEL>.
• ci-local: the pin variable is set to 0, a missing claude or an unreadable pin drops it, and the tests cover both.
• Commits: every non-merge commit has a Signed-off-by.

[antonwolfy]

An operator-host bare capture with extra skills keeps an empty <system-reminder> shell, so it never matches the golden.

When every listing entry is excluded, the block is cut down to the wrapper, and rest or None does not see it as empty. The guard at line 286 only runs when a reminder was removed.

• Input: <system-reminder>\nThe following skills are available for use with the Skill tool:\n\n- dataviz: x\n- loop: y\n</system-reminder>, _keep_text(text, ("sum-fast",)).
• Output: '<system-reminder>\n\n\n</system-reminder>'. normalize_request puts it into messages as its own text block.
• bare.json has no listing block, so diff_requests(capture, operator_view(golden)) is non-empty on any host that has skills other than sum-fast. That is the case this mode exists for.

No test covers a listing that is only a wrapper: test_excluded_only_listing_keeps_the_text_around_it has loose text after it.

Suggestion: after kept = _keep_listing(...), return None when re.sub(r"</?system-reminder>|\s", "", rest) is empty, as line 286 does, and add a test with a wrapper-only listing.

[antonwolfy]

Operator-host mode is not pinned by any test that runs without the pinned claude, so two ways to break it pass the suite.

Mutation check, both pass test_fidelity_golden.py and test_refresh_fidelity_golden.py (40 passed):

• operator_view strips with () instead of (FIXTURE_SKILL_NAME,). The existing test uses bare.json, which has no listing, so the skill golden would lose its sum-fast entry on the golden side.
• capture_arm line 551 as keep = None. No test sets operator_host=True, so the flag can be ignored.

Only test_docker_claude_payload_matches_golden would catch either, and it skips without the pinned binary.

Suggestion: add a test that operator_view of skill.json keeps the sum-fast entry and drops the reminders, and a fake-client test that capture_arm(..., operator_host=True) removes an extra skill from the listing while operator_host=False keeps it.

[antonwolfy]

Nothing tests the residue gate that keeps a dirty golden from being written.

Mutation check: if dirty: replaced by if False:, and its return 1 changed to return 0, both pass the full fidelity test files. The refresh tests stub capture_arm and never return a request with a date, path or raw model id. A refactor that drops the gate writes a dirty golden, and the golden's own residue test only fails later.

The call at line 67 is also unpinned: adding operator_host=True passes. That would write a golden with the reminders and the extra skills removed, and CI would no longer see a change in either.

Suggestion: add a test in test_refresh_fidelity_golden.py where the stub returns {"x": "/home/bob/a"}: assert main() returns 1 and writes no file. Assert that the stub received operator_host as false and skill_dir as the fixture.