docs: read the reviews before merging; authorship will not identify them
CI / Build all bundles (push) Has been cancelled
CI / Test and lint (host) (push) Has been cancelled
CI / wasm32 check (browser bundle crates) (push) Has been cancelled
CI / Browser JS unit tests (node --test) (push) Has been cancelled
CI / Knowledge base consistency (push) Has been cancelled
CI / Lint the harness crate (push) Has been cancelled
CI / Browser harness (OPFS contract + shell) (push) Has been cancelled
CI / Deploy to GitHub Pages (push) Has been cancelled

I merged twenty PRs this campaign without reading their REVIEWS. CI green plus
a dispatched adversarial agent felt like the whole gate and was not. Two
ChatGPT Codex reviews were sitting on merged PRs; both were correct.

improvise-x1ns (P2, filed, live on main): encoded_width's categorical branch
counts only distinct PRESENT values, while build_categorical maps every
missing cell to MISSING_CAT and emits an additional one-hot. A forced category
with N present values plus one missing coordinate passes the preflight as N
and materializes N+1 — caught only by the later exact check, which is the
allocation the preflight exists to prevent.

The keeper from that review: 'bounded by the later exact check' is not a
defence, because the exact check runs AFTER the allocation. It bounds the
error message, not the cost. Our own review had seen the shape and reasoned it
away on exactly that ground.

The PR #91 review described round 1 and round 2 fixed it — verified with a
probe rather than assumed, both directions. But the regression it asked for
does not exist, so improvise-4dpj now carries it.

Three things make a Codex review easy to miss, all now in AGENTS.md: it
arrives as a review rather than a comment; it posts under the owner's account,
the same one our agents use, so authorship cannot distinguish it — the tell is
the TRAILING '_Automated review by ChatGPT Codex_' where ours OPEN with
'_Posted by Claude_'; and it may target an earlier head, so a finding can be
already fixed or still live and you must check which.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
2026-09-21 10:15:30 -07:00
parent ce895ff560
commit ea206bf8de
3 changed files with 30 additions and 5 deletions
+2
View File
@@ -287,6 +287,8 @@ doctrine(the_elsewhere_path_is_only_as_correct_as_the_inputs_it_can_rebuild,'pro
doctrine(a_conflicting_pr_runs_no_ci_because_it_has_no_merge_ref,'A pull_request workflow runs against refs/pull/N/merge, and GitHub cannot build that ref for a CONFLICTING pull request -- so NO RUN IS EVER CREATED and the PR simply shows zero checks. That is indistinguishable from a dropped webhook if you only look at the runs list. PR #92 sat with zero checks across two pushes and a close/reopen; the cause was that PR #91 had merged and both had appended to the same four context/ files. DIAGNOSIS ORDER: before theorising about event delivery, ask \`gh pr view N --json mergeable\` -- one call, and it answers first. The orchestrator ran a genuinely good control (push to another PR, watch a run appear in five seconds) which correctly proved delivery works repo-wide, and then drew the WRONG conclusion from a sound experiment, because the hypothesis space had two entries and the truth was a third. A control discriminates only between the hypotheses you listed.').
doctrine(no_checks_reported_is_a_merge_state_symptom,'A pull_request-triggered workflow builds refs/pull/N/merge, and a CONFLICTING pull request HAS NO MERGE REF, so no run is ever created -- \"no checks reported\" then looks IDENTICAL to a dropped webhook. Measured on PR #92, 2026-09-21: 34 minutes of polling actions/runs?head_sha=, a close/reopen and an empty commit all produced zero runs; one git merge main produced a run in seconds. ASK gh pr view --json mergeable,mergeStateStatus FIRST -- one call answers what no amount of polling the runs API can distinguish. Both the agent and the orchestrator hypothesised a dropped event and a stalled delivery; NEITHER LISTED THE THIRD CASE, and the orchestrator control -- pushing to another PR and watching a run appear in five seconds -- is consistent with all three. A control discriminates only between the hypotheses you listed.').
doctrine(grep_over_git_diff_is_silently_empty_under_an_external_differ,'This repo configures diff.external = difftastic, which prints a SIDE-BY-SIDE format carrying no unified-diff markers. So \`git diff | grep \"^-\"\` matches nothing REGARDLESS OF CONTENT, and an agent verifying \"nothing was dropped in the merge\" read three consecutive empty outputs as three clean bills. Verified by the orchestrator: on a file with one real deletion, git diff | grep -c \"^-\" gives 0 and git diff --no-ext-diff | grep -c \"^-\" gives 1. Use --no-ext-diff in any programmatic read of a diff, or --numstat / --name-only, which are unaffected. The general shape, and it is the sharpest form of check_the_checker: A CHECK THAT RETURNS EMPTY FOR THE WRONG REASON IS INDISTINGUISHABLE FROM ONE THAT PASSES -- and an empty result is the one outcome nobody investigates.').
doctrine(read_the_reviews_before_merging_and_authorship_will_not_identify_them,'Twenty PRs were merged in one campaign without reading their REVIEWS -- CI green plus a dispatched adversarial agent felt like the whole gate, and it was not. Two ChatGPT Codex reviews were found afterwards on merged PRs; both were correct and one was a live P2 (improvise-x1ns). Three things make a Codex review easy to miss. It arrives as a REVIEW, not a comment, so scanning the comment thread does not show it. It posts under the repository OWNER account -- the same account our own agents post under -- so AUTHORSHIP DOES NOT DISTINGUISH IT; the tell is the trailing line \"Automated review by ChatGPT Codex\", where ours OPEN with \"Posted by Claude\". And it may target an EARLIER head, so a finding can be already fixed or still live and you must verify which rather than assume from the date. Reviews arrive as the owner quota allows, so absence is not evidence of approval. Answer a review on the thread even after the PR has merged: that is where the next reader looks.').
doctrine(bounded_by_a_later_check_does_not_bound_the_cost,'improvise-x1ns: encoded_width is a PREFLIGHT whose whole purpose is to refuse an expansion BEFORE allocating it, and its categorical branch under-counted by the missing level that build_categorical actually emits. The wave own review had seen the shape and reasoned it away as \"bounded by the exact check\". Codex rebuttal is the keeper: THE EXACT CHECK RUNS AFTER THE ALLOCATION, so it bounds the error MESSAGE, not the cost. When a guard exists to prevent work rather than to report a wrong answer, a downstream correctness check is not a backstop for it -- the thing you were protecting has already happened. Second instance on this path: improvise-zv8t own round found forcing by NAME while budgeting by INDEX, repaired as a data dependency so the ordering became a compile error. Both are the same shape -- two halves of one decision disagreeing about what a column costs.').
work_done('improvise-8tk',bug_fix,'ZZ/:wq quit even when the save failed, silently discarding the model. Save now aborts the effect batch on failure; save_to_current_path/1 returns Result<String,String>; three tests in ui/effect.rs.').
work_done('improvise-907',bug_fix,'Import wizard step machine redesign: validation_error() gate at ReviewProposals (no-category and over-limit), history-stack previous() bound to Left on every step, and Done made a real confirmation step holding the built workbook. Superseded improvise-gfq, improvise-o1c, improvise-c6j.').
work_done('improvise-6sq',wont_do,'Merging EditOrDrill and EnterEditAtCursorCmd into one EnterEdit{mode} is not safe: they differ in WHEN the cell value is read, not in a mode parameter. Added ui/app.rs::tests::i_and_o_diverge_on_an_empty_records_grid as executable evidence.').