From a70a0fbbd4953ca657dfe64dee9b9aa22fe25056 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Sun, 30 Aug 2026 10:46:25 -0400 Subject: [PATCH] docs(harness): two costs that are only visible across branches git cherry compares patch-ids and a patch-based merge rewrites them, so it reports landed commits as unlanded on any long branch - three instances today. And a serde struct plus a struct literal in a test broke ten fixtures across four structs, each on whoever rebased next. Change-Id: I007d97e25ab86d95527a85c9794774bff6d05f03 --- plan/harness.md | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/plan/harness.md b/plan/harness.md index b00790c..870b6ac 100644 --- a/plan/harness.md +++ b/plan/harness.md @@ -682,3 +682,61 @@ diff. So: Same family as the sweep and the record key above: an operation that is right at every site but one, and the exception is invisible because it is the thing you were looking at. + +### `git cherry` is reliable on short branches and quietly wrong on long ones + +`git cherry origin/main ` prints `+` for commits it believes are not +upstream. It compares **patch-ids**, and a patch-based merge rewrites them, so +on a branch of any length it starts reporting commits as unlanded that are +already in. Three instances in one session: + +- `claude/volley-cost` reported **four** unlanded; all four were on `main` under + rewritten SHAs, and the branch was about to be restacked with nothing in it. +- `claude/move-profiles` and `claude/move-retry` reported 17 and 18 unlanded + after a merge that had landed most of them. +- The same `move-retry` later reported **27** unlanded, of which a rebase + identified **8** as already applied. + +**The cheap check is `git rebase origin/main` itself.** Its +`skipped previously applied commit` lines answer the question directly, because +the rebase machinery compares *content* where `git cherry` compares patch-ids: + +``` +git rebase origin/main 2>&1 | grep -c 'skipped previously applied' +``` + +Better than grepping for a distinctive symbol, which is the other way to settle +it and needs you to already know which symbol is distinctive. Use the symbol +check to answer "did *this feature* land"; use the rebase to answer "how much of +this branch landed". + +### A serde struct plus a struct literal in a test is a standing trap + +**Ten instances today, across four structs.** A struct that is deserialised from +the wire gains a field; every test fixture that builds it as a *struct literal* +stops compiling; and the breakage lands on whoever rebases next, as a compile +error in a branch whose author touched nothing near it. + +| struct | fields gained | fixtures broken | +|---|---|---| +| `Weapon` | six, incl. `anti_mek`, `classes`, `avg_damage_minimum` | `carry.rs`, six separate times | +| `BoardHex` | `kind`, from the terrain-cost merge | `carry.rs`, and two in `stands.rs` | +| `Walker` | `ground_turn_mp`, `terrain` | its own fixtures | + +It is not that two instances make a category. It is that **the same category +recurred six more times in one afternoon of routine restacking**, and nobody had +counted it because each payer books it as their own overhead - the same +accounting error as the arc corpus above. + +**The fix is to deserialise the fixture rather than construct it.** Every one of +those fields carries `#[serde(default)]`, so `serde_json::from_str` fills them +and the next addition costs nothing: + +```rust +let weapon: Weapon = serde_json::from_str(r#"{"id": 1, "name": "PPC", ...}"#).unwrap(); +``` + +**The honest limit:** this only works for structs that are actually deserialised. +`Walker` is not - it is built in code, so its fixtures have to be updated by +hand, and the rule does not cover it. A rule that overstated its coverage would +be the thing this file exists to warn about. -- 2.51.2