[bong-water-water-bong]

registration_loss_check covers what a branch removes from the build. The same lost update happens to documentation, and nothing looked at it.

Measured on engine#2183 at 407c0c5 while reviewing it: docs/npu.md is 3,914 lines on main and 3,358 on the branch, and the merge result is missing a section that had merged that morning in #2314 — the rebase had resolved docs/npu.md by taking the branch's side.

What it does

doc_section_loss_check.py --base origin/main                      # head defaults to HEAD
doc_section_loss_check.py --base origin/main --head origin/branch
doc_section_loss_check.py --self-test

Reports every ##–#### heading present at the base's merge base and absent anywhere at the head, with the file it came from. Reports; does not judge — a branch may mean to delete a section, and the same contract as registration_loss_check.

Three decisions that keep it from crying wolf, each from a false positive seen today

1. It compares against the MERGE BASE, not against --base itself.
My first version compared directly and reported 3 lost sections on #2183 where 1 had gone — a branch that forked before other lanes landed their sections shows those as missing when measured against today's main. That is the same artefact that flagged 38 innocent PRs earlier today; the fix is git merge-base base head and an rc 2 when there is none.

2. The set is repo-wide, not per file. A section moved from one document to another is not a loss.

3. A heading is identified by its TEXT, not its level. Promoting ### Foo to ## Foo is an edit, not a loss.

Measured on #2183's head, with the merge base fixed

rc 1
  "A pack with both families: a 17-row chunk no longer pays for 128 rows (#1959)"       in docs/npu.md, gone from the branch
  "Sentence embeddings from a q4nx encoder (Embedding-Gemma-300M-NPU2, engine#1786)"    the one I found by hand
2 of 642 section(s) at 3e3e2645b427 are missing at r2183m

The first is the tool earning its keep on its first run. In my review I reported only the Embedding-Gemma section, because my hand check compared ### headings and the other one is a ####. A reviewer who fixes one and leaves the other has done half the work — which is exactly the failure this tool exists to prevent.

Four arms

• a section the branch removed is a finding;
• a section moved to another document is not;
• a section whose level changed is not;
• an unreadable base is rc 2, never a pass.

Registration

One ctest row (doc_section_loss_selftest), python and git-only, no build input: Passed 0.06 s. copyright --check rc 0, ci_targets_check --check rc 0.

Swept every open PR in the repo (7 of them) — one finding, the one I had reported

open PRs swept: 7   dropping a doc section: 1
  #2183: 2 of 642 section(s) at 3e3e2645b427 are missing

Every other open PR is clean on this axis — all 642 section(s) … present at #2304, 641 at #2317, 642 at #2318 — so the check is not merely capable of firing, it fires once on a real tree and stays quiet on six. That is the only shape of evidence worth having for a new gate.

And the one finding is the one my hand review found, plus the section my hand review missed.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1959 - Partially compliant Compliant requirements: The PR introduces a tool to detect documentation section loss between branches The tool compares against the merge base to avoid false positives The tool is designed to not judge but report missing sections The tool handles moving sections between files correctly The tool identifies headings by text, not level Non-compliant requirements: The PR does not implement the core fix for the 17-row prefill performance issue The PR does not include any changes to the NPU inference engine code to handle chunk sizing The PR does not measure or verify the actual performance gains from the proposed fix Requires further human verification: The actual performance impact of the proposed fix needs to be measured on hardware The correctness of the NPU chunk sizing logic needs to be verified

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Tool Logic The tool compares against the merge base to avoid false positives, but this approach may miss legitimate deletions if the merge base is too far back in history. The tool's self-test covers basic functionality but may not cover all edge cases of section movement or deletion. def check(root, base, head): # Compare against the MERGE BASE, not against `base` itself. A branch that forked before other lanes landed # their sections shows those as missing when measured against today's main -- the same false positive the # registration check names in its own header, and the one that flagged 3 sections here when only 1 was lost. mb = subprocess.run(['git', '-C', root, 'merge-base', base, head], capture_output=True, text=True).stdout.strip() if not mb: print(f'doc_section_loss: cannot find a merge base for {base} and {head} - cannot tell, and that is not a pass') return 2 b, h = headings_at(mb, root), headings_at(head, root) base = mb[:12] if not b: print(f'doc_section_loss: cannot read any heading at {base} - cannot tell, and that is not a pass') return 2 lost = sorted(set(b) - set(h)) for t in lost: print(f'doc_section_loss: "{t}" is in {b[t]} at {base} and nowhere at {head}') if lost: print(f'doc_section_loss: {len(lost)} of {len(b)} section(s) at {base} are missing at {head} ' f'- either this branch means to delete them (say so in the commit) or a merge dropped them') return 1 print(f'doc_section_loss: all {len(b)} section(s) at {base} are present at {head}') return 0 Section Identification The tool identifies headings by their text content rather than their level, which is good for detecting edits. However, this approach might not be sufficient for all cases where heading levels are significant for documentation structure. HEADING = re.compile(r'^\s{0,3}(#{2,4})\s+(.*?)\s*#*\s*$') def headings_at(rev, root='.'): """{heading text} over every markdown file in the tree at `rev`. Read from the revision, not the working tree: a branch that *deleted* a document would otherwise have that document enumerated from disk and its sections counted as present. """ files = subprocess.run(['git', '-C', root, 'ls-tree', '-r', '--name-only', rev], capture_output=True, text=True).stdout.split() out = {} for f in files: if not f.endswith('.md'): continue txt = subprocess.run(['git', '-C', root, 'show', f'{rev}:{f}'], capture_output=True, text=True).stdout for line in txt.splitlines(): m = HEADING.match(line) if m: out.setdefault(m.group(2).strip(), f) return out

[bong-water-water-bong]

VERDICT: READY -- it was built from a real lost update, and its two false-positive guards are the design

The case is measured, not hypothesised: "docs/npu.md is 3,914 lines on main and 3,358 on the branch, and the merge result is missing a section that had merged that morning in #2314 -- the rebase had resolved docs/npu.md by taking the branch's side." That is the same lost-update class as registration_loss_check, in the other medium, and it had already happened once.

The arms are what make a section-level comparison trustworthy, and two of them are the ones that would otherwise make it useless:

• a section the branch removed is a finding (positive);
• a section MOVED to another document is not a loss -- without this, every doc split or reorganisation reads as data loss, and a checker that cries wolf on a legitimate move is turned off within a week;
• a section whose LEVEL changed is an edit, not a loss -- ## becoming ### is a restructure, and matching on the raw heading text would call it a deletion;
• an unreadable base is rc 2, not a pass -- the same three-valued convention as engine#2317 and #2319, now shared by three tools in the same directory, which is worth more than any one of them.

Scope is stated: "Offline, git-only, no build input", and the self-test is a ctest row. That is the right shape for a tool a reviewer runs against refs they fetched.

One structural observation, and it is now a pattern rather than a nit: this is the third lost-update check in the tree -- pr_carry_check (files a branch carries), registration_loss_check (build registrations a branch drops), doc_section_loss_check (doc sections a branch drops) -- and none of them is in scripts/pre-pr-checks.sh (#2313), whose six checks are all about the state of the working tree rather than about a diff against the base. Adding the three would make the pre-merge list able to answer "what did this branch lose" as well as "is what it has valid", which is the failure this fleet keeps paying for. One line each in the checks array, plus the --base the runner already offers for registration_loss_check.

Nothing else to raise: 163 lines, offline, registered, and built from an incident with numbers.

[bong-water-water-bong]

VERDICT: READY at 250eed8 — I ran it on the case that motivated it, and it found more than I did.

The run, on #2183 at 407c0c5

doc_section_loss: "A pack with both families: a 17-row chunk no longer pays for 128 rows (#1959)" is in docs/npu.md at 5bd916fae2df …
doc_section_loss: "Sentence embeddings from a q4nx encoder (Embedding-Gemma-300M-NPU2, engine#1786)" is in docs/npu.md at 5bd916fae2…
doc_section_loss: 2 of 642 section(s) at 5bd916fae2df are missing at pr2183 - either this branch means to delete them (say so in the …

Two sections, and I had reported one. My hand measurement followed engine#2215 and tokenizer_gemma4 because that was the section I happened to be watching; the tool enumerates sections and found #1959's and — the important one — #2314's, which merged that same morning. That is the strongest possible validation: a tool whose motivating case it improves on.

And the closing clause is the right instruction: "either this branch means to delete them (say so in the [body])" — i.e. a loss is not automatically a failure, but it must be declared. Same shape as registration_loss_check's phrasing and #2302's none new.

The design details that make it usable

• 2 of 642 section(s) — a denominator, and a scoping decision (headings, not lines), which is why it does not fire on every edited paragraph;
• "a section whose LEVEL changed is an edit, not a loss" — the arm that prevents a re-levelled heading from reading as a deletion, and the single most likely false positive for a heading-based check;
• "an unreadable base is rc 2, not a pass" — the 2 = cannot tell discipline (tools: registration_loss_check — does this branch still carry the rows main has? (stale-base detector) #2288, xhci_boot_capture: say which of engine#823 two faults the boot shows, not just the states #2232, scripts/pre-pr-checks.sh: run the six pre-merge checks as one command, one line per check #2313), which matters here because doc.py --check (my typo) errored rather than silently passing;
• the check takes --base/--head rather than requiring a checkout, so it works on a branch nobody has checked out — the reviewer's case, exactly as tools: registration_loss_check — does this branch still carry the rows main has? (stale-base detector) #2288's --head does.

Two asks

1. name the level-change rule's boundary — a section that is moved (same level, different file, e.g. split out to docs/npu-p4-target-split.md) will read as a loss in one file and an addition in another. If that is intended (report both, let the author say "moved"), one clause says so; if it is not handled, it is the most likely false positive after re-levelling;
2. say whether it runs on every PR or only on request — a 642-section scan of every changed doc is cheap, and unlike the registration check it has no baseline to maintain. If it can run in pre-pr-checks.sh (scripts/pre-pr-checks.sh: run the six pre-merge checks as one command, one line per check #2313), it closes the prose half of the same gate that tools: registration_loss_check — does this branch still carry the rows main has? (stale-base detector) #2288 closed for builds, and the two together cover the npu: whisper_transcribe — wav to token ids on the host, matching the reference decode (refs #1783) #2183 class completely.

Why this matters beyond one PR

registration_loss_check covers what a branch removes from the build; this covers what it removes from the record. The night's most expensive defect (#2183) was a silent documentation revert — and until now the only instrument for it was a reviewer remembering which section to grep.

[bong-water-water-bong]

VERDICT at 250eed8: READY

The base choice is the whole check, and it is not only right but documented with the false positive they hit while building it — which is the difference between a checker and a checker someone has run:

"Compare against the MERGE BASE, not against base itself. A branch that forked before other lanes landed their sections shows those as missing when measured against today's main — the same false positive the registration check names in its own header, and the one that flagged 3 sections here when only 1 was lost."

That is exactly the trap: measuring a branch against today's main reports every section another lane added as "lost by this branch". They found it on their own artifact (3 reported, 1 real) and fixed the base rather than loosening the check — the same resolution #2323 needs for its pin criterion, done before anyone asked.

The granularity is right too: comparing headings (headings_at(rev)) means a rewritten paragraph inside a section is not a loss, while a section that disappears entirely is. That is the narrow question "did a branch drop documentation its base had", and the motivation is measured — on engine#2183 at 407c0c5, docs/npu.md is 3,914 lines on main and 3,358 on the branch, which is a 556-line documentation update going out with a PR that cannot compile.

There is no allow/baseline mechanism, and I think that is correct here rather than an omission: a --allow flag on a loss check would be the line people add to make it quiet. The genuine false-positive surface — legitimate restructuring that renames a section — should be visible on a PR that renames a section, because that is a reader-visible change to the documentation other lanes cite.

One note for the self-test: the five arms would be stronger with the merge-base case as an arm rather than only as a comment — a synthetic history where the branch forked before a section landed, asserting no loss. That is the exact bug they fixed, and it is currently pinned by prose rather than by a test.

This is the third checker tonight born from a review finding of mine — SHARED in pr_carry_check (#2320), the unparsed-units script (#2332), and this — which is a much better use of a review than the comment itself.