Repository navigation
docs(plan): record the package-rename decision, its evidence and the user-facing docs - #277
Conversation
Whether to rename io.confluent.parallelconsumer.* to bz.stub.parallelconsumer.* is an open decision that expires when 0.6.0.0 ships: nothing is published under the fork's groupId yet, so today the rename costs downstream users nothing, and the moment v6 is on Central it costs every adopter a second migration. There is no third moment, so the decision is gated on the release rather than parked in the backlog. This commit records the decision and the evidence behind it. It does not perform the rename, and nothing in the tree is renamed by it. What the investigation established, so nobody redoes it: - No wire-format exposure. Offset metadata is a magic byte plus a bitset or run-length payload plus base64, and OffsetEncoding dispatches on the byte, so no class name reaches the wire. There is no Serializable, no serialVersionUID and no reflective class lookup in main source. Metric names, MDC keys and thread names are hand-written literals, not package-derived. This was the main risk and it is closed. - The work is the copyright-provenance model, not the sed. bin/check-copyright-headers.sh resolves provenance by exact path match against the fork-point listing, so moving the package directories makes every upstream-derived file look fork-original and its retained Confluent header an error. Measured by performing the rename in a throwaway clone rather than predicting it: 197 violations, against a clean baseline today. - Three mechanisms fail green rather than red. Two escaped-regex occurrences in bin/ci-mutation-test.sh (plus one in bin/lib/quarantine-common.sh) are invisible both to a find-and-replace and to the habitual verification sweep, and a stale one makes the mutation lane exit 0 while scoring zero mutants. An ArchUnit rule and six @AnalyzeClasses declarations pin package strings and pass vacuously when stale. The plan therefore makes two verification conditions non-negotiable: grep with a pattern that can see the escaped form, and assert the mutation lane scores mutants rather than accepting the tick. Apache 2.0 permits the rename outright (section 4) and grants no trademark rights (section 6), so moving off io.confluent reduces exposure. The trap is reading that as licence to drop attribution: the code stays a derivative work, so the Confluent headers and NOTICE stay regardless, and section 4(c) adds a modifications line to every file the rename touches. README changes, which are true today and independent of the decision: the Upgrading section gained a "From 0.5 to 0.6" subsection documenting the Maven groupId change that 0.6.0.0 already makes, filling a real gap (it previously covered only 0.4 to 0.5), and the drop-in claim near the top now says precisely what is unchanged and names the single edit a user makes. Both are written in the template and regenerated into README.adoc. The README deliberately says nothing about the package rename, because it is not decided and the README is a published artefact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
Checked-in IDE run configurations are a maintenance surface with no consumers - they only work in one IDE, they are invisible to CI, and nothing tells you when they rot. This one had already rotted: it pointed at `io.confluent.parallalconsumer.examples.core.*`, misspelled, two lines above a correctly spelled reference in the same file. It has presumably been broken for as long as it has existed and nobody noticed, which is the argument against the whole category. It is also a trap for the package rename: a find-and-replace on the correct spelling skips the misspelled line silently, leaving a dead reference behind while the sweep reports itself complete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
|
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 42s —— View job Claude finished reviewing this PR
|
✅ 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). |
Second of the checked-in run configurations to go, for the same reason as All_examples.xml: they work in one IDE, CI never executes them, and nothing reports when they rot. This one pinned `io.confluent.parallelconsumer.internal.ProducerManagerTest` by fully-qualified name in an XML attribute, so it was also a silent target for the package rename - a config file the compiler cannot check, holding a class name that a Java-scoped sweep would never visit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
|
Claude finished @astubbs's task in 4m 32s —— View job Claude finished reviewing this PR
SummaryThis is a documentation-only PR (no source/behavior changes). I fact-checked essentially every concrete claim in the new 377-line plan doc against the actual repository — file paths, line numbers ( Two issues survived verification and were posted as inline PR review comments:
No blocking correctness issues in the docs otherwise; both findings are worth a quick fix before merge but neither undermines the go/no-go decision itself. |
…ecided The README asserted twice that imports are unaffected - "your imports and your code are unaffected" in the intro, and "no import or source edit is needed" in the upgrade steps. Both stop being true the moment the rename lands in 0.6.0.0, and both were the most load-bearing user-facing claims in the document. Worse, the previous pass strengthened them. Written while the rename was still an open question, it made the drop-in promise more emphatic than the text it replaced, immediately before the decision that invalidates it. The intro now separates what actually is drop-in - behaviour, a source-compatible API, and an unchanged offset format, so a consumer group upgrades in place - from the two identifiers that change, which is the honest split. The Upgrading section gives both steps: the groupId, then a find-and-replace across imports, with a worked example. This makes the docs deliberately ahead of the code. The inflight note and the release blocker both now record that coupling: if the rename slips out of v6, this README has to be reverted in the same breath rather than left telling users to import a namespace that does not exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
|
Claude finished @astubbs's task in 2m 14s —— View job Claude finished reviewing this PR
SummaryThis synchronize added one new commit, Both previously-flagged issues are still present, unchanged, at the same lines — this commit didn't touch them:
New finding from The No blocking correctness issues beyond what's flagged; this remains a well-evidenced, mostly self-consistent set of docs, and the one new issue is a natural consequence of a genuine framing change made partway through the PR rather than a fresh factual error. |
… fix Two measured findings the plan predated. Commit shape: the move goes in its own commit. Squashing does not merely lose a rename, it invents four, pairing the five per-module ArchUnit test files into a cross-module cycle that a bare R-count reads as healthy. No threshold can separate them - they score R071-R073 against their own true former selves, the same band as against their siblings - and -B was byte-identical to omitting it while --find-copies-harder made it worse. The stronger reason is independent of the measurement: git's rename detection cannot be configured from the repository. There is no .gitattributes mechanism for it, so any "set this flag" answer holds on one clone and silently does not on CI or a contributor's machine. The default has to be correct. The merge hazard is separate and worse, and the plan did not have it at all. Commit shape is irrelevant to merges, which resolve over the tree delta. What matters is whether the other branch has been renamed: merging renamed master into one that has not exits 0 with no conflicts and puts one module's edit into another module's file. Nothing catches it - not merge.directoryRenames, not renameLimit, not a committed .gitattributes. Running the script on every branch first is the only defence there is, so the plan now says so in those terms rather than as advice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
|
Claude finished @astubbs's task in 5m 40s —— View job Claude finished reviewing this PR
SummaryThis synchronize adds one new commit, I ran the
Net: the new commit itself ( |
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
|
Claude finished @astubbs's task in 7m 39s —— View job Claude finished reviewing this PR
SummaryThe new commit since the last review pass,
No new defects found in I then ran the full
Also still present and unaddressed by Net: this remains a well-evidenced set of docs, and the recurring theme across every review pass on this PR is the same one — the "Decided" reframing introduced partway through ( |
The rename this branch argued for has landed on master. That turns the branch from a proposal into a record, and every conflict here is the same question: which side describes the finished thing better. README_TEMPLATE.adoc - this branch's prose wins, on merit rather than as a tie-break. Master's version tells a reader that imports move; this one also tells them the committed offset format is unchanged, so an existing consumer group upgrades in place with no reset and no offset migration. For a Kafka library that is the single most important migration fact, and master's wording omits it entirely. Master's two additions are folded in rather than dropped: the tree-wide sed with its BSD/macOS note, and the note that the shared internal utilities landed under bz.stub.parallelconsumer.internal.utils. Every old-spelling mention now sits inside a freeze region. This branch predates that mechanism, so its text was unprotected: the upgrade section reuses master's freeze-begin(upgrade-instructions), and the intro paragraph - which names the old namespace and had no equivalent on master - gets its own freeze-begin(fork-summary). --verify-only reports both regions and all twelve lines they hold, so the exemptions are audited rather than assumed. One sentence was worded to keep a tripwire alive. rename-packages.sh pins the corrected drop-in claim by matching "Java packages *move*"; this branch's rewording matched neither the false claim nor its correction, and the self-test failed exactly as designed - a guard that matches nothing is a dead guard. The intro now carries the pinned phrase without losing anything it said. README.adoc is generated, so it was regenerated from the template rather than hand-merged, and checked against the template rather than against maven's exit code. docs/inflight/branch-package-rename.md - both accounts combined into one entry, as that file's own header instructs. The decision record from this branch, the legacy-token finding and the asciidoc-tag warning from master, and each of the three "do not trust the sweep" traps annotated with where it ended up: all three were real and all three are handled on master. docs/inflight/release-0600-blockers.md - master's documentation-data bullet kept. This branch's rename blocker is resolved, but not empty: the 0.6.0.0 changelog section is regenerated from the commit log at release time, and generation reads commits, so it can silently drop the corrected claim. The bullet now states that release-day check instead of a landed blocker. docs/plans/2026-08-11-001-refactor-package-rename-plan.md - kept, and its status moved from "decision open, nothing performed" to executed. Master's bin/rename-packages.sh cites this document and it does not exist on master, so the merge closes a dangling reference. .idea/runConfigurations - this branch deleted All_examples.xml and _Tag__transactions__.xml rather than renaming them, and the deletion stands. It was a deliberate, argued change (IDE-only, never run by CI, and All_examples.xml had carried a misspelt package for years without anyone noticing), and a merge must not silently resurrect what a branch deliberately removed. Master's rename of those two files is superseded, not lost - the misspelling it normalised is already recorded in the tool's header and its self-test fixture. Gates, all green: rename-packages.sh --verify-only 0 non-excluded matches; test-rename-packages.sh 119 assertions, all pass; check-docs-data.sh 39 files valid; check-copyright-headers.sh 234 files, 0 violations. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015pmm1oZoLeX9hev9Snqv5P
🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its |
…iting the record The plan predates the rename it proposed; the rename has since landed on master (#294), and four passages went stale rather than wrong-at-the-time. Each is corrected in place with a dated note rather than edited silently, because a dated plan document is a record and rewriting it makes it say something it did not say (the rule this plan states for itself, R5). - s8's pre-drafted sed one-liner is BSD/macOS-only: `sed -i ''` passes -i with no attached suffix, so GNU sed takes '' as the script and the s/.../.../g expression as a filename - a Linux reader who pasted it would get no substitution at all. The draft was superseded before it was ever pasted; what shipped in README_TEMPLATE.adoc already uses the GNU form with an explicit BSD note. The broken block is kept as the record and marked do-not-paste. - s9 was titled for a decision that has since been taken, and its two bullets describe a README that no longer exists. Marked superseded, with a correction naming what the README says now and why the constraint was lifted rather than violated. - R5's IntelliJ bullet still told a future session to edit two run configurations this same PR deletes. Recorded as resolved by deletion, keeping the reason they were rename traps. - Section numbering skipped 4.4. Nothing was cut - 4.5 and 4.6 were mis-numbered when they were added - so they are renumbered 4.4 and 4.5, and the header's cross-reference follows. Gates run locally: bin/rename-packages.sh --verify-only (0 non-excluded matches), bin/test-rename-packages.sh (119 assertions), bin/check-docs-data.sh, bin/check-copyright-headers.sh, bin/todo-index.sh --check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015pmm1oZoLeX9hev9Snqv5P
…remediation #263 was squash-merged, so GitHub retargeted this PR onto master and the content this branch already carried came back as three add/add and content conflicts - the squash gave the same documents a second, unrelated history. master also gained #260 and #277. All three conflicts are the same shape: master's copy is #263's merged state, and this branch's copy is that state plus the corrections made here afterwards. Verified rather than assumed - master's audit is byte-identical to #263's tip, and nothing was lost on master. - inactive-tests-audit-2026-08-08.md: kept this branch's copy. master's is the earlier draft; this one carries the "Corrected 2026-08-08" pass (the nine restated claims, the §4 rewrite, the disposition of all ten deleted stubs). - refactoring.md: kept this branch's copy. Taking "both sides" would have been wrong here - master still lists the three `@Timeout(60000L)` annotations as work, which this branch deliberately moved to "Not listed as work" because #206 owns them, recording that `@Timeout(60)` would have been wrong (two of those tests wait 45s and 50s internally). master also still says the OOM diagnostics are unsalvaged; they are salvaged. - inflight/branch-stale-and-diagnostic.md: same - master's copy predates the OOM salvage this branch records as done. Verified on Temurin 17 after the merge, not deferred to CI: full reactor test-compile clean, core unit suite 338 tests 0 failures under -Pci (up 10, from #260's new KafkaTestUtils and ParallelEoSStreamProcessor tests), and all four repo gates pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d-waits Brings in #264 (now merged), which was this PR's declared dependency, plus #277, #260 and #263. One conflict, in docs/todo-index.md - a generated file. Master's bin/todo-index.sh changed the header format (the marker count moved out of the prose) while this branch still carried the old shape, so both sides had edited the same generated line. Resolved by regenerating from the merged tree rather than picking a side: 84 markers, and `bin/todo-index.sh --check` reports it current. Master did not touch BlockedThreadAsserter or junit-platform.properties in these four commits, so this PR's two central changes had no semantic collision to resolve. Verified after merging: junit-platform.properties still deleted, core's pom still carries the surefire/failsafe configurationParameters, and the asserter rewrite plus its negative-control test are intact. Local verification: core 354 tests and vertx 18, 0 failures, 0 errors. bin/check-quarantine-registry.sh and bin/check-copyright-headers.sh both clean.
…298) Two merged PRs, #264 and #277, carry no LGTM in any form - no review, no comment. The requirement is that the owner reviews everything that goes in, and the gap was not a lapse of intent but of visibility: "have I read this one myself yet?" is a thing to carry across a dozen open PRs, and carrying it does not work. So it becomes a check with its own name. `bin/check-human-lgtm.sh` asserts one thing: the owner has left a REVIEW whose body contains "lgtm", any case, anywhere in the body. The reviews endpoint is the rule rather than an implementation detail - "lgtm" typed into the ordinary comment box is not the deliberate act being asked for - and the endpoint gives that distinction for free. The matching rule was settled by looking rather than by taste. All 50 owner LGTMs this repo has received, across 38 PRs from #63 to #292, are COMMENTED reviews; 49 are lower-case `lgtm` and one, on #84, is `Lgtm`. Four carry a trailing clause, one of them ending in a question mark. A case-sensitive or whole-word rule would have gone red on real history, so both of those real spellings are pinned as cases. NOT HEAD-SENSITIVE, deliberately: an LGTM on any commit counts for the whole PR, permanently, because the owner only stamps a PR once it is near merge. That is why review STATE is not consulted at all - the master ruleset sets dismiss_stale_reviews_on_push, so reading state would silently un-stamp a PR the owner had already stamped. A SEPARATE JOB, NOT A SECOND STEP IN claude-review, and that is the substance of this change rather than a packaging choice. Two checks say WHICH half is missing straight from the checks list - "claude-review" red means no automated review, "review: human LGTM" red means the owner has not read it yet - and one combined check is red either way, which is exactly the question the human half exists to answer. It also leaves claude-review byte-identical to master: no rename, so no transitional duplicate job and no ordered ruleset swap, and a required check matched by name cannot be left pointing at nothing. Making the new context required is additive, once it has merged and is reporting. NO BOT EXEMPTION, unlike the automated half. A Dependabot PR does not need an AUTOMATED review, but it is still a change going in. Having no guard also means there is no job to skip - and a skipped job satisfies a required check, so a guard keyed on github.event.sender would have let a human PR synchronized by a bot go green having asserted nothing. The checker is 15 lines. An earlier revision was 593, with 855 lines of self-test, parsing code fences, blockquotes, negation forms, glued repeats, CRLF and typographic apostrophes - defences against an attacker who is also the only person the check protects. Filtering the reviews with jq on .user.login before any text is read removed both the attack and the unguessable per-run marker token it needed, since a stranger's body can no longer be attributed to the owner. Verified against real data, not only fixtures: #206 reads LGTM-present, #298 and #299 read absent. The self-test carries a negative control proving the check can fail at all.

Description
depends on #280
The merge-order condition this PR carried has since been met. Its README documents the new
namespace, so it could not reach master until the packages had actually moved. That needed
#280 (the tooling) and then the rename executed with it — both have
landed: the tooling merged, and the rename went in on master as #294. Master
now ships
bz.stub.parallelconsumer.*, this branch has been merged with it, and the orderingwarning that used to sit here no longer applies.
Records the decision to rename the Java packages in 0.6.0.0, and writes the user-facing docs for
it. Performs no rename itself — the code change landed separately, driven by the script.
Java packages were
io.confluent.parallelconsumer.*while the Maven coordinates had already movedto
bz.stub.parallelconsumer, so the published artifact and every import disagreed. Decided: theyget aligned in 0.6.0.0 — and they now are.
It has to happen before v6 ships. Nothing has ever been published under the fork's
bz.stub.parallelconsumergroupId — no tags, nothing on Central — so no downstream code imports ourpackages yet and the change costs users nothing today. After v6 it asks everyone who adopted the
fork to migrate a second time. There is no third moment.
What's here
docs/plans/2026-08-11-001-refactor-package-rename-plan.md— the full task inventory,risk-ordered by how quietly each item fails rather than by effort, with
file:linereferencesand re-measurement commands rather than bare counts. It now carries dated corrections where
execution falsified it: §8's pre-drafted
sedone-liner was BSD/macOS-only and was supersededrather than pasted (the shipped README uses the GNU form), §9's pre-decision constraint has
expired, and R5's IntelliJ bullet was resolved by deleting the files rather than editing them.
The corrections are additive and dated rather than rewrites, because a dated plan document is a
record — the rule the plan states for itself in R5.
docs/inflight/branch-package-rename.md— the decision, the traps, and the settled facts, forany branch or session that needs to inherit them.
docs/inflight/release-0600-blockers.md, recording that this must land beforethe release.
=== From 0.5 to 0.6upgrading section givingboth steps — the
groupId, then a find-and-replace across imports with a worked example — and anintro that separates what genuinely is drop-in from what changes.
The docs were written ahead of the code, and the code has since caught up
These README changes were written while the source still said
io.confluent.parallelconsumer.*,deliberately, because 0.6.0.0 is unreleased and this is what it would ship. The coupling was
recorded in both the inflight note and the release blocker, so that a rename slipping out of v6
would have taken this README with it. That risk is now discharged: the rename landed on master in
#294, and the README describes what the code actually does.
Two claims the previous state got wrong and this corrects: the intro said "your imports and your
code are unaffected", and the upgrade steps said "no import or source edit is needed". Both were
written while the rename was still an open question, and both would have shipped as false.
The three findings that matter
No wire-format exposure. Offset metadata is magic-byte plus bitset/run-length plus base64; no
class name reaches the wire. A rename cannot break offset compatibility, and a consumer group still
upgrades in place. This was the main risk and it is closed.
A clean
grep -rn "io\.confluent"is not evidence of completeness. Three files encode thepackage as an escaped regex (
io\.confluent\.parallelconsumer\.), invisible both to afind-and-replace and to that verification sweep. Worst case is
bin/ci-mutation-test.sh: stale, itmatches nothing, prints
PIT: no core main-source classes changed - nothing to mutate, skippingand exits 0 — the mutation gate reports green forever while scoring zero mutants. An ArchUnit
rule that pins a fully-qualified class name as a string goes vacuous the same way, without
failing.
The real work is not the
sed.bin/check-copyright-headers.shdecides provenance by exactpath match against the fork-point listing, so moving the package directories makes every
upstream-derived file look fork-original and its retained Confluent header an error — 197
violations, measured by performing the rename in a throwaway clone rather than predicting it.
Redesigning that provenance model is the engineering; 121 files then needing a
Modifications Copyrightline is bookkeeping on top.Also here: two deleted IDE run configurations
All_examples.xmlpointed atio.confluent.parall**a**lconsumer— misspelled, two lines above acorrectly spelled reference in the same file, broken for as long as it existed.
_Tag__transactions__.xmlpinned a test class by fully-qualified name in an XML attribute. Both arerename traps: the compiler cannot check either, and a find-and-replace on the correct spelling skips
the misspelled one silently while reporting itself complete.
Checklist
N/A: documentation plus two deleted IDE configs; no product behaviourchanges. The measurements it records were obtained by performing the rename in a throwaway
clone and running the real gates against the result.
N/A: no workflow, runner or CI logicis changed here. The plan documents CI scripts a future rename must update, and changes none
of them.