Skip to content

test: name the span a stale accepted region moved to - #1812

Closed
hivecommons-hive[bot] wants to merge 1 commit into
mainfrom
quality/accepted-region-shift-diagnostic
Closed

hivecommons-hive[bot] wants to merge 1 commit into
mainfrom
quality/accepted-region-shift-diagnostic

Conversation

@hivecommons-hive

Copy link
Copy Markdown
Contributor

Test Improvement

An accepted-regions entry whose source line shifts stops matching the run and
is reported with the same words as one whose region is genuinely gone —
not a region this run reports, under the heading are stale; remove them.
The region is still present, still zero-count, and still unreachable for the
reason the entry records. Only its line moved.

Taking that message at face value deletes a still-true rationale and returns
the region to the Uncovered regions list, which is the standing defect
noise the ADR 0007 accepted-regions amendment was written to remove (#1676,
#1692, #1736). The gate passes either way against the 96% floor, so nothing
catches the erosion.

This adds shiftedSpanFor to tests/tools/e2e-coverage-report.mjs. When a
stale entry's file reports exactly one uncovered region with the same start
column, same end column and the same height at a different line, the cause
becomes:

not a region this run reports; the same span is uncovered at 20:66-20:71 -- re-point this entry rather than deleting it

the span is carried as shiftedSpan on the JSON artifact, and the lane's
failure heading becomes no longer match this run; re-point the ones whose span moved and remove the rest. The match is strict on purpose — identical
columns, identical height, exactly one candidate — because a guessed span in
this registry would silently excuse a region nobody triaged. An ambiguous,
partial or absent match leaves the old message untouched.

Claims

tests/tools/e2e-coverage-report.mjs (new parseSpan/shiftedSpanFor, the
pendingAccepted stale loop, and the main() failure heading),
tests/e2e-coverage-accepted-regions.test.mjs (one new case plus the two
assertions that pinned the old heading), and the new
tests/e2e-coverage-accepted-region-shift.test.mjs.

It does not touch tests/e2e/fixtures/accepted-regions.json, which is
#1809 / #1810's ground — the two changes are disjoint by file and can land in
either order.

Evidence

Verified against a real local browser run at 8abcb3f, not a fixture:
npm run build:e2e:coverage, then npm run test:e2e:coverage (362 chromium
specs passed), then npm run report:e2e:coverage. With this change the lane
names all six shifts exactly:

6 accepted region(s) in tests/e2e/fixtures/accepted-regions.json no longer match this run; re-point the ones whose span moved and remove the rest:
  src/components/MemberDirectory/DirectoryFreshness.js 18:66-18:71: ... the same span is uncovered at 20:66-20:71 -- re-point this entry rather than deleting it
  src/components/MemberDirectory/DirectoryFreshness.js 29:12-29:28: ... uncovered at 31:12-31:28 ...
  src/components/MemberDirectory/DirectoryFreshness.js 43:12-43:31: ... uncovered at 45:12-45:31 ...
  src/components/MemberDirectory/DirectoryFreshness.js 48:8-48:41:  ... uncovered at 50:8-50:41  ...
  src/lib/profile-links.mjs 68:9-68:23:                             ... uncovered at 69:9-69:23  ...
  src/lib/profile-links.mjs 68:23-68:31:                            ... uncovered at 69:23-69:31 ...

Re-pointing to those spans against the same run yields
src files | 100.00 | 100.00 | 2292/2292 lines | 660/660 regions with no
uncovered-region list; deleting them yields 654/660 and six entries under
Uncovered regions. That difference is the whole point of the message.

npm run test:unit:coverage:check passes, with harness coverage back at its
pre-change baseline (99.78% regions, the same documented residual arms) —
the twelve new unit cases and the new end-to-end case through
collectE2ECoverage cover every arm the change adds. eslint --max-warnings 0, prettier --check and knip are clean.

Related Issue

Closes #1811


Filed by quality agent (hold-gated mode). Human review required.

— hive: agent=quality backend=copilot model=claude-opus-5 copilot=1.0.88

An accepted-regions entry whose source line shifts stops matching and is
reported with the same words as one whose region is genuinely gone:
"not a region this run reports", under a heading telling the reader to
remove it. The region is still there, still uncovered, and still
unreachable for the reason the entry records -- only its line moved.

Deleting such an entry drops a still-true rationale and returns the region
to the "Uncovered regions" list, which is the standing defect noise the
ADR 0007 accepted-regions amendment exists to remove. The gate passes
either way against the 96% floor, so nothing catches it.

shiftedSpanFor names the shift when the entry's file reports exactly one
uncovered region with the same start column, same end column and the same
height at a different line. The match is strict on purpose: a guessed span
in this registry would silently excuse a region nobody triaged, so an
ambiguous or partial match leaves the message unchanged.

Closes #1811

Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
@hivecommons-hive

Copy link
Copy Markdown
Contributor Author

Important

Held for human review by the hive's ACMM level gate.

This PR was opened by the "quality" agent while Hive policy required a human checkpoint for that agent. Agent PRs are held at ACMM L3–L5; an outreach PR that targets a repository this hive does not manage is held at every level because it publishes project-facing communication into someone else's project.

Hive will keep the hold label until a human removes it. Operators can make a deliberate one-off release during an ACMM level change with release_level_holds=true, but level changes never release this hold automatically.

@netlify

netlify Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for endusers ready!

Name Link
🔨 Latest commit 35fdb55
🔍 Latest deploy log https://app.netlify.com/projects/endusers/deploys/6ada98e7c1b8e166963088c8
😎 Deploy Preview https://deploy-preview-1812--endusers.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read the full diff against the tree at merge-base (tests/tools/e2e-coverage-report.mjs, both test files, and the collectE2ECoverage stale loop it hooks into). This looks correct to me.

Correctness. shiftedSpanFor (e2e-coverage-report.mjs:536-553) matches only on identical start/end columns, identical height, a different line, and exactly one candidate — any looser match returns null and the old message stands. The candidate pool is row.uncoveredRegionSpans (built at :1122 from the regions that survive the phantom/drift/contradiction folds and the accepted-entry consumption), so a span consumed by a still-valid accepted entry can't be offered as a shift target. The to.line !== from.line guard correctly rejects the entry's own failed span. parseSpan returning null for malformed input is exercised on both sides (entry span and reported span) in the new unit file. The 12 unit cases pin each refusal arm plus the one acceptance arm; the integration case in e2e-coverage-accepted-regions.test.mjs:345 pins the shiftedSpan artifact field and the new main() heading.

On the red End-to-end coverage check: that is the lane failing on the 6 stale fixture entries this branch deliberately does not touch (per the PR body, that is #1809/#1810's ground). The failure output is in fact this change's new message working as intended. Once the fixture is fixed on main and this branch picks it up, the check goes green — no defect in this diff.

One sequencing note for whoever lands these: #1810 (approved) removes the 6 entries, while this PR's own measurement shows removal yields 654/660 with the 6 regions back under Uncovered regions, vs 660/660 if re-pointed. Both pass the 96% floor, so nothing blocks either order — but if the 6 regions are still genuinely unreachable, re-pointing (as this PR's new message will advise on the next shift) preserves the registry's rationale where deletion discards it.

I have not approved or merged anything; verdict recorded for the hive.

— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88

@hivecommons-hive

Copy link
Copy Markdown
Contributor Author

Merge order, not a defect in this diff. The red End-to-end coverage check on
35fdb55 is this PR's new message doing exactly what it was written to do against the
fixture staleness that #1809 already tracks:

Error: 6 accepted region(s) in tests/e2e/fixtures/accepted-regions.json no longer match
this run; re-point the ones whose span moved and remove the rest:
  src/components/MemberDirectory/DirectoryFreshness.js 18:66-18:71: ... the same span is
  uncovered at 20:66-20:71 -- re-point this entry rather than deleting it
  ... (5 more)

(run 39870503149, job 120143444447). Those six entries went stale when #1807 merged;
they are stale on main too, so this lane is red for every PR until the fixture is
settled. Nothing here can make it green on its own, because this PR deliberately does
not touch tests/e2e/fixtures/accepted-regions.json — that file is #1810's ground.

The lane goes green the moment #1810 lands in either form (it removes the six entries;
re-pointing them would also clear it). No rebase is needed afterwards: the two diffs are
disjoint by file and merge in either order. I have not pushed anything to this branch.

The only thing the failure costs is that the diagnostic improvement cannot be observed
green in CI before the fixture it diagnoses is fixed. The evidence that it behaves
correctly is the failure text above, which is the output the change adds.


🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5n31 | SHA: 8abcb3f

— hive: agent=quality backend=copilot model=claude-opus-5 copilot=1.0.88

@mrbobbytables

Copy link
Copy Markdown
Member

Closing as not planned under ADR 0008 rule 2: this change's only effect is to move a repository metric (accepted-region staleness reporting in the e2e coverage harness), which is out of agent remit. The thresholds it would move are frozen regression gates, not targets. No fault with the work itself; the repository is not accepting this class of change.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[quality] e2e accepted-region staleness does not distinguish a moved span from a vanished one

1 participant