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 12a2b1109d8 Strengthen AI implementation boundaries and review loop
(#39245)
12a2b1109d8 is described below
commit 12a2b1109d8a990c714e03fd00db7f4a18b1a95f
Author: Liang Zhang <[email protected]>
AuthorDate: Mon Jul 27 16:35:51 2026 +0800
Strengthen AI implementation boundaries and review loop (#39245)
* Strengthen AI implementation boundaries and review loop
* Strengthen AI implementation boundaries and review loop
---
AGENTS.md | 50 ++++++++++++++++++++++++++++++--------------------
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/AGENTS.md b/AGENTS.md
index 7fb78e0e06f..bf26aa67cdf 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -25,19 +25,22 @@ This guide is written **for AI coding agents only**. Follow
it literally; improv
## Quality Standards
### Engineering Principles
-- **Architecture**: follow SOLID, DRY, separation of concerns, and YAGNI
(build only what you need).
+- **Architecture**: preserve existing architecture by default; SOLID, DRY,
symmetry, anticipated reuse, and testability do not by themselves justify an
unrequested abstraction or architectural boundary.
- **Code Quality**:
- Use clear naming and reasonable abstractions.
- **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.
+ Before adding an implementation, prove that direct reuse cannot satisfy
the goal; then prefer composition or delegation, and use an existing extension
point only when the earlier options cannot satisfy the goal within the
confirmed boundary or the task requires that extension contract.
+ Every new abstraction must own independent production behavior, state,
or contract that existing types cannot express; forwarding, renaming, type
distinction, structural symmetry, shorter call sites, test convenience, and
anticipated reuse are insufficient.
+ Reuse never authorizes changes to class or method `final`, visibility,
inheritance, or shared-code ownership; follow the Architecture change gate when
such a change is required.
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.
+ Add a top-level or public helper only when existing types cannot
reasonably own independent production behavior, state, or contract required by
the task; multiple callers, readability, direct test value, symmetry, and
anticipated reuse are insufficient.
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.
@@ -107,6 +110,8 @@ This guide is written **for AI coding agents only**. Follow
it literally; improv
protocol or payload contract generation, or another non-trivial behavior
owned by the target class.
If a method only passes data through, delegates directly, returns a
constant, exposes a getter or setter, or wires an object without adding logic,
do not add a dedicated unit test for that method unless the pass-through
itself is a documented public contract or part of an externally visible
protocol boundary.
+ Never add tests whose subject is a test class, test fixture, mock
helper, or other test-only code; test only the production behavior it supports.
+ Test-support code distributed as an independent artifact with an
external contract is production code for this rule.
- A unit test is meaningful only if it would fail for a realistic bug that
matters to users, callers, protocol clients, or maintainers of the class
contract.
If a test only proves that implementation structure remains the same,
rather than proving behavior is correct, remove it.
- Do not add unit tests that only assert constant literal values, enum
names, field names, method names, YAML anchor names,
@@ -176,8 +181,12 @@ Dangerous operation detected! Operation type: [specific
action] Scope of impact:
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.
+ If no applicable precedent exists, do not invent architecture; record the
search evidence and follow the Architecture change gate before proposing a new
boundary.
Do not use this analysis to expand the task or refactor unrelated code.
+- **Architecture change gate**: changes to class or method `final`,
visibility, inheritance, public contracts, SPIs or extension points, loading or
registration contracts, module or dependency boundaries, or shared-code
ownership are architecture changes; local-variable `final` is only a style
concern.
+ Before editing, report existing behavior and ownership, reuse analysis,
compatibility impact, and the minimum affected files.
+ An exact architecture change explicitly requested by the user is authorized
after that report; otherwise stop and obtain explicit confirmation.
+ Adding, changing, or removing an SPI is always an architecture change.
- **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.
@@ -185,13 +194,14 @@ Dangerous operation detected! Operation type: [specific
action] Scope of impact:
- **Execution mode discipline**: in confirmation-only tasks, surface unclear
boundaries, design conflicts, and rule conflicts as questions or
recommendations before editing.
In authorized implementation tasks, produce a compliant result directly when
the issue can be resolved within the declared boundary; do not stop merely to
ask permission or explain alternatives.
Stop only for missing scope, required scope expansion, dangerous operations,
external approvals, or true impasses that cannot be resolved from local
evidence.
-- **Scope declaration gate**: before planning or editing, determine and
declare the requested change boundary.
- If the boundary is clear from the user request, state the inferred scope
explicitly before making changes.
- If the boundary is missing or ambiguous, pause and ask the developer to
confirm it before making changes.
- The boundary may be a module name, package name, class name, file path,
public API, issue/PR scope, or specific behavior/test scenario.
- Do not determine the scope after editing, and do not infer a broader scope
from nearby code, unrelated failures, or cleanup opportunities.
- The declared or confirmed boundary is the maximum allowed scope for
production, test, documentation, and configuration changes.
- If implementation evidence shows the declared or confirmed scope is
insufficient, pause again and explain the required expansion before editing
outside it.
+- **Scope declaration gate**: after reference analysis and before planning or
editing, record the verifiable goal, non-goals, behavior owners and evidence,
expected changed files, prohibited paths, reuse and architecture decisions,
required production and test protection, scoped verification, and response
constraints as the acceptance checklist.
+ If any item is missing or ambiguous, pause and ask the developer to confirm
it before making changes.
+ The checklist is the maximum allowed scope for production, test,
documentation, and configuration changes.
+ Every changed file and hunk must be necessary for a checklist item;
prohibited files and modules must have zero diff.
+ Do not infer broader scope from nearby code, unrelated failures, cleanup
opportunities, or rule compliance.
+ If implementation evidence proves the checklist insufficient, stop before
editing outside it, explain the required expansion, and obtain confirmation.
+ If the actual diff instead contains unnecessary current-task changes, stop
the current design, remove only those changes, preserve unrelated user changes,
and restart diff review from the PR base or last confirmed baseline.
+- **Rejected design reset**: if the user rejects a design, stop patching it.
Remove only changes provably introduced by the current task and rejected
design, preserve unrelated changes, and redesign the smallest solution from the
last confirmed baseline before coding again. If ownership is unclear, stop and
request confirmation.
- **Goal-driven execution**: convert the request into verifiable outcomes
before implementation, then validate those outcomes with scoped checks.
## Coding Skill Guidance
@@ -264,7 +274,7 @@ Dangerous operation detected! Operation type: [specific
action] Scope of impact:
If the boundary is clear from the request, state the inferred module,
package, class, file path, public API, issue/PR scope, or behavior/test
scenario.
If the boundary is missing or ambiguous, ask the developer to confirm it
before making changes.
- **Risk gate:** if any action fits the Dangerous Operation Checklist, pause
and use the confirmation template before proceeding.
-- **Planning rules:** use Sequential Thinking with 3-10 actionable steps (no
single-step plans) via the plan tool for non-trivial tasks; convert all hard
requirements (SPI usage, mocking rules, coverage/test naming, forbidden APIs)
into a checklist inside the plan and do not code until each item is addressed
or explicitly waived.
+- **Planning rules:** use Sequential Thinking with 3-10 actionable steps (no
single-step plans) via the plan tool for non-trivial tasks; convert all hard
requirements (SPI usage, mocking rules, coverage/test naming, forbidden APIs)
into a checklist inside the plan and do not code until each item is addressed
or explicitly waived by the user.
- **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.
@@ -272,12 +282,10 @@ Dangerous operation detected! Operation type: [specific
action] Scope of impact:
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,
- inspect the final diff and surrounding changed context against every
`AGENTS.md` and `CODE_OF_CONDUCT.md` rule that applies to the requested scope,
- touched files, and changed behavior.
- Do not rely on scripts, search output, Checkstyle, Spotless, tests, or
compilation alone as proof that the change satisfies semantic rules.
- Use tools such as `git diff`, `rg`, tests, Checkstyle, and Spotless to
collect evidence and candidate violations,
- then make an explicit semantic judgment from the code, contracts, existing
patterns, and user request.
- Verify that every actual change is simple, necessary, and directly tied to
the user request or an explicitly applicable rule requirement.
+ inspect the final diff and surrounding changed context against the
acceptance checklist and every applicable `AGENTS.md` and `CODE_OF_CONDUCT.md`
rule.
+ Scripts, search output, Checkstyle, Spotless, tests, and compilation provide
evidence, not proof of semantic compliance.
+ Make an explicit semantic judgment from the code, contracts, existing
patterns, and user request.
+ Verify that every changed file and hunk maps to a checklist item; prohibited
paths have zero diff; direct reuse was considered first; no unjustified
abstraction or public type was added; class or method `final`, visibility,
inheritance, and SPI changes were exactly authorized; and every test protects
production behavior.
Keep necessary formatting changes inside touched hunks, but remove unrelated
file or hunk changes introduced by the current task.
If an applicable rule is violated, or if a change is meaningless,
speculative, cosmetic, convenience-only, or unnecessary, fix or remove the
current-task edit before completion.
Do not treat reporting a violation, waiver, or residual risk as a substitute
for fixing an in-scope issue that can be fixed safely.
@@ -297,7 +305,8 @@ Dangerous operation detected! Operation type: [specific
action] Scope of impact:
- **Post-task self-check (before replying):** confirm all instructions were
honored; verify no placeholders/unused code; ensure Checkstyle/Spotless gates
for touched modules are satisfied or explain why not run and what to run; list
commands with exit codes; call out risks and follow-ups; complete all
applicable checks before replying and do not rely on users to find missed rule
violations.
- **End-of-task format/style gate:** for any task that edits files, run
`./mvnw spotless:apply -Pcheck -T1C` after the final edit, then run `./mvnw
checkstyle:check -Pcheck -T1C` when production, test, or project-rule files are
touched.
Do not perform manual formatting or whitespace cleanup after the final
Spotless run; if a later cleanup is required, repeat Spotless and then
Checkstyle before the final response.
-- **Final response template:** include intent/why, changed files with paths,
rationale per file/section, commands run (with exit codes), verification
status, and remaining risks/next actions (if tests skipped, state reason and
the exact command to run); include a concise self-check result statement
confirming final clean status after fixes.
+- **Final response template:** lead with the shortest complete outcome; add
detail only when requested or needed to resolve ambiguity, risk, or the next
action, and never repeat information that does not change the decision.
+ Never omit required evidence, blockers, or validation failures. Include
intent, changed files and rationale, commands with exit codes, verification
status, remaining risks or next actions, and a concise self-check result; if a
check was skipped, state why and give the exact command to run.
## Final Self-Iteration Gate
- Apply this gate only to implementation tasks where the user has requested or
authorized code or documentation edits.
@@ -306,7 +315,7 @@ Dangerous operation detected! Operation type: [specific
action] Scope of impact:
whether the changed implementation can be simpler without changing behavior,
whether existing public behavior and contracts are preserved, and whether
`code-review-and-quality` or equivalent review still has in-scope required
findings.
- Use a bounded fresh-context or adversarial self-review to keep raising and
resolving valuable in-scope questions until the stop condition is met.
- Stop when no actionable findings remain, the same findings repeat, 3 doubt
cycles complete, or the user explicitly overrides.
+ Stop only when no actionable findings remain. Repeated findings must be
fixed or, when the fix would exceed the confirmed boundary, reported as a
blocker requiring confirmation; cycle count is never a completion condition.
- Before finishing any implementation task, compare the final diff against the
declared or confirmed change boundary.
Use `git diff` to verify the final changed file set and changed hunks
against the declared or confirmed boundary.
If any edit is outside that boundary and has no documented reason required
by the task or an applicable rule,
@@ -314,6 +323,7 @@ Dangerous operation detected! Operation type: [specific
action] Scope of impact:
Do not keep out-of-scope changes for convenience, and do not use destructive
git commands or revert unrelated user changes.
If the task cannot be completed after removing the out-of-scope edits, stop
and ask the developer to confirm the required scope expansion.
- If any answer reveals an in-scope, behavior-preserving, low-risk required
fix, make the fix and rerun relevant checks.
+- Any file-changing fix invalidates the previous diff review and verification;
rerun affected checks and restart the final review.
- Repeat the review-fix-verify loop until no in-scope required findings remain.
- Do not continue iterating for optional polish, broad cleanup, risky
refactors, or unrelated code. Record those as follow-up suggestions instead of
expanding the task.
@@ -369,7 +379,7 @@ Dangerous operation detected! Operation type: [specific
action] Scope of impact:
## Mocking & SPI Guidance
- Favor Mockito over bespoke fixtures; only add new fixture classes when mocks
cannot express the scenario.
-- Use marker interfaces when distinct rule/attribute types are needed; reuse
SPI types such as `ShardingSphereRule` where possible.
+- Reuse an existing SPI only when its contract matches the required behavior;
one caller, one known implementation, code sharing, type distinction, symmetry,
anticipated reuse, or test convenience does not establish a new SPI contract.
- Name tests after the production method under test; never probe private
helpers directly—document unreachable branches instead.
- Mock heavy dependencies (database/cache/registry/network) and prefer mocking
over building deep object graphs.
- When static methods or constructors need mocking, prefer
`@ExtendWith(AutoMockExtension.class)` with `@StaticMockSettings` (or the
extension's constructor-mocking support); when a class is listed in
`@StaticMockSettings`, do not call `mockStatic`/`mockConstruction`
directly—stub via `when(...)` instead. Only if `AutoMockExtension` cannot be
used and the reason is documented in the plan may you fall back to
`mockStatic`/`mockConstruction`, wrapped in try-with-resources.
