Repository navigation
Conversation
… pass The plan as first written would not have compiled. Five reviewers ran in separate contexts; two independently found that KTD6's reactor design is impossible, and the corroboration is what makes it certain rather than an opinion. The defects that mattered: - limitRate is a Flux-only operator and ReactorProcessor.react() hands the user function one publisher per record, so the reactor example could not compile - and the version that does compile makes request(n) a prefetch constant rather than evidence of demand. Each reading now expands into its interval samples so limitRate has something real to throttle. This was the plan's own failure mode one level up: the probe ran on a standalone Flux and the result was generalised to a per-record context where it does not hold. - PC forces the external engines' dispatch pool to a single thread, so the barrier KTD2 mandated universally would deadlock in the vertx and reactor examples on every machine, not just under load. Placement is now per-module, and the failure mode is specified: a broken barrier drives PC into an unbounded retry loop whose symptom reads as "offsets never commit". - The vertx barrier would have proved leg concurrency, not record concurrency, passing on exactly the regression it exists to catch. - ExampleMockConsumers would have landed in the core test-jar, which is signed and published to Maven Central. - Two release workflows exclude examples by a hand-maintained module list, so a sixth module needs both edited. - The metrics test sits in a package surefire excludes, so the stated verification would have reported green without ever running it. Also records the role change these examples are taking on: they demonstrate the library's capabilities rather than demonstrating how to call its API. That is not cosmetic - it makes reader-runnability a requirement, adds an outcome criterion to a Definition of Done that was entirely mechanical, and gives the logistics example a domain UI so a reader watches concurrency happen instead of reading that it did. Three open questions are left open rather than silently settled; the scale one touches a user-settled decision and is not mine to close.
Folding a domain UI into an example module was making both jobs worse. An example is something you read; a demonstration is something you run. Trying to be both meant a tagged README region polluted with UI code, and a scale tension with no good answer: the barrier-forced test wants tens of records, while a persuasive demonstration wants a number not dominated by PC's own startup cost. Splitting them resolves it rather than deferring it. The four module examples stay simple - a readable file, a tagged region, a simulated dependency, logging, one basic test. No UI, no embedded server, no run harness, no second execution mode. The showcase becomes one single demonstration module with its own plan and its own PR. That also closes the open scale question outright: scale belongs to the demonstration module, where nothing constrains it, and the user-settled decision separating basic tests from load tests is untouched. Design seeds recorded so they survive the split: a side-by-side race against a vanilla consumer is the honest form of "wow", because the serial arm is measured in the same run under the same conditions instead of computed as records x latency and labelled "implied" - the credibility problem the adversarial reviewer raised disappears rather than being caveated. One engine, one domain, deep: which async framework PC composes with is academic for that purpose. And it has a predecessor to replace - the README's Illustrative Performance Example is a static chart, where this is the same argument a reader can execute.
OQ2: a meter reading expands into 48 half-hourly settlement periods - the number UK energy settlement actually uses, so limitRate(8) shows six real demand replenishments rather than an invented count. OQ3: leave wiremock on its default thread count. The concern assumed ~3N blocked handler threads; the per-record barrier gate already added for a different reason means only N park, so a barrier of 3 against Jetty's default 10 has headroom. Also records the demonstration module's settled inputs so they survive into its own plan: real broker via Testcontainers, its tests gate CI, basic UI tests only, and it reuses the parcel-logistics domain.
Sample code inverts the usual DRY rule at the boundary of the copied region: the reader copies the tagged block, so anything they must chase into a helper is a failure of the example. This module holds the other side of that line - ConcurrencyObserver, RunSummary, SimulatedService and DemoRecords - so the four industry examples can keep their PC setup inline and duplicated on purpose without handing the duplicate-code check four copies of the instrumentation. RunSummary is built to be hard to misread as a benchmark. It states the partition count and peak in-flight per partition, because "peak in-flight 4" means nothing without knowing what it ran against and concurrency beyond the partition count is the whole claim. It times from the observer's first enter to its last exit rather than app start, since at these record counts PC's own startup is comparable to the processing window. It labels the serial figure implied rather than measured, and it prints no speed-up multiple at all - that ratio is arithmetically determined by min(maxConcurrency, distinct keys), so a bare multiple invites a reading the numbers cannot support. ExampleMockConsumers lives here rather than in parallel-consumer-core's test sources: that test-jar is signed and published to Maven Central, and an examples-only scaffolding class does not belong in a published artifact's surface where removing it later becomes a compatibility question. It also folds a Kafka mock-consumer workaround that was copy-pasted into four example tests into one place, before this branch made it eight. Both release workflows exclude examples from Central by a hand-maintained module list rather than a pattern, so a sixth module needs adding to both or it silently enters the deploy reactor.
The web-gui branch generalised the chaos conductor into a Scenario / ScenarioRunner / ScenarioActions framework, and ShowcaseScenario already consumes it from outside its module with bin/dashboard-demo.sh as the front door. The demonstration module should declare a race scenario and inherit that, not build a run harness. Two of its decisions answer questions this plan had left open. The same scenario is both the demo and the gating test - LOOP to watch, --once where a phase that quietly stopped demonstrating its thing is a run failure - which is the shape 'its tests gate CI' should take. And its rule that a postcondition only holding when the RNG cooperates is a flaky test dressed up as a demo is the same discipline the reviewers applied to this plan's barrier design, arrived at independently. Test-scope execution is settled: the reader invokes a front-door script, so the classpath is an implementation detail they never see.
…a workaround Two behaviour-preserving changes the new examples need underneath them. The core, vertx, reactor and support example modules had no TestConventionsArchTest, so none of the shared conventions was enforced there - including the rule that a test class must be named so surefire collects it. A mis-named test in those modules silently never runs and the suite stays green without it, which is exactly the failure the rule exists to catch and exactly the risk of adding four new test classes to them. The stub is the metrics one verbatim, differing only in the package it scans. The rule was proven to fire before being trusted: a scratch FooChecker with a @test method in the core example failed the new arch test with the rename-it message, then was reverted. An assertion nobody has seen fail is decoration. Separately, four example tests each carried their own copy of the same MockConsumer.groupMetadata() workaround - one Kafka defect duplicated four times, recorded as such in docs/refactoring.md. This branch would have made it eight. They now share ExampleMockConsumers from the support module, and that refactoring.md entry is closed rather than left reading as open work. The extraction preserves each test's differences rather than normalising them: vertx and reactor keep their updateBeginningOffsets call and null serializers, core keeps its String serdes. Only the duplicated stubbing moved. Verified before and after in all four modules, including the metrics integration test against a real broker rather than settling for compile-only.
It printed 'records processed: 12' beside 'records completed: 14'. The second counts user-function invocations, so a retried record is counted per attempt - but read together the two lines invited the reader to conclude two records went missing. Relabelled once here rather than confusing readers in four examples. Found while implementing the first consumer of this class, which is the argument for landing a shared module before its consumers.
…e lookup The core module's existing CoreApp increments a counter - it shows the API compiles, not why anyone would reach for the library. This adds a second example beside it: parcel scan events keyed by consignment, where each scan needs an address lookup from an external service at ~120ms. That shape is the library's case. More partitions cannot fix a slow per-record call, and the ordering requirement is real rather than decorative - a consignment's status must not go backwards, which is what makes ProcessingOrder.KEY the right setting instead of an arbitrary one. Thread.sleep is deliberate here: PC core's value proposition IS giving you a pool to park blocked threads on, so a blocking geocode faithfully models JDBC or a blocking HTTP client. A deterministic failure fraction makes PC's retry behaviour visible rather than hypothetical. The tagged README region covers the options builder and the poll call only. The observer scope and in-flight logging sit in a wrapper outside it, because the reader copies that block and the support module is never published to Maven Central - a snippet referencing it would not compile for them. The test proves the claim rather than the plumbing. A CyclicBarrier sized three - one below the consignment count, so a key parked on retry backoff cannot starve the cohort - forces three scans to be inside the user function at once on a single partition. Shortfalls are recorded and asserted, never awaited bare: a broken barrier would otherwise drive PC into an unbounded retry loop that presents as "offsets never commit". A separate assertion fails if anyone switches this to UNORDERED. No wall-clock value is asserted anywhere. Also adds the support module at compile scope, which U2 gave only to tests.
… the logs
The reactor module's existing example returns Mono.just("something todo").
This adds a second one beside it: half-hourly smart-meter readings, where
each reading expands into its 48 settlement periods - a real day of UK
energy settlement, so the number is domain-true rather than invented.
The multi-element shape is forced, not stylistic. limitRate is a Flux-only
operator and ReactorProcessor hands the user function one publisher per
record, so a single-reading Mono cannot carry it - and wrapping one reading
in a one-element Flux compiles but makes request(n) a prefetch constant
rather than evidence of demand. Expanding a reading into its periods gives
backpressure something real to throttle, and the example states the honest
scope: maxConcurrency bounds readings across meters, limitRate bounds
periods within one reading.
doOnRequest sits upstream of limitRate. Downstream it logs
request(unbounded) forever and demonstrates nothing, which is the single
likeliest way to write a backpressure example that silently teaches nothing.
Both wrong placements were tried and both fail the test - including the
seductive one just below limitRate, which reports request(1) from
concatMap's inner serialisation and looks like tight backpressure.
The barrier sits inside the returned publisher on a boundedElastic step.
PC forces this engine's dispatch pool to one thread, so a barrier in the
user function body would deadlock on every machine, and delayElement
continues on Schedulers.parallel(), sized to the core count, where it would
deadlock on a 2-core runner.
Records an open library defect found on the way: ReactorProcessor completes
a record once per emitted element rather than once per publisher, so a
multi-element Publisher - a documented input - corrupts offset state. See
docs/inflight/bug-reactor-multi-element-publisher.md. Not fixed here: it is
library behaviour needing its own regression test, not a rider on a
docs-shaped change. The example works around it with .count() and says so.
… per record The vertx module's existing example builds one RequestInfo per record. This adds a second one beside it that uses the module for what it is actually good at: each search request fans out three concurrent non-blocking HTTP calls to different fare providers and picks the cheapest. Built on vertxFuture, deliberately not the JStream API the existing example uses - that deque is never cleared on close (confluentinc#912, fix unmerged) and there is standing intent to deprecate and remove it. A new example should not be built on a leaking, doomed API. Three Vert.x 4.5 details the naive version gets wrong. Future.all replaces the deprecated raw-list CompositeFuture.all. expecting() is declared on Future, not HttpRequest, so the check goes on what send() returns - client.get(...).expecting(...).send() does not compile. And Future.all fails fast, so each leg recovers before the join, which is also the domain-correct behaviour: one unreachable provider must not fail the search. Removing that recover does not merely change an assertion - offsets never commit at all. The WebClient is built once and sized to providers x maxConcurrency, because PC's own default assumes one request per record. Forcing the pool to 1 pushes the leg spread from 0ms to 180ms and fires the test, so the comment explaining it is backed by something. The barrier is gated to one arrival per record. Ungated, three legs of a single record trip a barrier of three: verified by mutation, where the cohort assertions passed and only the gating assertion caught it - a test that would otherwise pass on a build dispatching records one at a time. Stub delays are 200/600ms rather than 30/300ms. The arrival spread this measures is dispatch jitter, so it stays in single-digit milliseconds while the bound moves with the leg delay: the margin went from threefold to absolute at a cost well under a second, without weakening what the assertion discriminates. Providers answer a trivial text body rather than JSON: wiremock-jre8's Jackson wins this module's test classpath, and pinning Jackson globally is what broke VertxTest before.
…y measurement KTD7 said configuring both publishPercentiles and publishPercentileHistogram silently discards the quantiles. Measured during U4: on this module's pinned 1.12.13 registry both families are published. The discard behaviour was researched against Micrometer 1.13.15 - a different code path that the pin means we never take. The recommendation is unchanged but the reason now survives measurement: client-side quantiles are per-process and cannot be aggregated into a fleet percentile at all, so they double the series count for an answer nobody can combine.
… shows The metrics module's existing example wires up Micrometer. This adds a second one beside it where the wiring is the means and the signals are the point: e-commerce orders keyed by customer, whose slow step is a warehouse allocation call. Meter choices are the ones that answer an operator's questions rather than the ones that are easy. A LongTaskTimer, because a plain Timer records only on completion and so cannot answer "how many right now" - with its histogram omitted, since default LongTaskTimer buckets start at 120 seconds and are useless for sub-second work. Throughput from the Timer's own _count rather than a redundant Counter. Failures tagged by a bounded outcome and deliberately never by order id, with a comment saying why, because that is the cardinality mistake this example exists to visibly avoid. Stays on the pre-1.13 io.micrometer.prometheus package. The module pins the registry below 1.13 precisely to keep it, and every current tutorial shows the newer one. Binds its own port and stops its server on close. The existing example never stops its HttpServer, which is why 7001 stays bound for the life of the failsafe JVM - forcing this app onto 7001 reproduces the collision. The in-flight assertion is barrier-forced rather than observed: the test thread is the gate's last party, so pc_inflight_records and the LongTaskTimer's activeTasks are read while the cohort is pinned at exactly N. The barrier timeout deliberately exceeds the run timeout so a stall fails rather than silently self-releasing. Two proof-first reds found real problems rather than confirming guesses. Closing the processor deregisters PC's meters, so the scrape must be captured before close - the first run failed on an empty pc_* block. And measurement refuted the plan's claim that percentiles and a histogram cannot coexist; the recommendation stands on aggregability instead, which is true.
… door at them Four new README sections, one beside each module's existing example, each leading with the problem a reader recognises rather than with API mechanics. Every section names which of the module's two examples is which: the existing one answers "how do I call this", the new one answers "why would I reach for this". The front door mattered most. The intro block, some 400 lines above Usage, included the counter demo under "Consume many messages concurrently with a single consumer instance" - so an evaluating reader met the weakest example first and might never reach the rest. The snippet stays, now followed by pointers to the four demonstrations. The existing Scenarios catalogue already argued the case in prose; each new section is now cross-linked to its closest bullet and back, so the prose is a promise and the code is its proof rather than two disconnected answers. The metrics section ships PromQL rather than a dashboard JSON that would rot, and gives the histogram-over-percentiles reason that survives measurement - client-side quantiles are per-process and cannot be aggregated into a fleet percentile - not the discard claim U4 refuted. Fixes a defect that would have shipped four broken examples. The plan and three implementers all believed AsciiDoc concatenates same-named tag regions in file order. This README is not built by Asciidoctor: the plugin's reader latches after the first end:: and discards later regions with the same name, so the first regeneration silently dropped every pollAndProduce, vertxFuture, react and poll call - the whole point of each snippet - while succeeding and looking plausible. Each processing block now has its own tag. Written up in docs/solutions, where the README pipeline previously had no recorded failure modes at all.
Dependency ReviewThe following issues were found:
License Issuesparallel-consumer-examples/parallel-consumer-example-core/pom.xml
parallel-consumer-examples/parallel-consumer-example-metrics/pom.xml
parallel-consumer-examples/parallel-consumer-example-reactor/pom.xml
parallel-consumer-examples/parallel-consumer-example-support/pom.xml
parallel-consumer-examples/parallel-consumer-example-vertx/pom.xml
OpenSSF Scorecard
Scanned Files
|
|
Claude finished @astubbs's task in 2m 1s —— View job Claude is 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)
|
✅ SpotBugs ReportNo bugs found (new bugs only — baseline from base branch excluded). |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40732c56ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| */ | ||
| public void close() { | ||
| if (parallelConsumer != null) { | ||
| parallelConsumer.close(); |
There was a problem hiding this comment.
When the application shuts down while records are locally queued but have not started, close() delegates to closeDontDrainFirst(), so those records are abandoned for later redelivery even though this method promises to drain before producing its summary. Use closeDrainFirst() here and in OrderFulfilmentApp.close() (line 470), as the Vert.x and Reactor examples already do. The required prior-art checks found the industry-examples plan, the README-generator solution, and the Reactor publisher in-flight note, with no other local matches; open/merged PR and issue checks could not run because gh is unauthenticated.
Useful? React with 👍 / 👎.
| if (windowSeconds <= 0d) { | ||
| return "not observed - the processing window was too short to measure"; | ||
| } | ||
| return format("%.2f records/sec", observer.getCompleted() / windowSeconds); |
There was a problem hiding this comment.
When an example retries failed work, observer.getCompleted() counts every user-function attempt, while the rendered field is labelled records/sec and recordCount is the number of successfully processed records. The parcel and fulfilment examples deliberately generate retries, so their summaries overstate record throughput; divide recordCount by the observed window here, or relabel the result as invocation throughput.
Useful? React with 👍 / 👎.
| file (`destFile` with `${surefire.forkNumber}`) and add `jacoco:merge` before the report. | ||
|
|
||
| ### `MockConsumer.groupMetadata()` workaround, duplicated x4 | ||
| ### `MockConsumer.groupMetadata()` workaround - DONE |
There was a problem hiding this comment.
This converts a completed refactor into a DONE narrative inside the deferred-work backlog, so future backlog scans continue to report already-landed work as an item. Remove the completed portion and, if the Kafka 4.x recheck is still actionable, retain only that deferred task under a heading that describes it.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
| .metricsTags(Tags.of(Tag.of("service", "order-fulfilment"))) // <4> | ||
| // Distinguishes this instance's pc_* series from another instance's. Stable, not a random | ||
| // UUID per restart: a fresh value on each deploy churns the series set for no benefit | ||
| .pcInstanceTag(PC_INSTANCE) // <5> |
There was a problem hiding this comment.
When multiple Parallel Consumer instances register metrics in the same registry, this constant gives every instance the identical meter identity. Micrometer then reuses the existing meters rather than exposing each processor independently, and closing one processor can remove meters needed by another; this also contradicts ParallelConsumerOptions, which requires a supplied pcInstanceTag to be unique. Derive the value from an instance identifier or omit it so PC generates a UUID.
Useful? React with 👍 / 👎.
| .The same question from Parallel Consumer's own side, for the panel beside it | ||
| [source,promql] | ||
| ---- | ||
| sum by (pcinstance) (pc_inflight_records{service="order-fulfilment"}) |
There was a problem hiding this comment.
When an allocation fails and waits for its deliberate retry, pc_inflight_records still includes that record, whereas order_allocation_active_seconds_active_count only counts calls currently inside the warehouse allocation. The example therefore labels two gauges as answering the same question even though its default failure behavior makes them diverge; describe the PC gauge as active-or-waiting work rather than placing it beside the active-call gauge as an equivalent signal.
Useful? React with 👍 / 👎.
The 'todo fix AK mock consumer' marker moved out of four example tests into the shared ExampleMockConsumers helper, so the generated index no longer matched the tree and the staleness gate failed. Four entries become one.
|
Claude finished @astubbs's task in 3m 22s —— View job Claude is reviewing this PR
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a277916a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+317
to
+319
| .limitRate(DEMAND_WINDOW) // <4> | ||
| // concatMap, not flatMap: a meter's own periods are written in time order. | ||
| .concatMap(sample -> ingestSample(reading, sample)) |
There was a problem hiding this comment.
Because count() requests unbounded demand while the following concatMap subscribes to only one ingestSample publisher at a time, limitRate(8) merely batches requests/prefetch from the in-memory iterable; it does not put eight time-series writes in flight, and every run actually has at most one sample ingestion active per reading. The README, class documentation, and logs repeatedly present this as an inner concurrency bound, so readers are taught the wrong backpressure semantics; either describe it as an upstream demand/prefetch window or use an operator that really permits bounded concurrent work if that is the intended lesson. The required prior-art checks found the industry-examples plan, the README-generator solution, and the Reactor publisher in-flight note; open/merged PR and issue checks could not run because gh is unauthenticated.
Useful? React with 👍 / 👎.
Comment on lines
+151
to
+152
| .allSatisfy((consignmentId, peak) -> | ||
| assertThat(peak.get()).as("peak in flight for consignment %s", consignmentId).isEqualTo(1)); |
There was a problem hiding this comment.
This assertion proves only that two scans for the same consignment did not overlap; if KEY scheduling regressed to process each key serially but in reverse or otherwise incorrect offset order, every peak would still equal one and this test would remain green even though the example's primary guarantee—statuses never going backwards—was broken. Record the successfully processed offsets or statuses per consignment and compare them with their input order, as the Reactor and Vert.x example tests do for settlement and departure dates.
Useful? React with 👍 / 👎.
Comment on lines
+21
to
+23
| <!-- Shared scaffolding the other example modules depend on. Listed first for readability only - | ||
| reactor order is derived topologically from declared dependencies, not from this list. --> | ||
| <module>parallel-consumer-example-support</module> |
There was a problem hiding this comment.
This aggregator POM, the four existing example-module POMs, and parallel-consumer-example-metrics/src/test/resources/prometheus.yml all existed at the pinned upstream fork point and receive substantive changes here, but each still carries only its Confluent copyright header. Add the required Modifications Copyright (C) 2026 Antony Stubbs and contributors line beneath the retained notice in each file; the current scanner reports clean only because it limits its provenance diff to *.java.
AGENTS.md reference: AGENTS.md:L235-L238
Useful? React with 👍 / 👎.
Five review findings on the industry examples, all of which made an example report something that was not true of its own run. - Parcel tracking and order fulfilment called close(), which is closeDontDrainFirst(): records queued locally but not yet started were abandoned, and the run summary was then built from that incomplete work. Both now closeDrainFirst(), as the vertx and reactor examples already did. The metrics app's scrape still happens BEFORE the close, since closing PC deregisters its own meters and a later scrape would print nothing. - RunSummary's throughput field is labelled records/sec but was derived from the observer's invocation count, so the two examples that deliberately inject retries overstated it by their retry rate. It now divides the record count by the observed window; "user function invocations" still reports the attempts, under the name that describes them. - OrderFulfilmentApp passed a constant to pcInstanceTag. That tag is the identity PC registers and clears its meters under, so a shared value means closing one instance deregisters meters another is still publishing. The tag is now left unset, so PC generates a unique one. The cardinality point the old comment made was real and is kept honestly: a per-instance tag is required for lifecycle correctness, it costs a label value per instance, and a long-lived deployment should set it to something identifying the instance rather than leave it fresh on every restart. - The README presented pc_inflight_records and the LongTaskTimer's active_count as the same question asked twice. They diverge by design: a record waiting out its retry delay is in flight for PC but is not inside the warehouse call. Documented as active-or-waiting, with the gap between them named as the retry-backlog signal it is. - docs/refactoring.md holds deferred work only, so the completed MockConsumer.groupMetadata() dedup no longer sits in it as a DONE narrative; only the still-actionable Kafka 4.x re-check remains, under a heading that names the deferred task. README.adoc regenerated from the template via `mvn -N process-sources`.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Claude finished @astubbs's task in 2m 44s —— View job Claude is reviewing this PR
|
3 of 4 tasks
astubbs
added a commit
that referenced
this pull request
Aug 12, 2026
…te it cold Two rehearsals are done and green; this is the plan for the real thing, written so a session with no memory of them can run it. Four phases, with A and B in parallel: stage master's rename as a PR (there is no PR for it yet), sweep 38 open PR branches into local <branch>-rename staging branches, review both together, then fast-forward each staging branch into its parent as a separate later step. Only the master rename is visible on GitHub; the 38 stay local and unpushed. Carries the parts that cost something to learn rather than re-deriving them: the per-branch recipe and its gates, the constraints that bite at 38x scale (one pinned tooling source, or two branches rename to different destinations and neither can be brought back; the asyncconsumer fix byte-identical everywhere, or 38 conflicts instead of none), the wrong paths already refuted, and the measured per-branch cost from both rehearsals - which is driven by PR size and deletions, not by the arch-test files. Also records what is NOT settled: #266/268/269/271 genuinely edit the five near-identical TestConventionsArchTest.java files with divergent content, and neither rehearsal covered that shape. And #280's own CI check for this tool fails in CI while passing 112/112 locally twelve times over - unresolved, and it gates Phase A. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 tasks done
…hanges) Directory move only, so every path is 100% similar and git's exact-rename detection cannot fail on it. The content edits follow in the next commit. Generated by bin/rename-packages.sh.
Text edits only. No file moves in this commit, so it cannot dilute the rename detection in its parent. Generated by bin/rename-packages.sh.
astubbs
added a commit
that referenced
this pull request
Aug 12, 2026
…ng fear Two swept branches merged against renamed master in throwaway worktrees: 3 conflicts on a small one, 22 on the largest. All mechanical. The rehearsal's 2-and-9 stands as the right shape but the wrong scale, and the new numbers name where the conflicts actually come from. The logback conflicts are avoidable noise, recorded as such: the sweep deletes the dead logger lines, #289 also collapsed the blank runs and removed one further comment, so the two sides differ by whitespace on ~8 files per pre-289 branch. Both sides are commented-out dead configuration. Transplant the hunks whole next time; re-running 30 green branches to buy it now was not worth it. The mis-pairing fear is retired with evidence rather than argument. Branches carry seven, eight and ten of the near-identical arch tests, not five, and every one paired inside its own module at R100. The move commit changes only paths, so nothing scores below 100 and git has no tie to break. That is a property of the two-commit shape, not of the pool size - which is also why a squash on #266 would now damage ten files instead of five. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
astubbs
marked this pull request as draft
August 19, 2026 03:11
astubbs
added a commit
that referenced
this pull request
Aug 31, 2026
…culum, one demo to converge on Eleventh bit of the follow-up Codex strategy conversation (2026-08-29). New note docs-executable-progression.md: structure the flagship example as numbered runnable stages and the learning path stops being authored - it is encoded in the transitions, so an agent diffing stage 05 -> 06 can generate the quickstart, tutorial, manual chapter, workshop, talk and the "coming from KafkaConsumer / Streams / Python" entries from one running application that cannot drift from compiling code. Generalises the trust the repo already places in generated docs (the tagged-source README) and builds on #266's parcel example and the #208 docs site. Per the owner: the note reconciles rather than adds a fourth demo artefact - the uber demo (#332, collapsed into #331), the three-reveal demo, and the realistic-domain Streams benchmark branch are cross-linked both directions, with the frame that the progression's domain IS the realistic domain, its later stages ARE the reveals, and any stage can feed the uber demo's language matrix. The owner also ruled this convergence strategy-level, so the engine thesis's briefing list gains it with its merge-gate-shaped consequence: no new standalone demo codebase when a stage of the progression could carry it. The engine-thesis note's "keep your code; replace the runtime underneath it" also gains the follow-up conversation's own qualification: a design pressure, not an absolute promise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016PFA3CXkGc2m32cBJMAWTK
6 tasks done
astubbs
added a commit
that referenced
this pull request
Sep 9, 2026
…tested through the sandbox Owner direction, 2026-09-10. The fluent API ships with an example per concern, mirroring the reference surface example list, a Spring example among them, added to the existing core example module rather than a module each. Each is a runnable main plus a sandbox-driven test with no broker, following the industry-grounded examples plan (#266) on harness, support module and domain, and each is a stage of the executable progression. The README quickstart is one of them and is the primary success signal. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LpkQjk4NxtZtPERHvyxeQu
astubbs
added a commit
that referenced
this pull request
Sep 10, 2026
… - twenty units in five milestones over the engine as it is The requirements plan gains its Planning Contract, Implementation Units, Verification Contract and Definition of Done in the same file, built from a repository research pass over the composition surface, the institutional learnings, a landscape pass on random-record generators, and a flow analysis that walked every key flow against the engine and found thirty-six edge cases, five of them correctness-critical. What the research settled, recorded as technical decisions: the fluent API is a package in core (owner decision: one dependency gives both APIs, and no boundary to dissolve when the engine goes native), excluded from the compatibility gate while it incubates; the sandbox is its own module on a Java 17 floor because every maintained generator library is; park rides the retry-delay hook with a facade-side attempt ledger, and the intent must be recorded before the throw because the engine asks for the delay on the way out; stop is a marker throw plus a close from the handle's own thread, since a worker cannot close the engine it runs in; the payload-percentage export waits for a small-tier accessor, so an explicit percentage is refused in the first milestone; format helpers resolve reflectively because registry deserialisers are not on Maven Central. Two Product Contract corrections the flow analysis forced, named in a preservation note: the retention clause now says today's engine never notices a parked record retention removed until a rebalance re-bootstraps the partition, so the lost counter is engine work; and tiny-tier batch mode is accepted on one-route definitions only, since an engine batch mixes topics. Four acceptance examples gained qualifiers (once per assignment, the stopping record not re-invoked, export on the next dispatch, tier tags). Milestone A is the next-release candidate: the fluent package, the sandbox, and the README quickstart as the build signal, with no engine code touched. Three open PRs are merge-first prerequisites for later milestones: the industry-grounded examples (#266) own the parcel domain the sandbox uses, the verdict-free return (#295) is the small tier's admission mechanism already built, and the producer-recovery stack gates transactional export. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LpkQjk4NxtZtPERHvyxeQu
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
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
Each example module today proves the API compiles. None shows a reader a problem they recognise:
CoreAppincrements a counter,ReactorAppreturnsMono.just("something todo"). This adds a second example beside each minimal one, so every module answers both "how do I call this" (the existing example, unchanged) and "why would I reach for this" (the new one).The new examples take on a different role from the existing ones - they demonstrate the library's capabilities rather than demonstrating how to call its API. That distinction decided several things here: it is why each one is grounded in a use case where PC's value proposition is the actual reason, and why the Definition of Done carries an outcome check rather than only mechanical ones.
One industry each, no repeats:
Two supporting units land underneath them. A shared
parallel-consumer-example-supportmodule holds what a reader does not copy - the concurrency observer, the run summary, the simulated-service shape - so each example can keep its PC setup inline and duplicated on purpose without handing the duplicate-code check four copies of the instrumentation. And the core, vertx and reactor example modules gain theTestConventionsArchTestthey never had, so a mis-named new test cannot silently never run.Things worth a reviewer's attention
An open library defect was found, recorded, and deliberately not fixed here.
ReactorProcessorcompletes a record once per emitted element rather than once per publisher, so a multi-elementPublisher- a documented input - reports the same offset successful repeatedly and corrupts offset state. It is invisible today only because every existing reactor example and test returns aMono. Seedocs/inflight/bug-reactor-multi-element-publisher.md. It is library behaviour needing its own regression test, not a rider on a docs-shaped change; the example works around it with.count()and says so.The README generator silently truncates repeated tag regions. This README is not built by Asciidoctor - the plugin's reader latches after the first
end::and discards later regions sharing a tag name. The first regeneration dropped everypollAndProduce,vertxFuture,reactandpollcall while succeeding and looking plausible. Each processing block now has its own tag. Written up indocs/solutions/, where the README pipeline previously had no recorded failure modes.Two claims in the plan were refuted by measurement and corrected, rather than quietly worked around: that configuring both Micrometer percentiles and a histogram discards the quantiles (false on this module's pinned pre-1.13 registry), and the tag-concatenation assumption above.
On the tests
Concurrency is proved with a starting gate, never a threshold or a wall clock. A
CyclicBarrierforces N records to be inside the user function simultaneously; if the test completes, they provably were. Wall-clock figures are printed, never asserted - on a loaded runner a concurrent example can be slower than its own serial baseline.Barrier placement is per-module because the naive placement deadlocks: PC forces the vertx and reactor dispatch pools to a single thread, so a barrier in those user functions would hang on every machine. In the vertx example the barrier is gated to one arrival per record - ungated, three legs of a single record trip it, and the test would pass on a build dispatching records one at a time.
No new broker tests. Every example test except the metrics one uses the mock-consumer harness the other example tests already use; the metrics example extends the module's existing Prometheus-container test and runs in the failsafe lane.
Deliberately out of scope
A single demonstration module - a domain UI showing PC's performance in stakeholder terms - is planned separately and is not here. Neither is wiring in
parallel-consumer-dashboard, nor a CI check for README template/generated drift, though that gap is real and recorded.Checklist
docs/solutions/write-up and adocs/inflight/defect notepublish.ymlandrelease.ymlare edited to add the new module to their hand-maintained Maven Central exclusion lists, so it is not published. No runner or permission changes.