Repository navigation
docs: give the three shouting docs the same lowercase names as everything else - #270
Merged
Merged
Conversation
…hing else docs/ carried two naming conventions at once. Durable reference prose is lowercase - refactoring.md, and every entry under docs/inflight/ - while three files shouted in ALL_CAPS_WITH_UNDERSCORES for no reason anyone recorded. Renames them to match the majority: docs/QUARANTINED_TESTS.md -> docs/quarantined-tests.md docs/TODO_INDEX.md -> docs/todo-index.md docs/SELF_HOSTED_RUNNER.md -> docs/self-hosted-runner.md Root-level names are deliberately untouched. AGENTS.md, CONCEPTS.md, README.adoc, CHANGELOG.adoc and NOTICE are ecosystem conventions that tools and humans expect in caps - renaming those would break a convention rather than fix an inconsistency. Only this repository's own idiosyncrasy is in scope. The rename is the small half. All three paths are load-bearing: two are enforced by CI gates and one is generated, so every reference had to move with them. Swept 26 files - nine workflows, eight shell scripts, four core test classes, both AGENTS.md files including the "Where things live" table, refactoring.md, a solutions write-up, two plans, and actionlint.yaml. The CHANGELOG reference sits in the unreleased 0.6.0.0 section, which the release regenerates anyway, and it was a link that would now 404 - a factual correction, the one edit the changelog rules permit. Verified rather than assumed, because a missed reference here fails CI rather than failing loudly at edit time: zero remaining matches for the old names anywhere in the tree; check-quarantine-registry.sh consistent; todo-index.sh --check up to date, so the generator and its output agree on the new name; check-quarantine-owners.sh clean; check-issue-refs.sh clean across all changed files; and the 31 tests in QuarantineRegistryScriptTest, QuarantinedAnnotationContractTest and CheckQuarantineOwnersScriptTest green - those exercise the scripts end to end against the renamed registry. Refs #240 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015GyZmjA6k1Fi9YQWavenke
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
Claude finished @astubbs's task in 2m 14s —— View job Claude's Review
Verdict: LGTM. This is a clean, purely mechanical rename with no behavioral changes. What I checked directly (not just trusted from the description):
No issues found. Nothing to fix. |
✅ 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
added a commit
that referenced
this pull request
Aug 10, 2026
The lowercase doc rename (#270) deliberately excluded this file, because it exists only on this branch and not on master. Merging that branch in therefore left it as the one place still naming docs/TODO_INDEX.md and docs/QUARANTINED_TESTS.md, both of which now 404. Refs #240 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015GyZmjA6k1Fi9YQWavenke
astubbs
added a commit
that referenced
this pull request
Aug 11, 2026
…w forfeit The reviewer could not lint workflow PRs or run the OSS Index guard scripts, because granting them means editing claude-code-review.yml - which makes it differ from the default-branch copy, so claude-code-action skips and REPORTS THAT SKIP AS SUCCESS. Whichever PR adds a script therefore forfeits its own review, which is why these sat ungranted. Landing them here, now, spends that forfeit on a review nobody needs: this PR's review has already completed on its current head with zero open threads. Doing it in a separate "non-workflow PR" as the ledger previously advised would cost another PR and another round-trip for a four-line allowlist change, and that advice assumed the forfeit was always expensive. It is not - it is free exactly once a PR is done being reviewed. Both spellings of each script path, per bin/AGENTS.md: a bare `bin/x.sh:*` grant does not match `./bin/x.sh`. The consequence is recorded rather than left implicit: any further review on this PR is a no-op, and its green review tick must not be read as "reviewed" - the same silent-success class this PR's own guard exists to catch. Also merges master, resolving AGENTS.md where this branch's new Dependency Audit entry met the docs/ lowercase renames from #270. Swept the branch for stale UPPERCASE doc references introduced by those renames: none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m
astubbs
added a commit
that referenced
this pull request
Aug 11, 2026
The merge of master was clean and wrong. git resolves renames and restructures without a conflict, so four citations on this branch kept pointing at files, line numbers and a sentence that no longer exist, and nothing in CI reads a prose citation. All four were silent. - `docs/TODO_INDEX.md` -> `docs/todo-index.md`. #270 lowercased the three shouting docs. This one is not cosmetic: it sits in the R5 sweep inventory as an actionable checklist item, so a future session executing the rename would have been sent to a path that does not exist, next to a correct instruction to regenerate it. - `pom.xml:723-724,750` -> `:743-744,770`. The surefire excludes and the failsafe include moved 20 lines when #259 added the central-publishing pluginManagement block. §6's claim - that the unit/integration split keys on the `integrationTest*` path segment and so survives the package move - was still true, but pointed at the compiler `<source>` element and an unrelated comment. Added what the lines are, so the next drift is detectable by reading rather than by counting. - The `=== Improvements` changelog quote, in both §8 and the release blocker. #276 deleted the exact sentence both quoted ("Java package names ... are unchanged, so imports are unaffected") while renaming the project, for a reason unrelated to this branch. The underlying warning survives - the `== 0.6.0.0` section still asserts the groupId is the only required change, in its opening paragraph and in the `=== Breaking` bullet - so both now name those two passages instead, and §8 records that the third is already gone so nobody hunts for it. - The "links to confluentinc that legitimately remain" item named `AGENTS.md` as one file. #272 split it into seven topic docs and three of them carry such links. Replaced the fixed filename list with the grep that produces it, since the same split will happen again. Verified rather than assumed, because every one of these fails silently: zero matches for the three old SCREAMING_CASE names anywhere outside .git; all 40 repo-relative backtick path citations in the branch's three docs resolve, the single exception being `bin/rename-packages.sh`, a deliberate forward reference to work in flight elsewhere; all 7 relative markdown links and all 8 adoc `link:` targets resolve; and the 24 `path:line` citations were checked by reading the cited line and confirming it still says what the prose claims. check-issue-refs, check-copyright-headers, todo-index --check and check-quarantine-registry are all green, and README.adoc regenerates byte-identical from the template, so master's branding rename merged into the generated file correctly rather than being hand-patched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
astubbs
added a commit
that referenced
this pull request
Aug 12, 2026
…he sweep tool a mode that could have caught it (#258) * docs(upstream-map): track the 28 issues closed by the 2023 admin sweep Upstream ran two bulk clearouts before going quiet, and neither was a triage: 2023-06-15 `eddyv` closed 35 unmerged PRs with "Closing - Stale." (34 of them ours), and 2023-07-07 `johnbyrnejb` closed 28 issues with "Closing Issue" - every one marked COMPLETED rather than "not planned". That state reason is why the cohort was invisible. GitHub renders `completed` as resolved, so the issues read as done at a glance, and `upstream-sweep.sh` searches `updated:>=last_swept`, so anything last touched in 2023 can never appear in a sweep. None of the 28 were in this manifest; the mirroring that produced the other 78 fork mirrors only ever sampled *open* upstream issues. All 28 were re-read in full - bodies plus all 58 comments - and each verified against fork source rather than trusted from its thread. That mattered: four issue bodies point at PRs that "fix" them (#372->#390, #319->#270, #203->#345, #191->#346) and every one of those PRs was itself swept unmerged. Result: 2 already fixed (#41 offset-scan removal, #319 shutdown CME), 6 partly addressed, 20 fully open. Grouped into work items rather than 28 near-duplicate entries, since several want designing together - per-topic handlers with separate consume/produce types, seek with subscription changes, retry expiry with stall detection. #57 folded into the existing log-noise entry. Swept PR head commits were checked and are all still reachable, so the drafts are recoverable; entries cite branch and SHA, not just the PR number. Upstream-Issue: confluentinc#154 Forwarded: no Applied-Upstream: no * docs(upstream-map): mirror the whole 2023 cohort, not the parts we rate The point of mirroring is a clean, known cutoff - "every issue upstream closed administratively is accounted for here" - not a curated pick of the good ones. A set filtered by our own value judgement is not a cutoff: the next person cannot tell "not mirrored because it was junk" from "not mirrored because it was missed", and that ambiguity is worth far more than the cost of carrying a few items nobody will ever action. Drops the decline recommendations from #53, #199, #246 and #57 in favour of neutral statements of the trade-off, and renames sweep-2023-decline-candidates to sweep-2023-small-items (status wontfix -> none). The facts that informed those recommendations are kept - they are useful to whoever picks the issue up - but they are now framed as input to that person's decision rather than as our verdict delivered in advance. Fork issues can always be closed later on their merits, which leaves a visible record; pruning before mirroring does not. Records the principle in the section header so a later pass does not re-prune. Upstream-Issue: confluentinc#154 Forwarded: no Applied-Upstream: no * docs(upstream-map): link the created fork mirrors back to their entries The 28 mirrors are live as #227-254 (label `upstream-admin-closed`), with confluentinc#41 -> #233 and confluentinc#319 -> #252 created and immediately closed as completed, carrying the code evidence that they are genuinely fixed. Records the fork numbers on each entry so the mapping is queryable rather than re-derived, which is the reason this file exists. Adds `fork.fork_issues` (the plural of the existing `fork_issue`) because several entries deliberately group upstream items that want designing together, and documents it in the schema block. Verified: all 28 mirrors are referenced, none missing, no strays. Also snapshots each upstream title verbatim into its mirror body header, dated 2026-08-07 - the fork titles may drift as these are worked, and the original wording is what makes an old thread findable. Upstream-Issue: confluentinc#154 Forwarded: no Applied-Upstream: no * fix(tooling): give upstream-sweep a mode that can see closures it never saw The default sweep is a window search - `updated:>=$since` - so it structurally cannot see an item whose last activity predates the window, and `last_swept` only moves forward, so the blind spot grows. That is why upstream's 2023 administrative closures went unnoticed for three years: 28 issues closed with "Closing Issue" and 35 unmerged PRs closed as "Closing - Stale.", all marked COMPLETED, none of it triage, none of it visible to us. Adds `--audit`: no time window, asks "which closed upstream items are neither tracked in the manifest nor mirrored in the fork?", and flags days where an implausible number of things closed at once. Run against the real repo it rediscovers both sweeps from scratch and reports zero unaccounted, which is also an end-to-end check that the new fork_issues linkage is correct. Bots are excluded from the PR analysis. Dependabot self-closes superseded bumps in batches that look exactly like a sweep - 2022-08-16, 2022-10-20, 2023-11-03 and 2024-01-25 are all dependabot - and unfiltered they buried the two real sweeps in noise. Filtering them surfaced two human PRs that had been sitting inside those batches (confluentinc#508, confluentinc#650), now recorded for review. The report says outright that a bulk day is not proof of a sweep, because a release triage looks identical from here, and that stateReason COMPLETED is not evidence of a fix - that assumption is what made this cohort invisible. Known remaining hole, recorded not fixed: a PR closed alone on a quiet day is still invisible, since detection keys on bulk. Upstream-Issue: confluentinc#154 Forwarded: no Applied-Upstream: no * fix(tooling): teach the audit about Discussions, a content type we never checked Discussions were invisible to everything: not in the manifest, not mirrored, not in any sweep mode. 74 of them upstream, and nothing we run would ever have mentioned one. They were NOT swept - no bulk closure, and the largest day (2024-04-02, six threads) is all answered=true housekeeping. So the failure mode is different from the 2023 cohort: not administrative closure, just questions nobody answered, in a place no issue search reaches. Handling differs accordingly - these want answering or converting, not mirroring wholesale. The audit now lists zero-reply discussions, excluding release-announcement threads by title: those legitimately have no replies, and counting them as neglect would put 9 false positives at the top of the report. Two finds justify the check on their own. Discussion 542 (zero replies) is a field report of a transactional consumer stuck in rebalance because it cannot acquire the produce lock, with the reporter suspecting the revoke-time flush - the same lock lifecycle as bug-producing-lock-double-release, and they raised the timeout without effect, which is evidence about the mechanism rather than the duration. Discussion 883 (zero replies) is the same complaint as fork mirror #187 but with a runnable reproducer, which #187 lacks. Upstream-Issue: confluentinc#154 Forwarded: no Applied-Upstream: no * docs(inflight): record the open obligation to account for every upstream item Two notes, both about what is NOT done. Discussions are not a queue to be converted. There is no pipeline turning threads into issues, and most should never become one. The rule is a judgement: if reading a discussion makes us think there is an issue, we raise one - a normal fork issue on its own merits, citing the discussion as where it came from. It exists because we believe the problem is real, not because a thread existed. That also keeps `upstream-mirror` meaning exactly "an upstream issue we carry", which is what makes the 2023 cutoff verifiable. The wider obligation is now written down: every upstream issue, PR and discussion must be accounted for - carried, declined, or genuinely resolved upstream - not sampled. The 2023 cohort was found only because it was a *bulk* event; `--audit` keys on bulk closures and zero-reply threads, and both are proxies that miss whole shapes of problem (a PR closed alone on a quiet day, a discussion with one dismissive reply, an issue marked COMPLETED with no linked PR). The audit narrows the field; only reading discharges the obligation. Also records what has been ruled out, so it is not re-investigated: wiki disabled, no advisories, all open-milestone issues already mirrored, the orphan branches accounted for (v0.6.x-dev is the lambda-actor work already captured via swept PRs; 0.5.3.x's regression fix IS on master as a908e16), and "upstream pushed today" being a pushed_at artefact rather than new activity. Left open: project boards need a read:project scope we do not have, and 169 forks are unexamined. * fix(upstream-map): use a group the schema actually allows Two new entries used `group: batching-ordering`, which is not in the GROUPS allow-list in scripts/upstream-map.py, so `upstream-map.py validate` failed on them. Caught in review on #258. My mistake was validating the wrong way: I checked the file with an ad-hoc yaml.safe_load plus a duplicate-id check, which passes happily on a group the schema rejects, instead of running the project's own validator that AGENTS.md tells contributors to run before committing manifest changes. Reassigned both to `features` rather than widening GROUPS. The rest of this cohort's feature entries already use `features`, so this keeps the cohort internally consistent, and the ordering/batching distinction is not lost - it lives where it belongs, on the issues themselves via the `area/batching-ordering` label (#244, #236). Adding a vocabulary term to a shared schema as a side effect of a mirroring PR is the larger, less reversible change. Also drops the redundant 345 from sweep-2023-broker-disconnect-commit's `related` list, where it already appears in `prs`. Verified: `upstream-map.py validate` reports OK on 27 entries, and `refs` and `table` both still render. * docs(upstream): graduate the durable parts of the sweep record out of inflight The three inflight notes stay - they hold undischarged work - but they were also carrying permanent content, and inflight is transient by its own charter. Moved to durable homes, each citing the manifest rather than restating it: - docs/upstream.md gains the upstream-admin-closed cohort note (with the "do not trust 2023-era closure states" rule), the discussions non-mirroring policy decided 2026-08-07, documentation of --audit and its known blind spots, and the ruled-out upstream surfaces (wiki, advisories, milestones, orphan branches, pushed_at). - docs/solutions/ gains the transferable lesson: a closure state is a rendering choice not a triage, "fixed by #N" must be checked against the merge bit, windowed watchers need a no-window audit, and bots must be filtered before hunting bulk events. - upstream-pr-analysis.adoc is corrected: it claimed issues were never bulk-closed, which the 2023-07-07 sweep disproves, and the PR sweep is now confirmed rather than suspected. The inflight notes now hold only open work, pointing at the new homes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(upstream): name the analysis doc's role (the plan) beside the manifest (the state) docs/upstream.md's opening enumerated everything it owns except the editorial analysis, and neither source-of-truth reference was a clickable link. Now the two roles are named explicitly: the .adoc is the plan, the manifest is the state tracker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(upstream): make the audit's discussion query actually paginate, and correct the sweep cohort Address PR review feedback (#258). - upstream-sweep.sh: `gh --paginate` only substitutes a variable named `endCursor`; calling it `$cursor` meant the cursor was never fed back and gh re-requested page 1 forever (reproduced: 469 identical pages at first:2). It only looked healthy because 74 discussions fit in one page of 100. The failure was also swallowed by `2>/dev/null || true`, which would have printed a clean "(none)" for a broken query - the one answer this mode must never invent. Fail loudly instead. - upstream-map.yaml: the sweep cohort listed 36 PRs against a documented count of 35. Upstream closed exactly 35 unmerged PRs on 2023-06-15; confluentinc#66 (closed 2022-10-19) was carried in by mistake. Listing a ref here marks it accounted for, so it was hiding an unrelated PR from every future audit. Recorded PR 258 and moved the entry to `pr-open` per the lifecycle rule. - next-upstream-coverage-completeness.md: dropped the cached counts - inflight notes must not record what a command can answer, and the "~100 partially mirrored" line already contradicted docs/upstream.md, which says all 78 open upstream issues are mirrored with backlinks. Kept only what the audit cannot say: what nobody has read yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(agents): let the manifest cache frozen upstream-issue facts, and say why The rule said "the manifest tracks upstream PRs only", so mirroring the 2023 sweep cohort into it read as a violation. The rule is what is out of date. An archived upstream's closed issue numbers and closure events are frozen. Caching them locally is a read-path optimisation, not a second tracker: grepping one file is instant, while the same answer from the mirrors costs dozens of API round-trips and burns rate limit shared across agents. The usual objection to duplication - the copy silently diverges - needs a source that can still move, and this one cannot. So the boundary is redrawn where it actually bites: the mirror owns an issue's *live state* and remains the only place you update it; the manifest may cache what is frozen (number, cohort, owning mirror). Cache what is frozen; never mirror what is moving. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(upstream): put the manifest-caching rule in the doc that owns it The previous commit wrote the rule into AGENTS.md, which is the one place it does not belong. AGENTS.md's own preamble calls itself a router - "if it only matters once you are already in a topic... it goes in that topic's doc" - and docs/upstream.md line 7 already claims ownership explicitly: "AGENTS.md carries only the pointer and the one-line rule that manifest upkeep is the agent's job." The rule was in fact already stated twice before this branch (AGENTS.md:442 and docs/upstream.md:23), so growing the AGENTS.md copy deepened a fragmentation that was already there. Now stated once, where an agent editing the manifest will actually be looking, with AGENTS.md left holding the pointer and the one binding rule whose failure is silent (keep the fork side in sync). Net effect on the router: one line shorter than master, despite covering more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(agents): make a pointer say who owns the rule, not just what is nearby Diagnosed from a live miss. A reviewer cited AGENTS.md's "the manifest tracks upstream PRs only", so that section was read and treated as authoritative - while docs/upstream.md had been stating the same rule, and claiming ownership of it, the whole time. The duplicate was then grown in the router rather than fixed at source. Nothing in the reading path revealed the mistake. AGENTS.md stated a complete-looking rule and followed it with a pointer listing four adjacent mechanics ("the manifest schema, the mirrors, the commit trailers and the upstream sweep"), which reads as *further detail*, not as *this is a summary and that doc wins*. The contract was written down - docs/upstream.md:7 - but only on the side nobody enters from, since AGENTS.md is what loads every session. The anti-duplication rule already existed here ("Never state a fact twice"). What was missing is the half that makes it discoverable: a cross-reference must name the owner and say the owner wins, so a reader can tell a stub from the whole rule before editing the copy. Applied to the rule itself and to the upstream pointer that failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2 of 4 tasks
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
docs/carried two naming conventions at once. Durable reference prose is lowercase —refactoring.md, and every entry underdocs/inflight/— while three files shouted inALL_CAPS_WITH_UNDERSCORESfor no reason anyone had recorded. This renames them to match the majority:docs/QUARANTINED_TESTS.mddocs/quarantined-tests.mddocs/TODO_INDEX.mddocs/todo-index.mddocs/SELF_HOSTED_RUNNER.mddocs/self-hosted-runner.mdRoot-level names are deliberately untouched.
AGENTS.md,CONCEPTS.md,README.adoc,CHANGELOG.adocandNOTICEare ecosystem conventions that tools and humans expect in caps — renaming those would break a convention rather than fix an inconsistency. Only this repository's own idiosyncrasy is in scope.The rename is the small half
All three paths are load-bearing: two are enforced by CI gates and one is generated, so every reference had to move with them. The sweep covers 26 files — nine workflows, eight shell scripts, four core test classes, both
AGENTS.mdfiles (including the "Where things live" table),refactoring.md, a solutions write-up, andactionlint.yaml.The
CHANGELOG.adocreference sits in the== 0.6.0.0 (unreleased)section, which the release regenerates anyway, and it was a link that would now 404 — a factual correction, which is the one edit the changelog rules permit. No frozen section is touched.Verified, not assumed
A missed reference here fails CI rather than failing loudly at edit time, so each gate was run directly:
bin/check-quarantine-registry.sh— consistentbin/todo-index.sh --check— up to date, so the generator and its output agree on the new namebin/check-quarantine-owners.sh— cleanbin/check-issue-refs.sh— clean across all changed filesQuarantineRegistryScriptTest,QuarantinedAnnotationContractTestandCheckQuarantineOwnersScriptTest— these exercise the scripts end to end against the renamed registryProvenance
Split out of #269, where it was noticed while naming a new doc. It is mechanical, cross-cutting, and has nothing to do with that PR's subject, so it reviews far more cleanly on its own.
Checklist
N/A, no behaviour change; existing quarantine script tests cover the renamed paths and were run🤖 Generated with Claude Code
https://claude.ai/code/session_015GyZmjA6k1Fi9YQWavenke