Skip to content

feat(ci): reconnect PPL lint validation to the candidate runtime grammar - #2

Open
Hanyu-W wants to merge 10 commits into
mainfrom
ppl-lint-contract-all-rules
Open

feat(ci): reconnect PPL lint validation to the candidate runtime grammar#2
Hanyu-W wants to merge 10 commits into
mainfrom
ppl-lint-contract-all-rules

Conversation

@Hanyu-W

@Hanyu-W Hanyu-W commented Jul 20, 2026

Copy link
Copy Markdown
Owner

What

Takes the cross-repository PPL lint check from the PoC to the design in ppl-lint-ci-validation-design.md. The detector half now lints against the candidate runtime grammar bundle the SQL PR builds — through OSD's production headless lint API — instead of the compiled analyzer or a hand-rolled reparse of OSD main's checked-in grammar. Both halves validate the same grammar, so a parser/semantic change that invalidates a lint rule reds the build.

Changes (SQL side; the OSD headless API ships separately)

  • PplLintRuleValidationIT — exports the candidate grammar bundle (GET /_plugins/_ppl/_grammar) + a target manifest {engineVersion, grammarHash, grammarBundle} while the cluster is alive; reads schema-v3 specs; selects the one expectations[] entry matching the backend version (zero / >1 fails before execution); records observed backend behavior per query for the differential.
  • integ-test/build.gradle — forwards -Dppl.lint.grammar.bundle / -Dppl.lint.target to the test JVM alongside the existing ppl.lint.* knobs.
  • run-frontend-contract.mjs — deserializes the candidate bundle via the OSD headless API and lints each query with lintQueryWithBundle (runtime-bundle surface, so the runtime-only arity rules fire); pins dataSourceVersion + knownVersion to the candidate version; asserts the detector-vs-backend differential; fails loud on a missing bundle.
  • Workflow — linear backend-validation → detector-validation → validation-result; artifacts are the only bridge between jobs. validation-result is the single always() required check (red unless both jobs succeed) and writes the per-rule PR summary; assemble-run-manifest.mjs emits run-manifest.json with the immutable SQL + OSD SHAs, mode, backend version, grammar hash, and enforced set. A workflow_dispatch osd_ref run is pre-merge evidence, not a protection result.
  • Contracts — migrate all 9 specs to schema v3 (named queries{role,query} + version-scoped expectations[]); partition manifest.json into enforced (eventstats, multisearch, union, replace), pendingReview (field-validation), and nonEnforcing. Union/multisearch triggers are query-initial, not pipe-first: OSD prepends a synthetic source=t prefix to pipe-first queries, which would desync the two halves.
  • scripts/ppl-lint/README.md — documents inputs, local reproduction, the contract format, and the failure table.

Testing

Verified end to end against a live cluster:

  • Backend IT exports a real 248 KB candidate bundle + target.json; BUILD SUCCESSFUL.
  • Detector runner agrees on all four enforced rules (triggers rejected + 1 diagnostic, controls accepted + 0) — green, exit 0.
  • An intentional expectation mismatch reds the runner (exit 1).
  • Manifest assembler emits a valid run-manifest.json.
  • Java compile + spotless, and JSON/YAML/JS/shell syntax all pass.

⚠️ CI dependency

detector-validation checks out OSD main, but the @osd/monaco/ppl-lint + headless_ppl_lint API it needs is only on OSD commit 8a11123dca, not yet merged to OSD main. Until the OSD change merges, detector-validation will fail on a normal PR run; use workflow_dispatch with osd_ref=8a11123dca for pre-merge evidence (per the design's merge order).

Hanyu Wei added 5 commits July 19, 2026 16:54
Add a cross-repository CI check that keeps the OpenSearch-Dashboards PPL
lint rule 'unsupported-window-function-in-eventstats' and the SQL backend
in agreement.

Frontend half: a SQL-owned Node script loads the compiled OSD analyzer
from an OSD checkout and asserts the rule's diagnostic counts. Backend
half: a Gradle integration test sends the same queries to the live
/_plugins/_ppl endpoint of the SQL plugin built from the checkout. Both
halves consume one shared contract file.

- integ-test/.../ppl-lint/unsupported-window-function-in-eventstats.spec.json
- scripts/ppl-lint/run-frontend-contract.mjs
- integ-test/.../calcite/remote/PplLintRuleValidationIT.java
- .github/workflows/ppl-lint-rule-validation.yml
- scripts/ppl-lint-rule-validation.sh

Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
Addresses shellcheck SC2006/SC2046 on the chown/su lines so actionlint
runs clean. Behavior is unchanged.

Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
The OpenSearch CI container is Amazon Linux 2 (glibc 2.26), but OSD
requires Node 22 whose prebuilt binary needs glibc >= 2.27. Running the
Node frontend contract inside that container failed with 'GLIBC_2.27 not
found'.

Split into two required jobs: 'frontend' runs the OSD analyzer contract
on a bare ubuntu-latest runner (modern glibc, actions/setup-node works),
and 'backend' keeps the Gradle integration test in the CI container where
the OpenSearch test cluster needs it.

Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
Generalize the single-rule PPL lint validation contract (eventstats PoC) into
a schema-v2 corpus that pins every reachable OSD analyzer rule to live
/_plugins/_ppl behavior. Both halves read the same reviewed contract files so
neither the analyzer diagnostic nor the engine behavior can drift without a
red build. Verified live end-to-end: frontend 13/13 against the OSD main
analyzer; backend 13/13 (pr) and 21/21 (nightly) against a live test cluster.

Contract schema v2 (integ-test/src/test/resources/ppl-lint/contracts/*.spec.json
+ manifest.json):
- backend.kind discriminator: rejection | result-shape | advisory (explain
  reserved for the nightly-only explain rule class once it lands on OSD main).
- per-contract backendFixture.clusterSettings so contracts that disagree on
  fallback/join settings each set what they need (eventstats needs
  calciteFallback=false; dedup-consecutive needs true) — validated in one run.
- per-case minVersionRequired/engineRequired so both halves skip identically.
- wiring block asserted deep-equal against the OSD catalog (drift tripwire).
- frontendContext.deriveFromMapping single-sources fields/typeMap for the
  field-validation existence pass.
- error.reason values snapshotted from the observed engine response, not
  hand-typed (join/multisearch AST-build-time throws yield generic
  "Invalid Query"; union/replace carry the specific message).

Frontend adapter (run-frontend-contract.mjs): contract discovery via manifest,
schedule + version/engine gating, catalog wiring assertion, compiled-simplified
and runtime-bundle grammar surfaces (runtime-only rules whose parser rules are
absent on the checkout's grammar assert wiring then skip cleanly), collect-all
failures, and a frontend-report.json for disagreement diffing. Nightly adds a
coverage assertion that every enabled catalog rule has a contract.

Backend IT: parameterized over the contract corpus with per-kind verifiers
(verifyRejectedCase / verifyResultShape / verifyAdvisory200), per-contract
cluster-setting apply+reset, GET / cluster-version gating, and a
backend-report.json recording observed status/type/reason per rejection.
Sends queries with a JSON-escaped body so contract queries containing quotes
(grok field=body "...") reach the engine faithfully instead of tripping a core
request-payload parse error. Honors -Dppl.lint.schedule=pr|nightly, forwarded
to the forked test JVM via integ-test/build.gradle.

Workflow + repro script: derive schedule (PR -> pr, cron -> nightly), read the
contract dir, upload frontend/backend reports + corpus artifacts.

Rules covered: eventstats window fn, division-by-zero, head-without-sort,
disabled-join-type, field-validation (shape + existence) on PR; plus
dedup-consecutive and the runtime-only union/multisearch/replace on nightly.

Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
@Hanyu-W
Hanyu-W force-pushed the ppl-lint-contract-all-rules branch from ea3d456 to 1745ed9 Compare July 20, 2026 16:33
Take the cross-repository PPL lint check from the PoC to the design in
ppl-lint-ci-validation-design.md. The detector half now lints against the
*candidate* runtime grammar bundle the SQL PR builds — through OSD's production
headless lint API — instead of the compiled analyzer or a hand-rolled reparse
of OSD main's checked-in grammar. Both halves validate the same grammar, so a
parser/semantic change that invalidates a lint rule reds the build.

SQL-side changes (the OSD headless API ships separately):

- PplLintRuleValidationIT: export the candidate grammar bundle
  (GET /_plugins/_ppl/_grammar) + a target manifest {engineVersion, grammarHash,
  grammarBundle} while the cluster is alive; read schema-v3 specs; select the one
  expectations[] entry matching the backend version (zero/>1 fails); record the
  observed backend behavior per query for the differential.
- integ-test/build.gradle: forward -Dppl.lint.grammar.bundle / -Dppl.lint.target
  to the test JVM alongside the existing ppl.lint.* knobs.
- run-frontend-contract.mjs: deserialize the candidate bundle via the OSD
  headless API and lint each query with lintQueryWithBundle (runtime-bundle
  surface, so the runtime-only arity rules fire); pin dataSourceVersion +
  knownVersion to the candidate version; assert the detector-vs-backend
  differential from the backend report; fail loud on a missing bundle.
- workflow: linear backend-validation -> detector-validation -> validation-result
  pipeline; artifacts are the only bridge between jobs. validation-result is the
  single always() required check (red unless both jobs succeed) and writes the
  per-rule PR summary; assemble-run-manifest.mjs emits run-manifest.json with the
  immutable SQL + OSD SHAs, mode, backend version, grammar hash, and enforced set.
  A workflow_dispatch osd_ref run is pre-merge evidence, not a protection result.
- contracts: migrate all 9 specs to schema v3 (named queries{role,query} +
  version-scoped expectations[]); partition manifest.json into enforced
  (eventstats, multisearch, union, replace), pendingReview (field-validation),
  and nonEnforcing. Union/multisearch triggers are query-initial, not pipe-first:
  OSD prepends a synthetic source prefix to pipe-first queries, which would
  desync the two halves.
- Add scripts/ppl-lint/README.md documenting inputs, local reproduction, the
  contract format, and the failure table.

Verified end to end against a live cluster: the backend IT exports a real
candidate bundle and the detector runner agrees on all four enforced rules
(triggers rejected + 1 diagnostic, controls accepted + 0); an intentional
expectation mismatch reds the runner.

Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
@Hanyu-W Hanyu-W changed the title feat(ci): extend PPL lint contract to all reachable rules (schema v2) feat(ci): reconnect PPL lint validation to the candidate runtime grammar Jul 20, 2026
Hanyu Wei added 4 commits July 20, 2026 16:22
The detector-validation step piped the runner through `tee`, so the step took
tee's (success) exit status and a real runner failure — e.g. the OSD headless
module being absent on OSD main — went green as a vacuous pass. The first live
PR run hit exactly this: the runner exited 2 with "Expected OSD module not
found ... headless_ppl_lint", yet detector-validation and validation-result
both reported success. Add `set -o pipefail` so node's non-zero exit propagates
and the required check correctly reds until the OSD headless API merges.

Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
detector-validation hardcoded repository: opensearch-project/OpenSearch-Dashboards,
so osd_ref could only resolve commits/branches that exist upstream. An unmerged
OSD change on a fork (e.g. the headless lint API before it lands on OSD main)
could not be validated end to end.

Add an osd_repo workflow_dispatch input (default
opensearch-project/OpenSearch-Dashboards) that the OSD checkout honors, thread
osd_repo through the detector job output into the run manifest + PR summary, and
treat any non-upstream-main target as osd-branch-evidence (never a required
check). The required pull_request run is unchanged: it still checks out upstream
OSD main.

Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
…te against the paired OSD ref

The detector-validation job imports OSD's headless lint API
(src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint), which is
not yet on OSD main — it lives on the paired branch
Hanyu-W/OpenSearch-Dashboards@ppl-lint-headless-api. The required
pull_request run hardcoded opensearch-project/OpenSearch-Dashboards@main, so
it failed on 'Expected OSD module not found' while the fork-targeting
workflow_dispatch run passed.

Resolve the OSD repo/ref in precedence order: workflow_dispatch input >
OSD_REPO/OSD_REF repo variables > canonical opensearch-project/...@main. The
committed default stays main; the temporary fork override lives in mutable
repo variables and reverts by deleting them once the OSD PR merges.

Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
Mirror the sibling SQL Java workflows explicitly (header comment) and make
the detector job more robust without changing what it validates:

- Note that the workflow reuses Get-CI-Image-Tag, the OpenSearch CI container
  + ci-image-start-command, and the chown/su non-root Gradle pattern from
  sql-test-and-build-workflow.yml, and that action SHAs match the siblings.
- Record measured CI cost: bootstrap (~2m13s, CPU-bound even with a warm yarn
  cache) dominates; the lint is ~2s. Hence no per-contract matrix, and why
  overlapping bootstrap with the backend job is a tracked follow-up.
- Download the backend artifact AFTER bootstrap so a flaky artifact download
  cannot waste a completed bootstrap.
- Add a bootstrap retry-with-backoff loop (mirrors the OSD build workflow).
- Add timeout-minutes: 30 to both validation jobs.

Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant