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 a1ce8109d06 Refine review-pr mergeability result handling (#38882)
a1ce8109d06 is described below
commit a1ce8109d06a8c7cf81615290d67e22bc5b6d041
Author: Liang Zhang <[email protected]>
AuthorDate: Sun Jun 21 22:02:26 2026 +0800
Refine review-pr mergeability result handling (#38882)
* Update release notes
* Refine review-pr mergeability result handling
- add Review Incomplete as a first-class review result
- clarify CI evidence judgment and blocker evidence thresholds
- move SQL parser-specific review guidance into a scoped reference
- update review-pr agent metadata for tri-state review output
* Refine review-pr mergeability result handling
- add Review Incomplete as a first-class review result
- clarify CI evidence judgment and blocker evidence thresholds
- move SQL parser-specific review guidance into a scoped reference
- update review-pr agent metadata for tri-state review output
---
.codex/skills/review-pr/SKILL.md | 425 +++++++--------------
.codex/skills/review-pr/agents/openai.yaml | 6 +-
.../review-pr/references/sql-parser-review.md | 49 +++
3 files changed, 180 insertions(+), 300 deletions(-)
diff --git a/.codex/skills/review-pr/SKILL.md b/.codex/skills/review-pr/SKILL.md
index 18198bd5826..398b43bf052 100644
--- a/.codex/skills/review-pr/SKILL.md
+++ b/.codex/skills/review-pr/SKILL.md
@@ -5,6 +5,7 @@ description: >-
assess side effects and regression risks, and determine whether it can be
safely merged.
If not mergeable, produce committer-tone change requests, or
needs-discussion feedback when
the PR direction, root-cause model, or problem framing should be reopened
before implementation continues.
+ If review cannot be completed from available public evidence, produce a
Review Incomplete result without patch-level advice.
Supports targeted comparison across GitHub-visible review rounds when prior
PR comments or review threads exist.
Before final output, internally self-iterate the review until no new
actionable findings are discovered.
---
@@ -13,12 +14,15 @@ description: >-
## Objective
-- Make merge decisions for ShardingSphere PRs with a "root-cause-first,
evidence-first" approach.
-- Output a single merge decision:
- - `Merge Decision`: `Mergeable` / `Not Mergeable`
+- Review ShardingSphere PRs with a root-cause-first, evidence-first approach.
+- Output exactly one `Review Result`:
+ - `Mergeable`: public code, tests, documentation, and relevant verification
support merge readiness.
+ - `Not Mergeable`: public evidence confirms a blocker in the current PR
scope.
+ - `Review Incomplete`: the reviewer cannot make a reliable mergeability
judgment because required public facts are unavailable, inaccessible, or not
attributable.
- For `Not Mergeable`, choose exactly one feedback mode:
- - `Change Request`: the direction is valid but the current patch needs
implementation, test, scope, or evidence changes.
+ - `Change Request`: the direction is valid, but the patch needs
implementation, test, scope, compatibility, or evidence changes.
- `Needs Discussion`: public evidence shows the PR direction, root-cause
model, or problem framing must be reopened before implementation continues.
+- For `Review Incomplete`, do not output patch-level change requests. State
what was verified, what required public fact is missing, and what must be
checked next.
## Trigger Scenarios
@@ -39,81 +43,49 @@ Before applying the numbered review gates, enforce the
public evidence boundary:
and verify it against public artifacts before output.
1. Verify root-cause repair first; fallback logic, defaults, null checks,
try-catch blocks, or swallowed errors cannot substitute for root-cause repair.
-2. Output exactly one `Merge Decision`, and choose one `Feedback Mode` for
every `Not Mergeable` result.
-3. If evidence is insufficient, risk is unclear, validation is incomplete, or
any required hard gate fails, set `Merge Decision: Not Mergeable`.
-4. If public evidence shows a wrong root-cause model, problem framing,
expected behavior, ownership boundary, or solution direction, use `Feedback
Mode: Needs Discussion`.
+2. Output exactly one `Review Result`. Choose one `Feedback Mode` only when
the result is `Not Mergeable`.
+3. Do not convert reviewer uncertainty, tool failure, inaccessible GitHub
data, or missing local verification into a PR blocker. Use `Review Incomplete`
when required public facts cannot be checked or attributed.
+4. If public evidence confirms a wrong root-cause model, problem framing,
expected behavior, ownership boundary, or solution direction, use `Review
Result: Not Mergeable` with `Feedback Mode: Needs Discussion`.
5. Review only the latest PR code version, and use GitHub PR metadata plus
`/pulls/{number}/files` as the authoritative scope boundary.
-6. Do not use GitHub Actions, CI status, or check-run completion for the merge
decision unless explicitly requested.
-7. Use repository-declared formatting/style gates as the formatting authority.
-8. Treat substantive unrelated changes or substantive scope expansion as merge
blockers; ignore non-behavioral import-only, whitespace-only, formatter-only,
or IDE cleanup churn unless it hides behavior,
+6. Use repository-declared formatting/style gates as the formatting authority.
+7. Treat substantive unrelated changes or substantive scope expansion as
blockers; ignore non-behavioral import-only, whitespace-only, formatter-only,
or IDE cleanup churn unless it hides behavior,
fails declared gates, touches broad unrelated areas, or violates explicit
scope rules.
-9. Before considering `Mergeable`, apply all triggered hard gates and
specialized review gates, including semantic compatibility, counterexamples,
blast-radius/shared-layer ownership,
- SQL parser official-doc and dialect-family checks, linked-issue
completeness, implicit-state review, high-frequency `computeIfAbsent` review,
and local verification freshness.
-10. Before any final output, complete the `Anti-Drip Review Gate` and
`Self-Iteration Gate`; do not expose intermediate findings, and output one
consolidated review.
+8. Before considering `Mergeable`, apply all triggered hard gates and
specialized review gates, including semantic compatibility, counterexamples,
blast-radius/shared-layer ownership,
+ linked-issue completeness, implicit-state review, high-frequency
`computeIfAbsent` review, CI evidence judgment when relevant, and local
verification freshness.
+9. Before final output, complete the `Pre-Publication Finding Audit`; do not
expose intermediate findings, and output one consolidated review.
+
+## Evidence Sufficiency and CI Judgment
+
+- `Not Mergeable` requires confirmed public evidence in the reviewed scope. A
blocker must be supported by at least one of:
+ - code, diff, or contract evidence that proves a defect or scope violation;
+ - relevant test, CI, check-run, log, or reproduction evidence that proves
failure;
+ - a required core behavior validation gap after the reviewer has checked
available public facts;
+ - compatibility, API/SPI, protocol, data, security, lifecycle, dependency,
or distribution risk with a clear executable path;
+ - confirmed scope, ownership, documentation, release-note, or migration
impact that the PR must resolve.
+- `Review Incomplete` is required when mergeability depends on public facts
that are unavailable, inaccessible, stale, or not attributable to the PR.
+- `Review Incomplete` is not a soft approval and must not contain patch-level
code requests. If a code issue is already confirmed, use `Not Mergeable`.
+- CI success never replaces code review, root-cause review, scope review, or
test adequacy review.
+- Relevant CI failure means the PR cannot be `Mergeable`. If the failure is
attributable to the PR, use `Not Mergeable`; if attribution is unclear, use
`Review Incomplete`.
+- Inspect CI, check-runs, or workflow logs when the PR goal, linked issue,
author/user statement, generated artifact, native image, E2E, test-infra, or a
candidate blocker depends on runtime verification.
+- Do not wait for or query CI when code, docs, or static evidence is
sufficient for the current review result. State the reason in `Verification`
when CI was not reviewed.
## Mergeable Hard Gates
-Apply these gates before considering `Merge Decision: Mergeable`.
+Apply these gates before considering `Review Result: Mergeable`.
If a gate is not applicable to the PR, state the reason briefly in the review
evidence or details.
Do not turn speculative risks, personal style preferences, or out-of-scope
polish into merge blockers.
-1. Root Cause Gate:
- - The PR must repair the true trigger point or required propagation path,
not only the final error point.
- - Fallbacks, defaults, null checks, try-catch blocks, or swallowed errors
cannot substitute for root-cause repair.
- - If the root-cause chain cannot be proven fixed, set `Merge Decision: Not
Mergeable`.
-2. Linked Issue Completeness Gate:
- - Keep and apply the existing linked-issue completeness rules whenever the
PR claims to fix, close, resolve, or address an issue.
-3. Scope & Ownership Gate:
- - Substantive unrelated changes, substantive scope expansion, and broad
cleanup outside the PR goal block mergeability.
- - For pluggable features, dialects, rules, registry centers, or protocol
modules, the fix should stay in the owning module by default.
- - Shared modules may be changed only for generic contracts or hooks that
make sense for all affected owners.
- - Target-specific names, lifecycle concepts, protocol state, database
strings, or comments in shared code are blockers unless proven to be an
intentional generic contract.
-4. Regression & Side Effect Gate:
- - The PR must not leave unresolved functional degradation, compatibility,
performance, config, API/SPI, SQL dialect, feature-disabled path, or
adjacent-feature risks.
- - "No side effects" means no identified but unresolved or unvalidated
side-effect risk in the reviewed scope; do not require impossible exhaustive
proof.
-5. Test Adequacy Gate:
- - New or changed production code or behavior needs corresponding test
evidence.
- - Bug fixes should have regression tests for the reported symptom or
root-cause path.
- - New features should cover the main success path and important boundary,
disabled, or error paths.
- - Existing tests may satisfy this gate only when they clearly exercise the
changed behavior.
- - Judge tests by behavior and root-cause coverage, not by coverage-rate or
environment breadth alone.
- - High-cost environment, native-image, distributed-system, or end-to-end
validation blocks mergeability only when lower-level public-path tests and code
evidence cannot prove the root-cause repair,
- or when the current PR itself owns that environment integration behavior.
- - For narrow split PRs whose code path can be proven locally, environment
validation may be delegated to the umbrella PR or integration test scope; state
that boundary in `Review Details` instead of turning it into a blocker.
- - Do not require coverage-rate proof, and do not block mergeability solely
because a coverage report was not produced.
-6. Code Quality Gate:
- - Block only concrete maintainability problems, such as unclear
responsibility, duplicated logic, dead code, over-complex control flow, hidden
state, magic values, or hard-to-read temporary design.
- - Do not turn ordinary naming/style preferences or optional nits into
blockers unless they violate repository rules or create real maintenance risk.
-7. Architecture Gate:
- - Trigger a deeper architecture review when the PR touches shared modules,
public/shared APIs, SPI contracts, metadata, rule owners, dialect owners,
session/executor state, or lifecycle state.
- - The PR must preserve module ownership, explicit contracts, SPI/metadata
boundaries, and clear state models.
- - Broken layering, bypassed owner modules, target-specific semantics in
shared code, or implicit lifecycle states block mergeability.
-8. Release Note Gate:
- - Required when the PR introduces or changes user-visible behavior that
users, DBAs, operators, or application developers need to know for upgrade,
troubleshooting, configuration,
- migration, rollback, compatibility assessment, or meaningful release
awareness.
- - Usually not required for test-only changes, pure refactoring with no
behavior change, formatter/import/typo/comment-only changes,
- internal bug fixes that restore already documented behavior without new
user action, or CI/build changes that do not affect released artifacts,
supported platforms, dependencies,
- or user-visible build behavior.
- - For split or staging PRs, a release note may be deferred to the umbrella
PR when the umbrella PR owns the user-facing release story and the split PR is
not expected to ship independently.
- If deferring, verify and state the delegation reason; do not require
duplicate release-note entries that would only create low-signal changelog
noise.
- - If the split PR can be released independently and the fix has meaningful
user-facing impact, require the release note in the split PR.
- - Required release notes must update `RELEASE-NOTES.md` in the proper
category and describe the user-visible outcome, affected module or feature,
- and important compatibility, configuration, upgrade, or rollback impact
when applicable.
- - Release notes must be understandable to users, DBAs, operators, and
application developers, not only maintainers.
- - Missing, misleading, implementation-only, wrong-category, or over-claimed
release notes block mergeability only after the gate determines that a release
note is required for the current PR.
-9. User Documentation Impact Gate:
- - If users need documentation to correctly use, configure, upgrade,
troubleshoot, or understand the changed behavior, check the relevant user docs.
- - Missing required docs for user-facing configuration, DistSQL, SQL
support, API/SPI usage, Proxy/JDBC behavior, or upgrade flow block mergeability.
-10. Breaking Change / Migration Impact Gate:
- - If the PR changes default behavior, config keys, API/SPI contracts,
protocols, metadata storage, SQL semantics, or released artifacts,
- require explicit compatibility, migration, upgrade, and rollback evidence.
- - Unexplained breaking or migration impact blocks mergeability.
-11. Error Message / Diagnostics Quality Gate:
- - If the PR changes exceptions, error codes, logs, or diagnostic output,
the result must be accurate, actionable, and safe for users.
- - Diagnostics that hide the real failure, mislead users, regress
troubleshooting, or expose sensitive information block mergeability.
-12. Dependency / Distribution Impact Gate:
- - If dependency manifests, lockfiles, distribution packaging, native-image
metadata, LICENSE, NOTICE, or release artifacts change,
- check security, license, compatibility, packaging, and release impact
before considering `Mergeable`.
+1. Root cause: the PR repairs the true trigger point or required propagation
path, not only the final error point. Fallbacks, null checks, defaults,
try-catch blocks, or swallowed errors do not substitute for root-cause repair.
+2. Linked issue completeness: every claimed fixed/closed/addressed issue
requirement maps to code and validation, or unresolved scope becomes `Not
Mergeable` or `Review Incomplete` under the evidence rules.
+3. Scope and ownership: no substantive unrelated changes, scope expansion,
target-specific leakage into shared code, or ownership bypass remains.
+4. Regression and side effects: no unresolved functional degradation,
compatibility risk, performance risk, config/API/SPI risk, dialect risk,
feature-disabled path risk, or adjacent-feature risk remains.
+5. Test adequacy: changed behavior has meaningful direct or existing coverage
of the root-cause path and important boundaries. Do not require coverage-rate
proof; high-cost environment proof may be delegated only when lower-level
evidence proves the current PR path.
+6. Code quality: no concrete maintainability blocker remains, such as unclear
responsibility, duplicated logic, dead code, over-complex control flow, hidden
state, magic values, or hard-to-read temporary design.
+7. Architecture: shared modules, public/shared APIs, SPI contracts, metadata,
rule owners, dialect owners, session/executor state, and lifecycle state
preserve explicit ownership and contracts.
+8. Release note and user docs: required entries are present when users need
upgrade, troubleshooting, configuration, compatibility, migration, rollback, or
release-awareness guidance; low-signal entries are not required for
internal/test-only fixes.
+9. Breaking change and migration: default behavior, config keys, API/SPI
contracts, protocols, metadata storage, SQL semantics, and released artifacts
have clear compatibility, migration, upgrade, and rollback evidence when
touched.
+10. Diagnostics: changed errors, logs, or diagnostic output remain accurate,
actionable, and safe.
+11. Dependency and distribution: manifests, lockfiles, packaging, native-image
metadata, LICENSE, NOTICE, and release artifacts are checked for security,
license, compatibility, packaging, and release impact when touched.
## Not Mergeable Feedback Mode
@@ -132,8 +104,8 @@ Choose the feedback mode before writing the GitHub-facing
review:
- Review PR code, tests, behavior, compatibility, regression risk, and scope.
- For GitHub PRs, derive the reviewed file list from the latest PR head and
GitHub `/pulls/{number}/files`, then use local git only to reproduce and
inspect that scope.
-- Do not inspect or use GitHub Actions, CI status, or check-run completion for
the merge decision unless the user explicitly asks for CI review.
-- Do not treat CI pending, failing, or passing as a review finding by default;
final approvers and mergers handle that gate.
+- Do not base the review result solely on GitHub Actions, CI status, or
check-run completion.
+- Inspect CI when it is relevant under `Evidence Sufficiency and CI Judgment`;
otherwise do not wait for CI just for formality.
- Use the repository-declared formatting and style gates as authority. For
ShardingSphere, Spotless and Checkstyle are the formatting/style gates.
- Do not treat `git diff --check` as a blocker when it conflicts with
Spotless/Checkstyle behavior, unless the user explicitly asks for that check.
@@ -142,7 +114,7 @@ Choose the feedback mode before writing the GitHub-facing
review:
- Still include import-only, whitespace-only, and formatter-only files in
`Reviewed Scope` when GitHub `/pulls/{number}/files` includes them.
- `import-only` includes normal imports, static imports, import ordering,
import grouping, and unused-import cleanup when there is no production or test
behavior change.
- Do not report import-only, whitespace-only, or formatter-only changes as
`Issues`, `Unrelated Changes`, or rollback requests by default.
-- Do not set `Merge Decision: Not Mergeable` solely because of import
ordering, unused-import cleanup, whitespace normalization, or IDE/formatter
cleanup.
+- Do not set `Review Result: Not Mergeable` solely because of import ordering,
unused-import cleanup, whitespace normalization, or IDE/formatter cleanup.
- Mention them only when they are excessive, obscure the real diff, fail
Spotless/Checkstyle, touch many unrelated files, or conflict with an explicit
reviewer/user/repo scope rule.
## PR Diff Boundary Rule
@@ -175,18 +147,12 @@ For the GitHub-facing review body, cite or summarize only
public evidence from t
Local verification may support the review, but output only sanitized command
summaries, exit codes, and repository-relative paths.
Do not output local absolute paths, random temporary file or directory names,
private/internal/downstream identifiers, private chat content, prompt text, or
intermediate reasoning notes.
-CI status and check-runs are out of scope unless explicitly requested by the
user.
+CI status and check-runs are review evidence only when relevant under
`Evidence Sufficiency and CI Judgment`.
For SQL parser reviews:
-- Prefer the target database's official SQL reference/manual as first-class
evidence.
-- When the PR or your review comment mentions a concrete SQL syntax form, cite
the exact official doc page that supports that form.
-- If the official docs do not clearly support the syntax, do not infer support
from another dialect, secondary article, or parser implementation alone.
-- Build a dialect-family evidence set when parser logic changes:
- - Check repo conventions for trunk/branch relationships first.
- - If the touched dialect is a trunk parser, inspect affected branch dialect
parser files, tests, and doc paths.
- - If the touched dialect is a branch parser, inspect the trunk parser and
any sibling branch dialects that may share or copy the same logic.
-- Check ShardingSphere docs/examples/release notes when parser behavior
changes, especially if the PR alters supported syntax, unsupported syntax, or
dialect-specific examples.
+- If SQL grammar, visitors, parser tests, SQL syntax docs, dialect parser
behavior, or parser-generated baselines are touched, read
`references/sql-parser-review.md` before reviewing.
+- Apply the official-documentation, dialect-family, docs/example, and
parser-baseline rules from that reference.
Forbidden sources:
@@ -220,7 +186,7 @@ Rules:
- In the current reply, prioritize `Summary`, blocking issues, and minimum
next actions.
- If the PR is obviously too large (too many files or too much churn), suggest
splitting first.
-- If full review cannot be completed immediately, provide high-risk blockers
first to avoid blocking the delivery chain.
+- If full review cannot be completed immediately, provide only confirmed
high-risk blockers; use `Review Incomplete` when required public facts are
still missing.
## Quick Triage
@@ -236,28 +202,25 @@ Before deep review, answer the smallest set of questions
needed to choose review
Triage policy:
- Information complete: proceed with full review.
-- Missing evidence: mark as "not mergeable" and request minimum additional
info.
-- Wrong problem model, root-cause model, or direction: mark as "not
mergeable", use `Feedback Mode: Needs Discussion`, and recommend `type:
discussion`.
-- Any substantive off-topic/unrelated changes or substantive scope expansion:
mark as "not mergeable" and require rollback or scope narrowing.
+- Confirmed blocker: set `Review Result: Not Mergeable` and request the
minimum required change or discussion.
+- Required public facts unavailable, inaccessible, stale, or unattributable:
set `Review Result: Review Incomplete` and request only the facts needed to
complete the review.
+- Wrong problem model, root-cause model, or direction: set `Review Result: Not
Mergeable`, use `Feedback Mode: Needs Discussion`, and recommend `type:
discussion`.
+- Any substantive off-topic/unrelated changes or substantive scope expansion:
set `Review Result: Not Mergeable` and require rollback or scope narrowing.
Ignore non-behavioral import-only, whitespace-only, and formatter-only churn
for mergeability unless it meets the Non-Behavioral Churn Rule escalation
conditions.
- Change set too large: request split first, and provide only blocker-level
feedback for current version.
-## Minimum Additional Information List (Fixed Template)
+## Minimum Required Information
-When information gaps block mergeability, request at least:
+When information gaps prevent a reliable review result:
-- Recheck scope for the PR latest version (files/modules).
-- ShardingSphere version and runtime topology (JDBC/Proxy +
Standalone/Cluster).
-- Database type and version.
-- Minimal reproducible input (SQL/request/config snippet) with expected vs
actual behavior.
-- Key logs or stack traces.
-- Test evidence mapped one-to-one with fix points (new or adjusted tests).
-- Release note, user documentation, migration, or diagnostics evidence when
the PR has user-visible impact.
-- For SQL parser reviews: official documentation links/pages for the exact
syntax and version, plus any affected ShardingSphere doc paths or examples.
+- Request only facts required by the unresolved gate; do not ask for a fixed
checklist by default.
+- Do not ask the author for evidence the reviewer can obtain from public PR,
issue, code, CI, or workflow data.
+- Map every requested item to the unresolved review gate.
+- Common requests include latest changed-file scope, runtime topology,
database type/version, minimal reproducible input, key logs, targeted test
evidence, docs/release impact, or SQL parser official documentation.
## Review Workflow
-CI/check-run review is not part of this workflow unless explicitly requested;
do not query or report it by default.
+CI/check-run review is not a substitute for code review. Query and report CI
only when it is relevant under `Evidence Sufficiency and CI Judgment`.
1. Define target and boundary: restate PR goal, impacted modules, target
topology (JDBC or Proxy, Standalone or Cluster).
2. Root-cause and linked-issue modeling: reconstruct "trigger condition ->
failure path -> result" from issue and code path.
@@ -279,11 +242,7 @@ CI/check-run review is not part of this workflow unless
explicitly requested; do
- Compatibility: behavior/config/API-SPI/SQL dialect versions
- Regression: similar statements, adjacent features, exception branches
- For parser, binder, routing, and default-schema changes, explicitly
compare the new behavior against official dialect semantics and check whether
precedence or shadowing rules changed
- - For SQL parser changes, build the dialect-family map first using repo
conventions, then expand the review to related trunk / branch dialects that
reuse, share, or copy the touched parser logic
- - For SQL parser changes, verify every changed syntax form in the PR
against the target database's official documentation, and reject syntax support
that cannot be proven from official docs
- - For SQL parser review comments, ensure every suggested SQL syntax example
or recommended acceptance/rejection rule is also supported by the target
database's official documentation; do not suggest parser behavior that official
docs do not support
- - For each related dialect in the same parser family, decide whether the
same root cause exists, whether the PR also fixes it, and whether extra review
feedback is required; do not silently treat unreviewed related dialects as safe
- - Check ShardingSphere docs, examples, and release-note expectations for
parser behavior changes; if docs and parser behavior diverge, require
correction or explicit explanation
+ - For SQL parser changes, read and apply `references/sql-parser-review.md`
- If shared code is touched, build a blast-radius list of affected
dialects/features and review at least one non-target example against its
documented semantics
- If config flags or temporary properties exist on the touched path, review
both enabled and disabled states
- If exceptions, error codes, logs, or diagnostics changed, check that
users can understand and act on the new output and that no sensitive data is
exposed
@@ -296,7 +255,7 @@ CI/check-run review is not part of this workflow unless
explicitly requested; do
- If native-image, container, cluster, performance, or other expensive
environment validation is missing, decide whether the current PR truly owns
that integration proof,
or whether public-path tests plus code evidence are enough for this split
PR and the environment proof belongs to a broader umbrella PR.
- Do not require coverage-rate proof as part of this review.
- - For SQL parser family scans, check whether each related dialect with the
same root cause has direct validation or explicit evidence for non-applicability
+ - For SQL parser family scans, use `references/sql-parser-review.md` to
check validation or non-applicability evidence
- Distinguish fixture-assisted validation from production-path validation;
if tests bypass the real assembly chain,
metadata loader, SPI discovery path, or routing path, state that gap
explicitly and downgrade confidence
- If the PR adds multiple static metadata definitions, verify that
regression tests cover the originally reported objects one-to-one; do not infer
coverage from a single representative object unless the code path is truly
identical and that equivalence is stated
@@ -322,64 +281,43 @@ CI/check-run review is not part of this workflow unless
explicitly requested; do
11. Latest delta plus full-path review:
- When new commits arrive after previous feedback, review the latest delta
to classify newly introduced risk.
- Re-run full-path review on the latest PR head; do not conclude
mergeability only because earlier comments were fixed.
-12. Self-iteration gate: repeat internal review passes until the latest pass
finds no new actionable findings with an independent fix boundary.
-13. Merge decision: output `Merge Decision`.
+12. Pre-publication finding audit: verify every blocker against the evidence
threshold before output.
+13. Review result: output exactly one `Review Result`.
14. Generate feedback: follow the output template below.
-## Anti-Drip Review Gate
+## Pre-Publication Finding Audit
Before producing the final review, build and freeze an internal review
inventory for the latest PR head.
-Do not expose intermediate findings, draft issue lists, or candidate blockers
before the inventory is frozen, unless the user explicitly asks for status or
early high-risk blockers.
+Do not expose intermediate findings, draft issue lists, or candidate blockers
unless the user explicitly asks for status or early high-risk blockers.
The inventory must cover:
- Authoritative scope: latest head SHA, base ref/SHA, merge-base, local file
list, and GitHub `/pulls/{number}/files` match status when available.
-- Changed file categories: production, tests, docs, release notes,
build/config, distribution, generated/baseline resources, and non-behavioral
churn.
-- Entry points and execution paths changed by the PR.
-- New public production types and whether each has direct focused tests.
-- New or changed public/shared methods, constructors, fields, return values,
cache keys, and session/executor state.
-- Stateful registries, caches, session fields, handles, lifecycle
begin/use/free/release/error paths, and cleanup ownership.
-- Supported feature matrix: what the PR accepts, rejects, or leaves
unsupported; flag unsupported-but-accepted inputs.
-- Boundary cases: empty, null, invalid, stale, repeated, split/coalesced,
disabled, fallback, release/free, and error paths.
+- Changed file categories, entry points, execution paths, public/shared APIs,
stateful registries or lifecycle handles, tests, docs, release notes, generated
resources, dependencies, and distribution impact.
+- Boundary cases relevant to the change: empty, null, invalid, stale,
repeated, disabled, fallback, cleanup, release/free, and error paths.
- Latest-commit delta risks when previous public feedback or review rounds
exist.
-- Release note, user documentation, migration, diagnostics, dependency, and
distribution impact.
-
-For each candidate issue, record internally:
-
-- Evidence path and line.
-- Whether it is caused by this PR, pre-existing on base, exposed by this PR,
newly introduced by latest commits, or newly discovered but present in earlier
PR revisions.
-- The minimum independent fix boundary.
-- Whether it duplicates another candidate issue.
-- Whether it is a confirmed blocker, missing-evidence blocker, non-blocking
risk, or out-of-scope note.
-
-Deduplicate findings by independent fix boundary before output.
-A fix boundary is independent when it requires a different code owner,
lifecycle hook, protocol/model contract, validation boundary, or test contract
to close safely.
-Merge duplicate symptoms into one issue; split only when fixes are genuinely
independent.
-
-## Self-Iteration Gate
-
-Before producing the final review output, run an internal self-review loop on
the latest PR version:
-
-1. Build the current candidate findings from the frozen review inventory.
-2. Ask explicitly:
- "If I review this same latest PR again from a fresh critical perspective,
can I find any new actionable issue, unresolved risk, missing evidence, or
scope problem not already captured?"
-3. Re-run the review against the authoritative PR scope, focusing on:
- - Missed root-cause, problem-model, or feedback-mode gaps.
- - Missed side effects, regression paths, test adequacy gaps, cross-dialect
paths, feature-disabled paths, fallback paths, or boundary cases.
- - Missed ownership, implicit-state, unrelated-change, release note, user
documentation, migration, diagnostics, dependency, distribution,
output-template, or evidence gaps.
-4. Include at least one explicit adversarial pass that assumes the PR is
unsafe and actively searches for:
- - one cross-dialect or adjacent-feature regression path,
- - one config-disabled or feature-flag-off path,
- - one original symptom path that is only partially covered by tests.
- If any of these remain unresolved, set `Merge Decision: Not Mergeable`.
-5. Include at least one latest-delta pass when new commits were added after
previous public feedback, and one full-path pass on the latest PR head.
-6. If the self-review finds any new actionable issue with an independent fix
boundary, add it to the inventory, deduplicate it against existing findings,
- update the merge decision and next steps if needed, and repeat the loop.
-7. Do not reset the loop for duplicate symptoms, optional polish, speculative
risks outside the PR scope, or already captured issues.
-8. Stop only after one full adversarial pass finds no new actionable issue
with an independent fix boundary.
-9. If the inventory cannot be completed because public evidence is
unavailable, state the minimum missing evidence and set `Merge Decision: Not
Mergeable` rather than emitting a partial approval.
-10. Do not expose intermediate review rounds, draft decisions, raw inventory,
or self-review transcripts in GitHub-facing output.
-11. Produce one consolidated final review with exactly one `Merge Decision`.
+
+For each candidate finding, record internally:
+
+- Evidence type: code/diff, test/CI/log, linked issue, docs/spec, generated
artifact, or local verification.
+- Evidence path, line, command, CI run, or public anchor.
+- Whether the finding is caused by this PR, pre-existing on base, exposed by
this PR, newly introduced by latest commits, or newly discovered but present
earlier.
+- Minimum independent fix boundary and duplicate relationship.
+- Classification: confirmed blocker, review-incomplete gap, non-blocking
observation, or out-of-scope note.
+
+Before any candidate enters `### Issues`, verify:
+
+- The evidence type matches the claimed root cause and linked issue.
+- No public counter-evidence invalidates the claim.
+- Reviewer uncertainty, skipped local verification, unavailable tools, or
inaccessible GitHub data are not being converted into a PR blocker.
+- The requested action is necessary in the current PR scope.
+- The blocker satisfies `Evidence Sufficiency and CI Judgment`.
+
+Run an adversarial pass on the latest head that looks for missed root-cause
gaps, side effects, feature-disabled paths, adjacent-feature regressions,
ownership issues, release/doc impacts, and required verification gaps.
+If the pass finds any new actionable finding with an independent fix boundary,
add it to the inventory, deduplicate and classify it, update the review result
if needed, and repeat the pass.
+Stop only after one full adversarial pass finds no new actionable finding.
+If the inventory cannot be completed because required public evidence is
unavailable or unattributable, output `Review Result: Review Incomplete`.
+Produce one consolidated review with exactly one `Review Result`.
## Root-Cause Validation Checklist (Must Answer Each)
@@ -395,7 +333,8 @@ Before producing the final review output, run an internal
self-review loop on th
- Do the target database official docs and ShardingSphere docs/examples both
support the resulting parser behavior, or is a documentation mismatch still
open?
- Does the config-disabled or feature-flag-off path still behave correctly?
-If the root-cause chain cannot be fully proven fixed, set `Merge Decision: Not
Mergeable`.
+If public evidence proves the root-cause chain is not fixed, set `Review
Result: Not Mergeable`.
+If the root-cause chain depends on required public facts that are unavailable
or unattributable, set `Review Result: Review Incomplete`.
## Linked Issue Completeness Gate
@@ -411,11 +350,15 @@ Must answer:
- Are any issue requirements left as future work, partial support, unsupported
topology, or untested behavior?
- Does the PR title/body over-claim the fix compared with the actual
implementation scope?
- If the PR narrows the issue scope, is that limitation explicit, technically
justified, and accepted by maintainers or the issue context?
+- What evidence type would prove the issue's root cause still exists, such as
generated metadata, protocol trace, SQL parser official syntax, runtime log,
config output, or changed code path?
+- Does the review claim use that evidence type, or is it inferring from an
adjacent fact that does not prove the root cause?
- If the issue expectation, compatibility boundary, or ownership boundary is
itself contradicted or unresolved in public evidence,
should the PR pause implementation and reopen discussion before patch-level
requests continue?
-If the linked issue cannot be read, the issue scope is ambiguous, or any
required issue behavior is only partially fixed or unvalidated,
-set `Merge Decision: Not Mergeable` and list the minimum missing
implementation or evidence.
+If public evidence proves that a required issue behavior is only partially
fixed or unvalidated after reasonable public verification,
+set `Review Result: Not Mergeable` and list the minimum missing implementation
or evidence.
+If the linked issue cannot be read, issue scope is ambiguous, or required
public evidence is unavailable or unattributable,
+set `Review Result: Review Incomplete`.
If public issue or PR evidence shows the current implementation direction is
based on a wrong root-cause model or unresolved expected behavior,
use `Feedback Mode: Needs Discussion` instead of patch-level change requests.
@@ -437,7 +380,7 @@ Must answer:
- Could the same behavior be expressed with a non-null generic key, explicit
state object, clear enum, target-module-owned lifecycle API, or documented
absence-return contract?
- Do tests validate generic shared behavior separately from target dialect
activation?
-If target-specific semantics leak into shared code, or if implicit state is
used as a mode switch in new/changed shared APIs, set `Merge Decision: Not
Mergeable`.
+If target-specific semantics leak into shared code, or if implicit state is
used as a mode switch in new/changed shared APIs, set `Review Result: Not
Mergeable`.
## Risk Checklist (Must Cover)
@@ -456,7 +399,7 @@ If target-specific semantics leak into shared code, or if
implicit state is used
- Documentation risk: user-visible behavior changes without matching
official-doc support, ShardingSphere docs/examples, or release notes that are
valuable and required for the current PR.
- Migration risk: breaking behavior, config, API/SPI, protocol, metadata
storage, SQL semantic, distribution, or rollback impact without clear
user-facing guidance.
- Diagnostics risk: error messages, error codes, logs, or troubleshooting
output that is misleading, unactionable, regressed, or unsafe.
-- Operational risk: config migration complexity, gray-release and rollback
complexity.
+- Operational risk: config migration complexity, staged rollout and rollback
complexity.
- Supply-chain/distribution risk: vulnerabilities, licenses, transitive
dependency changes, packaging changes, native-image metadata, LICENSE, NOTICE,
or release artifact changes.
## Boundary Validation Review Guidance
@@ -518,143 +461,29 @@ When GitHub-visible previous-round feedback exists,
perform incremental comparis
## Output Structure
-### GitHub Review Markdown Requirements
-
-- Format every review as GitHub-flavored Markdown that can be pasted directly
into a PR comment or review body.
-- The GitHub-facing review body must not be wrapped in a code fence,
blockquote, XML/HTML container, or plain-text transcript.
-- Use the same natural language as the user request unless the user explicitly
asks for another language.
-- Use Markdown headings (`### Summary`, `### Issues`, etc.) with a blank line
before and after each heading.
-- Keep the GitHub Markdown structure unchanged regardless of output language.
-- Keep `Merge Decision: ...` as a bold bullet in `Summary`, and output exactly
one merge-decision line.
-- Keep stable review labels in English, such as `Merge Decision`, `Feedback
Mode`, `Reason`, `Expert Review Needed`, `Reviewed Scope`, `Not Reviewed
Scope`, and `Verification`,
- so they remain searchable and consistent.
-- Do not include internal self-iteration rounds, draft decisions, or
self-review transcripts in the GitHub-facing review body.
-- Keep `Reason` to one sentence or one short bullet; put proof in `Evidence`,
and put confirmed blockers plus missing required proof in `Issues`.
-- Use `Evidence` for facts found in the reviewed artifacts, such as code
paths, tests, documentation, compatibility checks, and root-cause proof.
-- Use `Verification` only for reviewer-run local commands and exit codes, or
why those commands were not run.
-- Use short unordered bullets under each heading; use bold inline labels such
as `Problem:`, `Impact:`, `Required Change:`, and `Discussion Needed:` for
issue details.
-- In `### Issues`, list every merge-blocking issue discovered in the reviewed
scope; do not imply that other unlisted blocking issues exist.
-- Use only `P0`, `P1`, or `P2` for issue severity: `P0` for security,
data-loss, metadata corruption, or broken core behavior; `P1` for confirmed
functional regressions,
- incomplete root-cause fixes, incompatible behavior, or high-risk side
effects; `P2` for lower-risk but still merge-blocking defects, missing targeted
validation, or required scope cleanup.
-- Omit optional nits from `### Issues` unless they are part of a
merge-blocking pattern.
-- If missing required evidence blocks mergeability, mention that in `Summary`
-> `Reason`, and include the detailed blocker in `### Issues`.
-- Only missing evidence that blocks mergeability belongs in `### Issues`;
non-blocking verification gaps belong in `Review Details` -> `Verification`.
-- Do not put blocking missing-evidence requests only in `Review Details`; that
section is for review scope and verification facts.
-- Omit `### Next Steps` unless it adds non-duplicative cross-issue sequencing,
verification commands, or minimum missing information.
-- Use repo-relative paths with line numbers for file evidence, for example
`infra/.../Foo.java:123`; do not use local absolute file paths in GitHub-facing
review text.
-- For non-file direction evidence, use public anchors such as `PR
description`, `linked issue`, `review thread`, or official documentation links.
-- Prefer bullets over tables. Use tables only for compact status summaries
that remain readable in GitHub's narrow review pane.
-- Keep command evidence in inline code or short fenced blocks; avoid long raw
JSON, full logs, or unrendered terminal transcripts.
-- Before final output, perform a formatting self-check on the inner
GitHub-facing review body:
- - The inner GitHub-facing review body is not wrapped in a code fence,
blockquote, XML/HTML container, or transcript.
- - The inner GitHub-facing review body contains the required `###` headings
for the selected decision template.
- - The inner GitHub-facing review body contains exactly one bold `Merge
Decision: ...` line.
- - File references are repo-relative paths with line numbers, and non-file
direction evidence uses public anchors.
- - Stable labels remain in English.
- - The inner GitHub-facing review body contains no non-public downstream
project names, private/internal repository names, customer-specific or
vendor-specific context,
- private chat content, prompt process, internal self-review notes,
local-only archive details, private migration background, or intermediate
discussion results.
- - Local verification evidence is sanitized: no local absolute paths, home
directories, random temporary file or directory names, tokens, private
addresses, or undisclosed vulnerability details.
-
-### Codex Chat Delivery
-
-- When returning the review in Codex chat for the user to copy, wrap the
GitHub-facing review body in a fenced `markdown` code block.
-- The fenced code block is only a chat delivery wrapper; it is not part of the
GitHub-facing review body.
-- Tell the user to copy only the content inside the fenced block.
-- Keep any copy instruction outside the fenced block, and keep it free of
non-public downstream, private/internal, customer-specific, vendor-specific,
- prompt-process, or intermediate-discussion details.
-- When posting directly to GitHub through an API or tool, submit only the
inner GitHub-facing review body and do not include the outer fence.
-- Apply the formatting self-check to the inner GitHub-facing review body, not
to the chat delivery wrapper.
-
-### A. Not Mergeable (Change Request)
-
-Use committer tone, gentle wording, no emojis; use this GitHub Markdown
skeleton for required sections:
-
-When required, add `- **Expert Review Needed:** ...` under `Summary`; omit
this bullet when no expert review is required.
-
-```markdown
-### Summary
-
-- **Merge Decision: Not Mergeable**
-- **Feedback Mode: Change Request**
-- **Reason:** ...
-
-### Issues
-
-- **[P0|P1|P2] Short issue title** (`path/to/File.java:123`)
- - **Problem:** ...
- - **Impact:** ...
- - **Required Change:** Please ...
-
-### Review Details
-
-- **Reviewed Scope:** ...
-- **Not Reviewed Scope:** ...
-- **Verification:** ...
-- **Release Note / User Docs:** ...
-```
-
-Optional sections for `Not Mergeable`; do not output optional headings with
placeholder text:
-
-- `### Positive Feedback`: insert after `Summary` only when there is a
genuinely correct direction-aligned change.
-- `### Unrelated Changes`: insert after `Issues` only when substantive
unrelated changes or substantive scope expansion exist, and explicitly ask for
rollback or scope narrowing.
-- `### Next Steps`: insert before `Review Details` only when it adds
non-duplicative cross-issue sequencing, verification commands, or minimum
missing information.
-- `### Multi-Round Comparison`: insert before `Review Details` only when
previous-round feedback exists in GitHub-visible PR comments, review threads,
or change requests.
-
-### B. Not Mergeable (Needs Discussion)
-
-Use this template when the current PR direction, root-cause model, problem
framing, expected behavior, or ownership boundary must be reopened before
implementation continues.
-Do not include patch-level `Required Change` bullets for the current
implementation direction.
-
-When required, add `- **Expert Review Needed:** ...` under `Summary`; omit
this bullet when no expert review is required.
-
-```markdown
-### Summary
-
-- **Merge Decision: Not Mergeable**
-- **Feedback Mode: Needs Discussion**
-- **Reason:** ...
+GitHub-facing review bodies must be GitHub Markdown, use the user's language
unless requested otherwise, and contain exactly one bold `Review Result: ...`
line under `### Summary`.
+Do not wrap GitHub-facing text in a code fence, blockquote, XML/HTML
container, or transcript.
+Use stable English labels: `Review Result`, `Feedback Mode`, `Reason`,
`Reviewed Scope`, `Not Reviewed Scope`, `Verification`, and `Release Note /
User Docs`.
+Use repo-relative file references with line numbers, public anchors for
non-file evidence, and sanitized verification summaries.
+Do not include internal drafts, self-review notes, private context, local
absolute paths, temp paths, tokens, or raw long logs.
-### Discussion Needed
+When returning a review in Codex chat for the user to copy, wrap only the
GitHub-facing body in a fenced `markdown` block.
+When posting directly through a tool, submit only the inner GitHub-facing body.
-- **Problem:** ...
-- **Impact:** ...
-- **Discussion Needed:** Please pause the current implementation direction and
confirm ...
-- **Suggested Label:** `type: discussion`
+Use these required structures:
-### Review Details
-
-- **Reviewed Scope:** ...
-- **Not Reviewed Scope:** ...
-- **Verification:** ...
-- **Release Note / User Docs:** ...
-```
+- `Mergeable`: `### Summary` with `Review Result` and `Reason`; `###
Evidence`; `### Review Details`.
+- `Not Mergeable`: `### Summary` with `Review Result`, exactly one `Feedback
Mode`, and `Reason`; `### Issues`; `### Review Details`.
+- `Review Incomplete`: `### Summary` with `Review Result` and `Reason`; `###
Incomplete Reason`; `### Verified Facts`; `### Required Evidence`; `### Review
Details`.
+- `Correction`: prepend `### Correction` with `Previous Finding`, `Current
Status` (`Retained`, `Withdrawn`, or `Changed to Review Incomplete`), and
`Reason`;
+ then output the applicable current result structure with exactly one bold
`Review Result` line under `### Summary`.
+For `Not Mergeable`, choose `Feedback Mode: Change Request` for patch-level
required changes or `Feedback Mode: Needs Discussion` when the current
direction, root-cause model, problem framing, expected behavior, or ownership
boundary must be reopened.
+Do not include patch-level `Required Change` bullets after choosing `Needs
Discussion`.
+Use `P0`, `P1`, or `P2` issue severity, and include `Problem`, `Impact`, and
either `Required Change` or `Discussion Needed`.
Use `status: need more info` instead of `type: discussion` only when missing
public evidence blocks root-cause or scope classification.
-
-### C. Mergeable
-
-Use this GitHub Markdown skeleton:
-
-When required, add `- **Expert Review Needed:** ...` under `Summary`; omit
this bullet when no expert review is required.
-
-```markdown
-### Summary
-
-- **Merge Decision: Mergeable**
-- **Reason:** ...
-
-### Evidence
-
-- Root-cause fix evidence.
-- Hard gate evidence covering scope/ownership, regression risk, tests, code
quality, architecture, and user-facing release/docs impact.
-
-### Review Details
-
-- **Reviewed Scope:** ...
-- **Not Reviewed Scope:** ...
-- **Verification:** Reviewer-run local verification and exit codes, or why
local verification was not run.
-- **Release Note / User Docs:** Required and verified, delegated to an
umbrella PR with reason, missing, not required with reason, or not reviewed.
-```
+Optional `Not Mergeable` sections are `Positive Feedback`, `Unrelated
Changes`, `Next Steps`, and `Multi-Round Comparison`; omit placeholder headings.
+`Review Incomplete` must not contain patch-level code suggestions. If evidence
already supports a concrete code change, use `Review Result: Not Mergeable`.
## Feedback Tone Guidelines
@@ -670,18 +499,20 @@ When required, add `- **Expert Review Needed:** ...`
under `Summary`; omit this
## Prohibited Items
-- Do not output `Mergeable` when evidence is insufficient or risks are unclear.
+- Do not output `Review Result: Mergeable` when evidence is insufficient,
risks are unclear, or relevant CI is failing.
+- Do not output `Review Result: Not Mergeable` for reviewer uncertainty,
inaccessible public facts, unavailable tools, or skipped local verification;
use `Review Incomplete` when those facts are required.
- Do not use "fallback logic passes tests" to replace proof of root-cause
repair.
- Do not treat fixture-injected or mocked-path tests as full end-to-end proof
without explicitly stating the gap.
-- Do not output `Mergeable` only because previous-round blockers were closed;
always do one fresh-pass semantic and regression scan on the latest head.
+- Do not output `Review Result: Mergeable` only because previous-round
blockers were closed; always do one fresh-pass semantic and regression scan on
the latest head.
- Do not ignore substantive unrelated changes.
- Do not reuse old conclusions after new commits are added without re-review.
- Do not include emojis in review feedback text.
-- Do not inspect or report GitHub Actions / CI status unless explicitly
requested.
+- Do not use CI success as the sole reason for `Mergeable`.
+- Do not ignore CI/check-run evidence when it is relevant under `Evidence
Sufficiency and CI Judgment`.
- Do not include non-public downstream project names, private/internal
repository names, customer-specific or vendor-specific context, private chats,
prompt process, internal archive details,
private migration background, intermediate discussion results, local
absolute paths, or unsanitized temporary file names in GitHub-facing review
output.
- Do not output patch-level `Required Change` requests after selecting
`Feedback Mode: Needs Discussion`.
-- Do not output `Mergeable` when a required hard gate remains unresolved,
including missing required test evidence, release notes, user docs, migration
guidance, or diagnostic quality evidence.
-- Do not output `Mergeable` for a shared-code change unless you have checked
at least one non-target dialect or feature that also uses the changed path.
-- Do not output `Mergeable` when local verification omitted `-am` on a
module-scoped Maven run and dependency freshness matters.
-- Do not output `Mergeable` when Proxy/JDBC DML/DQL high-frequency SQL paths
directly call `ConcurrentHashMap#computeIfAbsent` without a preceding `get`
miss check.
+- Do not output `Review Result: Mergeable` when a required hard gate remains
unresolved, including missing required test evidence, release notes, user docs,
migration guidance, or diagnostic quality evidence.
+- Do not output `Review Result: Mergeable` for a shared-code change unless you
have checked at least one non-target dialect or feature that also uses the
changed path.
+- Do not output `Review Result: Mergeable` when local verification omitted
`-am` on a module-scoped Maven run and dependency freshness matters.
+- Do not output `Review Result: Mergeable` when Proxy/JDBC DML/DQL
high-frequency SQL paths directly call `ConcurrentHashMap#computeIfAbsent`
without a preceding `get` miss check.
diff --git a/.codex/skills/review-pr/agents/openai.yaml
b/.codex/skills/review-pr/agents/openai.yaml
index 8704b7c5e31..68243ba55b1 100644
--- a/.codex/skills/review-pr/agents/openai.yaml
+++ b/.codex/skills/review-pr/agents/openai.yaml
@@ -20,6 +20,6 @@ interface:
short_description: "Root-cause-first merge review and discussion triage"
default_prompt: >-
Use $review-pr to check whether this ShardingSphere PR fixes the root
cause,
- assess design/performance/compatibility risks, produce a merge decision,
and
- output committer-style change requests or needs-discussion feedback in the
same
- language as the user.
+ assess design/performance/compatibility risks, produce a Mergeable,
+ Not Mergeable, or Review Incomplete result, and output committer-style
+ feedback in the same language as the user.
diff --git a/.codex/skills/review-pr/references/sql-parser-review.md
b/.codex/skills/review-pr/references/sql-parser-review.md
new file mode 100644
index 00000000000..022fa0918b1
--- /dev/null
+++ b/.codex/skills/review-pr/references/sql-parser-review.md
@@ -0,0 +1,49 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+# SQL Parser Review Reference
+
+Read this reference before reviewing PRs that touch SQL grammar, SQL visitor
classes, parser tests, SQL syntax docs, dialect parser behavior, or
parser-generated baseline resources.
+
+## Official Evidence
+
+- Prefer the target database's official SQL reference/manual as first-class
evidence.
+- When the PR or review comment mentions a concrete SQL syntax form, cite the
exact official documentation page that supports that form.
+- If official docs do not clearly support the syntax, do not infer support
from another dialect, secondary article, parser implementation, or AI-reposted
content alone.
+- Reject syntax support that cannot be proven from official documentation
unless the PR explicitly scopes it as ShardingSphere-specific behavior and
maintainers accept that scope.
+
+## Dialect Family
+
+- Build the dialect-family map from repository conventions before judging
parser scope.
+- If the touched dialect is a trunk parser, inspect affected branch dialect
parser files, tests, and docs.
+- If the touched dialect is a branch parser, inspect the trunk parser and
sibling branch dialects that may share or copy the same logic.
+- For each related dialect, decide whether the same root cause exists, whether
the PR fixes it, and whether validation or non-applicability evidence is
present.
+- Do not silently treat unreviewed related dialects as safe.
+
+## Review Questions
+
+- Does the PR preserve precedence, name resolution, and shadowing semantics
for adjacent valid cases?
+- Would same-name or shadowing cases now take a different path?
+- If shared parser code is touched, would another dialect now diverge from its
documented semantics?
+- Are accepted, rejected, and unsupported syntax boundaries explicit?
+- Do parser tests cover each affected dialect or explain why the dialect is
not affected?
+- Do ShardingSphere docs, examples, release notes, and generated baselines
remain consistent with the parser behavior?
+
+## Output Requirements
+
+- In `Reviewed Scope`, name the target dialect, related trunk or branch
dialects checked, official documentation pages used, and repo docs or examples
checked.
+- If syntax evidence is missing or contradicted, classify the result using the
main skill's evidence sufficiency rules instead of guessing.
