This is an automated email from the ASF dual-hosted git repository.
terrymanu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new d8fe562514b Refine AI implementation quality rules (#39214)
d8fe562514b is described below
commit d8fe562514b1703d331660ba2718305f40b87b90
Author: Liang Zhang <[email protected]>
AuthorDate: Wed Jul 22 21:45:37 2026 +0800
Refine AI implementation quality rules (#39214)
- require reference-first design for databases, dialects, plugins, and
modules
- restrict validation to real contract boundaries
- consolidate code economy rules and remove duplicated guidance
---
AGENTS.md | 114 +++++++++++++++++++-------------------------------------------
1 file changed, 34 insertions(+), 80 deletions(-)
diff --git a/AGENTS.md b/AGENTS.md
index a4ea8f7f0b1..7fb78e0e06f 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -28,34 +28,21 @@ This guide is written **for AI coding agents only**. Follow
it literally; improv
- **Architecture**: follow SOLID, DRY, separation of concerns, and YAGNI
(build only what you need).
- **Code Quality**:
- Use clear naming and reasonable abstractions.
- - **Meaningful Code Gate**:
- - **Purpose traceability**: every added line, identifier, literal,
YAML anchor, helper, abstraction, and configuration entry must have a traceable
purpose.
- Before keeping AI-generated code, trace it back to one concrete
need: production behavior, public contract, regression protection, user-facing
diagnostics,
- security or safety, readability, or removal of real duplication.
- If the purpose cannot be explained in one specific sentence, remove
it.
- - **Meaningless code definition**: meaningless code is code that does
not change or protect behavior, clarify a real contract,
- reduce real duplication or complexity, improve diagnostics or
safety, or make the code easier to read.
- Code is also meaningless when it exists only for formal symmetry,
complete-looking groups, tidy-looking structure, coverage appearance,
- or possible future flexibility.
- Do not keep code merely because it was generated, looks tidy, might
be useful later, or makes a diff look more complete.
- - **YAML anchor rule**: do not add YAML anchors unless they are
actually reused in the same file and reduce meaningful duplication.
- A YAML anchor with no aliases, an anticipatory anchor for possible
future reuse, or an anchor that makes nearby YAML harder to read is forbidden.
- Prefer repeating a small YAML block when repetition is clearer than
indirection.
+ - **Code economy**: every added line, identifier, literal, helper,
abstraction, and configuration entry must serve production behavior, a public
contract,
+ regression protection, diagnostics, safety, readability, or removal of
real duplication.
+ Do not add code for formal symmetry, coverage appearance, hypothetical
reuse, structural completeness, or test convenience.
+ Avoid unnecessary locals, thin wrappers, helpers, collection copies,
comments, guards, and abstractions; inline single-use locals unless reuse or
readability justifies them.
+ Use the smallest clear implementation, and remove obsolete code within
scope only after verifying its usages.
+ - **YAML anchor rule**: do not add YAML anchors unless they are reused in
the same file and reduce meaningful duplication.
+ Prefer repeating a small block when an anchor has no aliases,
anticipates future reuse, or obscures nearby YAML.
- Do not introduce package-private top-level helper types by default.
Keep very small, single-owner state or continuation helpers as private
nested types, but avoid accumulating multiple nested collaborators inside one
class.
When a helper has cohesive behavior, multiple callers, direct test
value, or enough logic to distract from the owner class, split it into a public
top-level type with a clear contract and direct tests.
If neither private nor public fits, do not add a helper; keep the
implementation in the approved owner or simplify the design within the declared
boundary.
- Every new public production type must have direct, focused tests.
Broad workflow tests do not replace public contract tests unless they
explicitly exercise that public type's behavior.
- - Do not change production code solely for test convenience.
- Test-only reuse, easier mocking, coverage convenience, fixture sharing,
or test-only construction must not justify adding production types,
- widening visibility, changing constructors, adding overloads, altering
signatures, moving test helpers into production, or introducing abstractions.
- Production changes must have an independent production reason, such as
fixing behavior, clarifying a real contract, reducing production duplication,
- or exposing a construction path that production code legitimately
supports.
- Prefer test-local fixtures, mocks, existing public constructors,
factories, builders, SPI loaders, or production APIs when construction is
incidental to the behavior under test.
- Do not split a simple production class into a public no-argument
constructor plus a package-private collaborator constructor merely to replace
an internally created collaborator in tests.
- For simple handlers, adapters, or payload builders with one or two
locally created collaborators, keep the production construction shape and adapt
or remove the test unless there is an independent production boundary reason.
- If a change is only needed by tests, do not make the production change.
+ - Do not add production types or abstractions, widen visibility, or change
constructors or signatures solely for test convenience.
+ Require an independent production reason and prefer test-local fixtures,
mocks, existing public construction paths, builders, or SPI loaders.
- New internal abstractions must reduce cognitive complexity instead of
merely wrapping branches in more types.
For simple internal two-path flows, avoid marker interfaces, multi-type
result hierarchies, or extra DTO-style helpers.
Add them only when they define a stable boundary, keep owner classes
readable, or remove meaningful duplicated logic.
@@ -65,43 +52,23 @@ This guide is written **for AI coding agents only**. Follow
it literally; improv
For public constructors and accessors, verify the generated signature,
access level, parameter order, annotations, and reflection or serialization
behavior before replacing manual code.
Keep manual members only when they contain real logic, annotations,
documentation, validation, defaulting, side effects, compatibility
requirements, framework or reflection semantics, or public-contract details
that Lombok would change or obscure.
Do not use broad Lombok annotations such as `@Data` unless every
generated behavior is intentionally required.
- - Do not add guard clauses, parameter checks, or exception throws only to
make code appear safer.
- Add a runtime guard only when it protects a real contract boundary, such
as external input, public API usage, persisted or parsed configuration,
- SPI or reflection input, invalid shared/asynchronous state, or an
immediately diagnosable failure mode.
- Treat the `CODE_OF_CONDUCT.md` rule that method parameters and return
values are not allowed to be `null` as a design contract,
- not as permission to scatter redundant `null` checks through
private/internal call paths whose callers already own the invariant.
- If an upstream parser, validator, factory, SPI loader, type contract, or
public constructor already guarantees an invariant,
- do not duplicate the check unless it improves a user-facing diagnostic
or protects a later boundary; record that reason in the plan or final response.
- - For production guard failures such as invalid state, invalid arguments,
missing resources, or unsupported operations, prefer
`ShardingSpherePreconditions` with lazy exception suppliers
- over manual `if (...) { throw ...; }` guards.
- Use manual throws only when the target module cannot depend on
`infra/exception`, the check lives inside `ShardingSpherePreconditions` itself,
- the surrounding API requires a different control flow, or the
precondition form would obscure the semantics; record the reason in the plan or
final response.
- - Do not add or keep unnecessary `throws` declarations in method
signatures.
- Declare checked exceptions only when the exception is part of the
caller-facing contract, required by an overridden or implemented method,
- required by an external API or framework boundary, or intentionally
preserved for public/source compatibility.
- Do not widen signatures to generic `Exception` or `Throwable` when a
narrower exception, local handling, or ShardingSphere exception conversion is
the real contract.
- Before adding or keeping a checked `throws`, verify the ownership chain:
where the exception originates, who can act on it,
- whether an existing boundary should catch and wrap it, and whether
callers/tests actually depend on the declaration.
- When changing code removes the last checked-exception source, remove
stale `throws` from private and internal methods by default;
- for public APIs, first apply the public contract propagation gate and
document any compatibility reason for keeping the declaration.
+ - **Validation discipline**: add null, size, state, or other runtime
guards only at real contract boundaries, such as external input, public APIs,
+ persisted or parsed configuration, SPI or reflection input, and shared
or asynchronous state, or when they provide concrete diagnostic value.
+ Do not recheck invariants guaranteed by callers or upstream contracts;
every added guard must map to a specific failure scenario or diagnostic benefit.
+ Prefer `ShardingSpherePreconditions` with lazy exception suppliers for
production guards.
+ Use manual throws only when the module cannot depend on
`infra/exception`, the check is inside `ShardingSpherePreconditions`,
+ or required control flow would otherwise be obscured; record the reason.
+ Remove stale checked `throws` from private and internal methods when
their last exception source disappears.
+ Keep them on public or overridden methods only when required by a
caller-facing, framework, external API, or compatibility contract; do not widen
them to generic `Exception` or `Throwable`.
- **Javadoc signal-to-noise gate**:
- - Follow `CODE_OF_CONDUCT.md`: keep concise summaries and required
`@param`, `@return`, and `@throws` tags for public APIs and SPIs; do not treat
this baseline as meaningless repetition.
- - Add prose beyond that baseline only for contracts not expressed by
names, signatures, types, annotations, or repository conventions that affect
caller or implementer behavior,
- such as concurrency, lifecycle, blocking, state transitions, protocol
or compatibility boundaries, and exception handling.
- - Do not restate those visible facts or generic collection properties.
In particular, do not add statements such as
- `The returned collection may be empty, but must not be null or contain
null elements.`
- - Document an empty collection or another special return value only when
it has domain meaning and requires the caller to act differently.
- - Every extra sentence must map to a concrete caller action, implementer
obligation, compatibility requirement, or failure-handling rule; otherwise
remove it.
- Review only Javadocs added or modified within the declared task scope.
- - Do not add or keep Javadocs on methods that only override or implement a
documented parent method.
- Keep the public contract on the declaring API, SPI, or interface.
- An overriding method should add Javadocs only when it documents
implementation-specific behavior, stricter preconditions, side effects,
exceptions, compatibility notes,
- or semantics not already covered by the parent declaration.
- When cleaning redundant override Javadocs, change comments only and
verify no public contract information is lost.
+ - Keep the concise summaries and required tags mandated by
`CODE_OF_CONDUCT.md` for public APIs and SPIs.
+ - Beyond that baseline, document only contracts not expressed by code
that change caller or implementer behavior,
+ such as concurrency, lifecycle, state, protocol, compatibility, or
failure handling.
+ - Do not restate names, signatures, visible collection properties, or
parent contracts unless an override adds implementation-specific obligations.
+ - Review only Javadocs within scope, and remove any sentence that does
not affect caller action, implementer obligation, compatibility, or failure
handling.
- Keep variable declarations adjacent to first use to satisfy Checkstyle
VariableDeclarationUsageDistance; do not mark local variables as `final`,
including ordinary local declarations, loop variables, enhanced `for`
variables, and try-with-resources resources.
For parameters, use `final` only on method parameters, constructor
parameters and `catch` parameters; leave lambda parameters without `final`
unless surrounding code style or tooling requires it.
- - Single-use local variables must be inlined by default; keep a local
variable only when it is reused (for stubbing/verification/assertions) or
materially improves readability.
- For collection declarations in production and test code, use the
least-specific type that expresses the required contract.
Prefer `Collection` for parameters, fields, local variables, and
internal return values when the code only iterates, checks emptiness or size,
or uses common collection operations.
@@ -110,28 +77,10 @@ This guide is written **for AI coding agents only**.
Follow it literally; improv
Use `Set` only when uniqueness, set semantics, set-specific APIs, or an
external API or public contract requires `Set`.
Do not declare implementation types such as `LinkedList`, `ArrayList`,
or `HashSet` unless implementation-specific APIs are required.
Choose concrete implementations according to `CODE_OF_CONDUCT.md`.
- - Do not add defensive collection copies, concrete collection re-wrapping,
or immutable wrappers by default.
- Avoid patterns such as `Collections.unmodifiableList(new
LinkedList<>(values))`, `Collections.unmodifiableSet(new
LinkedHashSet<>(values))`,
- `new LinkedList<>(values)`, `new ArrayList<>(values)`, `List.copyOf`,
`Set.copyOf`, `Map.copyOf`,
- `Collections.unmodifiableList`, `Collections.unmodifiableSet`,
`Collections.unmodifiableMap`,
- `Collectors.toUnmodifiableList`, `Collectors.toUnmodifiableSet`,
`Collectors.toUnmodifiableMap`,
- Guava `ImmutableList` / `ImmutableSet` / `ImmutableMap`, or similar
explicit immutable copy/wrapper APIs
- when the only reason is defensive programming, possible mutability, or
possible ordering.
- - Do not create semantics-free collection copies.
- Before adding `new ArrayList<>(...)`, `new LinkedList<>(...)`, `new
HashSet<>(...)`, `new LinkedHashSet<>(...)`, `new HashMap<>(...)`, or `new
LinkedHashMap<>(...)`,
- verify the concrete semantic reason.
- Without such a reason, forbidden patterns include `new
LinkedHashSet<>(List.of(...))`, `new LinkedList<>(List.of(...))`, `new
LinkedHashMap<>(Map.of(...))`,
- copying the fresh result of a loader, builder, parser, or factory that
already returns an owned collection,
- copying a local collection freshly created in the same method only to
update its element contents,
- or creating `new HashSet<>(values)` only for `contains` when an existing
set already contains the same values.
- - Ordinary collection literals or direct transformation results are
allowed when they express data construction or transformation.
- Do not flag `List.of`, `Set.of`, `Map.of`, or `Stream.toList()` by
default, and do not replace `Stream.toList()` with a mutable collector
- unless the code has a concrete mutability requirement.
- - Explicit collection copies or wrappers are allowed only with a concrete
semantic reason, such as enforcing a documented public API contract,
- preserving a snapshot across shared ownership or asynchronous execution,
protecting cached/global state from mutation,
- isolating later local mutation, or satisfying an external API
requirement.
- In tests, wrap `List.of`, `Set.of`, or `Map.of` in a mutable
implementation only when that exact instance is mutated or when mutability is
the scenario under test.
- Record the reason in the plan, review note, final response, or nearby
code rationale.
+ - Do not copy, wrap, or re-materialize collections merely for defensive
programming, possible mutability or ordering, concrete-type symmetry, or
convenience.
+ A copy or wrapper requires a documented public contract, a snapshot of
shared, asynchronous, or cached state, isolation for later mutation, or an
external API requirement.
+ Ordinary collection literals and transformation results need no mutable
replacement; in tests, use a mutable copy only when that instance is mutated or
mutability is the scenario.
+ Record the concrete reason for any explicit copy or wrapper.
- **Complete Implementation**: no MVPs/placeholders/TODOs—deliver fully
runnable solutions.
### Dead Code Verification
@@ -223,7 +172,12 @@ This guide is written **for AI coding agents only**.
Follow it literally; improv
Dangerous operation detected! Operation type: [specific action] Scope of
impact: [affected area] Risk assessment: [potential consequence] Please confirm
whether to continue. [Requires explicit “yes”, “confirm”, or “proceed”]
## Coding Execution Principles
-- **Think before coding**: inspect existing code, contracts, tests, and
relevant standards before editing; do not guess, hide uncertainty, or invent
unsupported facts.
+- **Reference-first implementation**: before coding, inspect the affected
code, tests, contracts, configuration, registration paths, and module
boundaries.
+ For each affected subsystem, follow the closest applicable, compliant, and
maintained pattern for architecture, extension points, dependencies, naming,
and tests.
+ When adding a database, dialect, plugin, or module, reuse applicable
framework and extension mechanisms;
+ a database or dialect may be derived or entirely new, and derived dialects
must keep shared behavior aligned and isolate only real differences.
+ If no applicable precedent exists, design the missing part and record the
search scope, selected references or absence of precedent, and rationale in the
plan and final report.
+ Do not use this analysis to expand the task or refactor unrelated code.
- **Simple first**: solve the verified goal with the smallest clear
implementation that preserves existing behavior.
- **Precise modification**: change only the files and code paths required by
the task; avoid drive-by refactors and unrelated cleanup.
- **Path portability**: when writing code, tests, scripts, or skills, do not
hard-code local machine paths or workspace-specific absolute paths.
@@ -257,7 +211,7 @@ Dangerous operation detected! Operation type: [specific
action] Scope of impact:
- Use Sequential Thinking when tasks need decomposition: 6-10 steps (fallback
3-5), one sentence each, actionable.
- Intake: choose the strategy for the task, confirm tool
availability/fallbacks, capture constraints (forbidden APIs, output format,
coverage/test expectations),
and use `source-driven-development` when available, or equivalent
source-checking, to verify facts that depend on authoritative sources.
-- Plan: inspect existing code with tools before edits, finish the plan before
coding, use a bounded fresh-context or adversarial self-review for non-trivial
decisions,
+- Plan: complete the required reference analysis and plan before coding, use a
bounded fresh-context or adversarial self-review for non-trivial decisions,
and set the quality/verification bar without invoking or offering
cross-model review.
- Implement: keep scope minimal, follow quality standards, record decisions,
and handle edge cases; honor instruction precedence from Core Principle #7.
- Validate: run the narrowest meaningful checks (see Verification & Commands)
and prefer scoped runs; note any sandbox or limit blocks and alternatives.
@@ -314,7 +268,7 @@ Dangerous operation detected! Operation type: [specific
action] Scope of impact:
- **Report root-cause rule:** for report, audit, review, or analysis tasks, if
a reported finding, verdict, or conclusion is challenged, disproved, or shown
to be inaccurate,
do not stop at patching the output artifact. First fix the highest-leverage
root cause in rules, workflow, schema, validators, prompts, regression cases,
or tests so the same class of error is less likely to recur. Update the
report artifact afterward as a consequence of that root-cause fix, unless the
user explicitly requests a one-off result correction only.
-- **Execution discipline:** inspect existing code before edits; keep changes
minimal; default to mocks and SPI loaders; keep variable declarations near
first use without marking local variables `final`; inline single-use locals by
default unless reuse/readability justifies retention; delete dead code and
avoid placeholders/TODOs.
+- **Execution discipline:** keep changes minimal; default to mocks and SPI
loaders; keep variable declarations near first use without marking local
variables `final`; delete dead code and avoid placeholders/TODOs.
Before handoff, inspect the Java diff for newly added `final` declarations
and remove any new meaningless local-variable `final`.
Verify code and skills do not contain local machine paths before handoff.
- **Final semantic fix gate:** after finishing code, test, documentation,
configuration, or generated-artifact changes and before considering the task
complete,
