[Commands API interface consistency #2] Remove generators - #3866
Conversation
b0fa44f to
b37a5c7
Compare
![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: b37a5c7fdc
ℹ️ 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".
uglide
changed the title
b37a5c7 to
ab6760c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab6760c67c
ℹ️ 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".
ab6760c to
c8f7d34
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8f7d34679
ℹ️ 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".
a75d0c8 to
f7c122d
Compare
f7c122d to
fab040a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fab040ad66
ℹ️ 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".
| - across the Java flavors (async, reactive, node-selection, aggregates) the | ||
| suite also verifies **generic parameter signatures** (not just erased types) | ||
| and **`@Deprecated` parity** with the sync method. |
There was a problem hiding this comment.
When a hand-edited method uses varargs, the advertised parameter-signature check still accepts an array declaration in another flavor: foo(K... keys) and foo(K[] keys) have the same erased and generic reflection parameter type, while TypeSignatures never compares Method.isVarArgs(). This leaves the suite green even though callers can pass multiple arguments only to one flavor, so include varargs parity in each Java counterpart check.
AGENTS.md reference: AGENTS.md:L89-L93
Useful? React with 👍 / 👎.
|
|
||
| Every command group (STRING, HASH, …) exists in up to six parallel interfaces — | ||
| sync, async, reactive, Kotlin coroutines, and the two cluster node-selection | ||
| flavors. All of them are **ordinary hand-edited source files**; there is no code |
There was a problem hiding this comment.
Although this new paragraph correctly states that there is no code generation, the earlier sentence at line 40 still calls the flavors “independently generated shapes.” The previously reported bottom-of-file statement has been corrected, but this additional occurrence remains, so an agent reading this owner document still receives contradictory instructions about whether these interfaces are generated or hand-edited.
AGENTS.md reference: AGENTS.md:L89-L93
Useful? React with 👍 / 👎.
| **Then add the method + Javadoc to the sync interface** | ||
| `src/main/java/io/lettuce/core/api/sync/<Group>Commands.java`. The sync signature is |
There was a problem hiding this comment.
When the command belongs to SENTINEL, this new instruction points to the nonexistent core/api/sync/RedisSentinelCommands.java; that interface actually lives under core/sentinel/api/sync, and the later implementation steps likewise need SentinelCommandBuilder, RedisSentinelAsyncCommandsImpl, RedisSentinelReactiveCommandsImpl, and the Sentinel coroutine implementation rather than the core classes. Add an explicit Sentinel branch to this workflow so following it does not edit or create the wrong files.
AGENTS.md reference: AGENTS.md:L34-L36
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
AI would probably figure it out, though it is still valid comment.
| methods return `Flow<E>` (not suspend); deprecated and streaming-channel | ||
| methods are omitted. The suite verifies presence and the suspend/`Flow` shape |
There was a problem hiding this comment.
The categorical statement that deprecated methods are omitted is false for flushallAsync, flushdbAsync, slaveof, slaveofNoOne, and slaves: KnownApiDeviations.COROUTINES_KEEP_DEPRECATED explicitly requires those methods to remain in the coroutine API, and they are currently declared there. Mention that registry exception here so maintainers do not remove these supported coroutine methods while applying the owner document's mapping rule.
AGENTS.md reference: AGENTS.md:L24-L29
Useful? React with 👍 / 👎.
| methods are omitted. The suite verifies presence and the suspend/`Flow` shape | ||
| only — scalar return types and nullability are **not** checked; review them by | ||
| hand. |
There was a problem hiding this comment.
For Kotlin coroutine methods, parameter names are part of the callable source API because callers can use named arguments, but commandFunctions matches only the method name and erased parameter classes and verifyShape never compares parameter names. A hand-edited strlen(redisKey: K) therefore passes against sync strlen(key) while breaking existing calls such as strlen(key = value); enforce the source-level names or add this limitation to the explicit manual-review caveat.
AGENTS.md reference: AGENTS.md:L89-L93
Useful? React with 👍 / 👎.
| **Then add the method + Javadoc to the sync interface** | ||
| `src/main/java/io/lettuce/core/api/sync/<Group>Commands.java`. The sync signature is |
There was a problem hiding this comment.
AI would probably figure it out, though it is still valid comment.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5ad6db7ad
ℹ️ 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".
c5ad6db to
b348dcb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f43bd8208
ℹ️ 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".
| - **The sync interface is the reference** — the other flavors are derived from it | ||
| by mechanical return-type mapping: async wraps in `RedisFuture<…>`; reactive | ||
| maps `T → Mono<T>` and `List<T>/Set<T> → Flux<T>`; Kotlin uses | ||
| `suspend fun`/`Flow`; node-selection wraps in `Executions`/`AsyncExecutions`. |
There was a problem hiding this comment.
These new bullets restate the precise return-type mappings and deviation-registry split even though api-consistency.md is designated as their owner below. Future mapping or registry changes now require edits in both files and can give agents contradictory instructions when one copy drifts; keep only the architectural overview here and link to the owner for these mechanics.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
3f43bd8 to
2d16734
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d167345b5
ℹ️ 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".
| <scope>test</scope> | ||
| </dependency> | ||
|
|
||
| <!-- CDI --> |
There was a problem hiding this comment.
On a clean checkout, removing this test dependency makes test-compile fail because JavadocConsistencyUnitTests still directly imports com.github.javaparser.JavaParser and several related AST/Javadoc classes, and no other dependency declares JavaParser. Retain javaparser-core while that consistency test remains in the regular unit-test suite.
AGENTS.md reference: AGENTS.md:L89-L93
Useful? React with 👍 / 👎.
| - Javadoc is duplicated across flavors by design; the tests do **not** check | ||
| Javadoc — keep the flavors in sync by hand, using the sync file as reference. |
There was a problem hiding this comment.
This blanket statement is false: JavadocConsistencyUnitTests parses the Java command-interface sources and checks documentation completeness, @param/@return, @since, deprecation tags, parameter names, and selected cross-flavor text parity. Describe those enforced Java checks and limit the manual-review caveat to unverified prose/Kotlin KDoc so this owner document accurately explains test failures.
AGENTS.md reference: AGENTS.md:L24-L29
Useful? React with 👍 / 👎.
ec69252 to
5e865f7
Compare
The command interfaces are now hand-edited source files kept in lockstep by the API consistency test suite; the generators had drifted from the committed files (re-running them produced noisy, API-changing diffs) and were no longer part of any workflow. - delete src/test/java/io/lettuce/apigenerator and src/main/templates - drop the javaparser-core test dependency and the templates entry of the formatter plugin - remove the api_generator test tag - replace .agents/docs/code-generation.md with .agents/docs/api-consistency.md and update architecture.md, AGENTS.md and the adding-a-redis-command/writing-javadoc skills to describe the hand-edit-all-flavors workflow Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The markers referenced the removed io.lettuce.apigenerator classes and instructed readers not to hand-edit files that are now the source of truth. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- architecture.md: drop the leftover claim that the sync interface is generated; it is a declaration implemented by the async-backed proxy - api-consistency.md: describe builder coverage against each group's registered builder (module groups have dedicated builders), document that Kotlin scalar return types/nullability are not verified, and mention the parameter-signature and @deprecated parity checks - adding-a-redis-command: keep the return-type mapping in its owner document instead of restating it, and rename the stale "checkpoint before generation" reference to "before mirroring" - writing-javadoc: extend the mirroring checklist to the node-selection and Sentinel interfaces Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5e865f7 to
071f211
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 071f211773
ℹ️ 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".
| **The sync interface is the reference.** Async/reactive/coroutine/node-selection | ||
| declarations are derived from it by the mapping rules below. (Two methods — | ||
| `setAutoFlushCommands`, `flushCommands` — exist on async/reactive/coroutines but | ||
| intentionally not on sync.) |
There was a problem hiding this comment.
When an agent uses this owner document to determine command-interface parity, this exception is misleading: repo-wide search finds setAutoFlushCommands and flushCommands only on src/main/java/io/lettuce/core/api/StatefulConnection.java:113-119, not on the async, reactive, or coroutine command interfaces covered by this suite. Remove this parenthetical so maintainers do not preserve or introduce exceptions that are outside the command API catalog.
AGENTS.md reference: AGENTS.md:L24-L29
Useful? React with 👍 / 👎.
| - **builder coverage** — every sync command has a same-named method on its | ||
| group's command builder, and every command-producing builder method is | ||
| reachable from an interface. Core groups map to `RedisCommandBuilder`; the |
There was a problem hiding this comment.
When a maintainer checks commands whose API and builder names intentionally differ, this same-name rule misdescribes what the suite enforces: KnownApiDeviations.BUILDER_ALIASES maps examples such as waitForReplication to wait, getMasterAddrByName to getMasterAddrByKey, and vClearAttributes to vsetattr. State that a command needs either a same-named builder method or a registered alias so agents do not add duplicate builder entry points merely to satisfy this owner document.
AGENTS.md reference: AGENTS.md:L24-L29
Useful? React with 👍 / 👎.
| ```bash | ||
| mvn -Dtest='*ConsistencyUnitTests,CommandBuilderCoverageUnitTests' \ | ||
| -Dsurefire.failIfNoSpecifiedTests=false test |
There was a problem hiding this comment.
When this focused command is used after adding a command group or deviation, it can report success without running CatalogCompletenessUnitTests or DeviationTableStalenessUnitTests, because neither class name matches *ConsistencyUnitTests and only the builder test is added explicitly. Include both tests in the filter so an unregistered interface or stale deviation entry is caught at the workflow's consistency checkpoint rather than only by a later full mvn test.
AGENTS.md reference: AGENTS.md:L97-L102
Useful? React with 👍 / 👎.
2 participants
Removes the template-based API interface generators now that consistency between the command interface flavors is enforced by the test suite from #3865 (this PR is stacked on it — its base is
im/api-consistency-tests; rebase ontomainafter #3865 merges).The generators had drifted from the committed interfaces — re-running them produced large, noisy diffs that silently changed the public API — and were no longer part of any workflow. The interfaces are now hand-edited source files; the consistency tests fail the build when a flavor is missed.
Changes
src/test/java/io/lettuce/apigenerator/(9 classes) andsrc/main/templates/(23 templates)javaparser-coretest dependency and the templates entry of the formatter pluginapi_generatortest tag@generated by io.lettuce.apigenerator.*Javadoc markers from all 139 interface files — they referenced deleted classes and instructed readers not to hand-edit files that are now the source of truth (mechanical commit: one Javadoc line per file).agents/docs/code-generation.mdwith.agents/docs/api-consistency.mdand updatearchitecture.md,AGENTS.mdand theadding-a-redis-command/writing-javadocskills to describe the hand-edit-all-flavors workflowNo production code changes beyond the Javadoc marker removal.
Make sure that:
mvn formatter:formattarget. Don't submit any formatting related changes.🤖 Generated with Claude Code
Note
Low Risk
Documentation and Javadoc-only edits on interfaces; no command dispatch or wire-protocol changes.
Overview
Removes the legacy template + JUnit generator pipeline for sync/async/reactive/Kotlin/cluster command interfaces now that #3865’s consistency tests enforce parity. Deleted pieces include
src/test/java/io/lettuce/apigenerator/,src/main/templates/, theapi_generatortag, andjavaparser-core/ formatter coverage for templates;code-generation.mdis replaced byapi-consistency.md.Agent and contributor docs (
architecture.md,AGENTS.md,adding-a-redis-command,writing-javadoc, integration-testing layout) now describe hand-editing the sync interface first, mirroring to every flavor, and running*ConsistencyUnitTestsinstead of running generators.On the public API surface, behavior is unchanged: ~139 command interfaces only lose the
@generated by io.lettuce.apigenerator.*Javadoc line so the committed files are clearly the source of truth.Reviewed by Cursor Bugbot for commit 071f211. Bugbot is set up for automated code reviews on this repo. Configure here.