From 349e3e7a83a5dbccb9e0a3cb35d9bf119e8e410b Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 12 Sep 2026 02:59:24 +0000 Subject: [PATCH] record(ENG-RECORD-CONFLICT-SURFACES): file two records-tooling defects Both were found while landing the ROCm IQ4_NL arm (PR #3149) and neither is that row's work, so both are filed here rather than folded into it. Neither is fixed in this change, deliberately: each repair is a design question rather than a typo, and both want a spec and a red-before case. agent-issue.py renders an issue file from a fixed IssueRecord schema and never reads the file forward, so every update and close deletes any section the schema does not name. The caller sees a success and no diff. Observed rather than theorised: closing a row's local issue dropped two dated Reconciliation sections that were the only record of why that row's scope changed twice, each citing commits and file:line anchors. They were restored by hand and nothing in the tool's output said they had gone. The normal success path of a sanctioned records tool deleting evidence inverts the Records rule, in the one file class this protocol treats as canonical, and does it invisibly. check-pr-size requires any change touching a governance checker to also change that checker's one named evidence test file, so every row entering the RUNNABLE_BASELINE population edits one shared file. That is the shape AGENTS.md Records calls defective and tells us to move to a per-row surface. Measured rather than argued: the same ERROR fires on three already-merged commits of this shape. What kept it invisible until a fresh review is that agent-preflight SKIPS it for want of arguments, so the first signal is CI. An earlier draft of this message also claimed the checker exits 0 while printing ERROR; that was wrong and is corrected in the issue body. It returns 1. The zero came from reading $? after a pipe, which reports tail's status. It is also stricter than the rule it enforces, since AGENTS.md accepts a red-before test OR MUTATION and this gate accepts only a changed line in one named file. Neither issue cites the IQ4_NL record by ID. That record lives on the #3149 branch, so a reference to it here resolves to nothing and reds check-agent-record; the pull request names it instead. Found the same way as everything else in this change, by reading a checker's OUTPUT after it printed ERROR and exited 0. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5-1m [claude-code] --- .../ISSUE-LOCAL-01M29QNAFS7STAT36D7PVMM9QE.md | 31 ++++++++++++++ .../ISSUE-LOCAL-01M29RSZ48ZW6DH833G505CNBJ.md | 42 +++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 .agents/issues/ENG-RECORD-CONFLICT-SURFACES/ISSUE-LOCAL-01M29QNAFS7STAT36D7PVMM9QE.md create mode 100644 .agents/issues/ENG-RECORD-CONFLICT-SURFACES/ISSUE-LOCAL-01M29RSZ48ZW6DH833G505CNBJ.md diff --git a/.agents/issues/ENG-RECORD-CONFLICT-SURFACES/ISSUE-LOCAL-01M29QNAFS7STAT36D7PVMM9QE.md b/.agents/issues/ENG-RECORD-CONFLICT-SURFACES/ISSUE-LOCAL-01M29QNAFS7STAT36D7PVMM9QE.md new file mode 100644 index 000000000..9e74fb7a6 --- /dev/null +++ b/.agents/issues/ENG-RECORD-CONFLICT-SURFACES/ISSUE-LOCAL-01M29QNAFS7STAT36D7PVMM9QE.md @@ -0,0 +1,31 @@ +ID: ISSUE-LOCAL-01M29QNAFS7STAT36D7PVMM9QE +Title: agent-issue.py silently discards any issue section outside its fixed schema +Row: ENG-RECORD-CONFLICT-SURFACES +State: OPEN +Kind: bug +GitHub: - +Mirror: PENDING +Availability: FULL +Created: 2026-09-12 +Updated: 2026-09-12 +Closed: - + +## Problem + +scripts/agent-issue.py renders an issue file from a fixed IssueRecord schema, so every update and close DELETES any section the schema does not name. Evidence is lost with no warning and no diff shown to the caller. + +Mechanism, read in the tree at 97cb6964b: + +- IssueRecord carries exactly id, title, row, state, kind, github, mirror, availability, created, updated, closed, problem, resolution (scripts/agent-issue.py:120-140 area). +- _write (:654-674) calls filesystem.atomic_write(path, render_issue_record(record)). The rendered text is generated from those fields ALONE; the file on disk is never read forward. +- So a section such as '## Reconciliation', '## Evidence' or any other heading an agent or a human appended is not represented in the record, is not re-rendered, and disappears on the next update or close. + +Observed, not theorised. Closing the local issue on row QUANT-GGUF-IQ4_NL (PR #3149; that record is named in the pull request and is deliberately NOT cited by ID here, because it lives on that branch and a reference to it would not resolve on this one) dropped TWO '## Reconciliation' sections that recorded why the row's scope changed twice: that #3097 had landed the ROCm gather, and that the CUDA arm was never missing because #2419 had already landed it. Both were dated, both cited commits and file:line anchors, and both were the only record of a scope correction. They were restored by hand; nothing in the tool's output said they had gone. + +Why this matters more than an ordinary bug. AGENTS.md 'Records' says to move superseded detail into .agents/completed/ and 'Never delete evidence to reduce context'. A tool whose normal success path deletes evidence inverts that rule, and it does so in the one file class the protocol treats as canonical. The failure is also invisible by construction: the caller sees a success, and the loss is only detectable by diffing the file before and after, which nobody does because the tool is the sanctioned way to edit it. + +Not filed as a same-flow fix, because the repair is a design question rather than a typo: the tool must decide whether unknown sections are preserved verbatim and where they are re-emitted relative to Problem and Resolution, whether ordering is stable across a round trip, and whether a lossy write should refuse rather than warn. That wants a spec and a red-before test that round-trips a file carrying an unknown section. + +## Resolution + +- diff --git a/.agents/issues/ENG-RECORD-CONFLICT-SURFACES/ISSUE-LOCAL-01M29RSZ48ZW6DH833G505CNBJ.md b/.agents/issues/ENG-RECORD-CONFLICT-SURFACES/ISSUE-LOCAL-01M29RSZ48ZW6DH833G505CNBJ.md new file mode 100644 index 000000000..2b7bdcf55 --- /dev/null +++ b/.agents/issues/ENG-RECORD-CONFLICT-SURFACES/ISSUE-LOCAL-01M29RSZ48ZW6DH833G505CNBJ.md @@ -0,0 +1,42 @@ +ID: ISSUE-LOCAL-01M29RSZ48ZW6DH833G505CNBJ +Title: check-pr-size forces every RUNNABLE_BASELINE re-pin through one shared test file +Row: ENG-RECORD-CONFLICT-SURFACES +State: OPEN +Kind: bug +GitHub: - +Mirror: PENDING +Availability: FULL +Created: 2026-09-12 +Updated: 2026-09-12 +Closed: - + +## Problem + +scripts/check-pr-size.py:704-722 requires any change touching a governance_checker path to also change that checker's recognized_evidence file with lines > 0 in the same diff. For scripts/check-gate-commands.py that file is tests/scripts/test_check_gate_commands.py. Every row that enters the RUNNABLE_BASELINE population must therefore edit one shared test file, which is the shape AGENTS.md Records names as defective: 'A gate often creates the lock. If a checker requires every change to edit one shared file, the checker is defective. Move the obligation to a per-row surface. Do not delete the obligation.' + +Measured, not argued. The same ERROR fires on three already-merged commits of the same shape, so this is repo-wide behaviour rather than one row's mistake: + + d1256c2fd BACKEND-TENSTORRENT-KEEPQUANT fires (also on check-agent-record.py) + 2f4001199 QUANT-EXL3-PERF fires (also on check-agent-record.py) + aa7bcc8c6 QUANT-EXL3-MUL1 fires + +Found on PR #3149 (row QUANT-GGUF-IQ4_NL), where the baseline move was genuinely required: deleting the single line 'QUANT-GGUF-IQ4_NL' from RUNNABLE_BASELINE reds 17 tests, including test_the_baseline_matches_the_shipped_record, test_the_baseline_re_pin_is_load_bearing and test_hf_model_download_earns_its_runnable_baseline_entry. So the ratchet is doing its job and the evidence requirement is the part that does not fit. + +One further fact a reader needs, and one CORRECTION this issue makes against its own first draft. + +The fact: agent-preflight.sh SKIPS check-pr-size for want of --base/--head/--branch, so no local gate surfaces this at all and the first signal is CI. That is how PR #3149 reached a fresh review with it outstanding. + +The correction: this issue first asserted that check-pr-size 'prints ERROR and EXITS 0', and that is FALSE. Measured both ways at the same revision: + + python3 scripts/check-pr-size.py ... | tail -2 -> $? == 0 + python3 scripts/check-pr-size.py ... > file -> $? == 1 + +main() returns 1 when errors is non-empty. The zero came from reading $? after a PIPE, which reports the exit status of tail and not of the checker. The wrong reading was the author's, not the tool's, and it is recorded here rather than quietly deleted because a filed defect that misstates the tool it accuses is worse than no filing. Nothing else in this issue depends on it: the shared-file lock is a property of check-pr-size.py:704-722 and is unaffected by how the process exits. + +The gate is also STRICTER THAN THE RULE IT ENFORCES. AGENTS.md says a semantic checker change needs 'a spec, a red-before test OR MUTATION, and green-after evidence'. A mutation satisfies AGENTS.md; check-pr-size accepts only a changed line in one named test file, which a pure data re-pin cannot produce without adding a per-row test class to that shared file. + +Not filed with a fix. The repair is a design question: whether a data-only re-pin of a baseline list should count as a semantic checker change at all, whether the evidence surface should become per-row (one file per row, read with a glob, as Records prescribes), and whether the checker should exit non-zero when it prints ERROR. That wants a spec and a red-before case. + +## Resolution + +-