Repository navigation
docs+ci: finish the issue-reference migration, and close the gate behind it - #219
Merged
Merged
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
✅ Duplicate Code ReportTwo engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check. ✅ PMD CPD
No new clones introduced by this PR. ✅ jscpd (language-agnostic)
No new clones introduced by this PR. Powered by astubbs/duplicate-code-cross-check |
✅ SpotBugs ReportNo bugs found (new bugs only — baseline from base branch excluded). |
astubbs
changed the title
Owner
Author
|
@claude review this |
|
Claude finished @astubbs's task in 3m 11s —— View job Review
Solid PR — I traced the regex logic in Gate fix (
Sweep accuracy
Minor, non-blocking observation
Caveat on verification: I wasn't able to execute No functional bugs found. This is a well-scoped, well-tested, mechanically careful PR — happy to see it merged with the suggested re-cut-rather-than-rebase-merge strategy from the description. |
Owner
Author
|
@claude review this |
|
Claude finished @astubbs's task in 2m 52s —— View job Review
I traced Gate fix (
Sweep accuracy — spot-checked, matches
One description claim that doesn't hold against the current diff
No functional issues found. The regex logic, test coverage (including both-direction fixtures), and sweep classification all check out against direct diff inspection rather than just the description's own claims. Nice attention to the false-negative-is-worse-than-false-positive principle throughout. |
astubbs
changed the title
The reference gate requires any #NNN below the threshold to name its repo, because the fork's numbering sits entirely inside upstream's range and a bare number is a coin flip. It checks added lines only, so it never fired on text nobody was editing - leaving the convention true of the files the original work touched and false of the rest of the tree. Two passes, on the same lines and so landing together: - 366 bare references gain their repo. All 77 distinct numbers were resolved against BOTH repos first: 62 of them exist in each, meaning different things, so the classification is per-occurrence rather than per-number. #188 and #195 are fork mirror issues in the release notes while confluentinc#188 and confluentinc#195 are the upstream bugs cited in test comments in the same tree. - 24 `upstream #NNN` uses become the owner form. That form passed the gate but names a relationship rather than a repository, and this fork is itself upstream to anyone who forks it. Three sets are deliberately NOT prefixed, because they are not references: author ordinals ("run #1", "produce #1/#2", "NUDGE #1/#2") annotating log excerpts, reworded to plain numbers; the changelog gate's fixture, which asserts that a *bare* #NN is not a citation and would have been destroyed by qualifying it, so it moves above the threshold as a fake #999104; and upstream-pr-analysis.adoc, which is exempt and written entirely in upstream terms. README link text keeps its qualifier even though the URL beside it already names the repo - the gate can see a link target, a reader cannot, and this fork has its own #12. Quoted upstream titles keep the quotation intact with the number appended rather than having the owner inserted mid-title. README is generated: the edit is in src/docs/README_TEMPLATE.adoc. @tag("#355") becomes @tag("confluentinc#355"). Verified nothing selects on that tag - no pom, workflow or script filters it - and both classes still collect and pass. The 14 upstream-derived Java files gain the "Modifications Copyright" line the provenance-aware header check requires of any file changed since the fork point. docs/inflight/next-qualify-remaining-refs.md is deleted: this is everything it tracked, and in-flight files do not outlive their work. No behaviour change.
… role form Two changes to the same rule, in the order they have to happen. FIRST, three shapes made stripQualified report a number as unqualified when the URL right beside it already named the repo: - asciidoc `URL[link text]` - the bare-URL rule matches \S+, so it consumed ".../issues/65[Enhanced" and left "retry epic #65]" looking like prose. The new rule must run BEFORE the bare-URL one, or the URL eats the opening bracket and there is nothing left to recognise. - asciidoc `{attr}/path[link text]` - no http prefix at all, so nothing stripped it. This is how the README writes upstream links. - an html anchor wrapped across two lines - suspectRefs works a line at a time, so the single-line rule never sees a complete element. Each half now strips only when its counterpart is absent, which leaves a complete anchor to the original rule and keeps a #NN sitting OUTSIDE the element visible. This was found by the sweep having to work around it: the qualifier had to sit immediately before the number, so "[confluentinc PR #291 ...]" failed while "[PR confluentinc#291 ...]" passed. With the macro stripped whole, either reads. SECOND, `upstream #NNN` is no longer accepted. The sweep removed its last use, and a tolerated form comes back the moment someone copies older text. Four edits that must move together: the alternation leaves stripQualified; "allows upstream prose" becomes "flags upstream prose"; "still flags trailing items" now expects #233 too, since the leading `upstream` no longer qualifies the first item; and formatFailure() stops telling authors the form still passes, with the assertion that pinned that wording updated to pin the new state rather than deleted. Stripping MORE is the direction that produces false negatives, and AGENTS.md is explicit that a wrong reference which resolves is worse than a broken one. So the fixtures assert both directions: link macros are accepted, and a bare ref elsewhere on the line, or after a split anchor closes, is still flagged. 30 assertions pass.
docs/inflight/AGENTS.md and docs/refactoring.md both still said a bare #NN means this fork. AGENTS.md itself described the `upstream #NNN` tolerance as live and pointed at a sweep that has now finished. This is why the backlog kept regrowing. All three are read precisely when someone is about to write a reference, so the stale instruction was producing new violations faster than opportunistic fixes removed them. Clearing the tree without correcting them would have left the source running. AGENTS.md also gains the rule the gate cannot enforce: a hyperlink satisfies the gate but not the reader, so link text names the repo anyway. Without it written down, the next reader strips those qualifiers back out on exactly the reasoning that they are redundant - which is true of the gate and false of a human. The two smaller guides now state the below-threshold rule, name the gate, and link the authoritative section rather than restating it a third time.
astubbs
force-pushed
the
docs/qualify-issue-refs-treewide
branch
from
August 7, 2026 00:37
83a7986 to
4891582
Compare
…ile only The note said "never fixed there", which is wrong and hid the more useful fact. confluentinc#864 did fix it, and is merged. What it did not do is fix the source. Verified: that PR touches README.adoc and nothing else, while the README is generated from CoreApp.java by asciidoc include - so upstream's source still reads 0.5 and the patch reverts the next time the README is regenerated. That is exactly how this fork inherited the patch and then lost it, and it is why the fix here is in CoreApp.java with README.adoc regenerated from it, rather than the other way round.
astubbs
force-pushed
the
docs/qualify-issue-refs-treewide
branch
from
August 7, 2026 00:39
4891582 to
e884dba
Compare
4 tasks done
5 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The reference gate requires any
#NNNbelow the threshold to name its repo, because the fork's numbering sits entirely inside upstream's range and a bare number is a coin flip. It checks added lines only, so it never fired on text nobody was editing - leaving the convention true of the files the original work touched and false of the rest of the tree.This finishes the migration and closes the gate behind it, so the forms it just removed cannot come back.
#NNN. 366 references across 69 files.upstream #NNNuses to the owner form. 24 across 11 files.upstream #NNN, which nothing uses any more.Resolved against both repos, not prefixed blind
All 77 distinct numbers in phase 1 were looked up in both repos first. 62 of the 77 exist in both, meaning different things - so classification is per-occurrence, not per-number.
astubbs#188andastubbs#195are fork mirror issues in the release-blocker notes, whileconfluentinc#188andconfluentinc#195are the upstream bugs cited in test comments in the same tree.The gate fix
Three shapes made
stripQualifiedreport a number as unqualified when the URL right beside it already named the repo:URL[link text]- the bare-URL rule matches\S+, so it consumed.../issues/65[Enhancedand leftretry epic #65]looking like loose prose. The new rule must run before the bare-URL one, or the URL eats the opening bracket and there is nothing left to recognise.{attr}/path[link text]- nohttpprefix at all. This is how the README writes upstream links.suspectRefsworks a line at a time, so the single-line rule never sees a complete element.Stripping more is the direction that produces false negatives, and
AGENTS.mdis explicit that a wrong reference which resolves is worse than a broken one. The fixtures cover both directions: link macros are accepted, and a bare ref elsewhere on the same line, or after a split anchor closes, is still flagged.Link text still names the repo, even though the gate no longer asks
A hyperlink satisfies the gate - it can see the target. A reader cannot. Someone reading
issue #12in this fork's README has no way to know it means confluentinc without hovering, and the fork has its own#12. So the README keeps[confluentinc issue #12], andAGENTS.mdrecords this as style rather than enforcement - precisely because the gate will not catch it, and the next person would otherwise strip it back out.The gate fix is what makes the good phrasing possible: the old bare-URL rule ate the
[confluentinctoken and then reported a barePR #291, forcing an awkward attached form. Now prose takes the house spaced form and quoted upstream titles keep the quotation intact with the number appended.Phase 2, and the three files that keep the old form on purpose
upstream #NNNpassed the gate but names a relationship rather than a repository - and this fork is itself upstream to anyone who forks it. Converted toconfluentinc#NNN, keeping the spaced form where the word carries a real distinction. The ledger offered the fork mirror as an alternative; phase 1 had already standardised these same files onconfluentinc#NNN, and mixing the two would leave a mirror number and an upstream number describing one bug in adjacent sentences.Three files keep it deliberately, each for a different reason:
AGENTS.mdstates the rule by showing the rejected form. Rewriting the example would edit the rule it documents - the trap the ledger records three previous sweeps falling into. It survives the gate because it sits in a code span, not because anything protects it.issue-ref-gate.test.jsis fixtures; the tightening flips them to expect a flag.upstream-pr-analysis.adocis inEXEMPT_PATHSand written entirely in upstream terms.The tightening, which has to move as one piece
stripQualified()drops theupstreamalternation.allows upstream prosebecomesflags upstream prose. This is the part that matters: without it, re-adding the alternation would pass its own test suite.still flags trailing items in a listnow expects#233too - it used to document that the leadingupstreamqualified the first item.formatFailure()stops telling authors the form "still passes", and the assertion that pinned that wording is updated to pin the new state rather than deleted.Ledger
docs/inflight/next-qualify-remaining-refs.mdis deleted - phases 1, 2 and the tightening were everything it tracked. Its dangling reference inAGENTS.mdis replaced with a line saying there is no backlog, so anything found later is drift rather than leftovers.Also fixed
docs/inflight/release-0.6.0.0.mdsaid the retry-delay bug was "never fixed" upstream. It was -confluentinc#864, merged. Verified that PR touchesREADME.adocand nothing else, while the README is generated fromCoreApp.java, so upstream's source still reads0.5and the patch reverts on the next regeneration. That is exactly how this fork inherited the patch and lost it.Note for reviewers
Three merges from master landed during this branch's life, one of which (#218) deleted a file this branch had edited, and two of which changed the gate and
AGENTS.mddirectly. All resolved; where master carried a newer fact - theOffsetEncodinghazard being fixed in #217 - master's version was taken wholesale.claude-reviewfails because this PR edits.github/workflows/claude-code-review.yml(7 comment-only references), which trips the action's workflow-validation guard: it will not review a PR that rewrites its own reviewer. That is the guard working. A manual@claude reviewruns via a different workflow file.Merge strategy: suggest re-cutting rather than rebase-merging. The branch contains an in-flight note that was created, amended twice and then deleted once the gate fix made it moot, plus three merge commits. Atomic commits along the four numbered lines above would bisect and revert cleanly.
Verification
upstream #NNNremaining outside the three intentional files, confirmed case-insensitively - the first pass was case-sensitive and missed# Upstream #857 familybin/check-issue-refs.shclean across all 87 changed fileschangelog-ref-gate.test.jsgreenbin/check-copyright-headers.sh- 0 violations across 233 filestest-compilecleanParallelConsumerOptionsTest+ProducerManagerTest(the@Tagrename) - 5 tests passChecklist
AGENTS.mdrecords the tightened rule and the link-text style; the two guides teaching the superseded convention are corrected; the in-flight ledger is closedissue-ref-gate.test.js, including new fixtures for all three link forms and the flippedupstreamexpectations