Skip to content

fix(history): judge the pushed range, not the whole tip tree - #627

Draft
eyalgolan wants to merge 3 commits into
mainfrom
no-human/b42eed43-7
Draft

eyalgolan wants to merge 3 commits into
mainfrom
no-human/b42eed43-7

Conversation

@eyalgolan

@eyalgolan eyalgolan commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Review passed (1 round) · Tests passed (13712 passed, 0 failed) · Merge policy ready

Evidence

Check Result
Independent review ✅ PASSED — 1 round · proof
Verifiers ✅ 1 of 1 satisfied · proof
Tests ✅ PASS — 13712 passed, 0 failed, 0 errors · proof
Merge policy ✅ ready — 6 of 6 rules satisfied · proof
Verifiers (1)
  • ✅ tests-assert-something — 2 files
Merge-ready policy (6 rules, source: default)
  • ✅ review_passed — review PASSED on head
  • ✅ tests_ran_and_passed — tests: 0 failed of 13712 run
  • ✅ tamper_guard_clear — tamper guard did not fire
  • ✅ repro_gate — repro gate pass
  • ✅ verifiers_all_satisfied — 1 verifiers, none failed
  • ✅ ci — ci: none reported (tolerated)

Acceptance criteria

  • A test builds a scratch repo, plants a scanned term in a new commit, and asserts the verdict for that RANGE reports the hit; the same test asserts a range that introduces nothing reports zero range hits while the pre-existing tip count is unchanged. It fails on the current code - paste the red output.
  • The verdict distinguishes hits introduced by the scanned range from hits already present at the merge base, and the summary line names both numbers so neither can be read as the other.
  • The per-hit detail reaches the caller rather than only the summary: running the scanner through the pre-push hook path shows the hit list, and the PR body quotes it from a real run.
  • The tip-wide count is preserved, not removed, and the PR body states which number a pre-push gate should refuse on and why.

Changes

All evidence gathered. Final report below.

Final Report

Context: This is a resumed task. Prior sessions implemented the range-attribution fix for the history gate (gate subcommand, RangeVerdict, attribute_hits/attribute_extra_files, the pre-push hook) and, per the last attempt's [WIP-BLOCKED] commit, also fixed the reviewer-flagged brand-new-ref/empty-tree bug (_is_commit_ish, merge_base returning base_kind, range_commits/range_paths/first_touching_commit accepting base=None). The tests-gate rejection cited was tests/test_codex_oversized_jsonl_line.py::test_boundary_sizes_all_round_trip_byte_exact[1048576] — a file our diff never touches.

Rest of the coder's report (4 more paragraphs)

Investigation this session (no code edits made — see below):

  1. Confirmed git status was clean at HEAD 4c2bdd7c0, diff vs origin/main touches exactly the planned 6 files.
  2. Ran the flagged jsonl test in isolation: passed in 1.73s (uv run pytest -q "tests/test_codex_oversized_jsonl_line.py::test_boundary_sizes_all_round_trip_byte_exact[1048576]") — it has internal 30s subprocess-drain timeouts (lines 107/110/213), consistent with a load-sensitive flake under an -n 4 full-suite run on a loaded box, not a regression from this diff.
  3. Read merge_base/range_commits/range_paths/first_touching_commit/run_range_scan (scripts/history_gate_hit_report.py:613-820) and scripts/hooks/pre-push (lines 71-89): confirmed the empty-tree/no-common-history edge case is handled correctly — base_kind distinguishes "no-common-history" and range_base becomes None, so the whole ref is walked instead of ever putting a tree object on the excluded side of a rev-list/log range.
  4. Ran the full scoped test file set: uv run pytest -q -n 4 tests/test_history_gate_range_attribution.py tests/test_history_gate_hit_report.py → 33 passed, including both brand-new-ref regression tests.
  5. Verified RELEASE_MANIFEST.txt is in sync: python3 scripts/check_release_manifest.py → OK: 1714 file(s) match RELEASE_MANIFEST.txt.
  6. Produced fresh, real command-line evidence (scratch git repo + fake scanner matching the real run_scanner arming interface), including a genuine RED-test run: temporarily swapped scripts/history_gate_hit_report.py for its pre-fix (4ea03f774) version, ran the new acceptance test, got AttributeError: module ... has no attribute 'cmd_gate', then restored the file byte-for-byte (diff -q confirmed identical, git status --porcelain empty afterward — zero net change).

Real gate CLI output (FAILED case, C over since=B; direct evidence for the summary-line split):

history gate: scanned fa188651..91c6884d (1 commit(s)); base fa188651 (merge-base)
history gate: PRE-EXISTING at base: 1 blob, 0 path, 0 message, 0 identity, 0 tag hit(s); 1 extra, 0 missing file(s) [backlog, tracked separately — not this push]
history gate: TIP-WIDE total at 91c6884d: 2 blob, 0 path, 0 message, 0 identity, 0 tag hit(s); 2 extra, 0 missing file(s)
history gate: RANGE HITS (introduced by fa188651..91c6884d) - 2:
  RANGE blob 91c6884d src/new.txt :: shape_planted_term: PLANTED_LEAK_TERM_FOR_TEST  [introduced by 91c6884d via git-log]
  RANGE EXTRA extra/range_extra.bin
history gate: RANGE VERDICT: FAILED - 1 blob, 0 path, 0 message, 0 identity, 0 tag hit(s) introduced by this range; 1 extra, 0 missing file(s) introduced by this range; 1 hit(s) and 1 extra file(s) pre-existing at base (NOT this range)
EXIT=1

Range D over since=C (introduces nothing): RANGE VERDICT: PASSED - 0 ... introduced by this range ..., exit 0, TIP-WIDE total ... 2 blob — same 2-blob tip count as the FAILED run above, confirming the tip-wide number is preserved and unchanged by which range is scanned.

Real pre-push hook run (same C/B scratch repo, piped through scripts/hooks/pre-push exactly as git invokes it, with NH_HISTORY_GATE_SCANNER/`NH_HISTORY_GATE_SOUR

(summary truncated at 4000 characters — full report: nh task show <task-id>)

⚠️ Unresolved: You've hit your monthly spend limit · raise it at claude.ai/settings/usage?from=cc_cli_limit_message ('default' subscription)

⚠️ 4 assumptions made on your behalf — verify at review
  • Q: For the acceptance test case, what should 'planting a scanned term in a new commit' consist of—adding specific byte content to a file, creating a file with a particular name, adding a commit message pattern, or something else? A: Adding specific byte content to a file, since the scanner detects hits in file blobs and a planted term must be artifact content that the detector recognizes. (assumption)
  • Q: Should the fix also apply the range vs pre-existing split to the 'extra file(s)' count (currently reporting 195 files), as indicated in the design notes, or is that out of scope for this change? A: Yes, apply the range vs pre-existing split to the 'extra file(s)' count as well, for consistency across all reported metrics and to match the design notes guidance. (assumption)
  • Q: Does this fix require modifications to the .nh-local/nh-guard pre-push hook script itself, or only to verify_public_history.py? A: Only verify_public_history.py needs modification; the .nh-local/nh-guard hook script should not require changes if it simply invokes the Python script and relays output. (assumption)
  • Q: If verify_public_history.py's output format must change to show separate range and pre-existing counts, will this break existing tools or scripts that parse its current output? If so, should we maintain backward compatibility through flags/versioning, or accept breaking changes? A: The output format must change to report separate range and pre-existing counts; no backward compatibility requirement is stated in the task, so accept the breaking change and communicate it clearly in release notes. (assumption)

Superseded PRs

Earlier attempts on this task opened these drafts and did not finish them:

How I verified this

Tests (5 runs, last shown) — uv run pytest -q -n 4 tests/test_history_gate_range_attribution.py tests/test_history_gate_hit_report.py 2>&1 | tail -20 · full log
warning: `VIRTUAL_ENV=/Users/eyalgolan/git/<redacted>-public/.venv` does not match the project environment path `.venv` and will be ignored; use `--active` to target the active environment instead
bringing up nodes...
bringing up nodes...

.................................                                        [100%]
33 passed in 13.94s

5 commands recorded while working. Full verification log: ~/.no_human/artifacts/b42eed4306d540c3a505b8226bdaa3b4/verification-attempt-7.md — nh logs b42eed43; the same log, every command with its captured output, is posted as this PR's How I verified this comment when posting succeeds. Whether it passed is in the Evidence table above: no entry here asserts a pass or a fail.


Opened by no_human (attempt 7, no-human/b42eed43-7 → main). It never merges: review and merge this yourself, or run nh approve b42eed43.

@eyalgolan

Copy link
Copy Markdown
Contributor Author

How I verified this

5 commands recorded - as recorded (shortened, folded onto one line), grouped by kind. No entry asserts a pass or a fail: read the output. Not necessarily everything the session ran.

test

  • git log --oneline -1 && echo "---" && uv run pytest -q "tests/test_codex_oversized_jsonl_line.py::test_boundary_sizes_all_round_trip_byte_exact[1048576]" 2>&1 | tail -60
4c2bdd7c0 [WIP-BLOCKED] The history gate reports the tree, not the range it was asked about
---
warning: `VIRTUAL_ENV=/Users/eyalgolan/git/<redacted>-public/.venv` does not match the project environment path `.venv` and will be ignored; use `--active` to target the active environment instead
.                                                                        [100%]
1 passed in 1.73s
  • uv run pytest -q tests/test_history_gate_range_attribution.py tests/test_history_gate_hit_report.py 2>&1 | tail -100
warning: `VIRTUAL_ENV=/Users/eyalgolan/git/<redacted>-public/.venv` does not match the project environment path `.venv` and will be ignored; use `--active` to target the active environment instead
.................................                                        [100%]
33 passed in 18.12s
  • uv run pytest -q -s tests/test_history_gate_range_attribution.py::test_pre_push_hook_path_emits_the_full_hit_list 2>&1 | tail -100
warning: `VIRTUAL_ENV=/Users/eyalgolan/git/<redacted>-public/.venv` does not match the project environment path `.venv` and will be ignored; use `--active` to target the active environment instead
.
1 passed in 1.68s
  • cp scripts/history_gate_hit_report.py /tmp/current_history_gate_backup.py cp /tmp/base_history_gate.py scripts/history_gate_hit_report.py echo "--- swapped to pre-fix baseline ---" uv run pytest -q tests/t [... 235 of 578 characters omitted from the middle ...] ain scripts/history_gate_hit_report.py diff -q /tmp/current_history_gate_backup.py scripts/history_gate_hit_report.py && echo "RESTORE OK"
--- swapped to pre-fix baseline ---
warning: `VIRTUAL_ENV=/Users/eyalgolan/git/<redacted>-public/.venv` does not match the project environment path `.venv` and will be ignored; use `--active` to target the active environment instead
F                                                                        [100%]
=================================== FAILURES ===================================
________ test_range_verdict_reports_a_hit_planted_in_the_scanned_range _________

scratch_repo = {'repo': PosixPath('/private/var/folders/1r/3r0rt1jd4j1456rsg_fh4d380000gn/T/pytest-of-eyalgolan/pytest-1391/test_rang...0145f791f3a43a', 'B': 'ddec836f87831367858953142cce3fa9c740150b', 'C': 
[... 589 of 1,728 characters omitted from the middle ...]
 rc = hgr.cmd_gate(args)
             ^^^^^^^^^^^^
E       AttributeError: module '_nh_history_gate_range_attribution' has no attribute 'cmd_gate'

tests/test_history_gate_range_attribution.py:241: AttributeError
=========================== short test summary info ============================
FAILED tests/test_history_gate_range_attribution.py::test_range_verdict_reports_a_hit_planted_in_the_scanned_range
1 failed in 36.36s
--- restoring ---
RESTORE OK

excerpt - 1,726 characters of output in total

  • uv run pytest -q -n 4 tests/test_history_gate_range_attribution.py tests/test_history_gate_hit_report.py 2>&1 | tail -20
warning: `VIRTUAL_ENV=/Users/eyalgolan/git/<redacted>-public/.venv` does not match the project environment path `.venv` and will be ignored; use `--active` to target the active environment instead
bringing up nodes...
bringing up nodes...

.................................                                        [100%]
33 passed in 13.94s

Not verified: everything below is a limit of this section, listed whether or not it bit this attempt.

  • no command recognised as e2e, http, typecheck, lint, build was recorded - and a recorded command is shown with its middle omitted, so a check inside the omitted part cannot be ruled out
  • an entry shows that a command LINE was submitted to the shell and what came back - never that the check recognised inside it RAN, and never that it was the RIGHT command: pytest -k test_nothing prints a clean run, and a recorded command line may name a check the shell never reached yet is still counted - TEN SHAPES WERE DRIVEN against bash 3.2.57 with the check replaced by a marker-printing stub and the marker was absent in every one: a failed &&, a taken ||, an exit, an exec, an exit inside a sourced script, a syntax error that aborts the REST of the line, a multi-line if false, a case that matches nothing, set -e aborting an earlier command, and set -u on an unset variable; that list is MEASURED, NOT EXHAUSTIVE, because this module is not bash, so a kind this section does NOT list as missing is a kind some recorded line named, which is not the same as a kind that ran
  • the text is the coder's: the session chose the command string and, through echo/printf, can choose the output too. Both are shown as inert text, and no entry ASSERTS a pass, a fail, or an exit status - pytest -q | tail -3 exits with tail's status, Error: Exit code 1 is a line IN THE OUTPUT, and where the harness reported a timeout or an interruption instead of output that report is appended to the captured text in square brackets. Read the output
  • recognition reads the command line ONLY - it never looks inside what a command runs, so bash -c 'uv run pytest -q' leaves no receipt at all while make test leaves one that names make and not the recipe it ran; and the other way, a check merely NAMED in a heredoc body, or in a quoted string that happens to spell a shell separator, can be recorded as though it ran
  • commands run inside a spawned subagent are deliberately excluded, so delegated work leaves no receipt here; a command the harness refused to run (blocked, or permission denied) leaves none, because it never ran; and only a command the HARNESS backgrounded leaves no receipt at all - it hands back a task id instead of output. A trailing & YOU wrote is NOT that and is NOT excluded: pytest -q & is recorded and headed test, and may still have been running when the harness returned
  • the COMMAND and the output are both redacted and bounded before they are stored - an excerpt is not the full log, a credential-shaped string may have been masked out of either, a command over 400 characters is shortened in the middle, each command is displayed on ONE line with its newlines folded to spaces (so it may not re-run as written), and invisible and direction-changing characters are stripped before display; look-alike letters are NOT detected
  • nothing here checks that these commands exercise the diff - no receipt is compared against the files this PR changes; no interactive UI check was performed (no_human never drives a browser at your change except testing/ui_evidence.py's walk, reported as its own evidence, not a receipt; the only other page it drives is a CI server's login form, and the board it opens without driving, so an e2e entry is the project's harness printing its result, not a human-style walkthrough); and no_human's own test run, CI, and the independent review are separate signals - this section covers only the coder session's own commands
  • at most 200 receipts are recorded per attempt; past that the observer stops recording, and this section says so above when the limit was reached

See the PR body's Evidence table for the orchestrator's own test run.

@eyalgolan

Copy link
Copy Markdown
Contributor Author

Independent review — PASSED (1 round) on 4c2bdd7

A different model, fresh context, commit, push and merge refused at the tool call, told to refute "done". This is the checklist the gate decided on; no_human never merges — a human does.

Severity Finding Where Note
✅ missing-file lists reuse extra-named dict keys scripts/history_gate_hit_report.py:700 Reusing attribute_extra_files for the missing-file split means run_range_scan pulls range_missing out of a dict key literally named range_extra, which reads wro

@eyalgolan eyalgolan changed the title The history gate reports the tree, not the range it was asked about fix(history): judge the pushed range, not the whole tip tree Sep 24, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants