From 1bb8d2152f859a88825ec8f2837b8bb52fb5b5ea Mon Sep 17 00:00:00 2001 From: Hanyu Wei Date: Tue, 4 Aug 2026 11:14:22 -0700 Subject: [PATCH 1/5] feat(ci): enforce exact PPL lint frontend contracts Pin exact diagnostic, deterministic-fix, and AI-action behavior for all active schema-v4 contracts, and promote the complete 13-contract corpus to required PR validation. Enforce the shipping census while retaining dormant detector contracts as report-only observations. Propagate frontend assertion failures through required and multi-version reports with inline annotations. Signed-off-by: Hanyu Wei --- .../ppl-lint-multiversion-validation.yml | 2 + .../workflows/ppl-lint-rule-validation.yml | 6 +- .../ppl-lint/contracts/agg-on-text.spec.json | 28 +- .../contracts/command-suggestion.spec.json | 13 +- .../contracts/division-by-zero.spec.json | 52 +- .../contracts/enabled-false-object.spec.json | 28 +- .../contracts/field-validation.spec.json | 142 +++- .../invalid-capture-group-name.spec.json | 76 ++- .../ppl-lint/contracts/manifest.json | 11 +- .../multisearch-min-subsearch.spec.json | 38 +- .../replace-wildcard-asymmetry.spec.json | 76 ++- .../contracts/rex-scan-cost.spec.json | 26 +- .../contracts/type-mismatch-numeric.spec.json | 28 +- .../contracts/union-min-datasets.spec.json | 38 +- ...ed-window-function-in-eventstats.spec.json | 114 +++- .../wildcard-source-zero-match.spec.json | 19 +- scripts/ppl-lint/README.md | 28 +- .../__tests__/aggregate-versions.test.mjs | 88 +++ scripts/ppl-lint/__tests__/annotate.test.mjs | 32 + .../__tests__/assemble-run-manifest.test.mjs | 73 ++- .../__tests__/contract-schema.test.mjs | 252 +++++++- .../__tests__/run-frontend-contract.test.mjs | 325 ++++++++++ scripts/ppl-lint/aggregate-versions.mjs | 168 ++++- scripts/ppl-lint/annotate.mjs | 19 + scripts/ppl-lint/assemble-run-manifest.mjs | 96 ++- scripts/ppl-lint/contract-schema.mjs | 238 ++++++- scripts/ppl-lint/run-frontend-contract.mjs | 609 +++++++++++++++--- 27 files changed, 2375 insertions(+), 250 deletions(-) create mode 100644 scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs diff --git a/.github/workflows/ppl-lint-multiversion-validation.yml b/.github/workflows/ppl-lint-multiversion-validation.yml index 8e31ab55918..df1aad11185 100644 --- a/.github/workflows/ppl-lint-multiversion-validation.yml +++ b/.github/workflows/ppl-lint-multiversion-validation.yml @@ -837,6 +837,8 @@ jobs: env "${surface_env[@]}" "${observe_env[@]}" \ PPL_LINT_CONTRACT_DIR="$GITHUB_WORKSPACE/integ-test/src/test/resources/ppl-lint/contracts" \ PPL_LINT_SCHEDULE=nightly \ + PPL_LINT_ENFORCE_CENSUS=1 \ + PPL_LINT_INCLUDE_DORMANT=1 \ PPL_LINT_TARGET_MANIFEST="$leg/target.json" \ PPL_LINT_REPORT="$leg/detector-report.json" \ node -r ./src/setup_node_env \ diff --git a/.github/workflows/ppl-lint-rule-validation.yml b/.github/workflows/ppl-lint-rule-validation.yml index 6f83c60a60b..7ffabbce2da 100644 --- a/.github/workflows/ppl-lint-rule-validation.yml +++ b/.github/workflows/ppl-lint-rule-validation.yml @@ -37,8 +37,8 @@ concurrency: # # Modes (design §3.4, §4.1.1): # - pull_request: SQL PR validation against the resolved OSD target. The ONLY -# enforcing mode; this is what branch protection pins to. Runs the fast -# schedule:pr subset. The committed default is `main` on the canonical repo; +# enforcing mode; this is what branch protection pins to. Runs all 13 active +# schedule:pr contracts. The committed default is `main` on the canonical repo; # it can be overridden by the OSD_REPO/OSD_REF repo variables — see the # "Resolve OSD ref" step. TEMPORARY: those repo variables are currently set to # the unmerged paired OSD branch that ships the headless lint API this job @@ -295,6 +295,8 @@ jobs: PPL_LINT_TARGET_MANIFEST: ${{ github.workspace }}/artifacts/target.json PPL_LINT_BACKEND_REPORT: ${{ github.workspace }}/artifacts/backend-report.json PPL_LINT_REPORT: ${{ github.workspace }}/detector-report.json + PPL_LINT_ENFORCE_CENSUS: '1' + PPL_LINT_INCLUDE_DORMANT: ${{ steps.schedule.outputs.value == 'nightly' && '1' || '0' }} run: | # pipefail so the runner's non-zero exit propagates through `tee` — # otherwise the pipeline takes tee's (success) status and a real diff --git a/integ-test/src/test/resources/ppl-lint/contracts/agg-on-text.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/agg-on-text.spec.json index 4c574aaa86c..ebec7087274 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/agg-on-text.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/agg-on-text.spec.json @@ -4,7 +4,7 @@ "channel": "lint", "note": "The standard engine accepts both text aggregations: avg(text) returns null while sum(text) returns a non-empty numeric result (observed as 0.0). The warning catches this misleading coercion rather than predicting backend rejection.", "grammarSurface": "both", - "schedule": "nightly", + "schedule": "pr", "wiring": { "detector": "agg-on-text", "enabled": true, @@ -58,7 +58,14 @@ "frontend": { "count": 1, "severity": "warning", - "matchMessage": "text field" + "messageEquals": "Numeric aggregation on a text field may return no value (null), because text is not stored as a number.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } }, "backends": { "standard": { @@ -81,7 +88,14 @@ "frontend": { "count": 1, "severity": "warning", - "matchMessage": "text field" + "messageEquals": "Numeric aggregation on a text field may return no value (null), because text is not stored as a number.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } }, "backends": { "standard": { @@ -102,7 +116,13 @@ }, "avg-numeric-control": { "frontend": { - "count": 0 + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } }, "backends": { "standard": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/command-suggestion.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/command-suggestion.spec.json index e1bb53ffb3b..021a37bb4b3 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/command-suggestion.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/command-suggestion.spec.json @@ -3,7 +3,7 @@ "ruleId": "command-suggestion", "channel": "syntax", "grammarSurface": "runtime-bundle", - "schedule": "nightly", + "schedule": "pr", "wiring": { "code": "UNKNOWN_COMMAND" }, @@ -47,8 +47,9 @@ "count": 1, "code": "UNKNOWN_COMMAND", "fixText": "where", - "matchMessage": "where", - "rawMessage": true + "matchMessage": "Unknown command \"wherre\". Did you mean \"where\"?", + "rawMessage": true, + "totalErrors": 1 }, "backends": { "standard": { @@ -71,6 +72,8 @@ "frontend": { "count": 0, "code": "UNKNOWN_COMMAND", + "fixText": null, + "rawMessage": false, "totalErrors": 0 }, "backends": { @@ -94,6 +97,8 @@ "frontend": { "count": 0, "code": "UNKNOWN_COMMAND", + "fixText": null, + "rawMessage": false, "totalErrors": 1 }, "backends": { @@ -117,6 +122,8 @@ "frontend": { "count": 0, "code": "UNKNOWN_COMMAND", + "fixText": null, + "rawMessage": false, "totalErrors": 1 }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/division-by-zero.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/division-by-zero.spec.json index 88c4226fd8e..b643094471b 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/division-by-zero.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/division-by-zero.spec.json @@ -50,8 +50,18 @@ "version": ">=0.0.0", "queries": { "divide-by-zero-literal": { - "detectorCount": 1, - "severity": "warning", + "frontend": { + "count": 1, + "severity": "warning", + "messageEquals": "Dividing by zero returns no value (null) instead of an error.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "result-shape", @@ -70,8 +80,18 @@ } }, "divide-by-decimal-zero-literal": { - "detectorCount": 1, - "severity": "warning", + "frontend": { + "count": 1, + "severity": "warning", + "messageEquals": "Dividing by zero returns no value (null) instead of an error.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "result-shape", @@ -90,7 +110,15 @@ } }, "divide-by-nonzero-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", @@ -109,8 +137,18 @@ } }, "modulo-by-zero-literal": { - "detectorCount": 1, - "severity": "warning", + "frontend": { + "count": 1, + "severity": "warning", + "messageEquals": "Dividing by zero returns no value (null) instead of an error.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "result-shape", diff --git a/integ-test/src/test/resources/ppl-lint/contracts/enabled-false-object.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/enabled-false-object.spec.json index b9ebc0eef91..4e222982589 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/enabled-false-object.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/enabled-false-object.spec.json @@ -4,7 +4,7 @@ "channel": "lint", "note": "The standard Calcite route can still project and filter enabled:false object values from _source. These result-shape oracles pin that observed acceptance; the warning communicates that the object is not indexed/searchable through ordinary OpenSearch field semantics.", "grammarSurface": "both", - "schedule": "nightly", + "schedule": "pr", "wiring": { "detector": "enabled-false-object", "enabled": true, @@ -60,7 +60,14 @@ "frontend": { "count": 1, "severity": "warning", - "matchMessage": "not searchable" + "messageEquals": "This field is stored but not searchable, so PPL returns null for it.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } }, "backends": { "standard": { @@ -83,7 +90,14 @@ "frontend": { "count": 1, "severity": "warning", - "matchMessage": "not searchable" + "messageEquals": "This field is stored but not searchable, so PPL returns null for it.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } }, "backends": { "standard": { @@ -104,7 +118,13 @@ }, "indexed-field-control": { "frontend": { - "count": 0 + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } }, "backends": { "standard": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/field-validation.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/field-validation.spec.json index 6225eaed87b..4094f0af7e0 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/field-validation.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/field-validation.spec.json @@ -62,8 +62,18 @@ "note": "This rule has an empty appliesTo, so it ships to users on EVERY engine, including pre-3.4. Detector behavior is live-verified identical from 2.19 up (1/1/0 on the compiled surface at 2.19.0, 3.0.0, 3.5.0, 3.7.0). The backend oracle deliberately omits error.type/reason: this engine's wording for an unknown field has not been observed live, and inventing one would either fail spuriously or get 'fixed' by pinning whatever CI first happened to see. A compiled-surface leg records the real wording; pin it then.", "queries": { "unknown-field-existence": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "Unknown field \"nonexistent_field\".", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -82,8 +92,27 @@ } }, "grok-field-slot-shape-typo": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "grok expects a field name here, not an expression.", + "deterministicFix": { + "offered": true, + "title": "Remove \"field=\" (use \"firstname\")", + "text": "firstname", + "range": { + "startLine": 1, + "startColumn": 48, + "endLine": 1, + "endColumn": 63 + }, + "expectedText": "field=firstname", + "appliedQuery": "source=opensearch-sql_test_index_account | grok firstname \"%{WORD:w}\"" + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "rejection", @@ -102,7 +131,15 @@ } }, "known-field-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", @@ -126,8 +163,18 @@ "version": ">=3.4.0 <3.7.0", "queries": { "unknown-field-existence": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "Unknown field \"nonexistent_field\".", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -154,8 +201,27 @@ } }, "grok-field-slot-shape-typo": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "grok expects a field name here, not an expression.", + "deterministicFix": { + "offered": true, + "title": "Remove \"field=\" (use \"firstname\")", + "text": "firstname", + "range": { + "startLine": 1, + "startColumn": 48, + "endLine": 1, + "endColumn": 63 + }, + "expectedText": "field=firstname", + "appliedQuery": "source=opensearch-sql_test_index_account | grok firstname \"%{WORD:w}\"" + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "rejection", @@ -182,7 +248,15 @@ } }, "known-field-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", @@ -206,9 +280,18 @@ "version": ">=3.7.0", "queries": { "unknown-field-existence": { - "detectorCount": 1, - "severity": "error", - "matchMessage": "nonexistent_field", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "Unknown field \"nonexistent_field\".", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -235,8 +318,27 @@ } }, "grok-field-slot-shape-typo": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "grok expects a field name here, not an expression.", + "deterministicFix": { + "offered": true, + "title": "Remove \"field=\" (use \"firstname\")", + "text": "firstname", + "range": { + "startLine": 1, + "startColumn": 48, + "endLine": 1, + "endColumn": 63 + }, + "expectedText": "field=firstname", + "appliedQuery": "source=opensearch-sql_test_index_account | grok firstname \"%{WORD:w}\"" + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "rejection", @@ -263,7 +365,15 @@ } }, "known-field-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", diff --git a/integ-test/src/test/resources/ppl-lint/contracts/invalid-capture-group-name.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/invalid-capture-group-name.spec.json index 4c2cfc36c6f..b4ba3df32c8 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/invalid-capture-group-name.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/invalid-capture-group-name.spec.json @@ -55,8 +55,18 @@ "engine": "calcite", "queries": { "rex-capture-name-underscore": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "Capture group name \"user_name\" is invalid. Start with a letter and use only letters and numbers.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -83,8 +93,18 @@ } }, "rex-capture-name-hyphen": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "Capture group name \"user-name\" is invalid. Start with a letter and use only letters and numbers.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -111,7 +131,15 @@ } }, "rex-capture-name-alphanumeric-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", @@ -136,8 +164,18 @@ "engine": "calcite", "queries": { "rex-capture-name-underscore": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "Capture group name \"user_name\" is invalid. Start with a letter and use only letters and numbers.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -164,8 +202,18 @@ } }, "rex-capture-name-hyphen": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "Capture group name \"user-name\" is invalid. Start with a letter and use only letters and numbers.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -192,7 +240,15 @@ } }, "rex-capture-name-alphanumeric-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", diff --git a/integ-test/src/test/resources/ppl-lint/contracts/manifest.json b/integ-test/src/test/resources/ppl-lint/contracts/manifest.json index 3e83f493e0c..20a6995acb8 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/manifest.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/manifest.json @@ -1,6 +1,6 @@ { "schemaVersion": 4, - "description": "Active PPL frontend/backend compatibility corpus for the approved 12 detector rules plus the command-suggestion syntax feature. New contracts remain nightly until their runtime and analytics oracles are reviewed; census drift is report-only until the paired OSD default-alignment change lands.", + "description": "Required PPL frontend/backend compatibility corpus for the approved 12 detector rules plus the command-suggestion syntax feature.", "contracts": [ "agg-on-text.spec.json", "command-suggestion.spec.json", @@ -41,14 +41,6 @@ "requiredSyntaxFeatures": [ "command-suggestion.spec.json" ], - "pendingReview": [ - "agg-on-text.spec.json", - "command-suggestion.spec.json", - "enabled-false-object.spec.json", - "rex-scan-cost.spec.json", - "type-mismatch-numeric.spec.json", - "wildcard-source-zero-match.spec.json" - ], "nonEnforcing": [ "agg-on-text.spec.json", "division-by-zero.spec.json", @@ -61,7 +53,6 @@ "enforced": "Reviewed lint error contracts with deterministic backend behavior.", "defaultError": "Exact approved six-rule detector error census. command-suggestion is an error-channel feature but is intentionally excluded because it is not a detector.", "requiredSyntaxFeatures": "Syntax-channel features validated through the production runtime grammar listener.", - "pendingReview": "Nightly contracts whose standard and analytics observations must be reviewed before promotion to the required PR schedule.", "nonEnforcing": "Oracle-quality classification for warning, info, advisory, and result-shape contracts; scheduling determines execution, not this list.", "dormantContracts": "Preserved default-off detector regression contracts. They do not count toward active shipping coverage and must force-enable their detector when run." } diff --git a/integ-test/src/test/resources/ppl-lint/contracts/multisearch-min-subsearch.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/multisearch-min-subsearch.spec.json index 21cdae63845..106ed504f01 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/multisearch-min-subsearch.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/multisearch-min-subsearch.spec.json @@ -52,8 +52,18 @@ "version": ">=3.4.0", "queries": { "multisearch-single-subsearch": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "The multisearch command requires at least two subsearches.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -80,8 +90,18 @@ } }, "multisearch-single-subsearch-with-where": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "The multisearch command requires at least two subsearches.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -108,7 +128,15 @@ } }, "multisearch-two-subsearches-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", diff --git a/integ-test/src/test/resources/ppl-lint/contracts/replace-wildcard-asymmetry.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/replace-wildcard-asymmetry.spec.json index 29def9f3b8a..c4b310bbac8 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/replace-wildcard-asymmetry.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/replace-wildcard-asymmetry.spec.json @@ -53,8 +53,18 @@ "engine": "calcite", "queries": { "replace-wildcard-count-mismatch": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "The replace match and replacement have different numbers of \"*\" wildcards. The counts must match.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -81,8 +91,18 @@ } }, "replace-wildcard-count-mismatch-reverse": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "The replace match and replacement have different numbers of \"*\" wildcards. The counts must match.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -109,7 +129,15 @@ } }, "replace-symmetric-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", @@ -134,8 +162,18 @@ "engine": "calcite", "queries": { "replace-wildcard-count-mismatch": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "The replace match and replacement have different numbers of \"*\" wildcards. The counts must match.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -162,8 +200,18 @@ } }, "replace-wildcard-count-mismatch-reverse": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "The replace match and replacement have different numbers of \"*\" wildcards. The counts must match.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -190,7 +238,15 @@ } }, "replace-symmetric-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", diff --git a/integ-test/src/test/resources/ppl-lint/contracts/rex-scan-cost.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/rex-scan-cost.spec.json index 918eaacb0cd..5230aefe49f 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/rex-scan-cost.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/rex-scan-cost.spec.json @@ -3,7 +3,7 @@ "ruleId": "rex-scan-cost", "channel": "lint", "grammarSurface": "both", - "schedule": "nightly", + "schedule": "pr", "wiring": { "detector": "rex-scan-cost", "enabled": true, @@ -52,7 +52,13 @@ "frontend": { "count": 1, "severity": "info", - "matchMessage": "every input row" + "messageEquals": "parse runs the pattern against every input row from text field \"email\", even when it finds no match.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } }, "backends": { "standard": { @@ -69,7 +75,13 @@ "frontend": { "count": 1, "severity": "info", - "matchMessage": "every input row" + "messageEquals": "grok runs the pattern against every input row from text field \"email\", even when it finds no match.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } }, "backends": { "standard": { @@ -84,7 +96,13 @@ }, "plain-field-control": { "frontend": { - "count": 0 + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } }, "backends": { "standard": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/type-mismatch-numeric.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/type-mismatch-numeric.spec.json index 0f3e14791f5..5589bdb79f7 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/type-mismatch-numeric.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/type-mismatch-numeric.spec.json @@ -3,7 +3,7 @@ "ruleId": "type-mismatch-numeric", "channel": "lint", "grammarSurface": "both", - "schedule": "nightly", + "schedule": "pr", "wiring": { "detector": "type-mismatch-numeric", "enabled": true, @@ -56,7 +56,14 @@ "frontend": { "count": 1, "severity": "warning", - "matchMessage": "not a number" + "messageEquals": "This field is numeric, but the compared value is not a number, so the comparison returns no rows.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } }, "backends": { "standard": { @@ -79,7 +86,14 @@ "frontend": { "count": 1, "severity": "warning", - "matchMessage": "not a number" + "messageEquals": "This field is numeric, but the compared value is not a number, so the comparison returns no rows.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } }, "backends": { "standard": { @@ -100,7 +114,13 @@ }, "numeric-string-control": { "frontend": { - "count": 0 + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } }, "backends": { "standard": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/union-min-datasets.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/union-min-datasets.spec.json index 6de9db9a7d6..ca16823381a 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/union-min-datasets.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/union-min-datasets.spec.json @@ -55,8 +55,18 @@ "engine": "calcite", "queries": { "union-single-dataset": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "The union command requires at least two datasets.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -83,8 +93,18 @@ } }, "union-single-dataset-with-fields": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "The union command requires at least two datasets.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -111,7 +131,15 @@ } }, "union-two-datasets-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", diff --git a/integ-test/src/test/resources/ppl-lint/contracts/unsupported-window-function-in-eventstats.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/unsupported-window-function-in-eventstats.spec.json index fda4d273fb9..6c017195e1a 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/unsupported-window-function-in-eventstats.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/unsupported-window-function-in-eventstats.spec.json @@ -49,8 +49,18 @@ "version": ">=3.4.0 <3.7.0", "queries": { "eventstats-rank": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -77,8 +87,18 @@ } }, "eventstats-dense-rank": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -105,7 +125,15 @@ } }, "eventstats-avg-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", @@ -129,8 +157,18 @@ "version": ">=3.7.0 <3.8.0", "queries": { "eventstats-rank": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -157,8 +195,18 @@ } }, "eventstats-dense-rank": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -185,7 +233,15 @@ } }, "eventstats-avg-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", @@ -209,8 +265,18 @@ "version": ">=3.8.0", "queries": { "eventstats-rank": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -237,8 +303,18 @@ } }, "eventstats-dense-rank": { - "detectorCount": 1, - "severity": "error", + "frontend": { + "count": 1, + "severity": "error", + "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } + }, "backends": { "standard": { "kind": "rejection", @@ -265,7 +341,15 @@ } }, "eventstats-avg-control": { - "detectorCount": 0, + "frontend": { + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } + }, "backends": { "standard": { "kind": "result-shape", diff --git a/integ-test/src/test/resources/ppl-lint/contracts/wildcard-source-zero-match.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/wildcard-source-zero-match.spec.json index df01303aefb..daf9920cab7 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/wildcard-source-zero-match.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/wildcard-source-zero-match.spec.json @@ -3,7 +3,7 @@ "ruleId": "wildcard-source-zero-match", "channel": "lint", "grammarSurface": "both", - "schedule": "nightly", + "schedule": "pr", "wiring": { "detector": "wildcard-source-zero-match", "enabled": true, @@ -48,7 +48,14 @@ "frontend": { "count": 1, "severity": "info", - "matchMessage": "matches no known index" + "messageEquals": "Wildcard source pattern matches no known index.", + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": true, + "commandId": "ppl.lint.aiFix" + } }, "backends": { "standard": { @@ -69,7 +76,13 @@ }, "matching-wildcard-control": { "frontend": { - "count": 0 + "count": 0, + "deterministicFix": { + "offered": false + }, + "aiAction": { + "offered": false + } }, "backends": { "standard": { diff --git a/scripts/ppl-lint/README.md b/scripts/ppl-lint/README.md index f79a3052513..62b7d157283 100644 --- a/scripts/ppl-lint/README.md +++ b/scripts/ppl-lint/README.md @@ -50,12 +50,10 @@ backend-validation ──(target.json, ppl-grammar-bundle.json, backend-report.j | `workflow_dispatch` (`osd_ref`) | OSD-branch evidence | the given commit/branch | No — pre-merge evidence only | | `schedule` (nightly) | full corpus + coverage | `main` | No | -The active corpus contains 12 detector contracts plus the -`command-suggestion` syntax contract. Seven reviewed contracts currently declare -`schedule: "pr"`; the six new contracts remain `nightly` until their standard and -analytics observations are reviewed. A contract that runs also asserts: neither -the IT nor the frontend runner consults the manifest's `enforced` list, so any -contract on the PR schedule can fail the required check. +The required PR corpus contains 12 detector contracts plus the +`command-suggestion` syntax contract. All 13 declare `schedule: "pr"` and can +fail the required check. The nightly mode runs the same shipping corpus across +the supported version and execution-backend matrix. `workflow_dispatch` inputs: @@ -214,8 +212,6 @@ rule cannot be validated end to end. `rules_catalog.json`; it contains exactly six detector rules. - `requiredSyntaxFeatures` — `command-suggestion` only. Syntax features never appear in `defaultError` or the detector catalog. -- `pendingReview` — the six nightly contracts awaiting oracle review and PR - promotion. - `nonEnforcing` — oracle-quality classification for warning, info, advisory, and result-shape contracts. Scheduling determines whether a contract runs. - `dormantContracts` — four preserved default-off detector contracts. They do @@ -410,8 +406,8 @@ different places a developer looks: The required single-version lane follows the same rule: frontend and backend failures with a `[rule/query]` identity anchor on that contract's `ruleId`. -Shipping-census findings anchor on `manifest.json` (as warnings while census -enforcement is report-only). Artifact and job failures without a trustworthy +Shipping-census findings anchor on `manifest.json` and fail the required lane. +Artifact and job failures without a trustworthy repository location remain file-less rather than pointing at a guessed line. Without the annotations the only thing above the summary is `Process completed @@ -470,10 +466,9 @@ node --test "scripts/ppl-lint/__tests__/*.test.mjs" ## Discovery corpus (harvested, never enforced) -The enforced corpus is hand-pinned, which is what lets a mismatch red the build — -and also why it is small (about one trigger per rule). One trigger is not enough to -tell a full engine fix from a partial one, so the `discovery` job builds a second, -much larger corpus that pins nothing. +The required corpus is hand-pinned, which is what lets a mismatch red the build. +The `discovery` job builds a larger unpinned corpus to distinguish full engine +fixes from partial behavior changes. ``` harvest-queries.mjs ──▶ discovery-corpus.json ──┬──▶ run-frontend-contract.mjs ──▶ detector report @@ -487,9 +482,8 @@ harvest-queries.mjs ──▶ discovery-corpus.json ──┬──▶ run-front (matched as a prefix, so `describe('rex-scan-cost (compiled surface)')` counts). A query with no rule-owning ancestor is recorded unattributed and dropped rather than guessed at. Indices are rewritten onto the fixture index; JS string escapes - are unescaped so the query matches what the test actually linted. Against OSD - `main` today this yields **~109 queries across 12 rules** versus 27 across 11 in - the enforced corpus. + are unescaped so the query matches what the test actually linted. The harvested + corpus is substantially larger than the curated 13-contract required corpus. Each file's **lint context** is harvested alongside its queries. Seven of the nineteen rules are `needsContext: true` and self-suppress without a `typeMap`, so diff --git a/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs b/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs index b498f594745..b42c825384d 100644 --- a/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs +++ b/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs @@ -119,6 +119,7 @@ function writeLeg({ grammarHash = `sha256:${version}`, surface = 'runtime-bundle', explicitIdentity = true, + censusEnforced = false, }) { const dir = makeTmp(`ppl-lint-leg-${version}-`); const target = { @@ -168,6 +169,20 @@ function writeLeg({ severities: c.severities || (c.detector > 0 ? ['error'] : []), severityMatched: c.severityMatched ?? true, messageMatched: c.messageMatched ?? true, + ...Object.fromEntries( + [ + 'deterministicFixMatched', + 'aiActionMatched', + 'actionDecisionMatched', + 'fixMatched', + 'rawMessageMatched', + 'totalErrorsMatched', + ] + .filter((field) => c[field] !== undefined) + .map((field) => [field, c[field]]) + ), + ...(c.assertions ? { assertions: c.assertions } : {}), + ...(c.mismatches ? { mismatches: c.mismatches } : {}), ...(explicitIdentity ? { executionBackend } : {}), }); backend.push({ @@ -200,6 +215,10 @@ function writeLeg({ surface, results, ...(defaultErrorRules !== null ? { defaultErrorRules } : {}), + enabledRules: [SPEC.ruleId], + activeContractRules: [SPEC.ruleId], + requiredSyntaxFeatures: [], + census: { enforced: censusEnforced }, }) ); fs.writeFileSync(path.join(dir, 'backend-report.json'), JSON.stringify(backend)); @@ -625,6 +644,51 @@ test('paired detector reports must be identical across execution backends', () = assert.match(stderr, /detector parity failed for union-min-datasets::trigger/); }); +test('identical exact-action mismatches across versions cannot aggregate green', () => { + const badCase = { + detector: 1, + rejected: true, + deterministicFixMatched: false, + assertions: { deterministicFix: false }, + mismatches: [ + { + field: 'deterministicFix', + expected: { offered: false }, + actual: { offered: true }, + }, + ], + }; + const legs = { + '3.7.0': writeLeg({ + version: '3.7.0', + cases: { + trigger: badCase, + control: { detector: 0, rejected: false }, + }, + }), + '3.8.0': writeLeg({ + version: '3.8.0', + cases: { + trigger: badCase, + control: { detector: 0, rejected: false }, + }, + }), + }; + + const { status, report } = run({ contracts: writeContracts(), legs }); + assert.equal(status, 1); + const actionDrifts = report.drifts.filter( + (drift) => drift.driftClass === 'frontend-contract-mismatch' + ); + assert.equal(actionDrifts.length, 2); + assert.ok( + actionDrifts.every((drift) => + drift.frontendAssertions.includes('deterministicFixMatched') + ) + ); + assert.ok(report.matrix.every((row) => row.status === 'drift')); +}); + test('target and detector execution identities must match', () => { const dir = writeLeg({ version: '3.8.0', @@ -1048,6 +1112,7 @@ test('a default-error rule with no contract file fails the check', () => { version: '3.7.0', cases: { trigger: { detector: 1, rejected: true }, control: { detector: 0, rejected: false } }, defaultErrorRules: ['union-min-datasets', 'brand-new-error-rule'], + censusEnforced: true, }), }; const { status, report, stdout } = run({ contracts: writeContracts(), legs }); @@ -1071,6 +1136,29 @@ test('a census matching the manifest keeps the check green', () => { assert.equal(report.result.missingContractCount, 0); }); +test('an enforced shipping census mismatch fails aggregation', () => { + const legs = { + '3.7.0': writeLeg({ + version: '3.7.0', + cases: { + trigger: { detector: 1, rejected: true }, + control: { detector: 0, rejected: false }, + }, + censusEnforced: true, + }), + }; + + const { status, report, stdout } = run({ contracts: writeContracts(), legs }); + assert.equal(status, 1); + assert.equal(report.result.passed, false); + assert.ok(report.result.blockingShippingCensusProblems > 0); + assert.equal(report.shippingCensus.blocking, true); + assert.match(stdout, /shipping census problem/); + assert.match(stdout, /CENSUS ENFORCED/); + assert.doesNotMatch(stdout, /CENSUS REPORT-ONLY/); + assert.match(stdout, /### Shipping census/); +}); + test('a schema-v2 detector report without a census fails closed', () => { const dir = writeLeg({ version: '3.8.0', diff --git a/scripts/ppl-lint/__tests__/annotate.test.mjs b/scripts/ppl-lint/__tests__/annotate.test.mjs index 1209c1b0637..c0aca6f5d33 100644 --- a/scripts/ppl-lint/__tests__/annotate.test.mjs +++ b/scripts/ppl-lint/__tests__/annotate.test.mjs @@ -180,6 +180,38 @@ test('an unvalidated rule has no file to point at', () => { assert.match(annotations[0].message, /manifest\.defaultError/); }); +test('shipping census errors point to manifest.json', () => { + const annotations = buildAnnotations( + { + shippingCensus: { + passed: false, + blocking: true, + problems: ['active lint rules do not equal enabled OSD rules'], + }, + }, + { + contractsDir: '/workspace/contracts', + workspace: '/workspace', + readFile: (_dir, file) => + file === 'manifest.json' + ? '{\n "schemaVersion": 4,\n "contracts": []\n}\n' + : undefined, + } + ); + + assert.deepEqual(annotations, [ + { + level: 'error', + file: 'contracts/manifest.json', + line: 3, + title: 'PPL lint shipping census mismatch', + message: + 'active lint rules do not equal enabled OSD rules\n' + + 'FIX: align the active SQL manifest with the approved OSD shipping catalog.', + }, + ]); +}); + test('paths are repo-relative so GitHub can render them inline', () => { // An absolute path still annotates the run, but never attaches to the diff. assert.equal( diff --git a/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs b/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs index a3ee6f10acd..7e2c1afa062 100644 --- a/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs +++ b/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs @@ -184,6 +184,77 @@ test('syntax-specific frontend mismatches fail artifact validation', () => { const result = run(dir); assert.notEqual(result.status, 0); - assert.match(result.stderr, /did not match its (fix|raw-message|total-error) assertion/); + assert.match( + result.stderr, + /did not match its (syntax-fix|raw-parser-error|total-error) assertion/ + ); } }); + +test('exact deterministic and AI action mismatches fail artifacts and summary rows', () => { + for (const field of ['deterministicFixMatched', 'aiActionMatched']) { + const dir = makeRun(); + validArtifacts(dir); + const file = path.join(dir, 'artifacts', 'detector-report.json'); + const detector = JSON.parse(fs.readFileSync(file, 'utf8')); + detector.results[0][field] = false; + detector.results[0].assertions = { + [field === 'deterministicFixMatched' ? 'deterministicFix' : 'aiAction']: false, + }; + detector.results[0].mismatches = [ + { + field: field === 'deterministicFixMatched' ? 'deterministicFix' : 'aiAction', + expected: { offered: false }, + actual: { offered: true }, + }, + ]; + fs.writeFileSync(file, JSON.stringify(detector)); + const summary = path.join(dir, 'summary.md'); + + const result = run(dir, { GITHUB_STEP_SUMMARY: summary }); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /did not match its (deterministic-fix|AI-action) assertion/); + assert.match(fs.readFileSync(summary, 'utf8'), /advisory-rule.*accepted.*Fail/); + } +}); + +test('report-only dormant rows do not affect the required manifest result or active set', () => { + const dir = makeRun(); + validArtifacts(dir); + const file = path.join(dir, 'artifacts', 'detector-report.json'); + const detector = JSON.parse(fs.readFileSync(file, 'utf8')); + detector.results.push({ + ruleId: 'dormant-rule', + queryName: 'trigger', + role: 'trigger', + expected: 1, + actual: 0, + severities: [], + severityMatched: false, + messageMatched: false, + assertions: { count: false }, + mismatches: [{ field: 'count', expected: 1, actual: 0 }], + executionBackend: 'standard', + reportOnly: true, + }); + fs.writeFileSync(file, JSON.stringify(detector)); + const backendFile = path.join(dir, 'artifacts', 'backend-report.json'); + const backend = JSON.parse(fs.readFileSync(backendFile, 'utf8')); + backend.push({ + ruleId: 'dormant-rule', + queryName: 'trigger', + role: 'trigger', + executionBackend: 'standard', + outcome: 'error', + error: 'report-only observation failed', + }); + fs.writeFileSync(backendFile, JSON.stringify(backend)); + + const summary = path.join(dir, 'summary.md'); + const result = run(dir, { GITHUB_STEP_SUMMARY: summary }); + assert.equal(result.status, 0, result.stderr); + const manifest = JSON.parse(fs.readFileSync(path.join(dir, 'run-manifest.json'), 'utf8')); + assert.deepEqual(manifest.validationSet, ['advisory-rule']); + assert.equal(manifest.result.passed, true); + assert.match(fs.readFileSync(summary, 'utf8'), /dormant-rule.*Report only/); +}); diff --git a/scripts/ppl-lint/__tests__/contract-schema.test.mjs b/scripts/ppl-lint/__tests__/contract-schema.test.mjs index 914dd31569f..5bb44957bf1 100644 --- a/scripts/ppl-lint/__tests__/contract-schema.test.mjs +++ b/scripts/ppl-lint/__tests__/contract-schema.test.mjs @@ -9,6 +9,7 @@ import { test } from 'node:test'; import { assertContractSchema, assertExactQueryCoverage, + assertShippingFrontendOracles, classifyBackendReportRow, contractChannel, indexBackendReport, @@ -508,11 +509,189 @@ test('missing channel remains a backwards-compatible lint contract', () => { channel: 'lint', count: 1, severity: 'warning', - matchMessage: undefined, + messageEquals: undefined, + deterministicFix: undefined, + aiAction: undefined, } ); }); +test('schema-v4 lint frontend normalizes exact message, fix, and AI action oracles', () => { + const contract = spec(4); + const frontend = normalizeFrontendOracle(contract, { + frontend: { + count: 1, + severity: 'warning', + messageEquals: 'Use a non-zero divisor.', + deterministicFix: { + offered: true, + title: 'Replace zero', + text: '1', + range: { + startLine: 1, + startColumn: 20, + endLine: 1, + endColumn: 21, + }, + expectedText: '0', + appliedQuery: 'source=t | eval x = 1', + }, + aiAction: { offered: false }, + }, + }); + + assert.deepEqual(frontend, { + channel: 'lint', + count: 1, + severity: 'warning', + messageEquals: 'Use a non-zero divisor.', + deterministicFix: { + offered: true, + title: 'Replace zero', + text: '1', + range: { + startLine: 1, + startColumn: 20, + endLine: 1, + endColumn: 21, + }, + expectedText: '0', + appliedQuery: 'source=t | eval x = 1', + }, + aiAction: { offered: false }, + }); +}); + +test('matchMessage remains available only to schema-v3 lint contracts', () => { + assert.equal( + normalizeFrontendOracle(spec(3), { + frontend: { count: 1, matchMessage: 'legacy substring' }, + }).matchMessage, + 'legacy substring' + ); + assert.throws( + () => + normalizeFrontendOracle(spec(4), { + frontend: { count: 1, matchMessage: 'not exact' }, + }), + /matchMessage is not valid/ + ); +}); + +test('schema-v4 action payloads fail closed on partial or extra fields', () => { + const contract = spec(4); + for (const [frontend, expected] of [ + [ + { count: 1, deterministicFix: { offered: false, title: 'unexpected' } }, + /must contain only offered/, + ], + [ + { + count: 1, + deterministicFix: { + offered: true, + title: 'Fix', + text: 'x', + range: { startLine: 0, startColumn: 0, endLine: 1, endColumn: 1 }, + appliedQuery: 'x', + }, + }, + /startLine must be a positive integer/, + ], + [ + { + count: 1, + deterministicFix: { + offered: true, + title: 'Fix', + text: 'x', + range: { startLine: 1, startColumn: 0, endLine: 1, endColumn: 1 }, + appliedQuery: 'x', + }, + }, + /expectedText must be a string/, + ], + [ + { + count: 1, + deterministicFix: { + offered: true, + title: 'Fix', + text: 'x', + range: { startLine: 1, startColumn: 2, endLine: 1, endColumn: 1 }, + expectedText: 'y', + appliedQuery: 'x', + }, + }, + /must end at or after its start/, + ], + [ + { count: 1, aiAction: { offered: true } }, + /commandId must be a non-empty string/, + ], + [ + { count: 1, aiAction: { offered: false, commandId: 'ppl.lint.aiFix' } }, + /must contain only offered/, + ], + ]) { + assert.throws(() => normalizeFrontendOracle(contract, { frontend }), expected); + } +}); + +test('active lint contracts require exact messages and explicit exclusive action modes', () => { + const contract = spec(4); + const expectation = structuredClone(contract.expectations[0]); + expectation.queries.trigger = { + frontend: { + count: 1, + severity: 'error', + messageEquals: 'Exact diagnostic.', + deterministicFix: { offered: false }, + aiAction: { offered: true, commandId: 'ppl.lint.aiFix' }, + }, + backends: { + standard: { kind: 'rejection', httpStatus: 400, body: { status: 400 } }, + analytics: { kind: 'rejection', httpStatus: 400, body: { status: 400 } }, + }, + }; + expectation.queries.control.frontend = { + count: 0, + deterministicFix: { offered: false }, + aiAction: { offered: false }, + }; + delete expectation.queries.control.detectorCount; + + assert.doesNotThrow(() => assertShippingFrontendOracles(contract, expectation)); + + const missingMessage = structuredClone(expectation); + delete missingMessage.queries.trigger.frontend.messageEquals; + assert.throws( + () => assertShippingFrontendOracles(contract, missingMessage), + /messageEquals is required/ + ); + + const missingSeverity = structuredClone(expectation); + delete missingSeverity.queries.trigger.frontend.severity; + assert.throws( + () => assertShippingFrontendOracles(contract, missingSeverity), + /severity is required/ + ); + + const simultaneousActions = structuredClone(expectation); + simultaneousActions.queries.trigger.frontend.deterministicFix = { + offered: true, + title: 'Fix', + text: 'fixed', + range: { startLine: 1, startColumn: 0, endLine: 1, endColumn: 3 }, + expectedText: 'bad', + appliedQuery: 'fixed', + }; + assert.throws( + () => assertShippingFrontendOracles(contract, simultaneousActions), + /cannot offer deterministic and AI actions together/ + ); +}); + test('syntax frontend assertions normalize stable code, fix, raw message, and error census', () => { const contract = { schemaVersion: 4, @@ -545,6 +724,77 @@ test('syntax frontend assertions normalize stable code, fix, raw message, and er assert.equal(assertContractSchema(contract), 4); }); +test('active syntax contracts require explicit fix, raw-message, and total-error assertions', () => { + const contract = { + schemaVersion: 4, + ruleId: 'command-suggestion', + channel: 'syntax', + wiring: { code: 'UNKNOWN_COMMAND' }, + queries: { + trigger: { role: 'trigger', query: 'source=t | wherre a > 1' }, + }, + }; + const expectation = { + queries: { + trigger: { + frontend: { + count: 1, + code: 'UNKNOWN_COMMAND', + fixText: 'where', + matchMessage: 'Unknown command "wherre". Did you mean "where"?', + rawMessage: true, + totalErrors: 1, + }, + }, + }, + }; + assert.doesNotThrow(() => assertShippingFrontendOracles(contract, expectation)); + + delete expectation.queries.trigger.frontend.fixText; + assert.throws( + () => assertShippingFrontendOracles(contract, expectation), + /fixText must explicitly assert/ + ); +}); + +test('syntax supports explicit fix absence and requires frontend code to match wiring', () => { + const contract = { + schemaVersion: 4, + ruleId: 'command-suggestion', + channel: 'syntax', + wiring: { code: 'UNKNOWN_COMMAND' }, + queries: { + suppressed: { role: 'suppression-control', query: 'source=t | zzzzzzzz' }, + }, + }; + assert.deepEqual( + normalizeFrontendOracle(contract, { + frontend: { + count: 0, + code: 'UNKNOWN_COMMAND', + fixText: null, + rawMessage: true, + totalErrors: 1, + }, + }), + { + channel: 'syntax', + count: 0, + code: 'UNKNOWN_COMMAND', + fixText: null, + rawMessage: true, + totalErrors: 1, + } + ); + assert.throws( + () => + normalizeFrontendOracle(contract, { + frontend: { count: 0, code: 'OTHER_ERROR' }, + }), + /does not match contract\.wiring\.code/ + ); +}); + test('lint and syntax frontend fields cannot cross channels', () => { assert.throws( () => diff --git a/scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs b/scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs new file mode 100644 index 00000000000..6b482125211 --- /dev/null +++ b/scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs @@ -0,0 +1,325 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; + +import { + assertActiveShippingContracts, + buildCensus, + evaluateFrontendAssertions, + selectManifestContractNames, +} from '../run-frontend-contract.mjs'; + +const RANGE = { + startLine: 1, + startColumn: 11, + endLine: 1, + endColumn: 14, +}; + +test('exact lint assertions materialize the effective deterministic edit', () => { + const result = evaluateFrontendAssertions({ + channel: 'lint', + query: 'source=t | bad', + matches: [ + { + ruleId: 'example-rule', + severity: 'warning', + message: 'Replace bad.', + range: RANGE, + fix: { + title: 'Replace bad', + text: 'good', + expectedText: 'bad', + }, + }, + ], + frontendOracle: { + severity: 'warning', + messageEquals: 'Replace bad.', + deterministicFix: { + offered: true, + title: 'Replace bad', + text: 'good', + range: RANGE, + expectedText: 'bad', + appliedQuery: 'source=t | good', + }, + aiAction: { offered: false }, + }, + decideAction: ({ hasDeterministicFix }) => ({ + kind: hasDeterministicFix ? 'deterministic' : 'ai', + commandId: 'ppl.lint.aiFix', + }), + }); + + assert.deepEqual(result.mismatches, []); + assert.deepEqual(result.assertions, { + severity: true, + message: true, + deterministicFix: true, + aiAction: true, + actionDecision: true, + }); + assert.deepEqual(result.deterministicFixActual, { + offered: true, + title: 'Replace bad', + text: 'good', + range: RANGE, + expectedText: 'bad', + appliedQuery: 'source=t | good', + }); +}); + +test('deterministic fixes require the production helper to choose the deterministic action', () => { + const result = evaluateFrontendAssertions({ + channel: 'lint', + query: 'source=t | bad', + matches: [ + { + severity: 'warning', + message: 'Replace bad.', + range: RANGE, + fix: { title: 'Replace bad', text: 'good', expectedText: 'bad' }, + }, + ], + frontendOracle: { + deterministicFix: { + offered: true, + title: 'Replace bad', + text: 'good', + range: RANGE, + expectedText: 'bad', + appliedQuery: 'source=t | good', + }, + aiAction: { offered: false }, + }, + decideAction: () => ({ kind: 'none' }), + }); + + assert.equal(result.deterministicFixMatched, true); + assert.equal(result.aiActionMatched, true); + assert.equal(result.actionDecisionMatched, false); + assert.equal( + result.mismatches.find(({ field }) => field === 'actionDecision')?.actual[0], + 'none' + ); +}); + +test('AI action identity and exact messages produce field-specific mismatches', () => { + let decisionInput; + const result = evaluateFrontendAssertions({ + channel: 'lint', + query: 'source=t | bad', + matches: [ + { + ruleId: 'example-rule', + severity: 'warning', + message: 'Different message.', + range: RANGE, + }, + ], + frontendOracle: { + messageEquals: 'Expected message.', + deterministicFix: { offered: false }, + aiAction: { offered: true, commandId: 'ppl.lint.aiFix' }, + }, + decideAction: (input) => { + decisionInput = input; + return { kind: 'ai', commandId: 'wrong.command' }; + }, + }); + + assert.deepEqual( + result.mismatches.map(({ field }) => field), + ['message', 'aiAction'] + ); + assert.deepEqual(result.aiActionActual, { + offered: true, + commandId: 'wrong.command', + }); + assert.equal(decisionInput.enableAIFeatures, true); + assert.equal(decisionInput.hasAiFixHandler, true); + assert.equal(decisionInput.aiAgentAvailableForSource, true); + assert.equal(decisionInput.aiFixEligible, true); +}); + +test('deterministic expectedText must match the source slice', () => { + const result = evaluateFrontendAssertions({ + channel: 'lint', + query: 'source=t | bad', + matches: [ + { + message: 'Replace bad.', + range: RANGE, + fix: { + title: 'Replace bad', + text: 'good', + expectedText: 'stale', + }, + }, + ], + frontendOracle: { + deterministicFix: { + offered: true, + title: 'Replace bad', + text: 'good', + range: RANGE, + expectedText: 'stale', + appliedQuery: 'source=t | good', + }, + }, + }); + + assert.equal(result.deterministicFixMatched, false); + assert.equal(result.deterministicFixActual.expectedTextMatchesSource, false); +}); + +test('AI assertions fail closed when the production decision export is unavailable', () => { + const result = evaluateFrontendAssertions({ + channel: 'lint', + query: 'source=t', + matches: [], + frontendOracle: { aiAction: { offered: false } }, + }); + + assert.equal(result.aiActionMatched, false); + assert.equal(result.mismatches[0].field, 'aiAction'); + assert.equal(result.aiActionActual.unavailable, true); +}); + +test('AI assertions report production decision errors instead of passing absence', () => { + const result = evaluateFrontendAssertions({ + channel: 'lint', + query: 'source=t | bad', + matches: [{ message: 'Bad', range: RANGE }], + frontendOracle: { aiAction: { offered: false } }, + decideAction: () => { + throw new Error('decision failed'); + }, + }); + + assert.equal(result.aiActionMatched, false); + assert.match(result.aiActionActual.error, /decision failed/); + assert.equal(result.mismatches[0].field, 'aiAction'); +}); + +test('syntax suppression checks raw parser errors outside the suggestion code filter', () => { + const parserErrors = [ + { + code: 'PARSER_ERROR', + message: 'Unexpected command.', + rawMessage: "mismatched input 'zzzzzzzz'", + }, + ]; + const result = evaluateFrontendAssertions({ + channel: 'syntax', + query: 'source=t | zzzzzzzz', + matches: [], + allFrontendFindings: parserErrors, + frontendOracle: { + fixText: null, + rawMessage: true, + totalErrors: 1, + }, + }); + + assert.deepEqual(result.mismatches, []); + assert.deepEqual(result.assertions, { + syntaxFix: true, + rawParserError: true, + totalErrors: true, + }); + + const withUnexpectedFix = evaluateFrontendAssertions({ + channel: 'syntax', + query: 'source=t | zzzzzzzz', + matches: [], + allFrontendFindings: [ + { + ...parserErrors[0], + fix: { title: 'Rewrite', text: 'where' }, + }, + ], + frontendOracle: { fixText: null, rawMessage: true }, + }); + assert.equal(withUnexpectedFix.syntaxFixMatched, false); + assert.equal(withUnexpectedFix.mismatches[0].field, 'syntaxFix'); +}); + +test('dormant manifest contracts are opt-in and remain tagged report-only', () => { + const manifest = { + contracts: ['active.spec.json'], + dormantContracts: ['dormant.spec.json'], + }; + assert.deepEqual(selectManifestContractNames(manifest), [ + { name: 'active.spec.json', reportOnly: false }, + ]); + assert.deepEqual(selectManifestContractNames(manifest, true), [ + { name: 'active.spec.json', reportOnly: false }, + { name: 'dormant.spec.json', reportOnly: true }, + ]); + assert.throws( + () => + selectManifestContractNames( + { + contracts: ['same.spec.json'], + dormantContracts: ['same.spec.json'], + }, + true + ), + /cannot be both active and dormant/ + ); +}); + +test('discovery mode accepts legacy generated specs without shipping oracles', () => { + const contract = { + file: 'generated.discovery.spec.json', + spec: { + schemaVersion: 3, + ruleId: 'generated-rule', + queries: { trigger: { role: 'trigger', query: 'source=t' } }, + expectations: [{ version: '', queries: { trigger: { detectorCount: 0 } } }], + }, + }; + + assert.doesNotThrow(() => + assertActiveShippingContracts([contract], { discovery: true }) + ); + assert.throws( + () => assertActiveShippingContracts([contract]), + /active shipping contracts must use schemaVersion 4/ + ); +}); + +test('shipping census rejects duplicate detector IDs and syntax features in the catalog', () => { + const syntaxContract = { + file: 'command-suggestion.spec.json', + spec: { + schemaVersion: 4, + ruleId: 'command-suggestion', + channel: 'syntax', + }, + }; + const census = buildCensus( + [syntaxContract], + { + contracts: ['command-suggestion.spec.json'], + defaultError: [], + requiredSyntaxFeatures: ['command-suggestion.spec.json'], + }, + [ + { id: 'command-suggestion', enabled: false, severity: 'error' }, + { id: 'duplicate-rule', enabled: false, severity: 'info' }, + { id: 'duplicate-rule', enabled: false, severity: 'info' }, + ] + ); + + assert.ok(census.problems.some((problem) => /duplicate rule IDs/.test(problem))); + assert.ok( + census.problems.some((problem) => /outside the detector catalog/.test(problem)) + ); +}); diff --git a/scripts/ppl-lint/aggregate-versions.mjs b/scripts/ppl-lint/aggregate-versions.mjs index a8c29560fb1..1a9abf03b0b 100644 --- a/scripts/ppl-lint/aggregate-versions.mjs +++ b/scripts/ppl-lint/aggregate-versions.mjs @@ -266,6 +266,37 @@ function normalizeDetectorReport(detector, target) { if (typeof entry.messageMatched !== 'boolean') { throw new TypeError(`detector report row ${key}.messageMatched must be a boolean`); } + for (const field of [ + 'deterministicFixMatched', + 'aiActionMatched', + 'actionDecisionMatched', + 'fixMatched', + 'rawMessageMatched', + 'totalErrorsMatched', + ]) { + if (entry[field] !== undefined && typeof entry[field] !== 'boolean') { + throw new TypeError(`detector report row ${key}.${field} must be a boolean`); + } + } + if (entry.assertions !== undefined) { + if ( + !entry.assertions || + typeof entry.assertions !== 'object' || + Array.isArray(entry.assertions) + ) { + throw new TypeError(`detector report row ${key}.assertions must be a JSON object`); + } + for (const [field, matched] of Object.entries(entry.assertions)) { + if (typeof matched !== 'boolean') { + throw new TypeError( + `detector report row ${key}.assertions.${field} must be a boolean` + ); + } + } + } + if (entry.mismatches !== undefined && !Array.isArray(entry.mismatches)) { + throw new TypeError(`detector report row ${key}.mismatches must be a JSON array`); + } } results.set(key, entry); } @@ -578,6 +609,16 @@ function detectorParityValue(entry) { typeof entry.rawMessageMatched === 'boolean' ? entry.rawMessageMatched : undefined, totalErrorsMatched: typeof entry.totalErrorsMatched === 'boolean' ? entry.totalErrorsMatched : undefined, + deterministicFixMatched: + typeof entry.deterministicFixMatched === 'boolean' + ? entry.deterministicFixMatched + : undefined, + aiActionMatched: + typeof entry.aiActionMatched === 'boolean' ? entry.aiActionMatched : undefined, + assertions: entry.assertions, + mismatches: entry.mismatches, + deterministicFix: entry.deterministicFix, + aiAction: entry.aiAction, code: entry.code, codes: entry.codes, totalErrors: entry.totalErrors, @@ -596,6 +637,9 @@ function assertDetectorParity(pair) { for (const key of keys) { const standardRow = standard.get(key); const analyticsRow = analytics.get(key); + if (standardRow?.reportOnly === true || analyticsRow?.reportOnly === true) { + continue; + } if (!standardRow || !analyticsRow) { fatal( `detector parity failed for ${key}: standard row=${!!standardRow}, ` + @@ -852,10 +896,61 @@ function readBackendObservation(backendEntry, detectorResult) { backendMismatch: verdict.backendMismatch, severityMatched: detectorResult ? detectorResult.severityMatched : undefined, messageMatched: detectorResult ? detectorResult.messageMatched : undefined, + deterministicFixMatched: detectorResult + ? detectorResult.deterministicFixMatched + : undefined, + aiActionMatched: detectorResult ? detectorResult.aiActionMatched : undefined, + fixMatched: detectorResult ? detectorResult.fixMatched : undefined, + rawMessageMatched: detectorResult ? detectorResult.rawMessageMatched : undefined, + totalErrorsMatched: detectorResult + ? detectorResult.totalErrorsMatched + : undefined, + assertions: detectorResult ? detectorResult.assertions : undefined, + mismatches: detectorResult ? detectorResult.mismatches : undefined, }, }; } +function failedExtendedFrontendAssertions(entry, frontendOracle) { + if (!entry) { + return []; + } + const failures = new Set(); + for (const field of [ + 'deterministicFixMatched', + 'aiActionMatched', + 'fixMatched', + 'rawMessageMatched', + 'totalErrorsMatched', + ]) { + if (entry[field] === false) { + failures.add(field); + } + } + for (const [field, matched] of Object.entries(entry.assertions || {})) { + if ( + matched === false && + field !== 'count' && + field !== 'severity' && + (field !== 'message' || frontendOracle.messageEquals !== undefined) + ) { + failures.add(field); + } + } + for (const mismatch of entry.mismatches || []) { + const field = mismatch && mismatch.field; + if ( + typeof field === 'string' && + field !== 'count' && + field !== 'severity' && + (field !== 'message' || frontendOracle.messageEquals !== undefined) + ) { + failures.add(field); + } + } + return [...failures].sort(); +} + /** * Check an out-of-scope rule for the one drift that still matters there: the * engine rejects a trigger query, but the rule's `appliesTo` excludes this @@ -1036,12 +1131,16 @@ function main() { const missingContracts = auditDefaultErrorCensus(legs, specs, enforcedRules); const shippingCensus = auditShippingCensus(legs, specs, manifest); const blockCensusDrift = !shippingCensus.available || shippingCensus.enforced; + shippingCensus.blocking = blockCensusDrift; + const blockingShippingCensusProblems = blockCensusDrift + ? shippingCensus.problems + : []; for (const entry of missingContracts) { entry.blocking = blockCensusDrift; } if (!shippingCensus.passed) { for (const problem of shippingCensus.problems) { - log(`CENSUS REPORT-ONLY: ${problem}`); + log(`CENSUS ${blockCensusDrift ? 'ENFORCED' : 'REPORT-ONLY'}: ${problem}`); } } @@ -1306,6 +1405,51 @@ function main() { ruleNotApplicable++; continue; } + const failedFrontendAssertions = failedExtendedFrontendAssertions( + detectorResult, + oracleSelection.frontend + ); + if (failedFrontendAssertions.length > 0) { + addDrift( + { + ruleId, + version: leg.version, + driftVersion: leg.version, + queryName, + role, + query, + driftClass: 'frontend-contract-mismatch', + evidence: + `${ruleId} @ ${leg.version} [${queryName}]: frontend assertion(s) failed: ` + + failedFrontendAssertions.join(', '), + frontendAssertions: failedFrontendAssertions, + frontendMismatches: detectorResult.mismatches || [], + remediation: { + action: 'update-detector', + target: + spec.detectorPath || + `packages/osd-monaco/src/ppl/lint/rules/${ruleId.replace(/-/g, '_')}.ts`, + detail: + `Reproduce this contract query against the reported OSD commit and candidate ` + + `grammar. Restore the exact message/action/fix behavior, or update the contract ` + + `only after confirming an intentional product change.`, + }, + }, + leg, + { + enforced: isEnforced, + contractFile: file, + expectationRange: expectation.version, + expectationEngine: expectation.engine, + } + ); + ruleDrifts++; + compared++; + if (role === 'trigger') { + triggersCompared++; + } + continue; + } const { observed, usable } = readBackendObservation(backendEntry, detectorResult); if (!usable) { // No comparable pair, so there is nothing to classify. Attempting it @@ -1627,6 +1771,7 @@ function main() { coverageHoles.filter((h) => h.enforced && !h.blocking).length, missingContractCount: missingContracts.length, blockingMissingContractCount: blockingMissingContracts.length, + blockingShippingCensusProblems: blockingShippingCensusProblems.length, enforcedInconclusive: enforcedInconclusive.length, // An inconclusive default-error rule fails too: "we could not check" must // never render as "it is fine". @@ -1634,6 +1779,7 @@ function main() { enforcedDrifts.length === 0 && enforcedHoles.length === 0 && blockingMissingContracts.length === 0 && + blockingShippingCensusProblems.length === 0 && enforcedInconclusive.length === 0, }, }; @@ -1668,7 +1814,8 @@ function main() { console.error( `[ppl-lint-multiversion] FAIL: ${enforcedDrifts.length} drift(s), ` + `${enforcedHoles.length} coverage hole(s), ${blockingMissingContracts.length} unvalidated ` + - `default-error rule(s) and ${enforcedInconclusive.length} inconclusive rule/version pair(s).` + `default-error rule(s), ${blockingShippingCensusProblems.length} shipping census problem(s), ` + + `and ${enforcedInconclusive.length} inconclusive rule/version pair(s).` ); process.exit(1); } @@ -1702,6 +1849,11 @@ function renderMarkdown(report, drifts, coverageHoles, legs) { if (report.result.missingContractCount) { reasons.push(`${report.result.missingContractCount} unvalidated rule(s)`); } + if (report.result.blockingShippingCensusProblems) { + reasons.push( + `${report.result.blockingShippingCensusProblems} shipping census problem(s)` + ); + } lines.push( // Name the surface when a leg is not the default runtime-bundle one, so a // reader knows a column speaks for OSD's compiled grammar rather than the @@ -1786,6 +1938,18 @@ function renderMarkdown(report, drifts, coverageHoles, legs) { lines.push(''); } + if (report.shippingCensus && !report.shippingCensus.passed) { + lines.push('### Shipping census'); + lines.push(''); + for (const problem of report.shippingCensus.problems || []) { + lines.push( + `- ${report.shippingCensus.blocking ? '**ENFORCED:**' : '**REPORT ONLY:**'} ${problem}. ` + + `Align \`manifest.json\` with the approved OSD shipping catalog.` + ); + } + lines.push(''); + } + if (coverageHoles.length > 0) { lines.push('### Coverage holes'); lines.push(''); diff --git a/scripts/ppl-lint/annotate.mjs b/scripts/ppl-lint/annotate.mjs index 9c9d6bf2c16..6cb2e031a9c 100644 --- a/scripts/ppl-lint/annotate.mjs +++ b/scripts/ppl-lint/annotate.mjs @@ -215,6 +215,25 @@ export function buildAnnotations(report, { contractsDir, workspace, readFile = r }); } + const shippingCensus = report.shippingCensus; + if (shippingCensus && shippingCensus.passed === false) { + const manifestText = contractText('manifest.json'); + for (const problem of shippingCensus.problems || []) { + const blocking = shippingCensus.blocking !== false; + annotations.push({ + level: blocking ? 'error' : 'warning', + file: contractRepoPath(contractsDir, 'manifest.json', workspace), + line: findJsonKeyLine(manifestText, manifestKeyFor(problem)), + title: 'PPL lint shipping census mismatch', + message: + `${problem}\n` + + (blocking + ? 'FIX: align the active SQL manifest with the approved OSD shipping catalog.' + : 'REPORT ONLY: align the active SQL manifest with the approved OSD shipping catalog before enabling census enforcement.'), + }); + } + } + return annotations; } diff --git a/scripts/ppl-lint/assemble-run-manifest.mjs b/scripts/ppl-lint/assemble-run-manifest.mjs index 4d0cd183503..5d875e593bb 100644 --- a/scripts/ppl-lint/assemble-run-manifest.mjs +++ b/scripts/ppl-lint/assemble-run-manifest.mjs @@ -46,6 +46,47 @@ function readJson(file, errors) { return undefined; } +function failedFrontendAssertions(entry) { + const failures = new Set(); + for (const [field, label] of [ + ['severityMatched', 'severity'], + ['messageMatched', 'message'], + ]) { + if (entry[field] !== true) { + failures.add(label); + } + } + for (const [field, label] of [ + ['deterministicFixMatched', 'deterministic-fix'], + ['aiActionMatched', 'AI-action'], + ['actionDecisionMatched', 'action-decision'], + ['fixMatched', 'syntax-fix'], + ['rawMessageMatched', 'raw-parser-error'], + ['totalErrorsMatched', 'total-error'], + ]) { + if (entry[field] === false) { + failures.add(label); + } + } + if ( + entry.assertions && + typeof entry.assertions === 'object' && + !Array.isArray(entry.assertions) + ) { + for (const [field, matched] of Object.entries(entry.assertions)) { + if (matched === false) { + failures.add(field); + } + } + } + for (const mismatch of Array.isArray(entry.mismatches) ? entry.mismatches : []) { + if (mismatch && typeof mismatch.field === 'string') { + failures.add(mismatch.field); + } + } + return [...failures].sort(); +} + function main() { const artifactErrors = []; const targetRaw = readJson(path.join(ARTIFACTS, 'target.json'), artifactErrors) || {}; @@ -118,6 +159,7 @@ function main() { artifactErrors.push('detector-report.json must contain a non-empty results array'); } const detectorKeys = new Set(); + const reportOnlyDetectorKeys = new Set(); for (const entry of Array.isArray(detector.results) ? detector.results : []) { const key = `${entry.ruleId}::${entry.queryName}`; if (!entry.ruleId || !entry.queryName) { @@ -128,6 +170,10 @@ function main() { artifactErrors.push(`detector-report.json contains duplicate row ${key}`); } detectorKeys.add(key); + if (entry.reportOnly === true) { + reportOnlyDetectorKeys.add(key); + continue; + } if (entry.executionBackend !== executionBackend) { artifactErrors.push( `detector row ${key} executionBackend ${JSON.stringify(entry.executionBackend)} does not match target ${JSON.stringify(executionBackend)}` @@ -143,23 +189,28 @@ function main() { `detector row ${key} count mismatch: expected ${entry.expected}, got ${entry.actual}` ); } - if (entry.severityMatched !== true) { - artifactErrors.push(`detector row ${key} did not match its severity assertion`); + if ( + entry.assertions !== undefined && + (!entry.assertions || + typeof entry.assertions !== 'object' || + Array.isArray(entry.assertions) || + Object.values(entry.assertions).some((matched) => typeof matched !== 'boolean')) + ) { + artifactErrors.push(`detector row ${key} assertions must contain only booleans`); } - if (entry.messageMatched !== true) { - artifactErrors.push(`detector row ${key} did not match its message assertion`); + if (entry.mismatches !== undefined && !Array.isArray(entry.mismatches)) { + artifactErrors.push(`detector row ${key} mismatches must be an array`); } - for (const [field, label] of [ - ['fixMatched', 'fix'], - ['rawMessageMatched', 'raw-message'], - ['totalErrorsMatched', 'total-error'], - ]) { - if (entry[field] === false) { - artifactErrors.push(`detector row ${key} did not match its ${label} assertion`); - } + for (const assertion of failedFrontendAssertions(entry)) { + artifactErrors.push( + `detector row ${key} did not match its ${assertion} assertion` + ); } } for (const [key, entry] of backendByKey) { + if (reportOnlyDetectorKeys.has(key)) { + continue; + } if (!detectorKeys.has(key)) { artifactErrors.push(`backend row ${key} has no matching detector row`); } @@ -192,7 +243,11 @@ function main() { // The selected validation set is the set of rules the detector run actually // evaluated (post schedule filtering). const validationSet = Array.from( - new Set((detector.results || []).map((r) => r.ruleId)) + new Set( + (detector.results || []) + .filter((row) => row.reportOnly !== true) + .map((row) => row.ruleId) + ) ).sort(); const manifest = { @@ -286,14 +341,17 @@ function writeSummary(manifest, detector, backend) { ? `HTTP ${be.observed ? be.observed.httpStatus : '4xx'}` : 'accepted'; const ok = - r.actual === r.expected && - r.severityMatched === true && - r.messageMatched === true && - !!be && - be.outcome === 'pass'; + r.reportOnly === true + ? undefined + : r.actual === r.expected && + failedFrontendAssertions(r).length === 0 && + !!be && + be.outcome === 'pass'; lines.push( `| \`${r.ruleId}\` | \`${r.queryName}\` | \`${manifest.engineVersion || '—'}\` | ` + - `\`${shortHash(manifest.grammarHash)}\` | ${detectorCell} | ${backendCell} | ${ok ? 'Pass' : 'Fail'} |` + `\`${shortHash(manifest.grammarHash)}\` | ${detectorCell} | ${backendCell} | ${ + ok === undefined ? 'Report only' : ok ? 'Pass' : 'Fail' + } |` ); } diff --git a/scripts/ppl-lint/contract-schema.mjs b/scripts/ppl-lint/contract-schema.mjs index ca3c5a809c2..b1da53617fa 100644 --- a/scripts/ppl-lint/contract-schema.mjs +++ b/scripts/ppl-lint/contract-schema.mjs @@ -8,7 +8,14 @@ const CONTRACT_SCHEMA_VERSIONS = new Set([3, 4]); const APPLICABLE_BACKEND_KINDS = new Set(['rejection', 'result-shape', 'advisory']); const CONTRACT_CHANNELS = new Set(['lint', 'syntax']); const QUERY_ROLES = new Set(['trigger', 'control', 'suppression-control']); -const LINT_FRONTEND_FIELDS = new Set(['count', 'severity', 'matchMessage']); +const LINT_V3_FRONTEND_FIELDS = new Set(['count', 'severity', 'matchMessage']); +const LINT_V4_FRONTEND_FIELDS = new Set([ + 'count', + 'severity', + 'messageEquals', + 'deterministicFix', + 'aiAction', +]); const SYNTAX_FRONTEND_FIELDS = new Set([ 'count', 'code', @@ -40,6 +47,13 @@ function requireNonEmptyString(value, label) { return value; } +function requireString(value, label) { + if (typeof value !== 'string') { + throw new TypeError(`${label} must be a string.`); + } + return value; +} + function requireNonNegativeInteger(value, label) { if (!Number.isInteger(value) || value < 0) { throw new TypeError(`${label} must be a non-negative integer.`); @@ -61,6 +75,85 @@ function assertKnownKeys(value, allowed, label) { } } +function normalizeRange(value, label) { + const range = requireObject(value, label); + assertKnownKeys( + range, + new Set(['startLine', 'startColumn', 'endLine', 'endColumn']), + label + ); + for (const field of ['startLine', 'endLine']) { + if (!Number.isInteger(range[field]) || range[field] < 1) { + throw new TypeError(`${label}.${field} must be a positive integer.`); + } + } + for (const field of ['startColumn', 'endColumn']) { + requireNonNegativeInteger(range[field], `${label}.${field}`); + } + if ( + range.endLine < range.startLine || + (range.endLine === range.startLine && range.endColumn < range.startColumn) + ) { + throw new Error(`${label} must end at or after its start.`); + } + return { + startLine: range.startLine, + startColumn: range.startColumn, + endLine: range.endLine, + endColumn: range.endColumn, + }; +} + +function normalizeDeterministicFix(value, label) { + const fix = requireObject(value, label); + if (typeof fix.offered !== 'boolean') { + throw new TypeError(`${label}.offered must be a boolean.`); + } + const allowed = new Set([ + 'offered', + 'title', + 'text', + 'range', + 'expectedText', + 'appliedQuery', + ]); + assertKnownKeys(fix, allowed, label); + if (!fix.offered) { + if (Object.keys(fix).length !== 1) { + throw new Error(`${label} must contain only offered when no fix is expected.`); + } + return { offered: false }; + } + + const normalized = { + offered: true, + title: requireNonEmptyString(fix.title, `${label}.title`), + text: requireString(fix.text, `${label}.text`), + range: normalizeRange(fix.range, `${label}.range`), + expectedText: requireString(fix.expectedText, `${label}.expectedText`), + appliedQuery: requireString(fix.appliedQuery, `${label}.appliedQuery`), + }; + return normalized; +} + +function normalizeAiAction(value, label) { + const action = requireObject(value, label); + if (typeof action.offered !== 'boolean') { + throw new TypeError(`${label}.offered must be a boolean.`); + } + assertKnownKeys(action, new Set(['offered', 'commandId']), label); + if (!action.offered) { + if (Object.keys(action).length !== 1) { + throw new Error(`${label} must contain only offered when no AI action is expected.`); + } + return { offered: false }; + } + return { + offered: true, + commandId: requireNonEmptyString(action.commandId, `${label}.commandId`), + }; +} + export function contractChannel(spec) { requireObject(spec, 'contract'); const channel = spec.channel === undefined ? 'lint' : spec.channel; @@ -75,9 +168,10 @@ export function contractChannel(spec) { /** * Normalize legacy detector fields and channel-specific frontend assertions. * - * Callers continue to receive `count`, `severity`, and `matchMessage` for lint - * contracts while syntax contracts can assert stable parser error identity, - * quick-fix text, raw-message preservation, and the total syntax error census. + * Schema-v3 lint contracts retain substring messages. Schema-v4 lint contracts + * assert exact messages and exact deterministic/AI action availability. Syntax + * contracts assert stable parser error identity, quick-fix presence or absence, + * raw-message preservation, and the total syntax error census. */ export function normalizeFrontendOracle(spec, queryExpectation) { const channel = contractChannel(spec); @@ -103,33 +197,72 @@ export function normalizeFrontendOracle(spec, queryExpectation) { : { count: queryExpectation.detectorCount, severity: queryExpectation.severity, - matchMessage: queryExpectation.matchMessage, + ...(queryExpectation.matchMessage !== undefined + ? { matchMessage: queryExpectation.matchMessage } + : {}), + ...(queryExpectation.messageEquals !== undefined + ? { messageEquals: queryExpectation.messageEquals } + : {}), + ...(queryExpectation.deterministicFix !== undefined + ? { deterministicFix: queryExpectation.deterministicFix } + : {}), + ...(queryExpectation.aiAction !== undefined + ? { aiAction: queryExpectation.aiAction } + : {}), }; - assertKnownKeys(frontend, LINT_FRONTEND_FIELDS, `[${spec.ruleId}] frontend`); + const allowed = + spec.schemaVersion === 3 ? LINT_V3_FRONTEND_FIELDS : LINT_V4_FRONTEND_FIELDS; + assertKnownKeys(frontend, allowed, `[${spec.ruleId}] frontend`); requireNonNegativeInteger(frontend.count, `[${spec.ruleId}] frontend.count`); assertOptionalString(frontend.severity, `[${spec.ruleId}] frontend.severity`); - if ( - frontend.matchMessage !== undefined && - typeof frontend.matchMessage !== 'string' - ) { - throw new TypeError( - `[${spec.ruleId}] frontend.matchMessage must be a string when present.` + if (spec.schemaVersion === 3) { + if ( + frontend.matchMessage !== undefined && + typeof frontend.matchMessage !== 'string' + ) { + throw new TypeError( + `[${spec.ruleId}] frontend.matchMessage must be a string when present.` + ); + } + } else { + assertOptionalString( + frontend.messageEquals, + `[${spec.ruleId}] frontend.messageEquals` ); } if ( hasFrontend && - (queryExpectation.severity !== undefined || - queryExpectation.matchMessage !== undefined) + ['severity', 'matchMessage', 'messageEquals', 'deterministicFix', 'aiAction'].some( + (field) => queryExpectation[field] !== undefined + ) ) { throw new Error( - `[${spec.ruleId}] severity and matchMessage must be nested under frontend when frontend is present.` + `[${spec.ruleId}] lint assertions must be nested under frontend when frontend is present.` ); } return { channel, count: frontend.count, severity: frontend.severity, - matchMessage: frontend.matchMessage, + ...(spec.schemaVersion === 3 + ? { matchMessage: frontend.matchMessage } + : { + messageEquals: frontend.messageEquals, + deterministicFix: + frontend.deterministicFix === undefined + ? undefined + : normalizeDeterministicFix( + frontend.deterministicFix, + `[${spec.ruleId}] frontend.deterministicFix` + ), + aiAction: + frontend.aiAction === undefined + ? undefined + : normalizeAiAction( + frontend.aiAction, + `[${spec.ruleId}] frontend.aiAction` + ), + }), }; } @@ -145,7 +278,15 @@ export function normalizeFrontendOracle(spec, queryExpectation) { assertKnownKeys(frontend, SYNTAX_FRONTEND_FIELDS, `[${spec.ruleId}] frontend`); requireNonNegativeInteger(frontend.count, `[${spec.ruleId}] frontend.count`); requireNonEmptyString(frontend.code, `[${spec.ruleId}] frontend.code`); - assertOptionalString(frontend.fixText, `[${spec.ruleId}] frontend.fixText`); + if ( + frontend.fixText !== undefined && + frontend.fixText !== null && + (typeof frontend.fixText !== 'string' || frontend.fixText.length === 0) + ) { + throw new TypeError( + `[${spec.ruleId}] frontend.fixText must be a non-empty string or null when present.` + ); + } if ( frontend.matchMessage !== undefined && typeof frontend.matchMessage !== 'string' @@ -170,9 +311,66 @@ export function normalizeFrontendOracle(spec, queryExpectation) { ); } } + if (spec.wiring && frontend.code !== spec.wiring.code) { + throw new Error( + `[${spec.ruleId}] frontend.code ${describe(frontend.code)} does not match ` + + `contract.wiring.code ${describe(spec.wiring.code)}.` + ); + } return { channel, ...frontend }; } +/** + * Active shipping contracts are stricter than dormant compatibility contracts: + * every lint finding pins its exact message and action mode, every lint control + * pins action absence, and every syntax case pins fix/raw-error/error-count state. + */ +export function assertShippingFrontendOracles(spec, expectation) { + if (spec.schemaVersion !== 4) { + throw new Error(`[${spec.ruleId}] active shipping contracts must use schemaVersion 4.`); + } + for (const queryName of assertExactQueryCoverage(spec, expectation)) { + const frontend = normalizeFrontendOracle(spec, expectation.queries[queryName]); + const label = `[${spec.ruleId}/${queryName}] frontend`; + + if (frontend.channel === 'syntax') { + if (frontend.fixText === undefined) { + throw new Error(`${label}.fixText must explicitly assert fix presence or absence.`); + } + if (frontend.rawMessage === undefined) { + throw new Error(`${label}.rawMessage must be explicitly asserted.`); + } + if (frontend.totalErrors === undefined) { + throw new Error(`${label}.totalErrors must be explicitly asserted.`); + } + if (frontend.count > 0 && frontend.matchMessage === undefined) { + throw new Error(`${label}.matchMessage is required for a syntax finding.`); + } + continue; + } + + if (frontend.deterministicFix === undefined) { + throw new Error(`${label}.deterministicFix must be explicitly asserted.`); + } + if (frontend.aiAction === undefined) { + throw new Error(`${label}.aiAction must be explicitly asserted.`); + } + if (frontend.count > 0) { + if (frontend.severity === undefined) { + throw new Error(`${label}.severity is required for a lint finding.`); + } + if (frontend.messageEquals === undefined) { + throw new Error(`${label}.messageEquals is required for a lint finding.`); + } + if (frontend.deterministicFix.offered && frontend.aiAction.offered) { + throw new Error(`${label} cannot offer deterministic and AI actions together.`); + } + } else if (frontend.deterministicFix.offered || frontend.aiAction.offered) { + throw new Error(`${label} must not offer actions when no finding is expected.`); + } + } +} + export function normalizeLintWiring(ruleId, wiring, label = 'wiring') { requireNonEmptyString(ruleId, `${label}.id`); requireObject(wiring, label); @@ -453,11 +651,7 @@ export function resolveBackendOracle(spec, queryExpectation, executionBackend) { const schemaVersion = assertContractSchema(spec); assertExecutionBackend(executionBackend); const frontend = normalizeFrontendOracle(spec, queryExpectation); - const detector = { - count: frontend.count, - severity: frontend.severity, - matchMessage: frontend.matchMessage, - }; + const { channel: _channel, ...detector } = frontend; let oracle; let missingReason; diff --git a/scripts/ppl-lint/run-frontend-contract.mjs b/scripts/ppl-lint/run-frontend-contract.mjs index 305f55fdbb6..68438d4632e 100644 --- a/scripts/ppl-lint/run-frontend-contract.mjs +++ b/scripts/ppl-lint/run-frontend-contract.mjs @@ -77,10 +77,12 @@ import fs from 'fs'; import path from 'path'; import { createRequire } from 'module'; +import { fileURLToPath } from 'url'; import { assertContractSchema, assertExactQueryCoverage, + assertShippingFrontendOracles, classifyBackendReportRow, contractChannel, indexBackendReport, @@ -105,6 +107,8 @@ const CATALOG_MODULE = 'packages/osd-monaco/ppl-lint'; // supports older checkouts (that is the coverage it adds), so fall back to the // source module, which `setup_node_env` transpiles on require anyway. const CATALOG_SOURCE_MODULE = 'packages/osd-monaco/src/ppl/lint/catalog'; +const ACTION_DECISION_MODULE = + 'packages/osd-monaco/src/ppl/lint/action_decision'; const DETECTOR_REGISTRY_MODULE = 'packages/osd-monaco/target/ppl/lint/detector_registry.js'; /** @@ -169,6 +173,54 @@ function loadContractFile(file) { return undefined; // unreachable } +export function assertActiveShippingContracts(contracts, { discovery = false } = {}) { + if (discovery) { + return; + } + for (const { file, spec } of contracts) { + for (const expectation of spec.expectations) { + try { + assertShippingFrontendOracles(spec, expectation); + } catch (error) { + throw new Error(`Invalid active shipping contract ${file}: ${error.message}`); + } + } + } +} + +export function selectManifestContractNames(manifest, includeDormant = false) { + if (!Array.isArray(manifest.contracts)) { + throw new TypeError('manifest.json must have a "contracts" array of file names.'); + } + if (new Set(manifest.contracts).size !== manifest.contracts.length) { + throw new Error('manifest.json "contracts" contains duplicate file names.'); + } + const active = manifest.contracts.map((name) => ({ name, reportOnly: false })); + if (!includeDormant) { + return active; + } + if (!Array.isArray(manifest.dormantContracts)) { + throw new TypeError( + 'PPL_LINT_INCLUDE_DORMANT=1 requires manifest.json "dormantContracts" to be an array.' + ); + } + if (new Set(manifest.dormantContracts).size !== manifest.dormantContracts.length) { + throw new Error('manifest.json "dormantContracts" contains duplicate file names.'); + } + const activeNames = new Set(manifest.contracts); + for (const name of manifest.dormantContracts) { + if (activeNames.has(name)) { + throw new Error( + `manifest.json contract "${name}" cannot be both active and dormant.` + ); + } + } + return [ + ...active, + ...manifest.dormantContracts.map((name) => ({ name, reportOnly: true })), + ]; +} + /** Load every *.spec.json under the contract dir, honoring manifest.json if present. */ function loadContracts() { const dir = process.env.PPL_LINT_CONTRACT_DIR; @@ -179,8 +231,16 @@ function loadContracts() { fatal(`Contract file not found: ${single}`); } const contract = loadContractFile(single); + try { + assertActiveShippingContracts([contract], { + discovery: process.env.PPL_LINT_DISCOVERY === '1', + }); + } catch (error) { + fatal(error.message); + } return { - contracts: [contract], + contracts: [{ ...contract, reportOnly: false }], + activeContracts: [contract], manifest: { contracts: [path.basename(single)] }, manifestPath: '', }; @@ -195,6 +255,7 @@ function loadContracts() { const manifestPath = path.join(dir, 'manifest.json'); let files; + let selectedFiles; let manifest; if (fs.existsSync(manifestPath)) { try { @@ -202,28 +263,69 @@ function loadContracts() { } catch (error) { fatal(`Invalid contract manifest ${manifestPath}: ${error.message}`); } - if (!Array.isArray(manifest.contracts)) { - fatal(`manifest.json must have a "contracts" array of file names.`); - } - if (new Set(manifest.contracts).size !== manifest.contracts.length) { - fatal(`manifest.json "contracts" contains duplicate file names.`); + try { + selectedFiles = selectManifestContractNames( + manifest, + process.env.PPL_LINT_INCLUDE_DORMANT === '1' + ); + } catch (error) { + fatal(error.message); } - files = manifest.contracts.map((name) => path.join(dir, name)); + files = selectedFiles.map(({ name }) => path.join(dir, name)); } else { files = fs .readdirSync(dir) .filter((f) => f.endsWith('.spec.json')) .sort() .map((f) => path.join(dir, f)); + selectedFiles = files.map((file) => ({ + name: path.basename(file), + reportOnly: false, + })); } - const contracts = files.map((file) => { + const contracts = files.map((file, index) => { if (!fs.existsSync(file)) { fatal(`Contract referenced by manifest not found: ${file}`); } - return loadContractFile(file); + return { + ...loadContractFile(file), + reportOnly: selectedFiles[index].reportOnly, + }; }); - return { contracts, manifest: manifest || { contracts: files.map(path.basename) }, manifestPath }; + const activeContracts = contracts + .filter(({ reportOnly }) => !reportOnly) + .map(({ file, spec }) => ({ file, spec })); + try { + assertActiveShippingContracts(activeContracts, { + discovery: process.env.PPL_LINT_DISCOVERY === '1', + }); + } catch (error) { + fatal(error.message); + } + return { + contracts, + activeContracts, + manifest: manifest || { contracts: files.map(path.basename) }, + manifestPath, + }; +} + +function resolveActionDecision(module) { + if (!module) { + return undefined; + } + for (const name of [ + 'decidePPLLintAction', + 'decidePPLDiagnosticAction', + 'getPPLDiagnosticActionDecision', + 'decideDiagnosticAction', + ]) { + if (typeof module[name] === 'function') { + return module[name]; + } + } + return undefined; } function loadOsd() { @@ -260,6 +362,7 @@ function loadOsd() { // so a checkout without the built export can still run the compiled surface. const catalogModule = resolveOsd(CATALOG_MODULE, { optional: true }) || resolveOsd(CATALOG_SOURCE_MODULE); + const actionDecisionModule = resolveOsd(ACTION_DECISION_MODULE, { optional: true }); const { getBundledCatalog } = catalogModule; const registry = resolveOsd(DETECTOR_REGISTRY_MODULE, { optional: true }); if (typeof getBundledCatalog !== 'function') { @@ -276,6 +379,7 @@ function loadOsd() { fatal(`PPLLanguageAnalyzer not found in ${ANALYZER_MODULE}.`); } const analyzer = new PPLLanguageAnalyzer(); + const headless = resolveOsd(HEADLESS_MODULE, { optional: true }); return { surface: SURFACE, // Same (query, grammar, context) shape as the bundle path so the main loop @@ -286,6 +390,10 @@ function loadOsd() { }, getBundledCatalog, getDetector, + decideAction: + resolveActionDecision(headless) || + resolveActionDecision(catalogModule) || + resolveActionDecision(actionDecisionModule), osdRoot, }; } @@ -310,6 +418,10 @@ function loadOsd() { deserializeBundleOrThrow, lintQuery: lintQueryWithBundle, validateSyntax, + decideAction: + resolveActionDecision(headless) || + resolveActionDecision(catalogModule) || + resolveActionDecision(actionDecisionModule), getBundledCatalog, getDetector, osdRoot, @@ -486,6 +598,9 @@ function selectExpectation(spec, version, isCalcite, failures, { allowMissing = function checkWiring(spec, catalog, getDetector, failures) { const { ruleId, wiring } = spec; if (contractChannel(spec) === 'syntax') { + if (!wiring) { + failures.push(`[${ruleId}] contract.wiring is required for strict syntax wiring.`); + } return { id: ruleId, syntaxCode: wiring && wiring.code }; } const entry = catalog.find((c) => c.id === ruleId); @@ -494,7 +609,8 @@ function checkWiring(spec, catalog, getDetector, failures) { return undefined; } if (!wiring) { - return entry; // no wiring block to assert + failures.push(`[${ruleId}] contract.wiring is required for strict catalog comparison.`); + return entry; } let expected; @@ -564,11 +680,300 @@ function buildContext(spec, engineVersion) { return context; } +function rangeOffsets(query, range) { + const lineStarts = [0]; + for (let index = 0; index < query.length; index += 1) { + if (query[index] === '\n') { + lineStarts.push(index + 1); + } + } + const offset = (line, column) => { + const lineStart = lineStarts[line - 1]; + if (lineStart === undefined) { + throw new Error(`range line ${line} is outside a ${lineStarts.length}-line query`); + } + const lineEnd = lineStarts[line] === undefined ? query.length : lineStarts[line] - 1; + if (lineStart + column > lineEnd) { + throw new Error(`range column ${column} is outside query line ${line}`); + } + return lineStart + column; + }; + return { + start: offset(range.startLine, range.startColumn), + end: offset(range.endLine, range.endColumn), + }; +} + +function materializeDeterministicFix(query, diagnostic) { + if (!diagnostic.fix) { + return undefined; + } + const range = diagnostic.fix.range || diagnostic.range; + const { start, end } = rangeOffsets(query, range); + const sourceText = query.slice(start, end); + const expectedTextMatchesSource = + diagnostic.fix.expectedText === undefined || + diagnostic.fix.expectedText === sourceText; + return { + offered: true, + title: diagnostic.fix.title, + text: diagnostic.fix.text, + range: { + startLine: range.startLine, + startColumn: range.startColumn, + endLine: range.endLine, + endColumn: range.endColumn, + }, + ...(diagnostic.fix.expectedText !== undefined + ? { expectedText: diagnostic.fix.expectedText } + : {}), + ...(!expectedTextMatchesSource + ? { expectedTextMatchesSource: false } + : {}), + appliedQuery: query.slice(0, start) + diagnostic.fix.text + query.slice(end), + }; +} + +function normalizeActionDecision(decision) { + if (typeof decision === 'string') { + return { kind: decision }; + } + if (!decision || typeof decision !== 'object' || Array.isArray(decision)) { + return { kind: 'invalid', value: decision }; + } + return { + kind: decision.kind || decision.type || decision.action, + commandId: decision.commandId || (decision.command && decision.command.id), + }; +} + +function exactEqual(left, right) { + return JSON.stringify(left) === JSON.stringify(right); +} + +export function evaluateFrontendAssertions({ + channel, + query, + matches, + allFrontendFindings = matches, + frontendOracle, + decideAction, +}) { + const assertions = {}; + const mismatches = []; + const record = (field, matched, expected, actual) => { + assertions[field] = matched; + if (!matched) { + mismatches.push({ field, expected, actual }); + } + return matched; + }; + + const severityMatched = + channel === 'syntax' || + !frontendOracle.severity || + matches.length === 0 || + matches.every((finding) => finding.severity === frontendOracle.severity); + if (channel === 'lint' && frontendOracle.severity !== undefined) { + record( + 'severity', + severityMatched, + frontendOracle.severity, + matches.map((finding) => finding.severity) + ); + } + + let messageMatched = true; + if (frontendOracle.matchMessage !== undefined) { + messageMatched = matches.some((finding) => + String(finding.message || '').includes(frontendOracle.matchMessage) + ); + record( + 'message', + messageMatched, + { contains: frontendOracle.matchMessage }, + matches.map((finding) => finding.message) + ); + } else if (frontendOracle.messageEquals !== undefined) { + messageMatched = + matches.length > 0 && + matches.every((finding) => finding.message === frontendOracle.messageEquals); + record( + 'message', + messageMatched, + { equals: frontendOracle.messageEquals }, + matches.map((finding) => finding.message) + ); + } + + let deterministicFixMatched = true; + let deterministicFixActual; + if (frontendOracle.deterministicFix !== undefined) { + const fixes = matches + .map((diagnostic) => materializeDeterministicFix(query, diagnostic)) + .filter(Boolean); + deterministicFixActual = + fixes.length === 0 + ? { offered: false } + : fixes.length === 1 + ? fixes[0] + : { offered: true, count: fixes.length, fixes }; + deterministicFixMatched = record( + 'deterministicFix', + exactEqual(frontendOracle.deterministicFix, deterministicFixActual), + frontendOracle.deterministicFix, + deterministicFixActual + ); + } + + let aiActionMatched = true; + let aiActionActual; + let actionDecisionMatched = true; + let actionDecisionActual; + if (frontendOracle.aiAction !== undefined) { + let decisions = []; + if (typeof decideAction !== 'function') { + aiActionActual = { + unavailable: true, + reason: 'production headless action-decision export is unavailable', + }; + actionDecisionActual = aiActionActual; + } else { + try { + for (const diagnostic of matches) { + decisions.push( + normalizeActionDecision( + decideAction({ + channel, + diagnostic, + hasDeterministicFix: !!diagnostic.fix, + aiFixEligible: diagnostic.aiFix?.eligible !== false, + enableAIFeatures: true, + hasAiFixHandler: true, + chatWired: true, + aiAgentAvailableForSource: true, + }) + ) + ); + } + } catch (error) { + aiActionActual = { + error: + `production action decision failed: ` + + `${error instanceof Error ? error.message : String(error)}`, + }; + actionDecisionActual = aiActionActual; + } + const invalidDecision = decisions.find( + (decision) => !['deterministic', 'ai', 'none'].includes(decision.kind) + ); + if (aiActionActual === undefined && invalidDecision) { + aiActionActual = { invalidDecision }; + } + if (aiActionActual === undefined) { + const actions = decisions + .filter((decision) => decision.kind === 'ai') + .map((decision) => ({ + offered: true, + ...(decision.commandId !== undefined + ? { commandId: decision.commandId } + : {}), + })); + aiActionActual = + actions.length === 0 + ? { offered: false } + : actions.length === 1 + ? actions[0] + : { offered: true, count: actions.length, actions }; + } + if (actionDecisionActual === undefined) { + actionDecisionActual = decisions.map((decision) => decision.kind); + } + } + aiActionMatched = record( + 'aiAction', + exactEqual(frontendOracle.aiAction, aiActionActual), + frontendOracle.aiAction, + aiActionActual + ); + const expectedDecisionKind = frontendOracle.deterministicFix?.offered + ? 'deterministic' + : frontendOracle.aiAction.offered + ? 'ai' + : 'none'; + const expectedDecisions = matches.map(() => expectedDecisionKind); + actionDecisionMatched = record( + 'actionDecision', + exactEqual(expectedDecisions, actionDecisionActual), + expectedDecisions, + actionDecisionActual + ); + } + + let syntaxFixMatched = true; + if (channel === 'syntax' && frontendOracle.fixText !== undefined) { + const fixes = allFrontendFindings + .filter((finding) => finding.fix) + .map((finding) => finding.fix.text); + const expected = + frontendOracle.fixText === null + ? { offered: false } + : { offered: true, text: frontendOracle.fixText }; + const actual = + fixes.length === 0 + ? { offered: false } + : fixes.length === 1 + ? { offered: true, text: fixes[0] } + : { offered: true, count: fixes.length, texts: fixes }; + syntaxFixMatched = record('syntaxFix', exactEqual(expected, actual), expected, actual); + } + + let rawMessageMatched = true; + if (channel === 'syntax' && frontendOracle.rawMessage !== undefined) { + const rawMessages = allFrontendFindings + .map((finding) => finding.rawMessage) + .filter((message) => typeof message === 'string' && message.length > 0); + const actual = rawMessages.length > 0; + rawMessageMatched = record( + 'rawParserError', + actual === frontendOracle.rawMessage, + frontendOracle.rawMessage, + actual + ); + } + + let totalErrorsMatched = true; + if (channel === 'syntax' && frontendOracle.totalErrors !== undefined) { + totalErrorsMatched = record( + 'totalErrors', + allFrontendFindings.length === frontendOracle.totalErrors, + frontendOracle.totalErrors, + allFrontendFindings.length + ); + } + + return { + assertions, + mismatches, + severityMatched, + messageMatched, + deterministicFixMatched, + deterministicFixActual, + aiActionMatched, + aiActionActual, + actionDecisionMatched, + actionDecisionActual, + syntaxFixMatched, + rawMessageMatched, + totalErrorsMatched, + }; +} + function equalSets(left, right) { return left.size === right.size && [...left].every((value) => right.has(value)); } -function buildCensus(contracts, manifest, catalog) { +export function buildCensus(contracts, manifest, catalog) { const problems = []; const byFile = new Map( contracts.map(({ file, spec }) => [path.basename(file), spec]) @@ -610,6 +1015,27 @@ function buildCensus(contracts, manifest, catalog) { .filter(({ spec }) => contractChannel(spec) === 'syntax') .map(({ spec }) => spec.ruleId) .sort(); + const catalogRuleIds = catalog.map((rule) => rule.id).sort(); + const duplicateCatalogRuleIds = catalogRuleIds.filter( + (ruleId, index) => catalogRuleIds.indexOf(ruleId) !== index + ); + if (duplicateCatalogRuleIds.length > 0) { + problems.push( + `catalog contains duplicate rule IDs: ${[ + ...new Set(duplicateCatalogRuleIds), + ].join(', ')}.` + ); + } + const syntaxRulesInCatalog = activeSyntaxRules.filter((ruleId) => + catalogRuleIds.includes(ruleId) + ); + if (syntaxRulesInCatalog.length > 0) { + problems.push( + `syntax features must remain outside the detector catalog: ${syntaxRulesInCatalog.join( + ', ' + )}.` + ); + } const enabledRules = catalog .filter((rule) => rule.enabled) .map((rule) => rule.id) @@ -629,6 +1055,12 @@ function buildCensus(contracts, manifest, catalog) { `expected one required syntax feature, found ${requiredSyntaxFeatures.length}.` ); } + if (!equalSets(new Set(requiredSyntaxFeatures), new Set(['command-suggestion']))) { + problems.push( + `required syntax features must equal ["command-suggestion"], found ` + + `${JSON.stringify(requiredSyntaxFeatures)}.` + ); + } if (activeContractRules.length !== 13) { problems.push(`expected 13 active contracts, found ${activeContractRules.length}.`); } @@ -669,7 +1101,7 @@ function main() { const reportPath = process.env.PPL_LINT_REPORT; const target = loadTarget(); const backendReport = loadBackendReport(target); - const { contracts, manifest, manifestPath } = loadContracts(); + const { contracts, activeContracts, manifest, manifestPath } = loadContracts(); const osd = loadOsd(); const { @@ -677,11 +1109,12 @@ function main() { getDetector, lintQuery, validateSyntax, + decideAction, osdRoot, surface, } = osd; const catalog = getBundledCatalog(); - const census = buildCensus(contracts, manifest, catalog); + const census = buildCensus(activeContracts, manifest, catalog); // The compiled surface lints with OSD's own checked-in grammar, so there is no // candidate bundle to load. On the runtime surface a missing bundle stays a hard @@ -698,6 +1131,7 @@ function main() { } const failures = []; + const reportOnlyFailures = []; // Contracts this surface did not score, recorded so the report says a rule was // skipped for surface rather than leaving its absence unexplained. const skippedForSurface = []; @@ -719,6 +1153,8 @@ function main() { observeAnalytics, observeOnly, differential: !!backendReport, + includedDormant: process.env.PPL_LINT_INCLUDE_DORMANT === '1', + reportOnlyFailures, // Census of the rules that ship enabled at ERROR severity, read from the OSD // catalog this run linted with. The multi-version aggregator enforces its // `defaultError` manifest set against this list, so a rule that becomes @@ -755,18 +1191,19 @@ function main() { `contracts=${contracts.length}` ); - for (const { file, spec } of contracts) { + for (const { file, spec, reportOnly = false } of contracts) { const ruleId = spec.ruleId; const index = spec.index; const channel = contractChannel(spec); - const entry = checkWiring(spec, catalog, getDetector, failures); + const scoringFailures = reportOnly ? reportOnlyFailures : failures; + const entry = checkWiring(spec, catalog, getDetector, scoringFailures); if (!entry) { continue; } // A contract runs on PR only when scheduled for PR; nightly runs everything. const contractSchedule = spec.schedule || 'pr'; - if (schedule === 'pr' && contractSchedule !== 'pr') { + if (!reportOnly && schedule === 'pr' && contractSchedule !== 'pr') { log(`SKIP ${ruleId} (schedule=${contractSchedule}, running ${schedule}) — ${path.basename(file)}`); continue; } @@ -796,6 +1233,7 @@ function main() { query: (queryDef.query || '').split('{{index}}').join(index), surface, executionBackend, + ...(reportOnly ? { reportOnly: true } : {}), outcome: 'not-applicable', notApplicable: `contract declares grammarSurface "${contractSurface}"`, }); @@ -804,7 +1242,7 @@ function main() { } const context = buildContext(spec, engineVersion); - const expectation = selectExpectation(spec, engineVersion, context.isCalcite, failures, { + const expectation = selectExpectation(spec, engineVersion, context.isCalcite, scoringFailures, { allowMissing: observeOnly, }); if (!expectation) { @@ -823,6 +1261,7 @@ function main() { query, surface, executionBackend, + ...(reportOnly ? { reportOnly: true } : {}), outcome: 'not-applicable', notApplicable: 'runtimeOnly rule does not run on the compiled-simplified surface', }); @@ -850,6 +1289,7 @@ function main() { query, surface, executionBackend, + ...(reportOnly ? { reportOnly: true } : {}), expected: 0, actual: matches.length, severities: matches.map((m) => m.severity), @@ -882,8 +1322,6 @@ function main() { } const frontendOracle = oracleSelection.frontend; const expectedCount = frontendOracle.count; - const expectedSeverity = frontendOracle.severity; - const expectedMessage = frontendOracle.matchMessage; // A `runtimeOnly` rule walks grammar productions that exist only in the // runtime bundle, so `lint_runner` skips it on the compiled surface. Its @@ -904,6 +1342,7 @@ function main() { query, surface, executionBackend, + ...(reportOnly ? { reportOnly: true } : {}), outcome: 'not-applicable', notApplicable: 'runtimeOnly rule does not run on the compiled-simplified surface', }); @@ -934,30 +1373,27 @@ function main() { `expected ${expectedCount}, got ${actual} — ${query}` ); - const severityOk = - channel === 'syntax' || - !expectedSeverity || - actual === 0 || - matches.every((m) => m.severity === expectedSeverity); - const messageOk = - !expectedMessage || - matches.some((m) => (m.message || '').includes(expectedMessage)); - const fixOk = - channel !== 'syntax' || - frontendOracle.fixText === undefined || - matches.some((m) => m.fix && m.fix.text === frontendOracle.fixText); - const rawMessageOk = - channel !== 'syntax' || - frontendOracle.rawMessage === undefined || - matches.some((m) => - frontendOracle.rawMessage - ? typeof m.rawMessage === 'string' && m.rawMessage.length > 0 - : m.rawMessage === undefined - ); - const totalErrorsOk = - channel !== 'syntax' || - frontendOracle.totalErrors === undefined || - allFrontendFindings.length === frontendOracle.totalErrors; + const evaluated = evaluateFrontendAssertions({ + channel, + query, + matches, + allFrontendFindings, + frontendOracle, + decideAction, + }); + const assertions = { count: ok, ...evaluated.assertions }; + const mismatches = [ + ...(ok + ? [] + : [ + { + field: 'count', + expected: expectedCount, + actual, + }, + ]), + ...evaluated.mismatches, + ]; const resultEntry = { ruleId, @@ -968,11 +1404,25 @@ function main() { expected: expectedCount, actual, severities: matches.map((m) => m.severity).filter(Boolean), - severityMatched: severityOk, - messageMatched: messageOk, - fixMatched: fixOk, - rawMessageMatched: rawMessageOk, - totalErrorsMatched: totalErrorsOk, + severityMatched: evaluated.severityMatched, + messageMatched: evaluated.messageMatched, + deterministicFixMatched: evaluated.deterministicFixMatched, + aiActionMatched: evaluated.aiActionMatched, + actionDecisionMatched: evaluated.actionDecisionMatched, + fixMatched: evaluated.syntaxFixMatched, + rawMessageMatched: evaluated.rawMessageMatched, + totalErrorsMatched: evaluated.totalErrorsMatched, + assertions, + mismatches, + ...(evaluated.deterministicFixActual !== undefined + ? { deterministicFix: evaluated.deterministicFixActual } + : {}), + ...(evaluated.aiActionActual !== undefined + ? { aiAction: evaluated.aiActionActual } + : {}), + ...(evaluated.actionDecisionActual !== undefined + ? { actionDecision: evaluated.actionDecisionActual } + : {}), ...(channel === 'syntax' ? { code: frontendOracle.code, @@ -980,39 +1430,16 @@ function main() { totalErrors: allFrontendFindings.length, } : {}), + ...(reportOnly ? { reportOnly: true } : {}), executionBackend, backendOracleStatus: oracleSelection.status, }; - if (!ok) { - failures.push( - `[${ruleId}/${queryName}] expected ${expectedCount} "${ruleId}" diagnostic(s), got ${actual} for: ${query}` - ); - } - if (!severityOk) { - failures.push( - `[${ruleId}/${queryName}] expected severity "${expectedSeverity}" for: ${query}` - ); - } - if (!messageOk) { - failures.push( - `[${ruleId}/${queryName}] expected message to contain "${expectedMessage}" for: ${query}` - ); - } - if (!fixOk) { - failures.push( - `[${ruleId}/${queryName}] expected fix text "${frontendOracle.fixText}" for: ${query}` - ); - } - if (!rawMessageOk) { - failures.push( - `[${ruleId}/${queryName}] expected rawMessage=${frontendOracle.rawMessage} for: ${query}` - ); - } - if (!totalErrorsOk) { - failures.push( - `[${ruleId}/${queryName}] expected ${frontendOracle.totalErrors} total syntax error(s), ` + - `got ${allFrontendFindings.length} for: ${query}` + for (const mismatch of mismatches) { + scoringFailures.push( + `[${ruleId}/${queryName}] frontend.${mismatch.field} mismatch: ` + + `expected ${JSON.stringify(mismatch.expected)}, got ` + + `${JSON.stringify(mismatch.actual)} for: ${query}` ); } @@ -1026,7 +1453,7 @@ function main() { resultEntry.reason = oracleSelection.reason; resultEntry.coverageMissing = oracleSelection.reason; if (!observeAnalytics) { - failures.push( + scoringFailures.push( `[${ruleId}/${queryName}] ${executionBackend} backend coverage missing: ${oracleSelection.reason}.` ); } @@ -1041,7 +1468,9 @@ function main() { if (backendReport) { const be = backendReport.get(`${ruleId}::${queryName}`); if (!be) { - failures.push(`[${ruleId}/${queryName}] no backend report entry (backend did not run this query).`); + scoringFailures.push( + `[${ruleId}/${queryName}] no backend report entry (backend did not run this query).` + ); } else { const backendObservation = classifyBackendReportRow(be); if (oracleSelection.status !== 'applicable') { @@ -1051,7 +1480,7 @@ function main() { resultEntry.backendOutcome = backendObservation.status; } else if (backendObservation.status !== 'observed') { resultEntry.backendOutcome = backendObservation.status; - failures.push( + scoringFailures.push( `[${ruleId}/${queryName}] backend report has no accepted/rejected verdict ` + `(outcome=${JSON.stringify(backendObservation.status)}).` ); @@ -1061,7 +1490,7 @@ function main() { const backendRejected = backendObservation.rejected; resultEntry.backendRejected = backendRejected; if (backendRejected !== expectRejected) { - failures.push( + scoringFailures.push( `[${ruleId}/${queryName}] differential: backend ${backendRejected ? 'rejected' : 'accepted'} ` + `but the contract's backend.kind="${backendKind}" expects ${expectRejected ? 'rejection' : 'acceptance'} for: ${query}` ); @@ -1090,7 +1519,7 @@ function main() { // the pairing rule is scoped to the rules it makes sense for. const detectorFlagged = actual > 0; if (role === 'trigger' && expectRejected && detectorFlagged !== backendRejected) { - failures.push( + scoringFailures.push( `[${ruleId}/${queryName}] differential: trigger detector ${detectorFlagged ? 'flagged' : 'passed'} ` + `but backend ${backendRejected ? 'rejected' : 'accepted'} for: ${query}` ); @@ -1099,7 +1528,7 @@ function main() { // query the rule has to stay quiet on. Unlike a trigger, that claim does // not vary with `backend.kind`. if (role === 'control' && (detectorFlagged || backendRejected)) { - failures.push( + scoringFailures.push( `[${ruleId}/${queryName}] differential: control must pass on both sides but detector ${detectorFlagged ? 'flagged' : 'passed'} ` + `and backend ${backendRejected ? 'rejected' : 'accepted'} for: ${query}` ); @@ -1108,7 +1537,7 @@ function main() { role === 'suppression-control' && (detectorFlagged || !backendRejected) ) { - failures.push( + scoringFailures.push( `[${ruleId}/${queryName}] differential: suppression control must retain a backend ` + `syntax rejection without a "${frontendOracle.code}" suggestion, but frontend ` + `${detectorFlagged ? 'suggested a rewrite' : 'did not suggest a rewrite'} and ` + @@ -1141,7 +1570,15 @@ function main() { process.exit(1); } + if (reportOnlyFailures.length > 0) { + log( + `REPORT-ONLY: ${reportOnlyFailures.length} dormant contract problem(s):\n- ` + + reportOnlyFailures.join('\n- ') + ); + } log(`PASS: all contracts agreed with the OSD detectors on the candidate bundle (schedule=${schedule}).`); } -main(); +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + main(); +} From 6d6e6388b1495883cc4ab9ed18255a6d710e620f Mon Sep 17 00:00:00 2001 From: Hanyu Wei Date: Tue, 4 Aug 2026 13:30:38 -0700 Subject: [PATCH 2/5] fix(ci): isolate PPL lint contract failures Signed-off-by: Hanyu Wei --- .../ppl-lint-multiversion-validation.yml | 10 +- .../workflows/ppl-lint-rule-validation.yml | 5 +- .../remote/PplLintRuleValidationIT.java | 8 +- .../ppl-lint/contracts/agg-on-text.spec.json | 11 - .../contracts/command-suggestion.spec.json | 149 ---- .../contracts/division-by-zero.spec.json | 15 - .../contracts/enabled-false-object.spec.json | 11 - .../contracts/field-validation.spec.json | 33 - .../invalid-capture-group-name.spec.json | 22 - .../ppl-lint/contracts/manifest.json | 11 +- .../multisearch-min-subsearch.spec.json | 11 - .../replace-wildcard-asymmetry.spec.json | 22 - .../contracts/rex-scan-cost.spec.json | 12 +- .../contracts/type-mismatch-numeric.spec.json | 11 - .../contracts/union-min-datasets.spec.json | 11 - ...ed-window-function-in-eventstats.spec.json | 33 - .../wildcard-source-zero-match.spec.json | 7 - scripts/ppl-lint/README.md | 38 +- .../__tests__/aggregate-versions.test.mjs | 63 +- .../__tests__/assemble-run-manifest.test.mjs | 46 +- .../__tests__/contract-schema.test.mjs | 51 +- .../__tests__/run-frontend-contract.test.mjs | 149 ++-- scripts/ppl-lint/aggregate-versions.mjs | 51 +- scripts/ppl-lint/assemble-run-manifest.mjs | 20 +- scripts/ppl-lint/contract-schema.mjs | 53 +- scripts/ppl-lint/run-frontend-contract.mjs | 701 +++++++++--------- 26 files changed, 600 insertions(+), 954 deletions(-) delete mode 100644 integ-test/src/test/resources/ppl-lint/contracts/command-suggestion.spec.json diff --git a/.github/workflows/ppl-lint-multiversion-validation.yml b/.github/workflows/ppl-lint-multiversion-validation.yml index df1aad11185..294197584f4 100644 --- a/.github/workflows/ppl-lint-multiversion-validation.yml +++ b/.github/workflows/ppl-lint-multiversion-validation.yml @@ -9,11 +9,10 @@ name: PPL lint multi-version validation # happens to be. A rule that is correct on main can be a false positive on 3.6 or # a false negative on 3.7, and nothing notices. # -# This workflow validates every active shipping contract — 12 detector rules and -# the command-suggestion syntax feature where its runtime grammar surface is -# available — against released engine versions plus the PR build. The aggregate -# still records the exact default-error census separately, but --all-rules makes -# warning/info omissions and syntax regressions visible too. +# This workflow validates all 12 active detector contracts against released +# engine versions plus the PR build. The aggregate still records the exact +# default-error census separately, but --all-rules makes warning/info omissions +# visible too. # # Shape — a per-version matrix of observation legs, then one aggregation: # @@ -837,7 +836,6 @@ jobs: env "${surface_env[@]}" "${observe_env[@]}" \ PPL_LINT_CONTRACT_DIR="$GITHUB_WORKSPACE/integ-test/src/test/resources/ppl-lint/contracts" \ PPL_LINT_SCHEDULE=nightly \ - PPL_LINT_ENFORCE_CENSUS=1 \ PPL_LINT_INCLUDE_DORMANT=1 \ PPL_LINT_TARGET_MANIFEST="$leg/target.json" \ PPL_LINT_REPORT="$leg/detector-report.json" \ diff --git a/.github/workflows/ppl-lint-rule-validation.yml b/.github/workflows/ppl-lint-rule-validation.yml index 7ffabbce2da..c218b7fbda3 100644 --- a/.github/workflows/ppl-lint-rule-validation.yml +++ b/.github/workflows/ppl-lint-rule-validation.yml @@ -37,8 +37,8 @@ concurrency: # # Modes (design §3.4, §4.1.1): # - pull_request: SQL PR validation against the resolved OSD target. The ONLY -# enforcing mode; this is what branch protection pins to. Runs all 13 active -# schedule:pr contracts. The committed default is `main` on the canonical repo; +# enforcing mode; this is what branch protection pins to. Runs all 12 active +# detector contracts. The committed default is `main` on the canonical repo; # it can be overridden by the OSD_REPO/OSD_REF repo variables — see the # "Resolve OSD ref" step. TEMPORARY: those repo variables are currently set to # the unmerged paired OSD branch that ships the headless lint API this job @@ -295,7 +295,6 @@ jobs: PPL_LINT_TARGET_MANIFEST: ${{ github.workspace }}/artifacts/target.json PPL_LINT_BACKEND_REPORT: ${{ github.workspace }}/artifacts/backend-report.json PPL_LINT_REPORT: ${{ github.workspace }}/detector-report.json - PPL_LINT_ENFORCE_CENSUS: '1' PPL_LINT_INCLUDE_DORMANT: ${{ steps.schedule.outputs.value == 'nightly' && '1' || '0' }} run: | # pipefail so the runner's non-zero exit propagates through `tee` — diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/PplLintRuleValidationIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/PplLintRuleValidationIT.java index c6a503fddaf..7410eb32023 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/PplLintRuleValidationIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/PplLintRuleValidationIT.java @@ -60,13 +60,13 @@ *

While the ephemeral cluster is alive, the test also exports the candidate runtime grammar * bundle it built ({@code GET /_plugins/_ppl/_grammar}) and a small target manifest pairing the * bundle with the backend version and grammar hash. These become workflow artifacts that the - * detector-validation job injects into OSD's headless lint and syntax APIs, so both halves validate - * against the SAME candidate grammar (design §4.2, §4.3). Export runs only when {@code + * detector-validation job injects into OSD's headless lint API, so both halves validate against the + * SAME candidate grammar (design §4.2, §4.3). Export runs only when {@code * -Dppl.lint.grammar.bundle} is set (CI); local runs without it are unaffected. * *

The suite honors {@code -Dppl.lint.schedule=pr|nightly} (default {@code pr}): a PR run skips - * contracts declaring {@code schedule: "nightly"}, while nightly runs all 13 active contracts. The - * filter holds new detector and syntax contracts back from PR runs while their standard and + * contracts declaring {@code schedule: "nightly"}, while nightly runs all 12 active detector + * contracts. The filter holds new detector contracts back from PR runs while their standard and * analytics oracles are still settling. * *

Note that a contract which RUNS also ASSERTS. This class does not consult the manifest's diff --git a/integ-test/src/test/resources/ppl-lint/contracts/agg-on-text.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/agg-on-text.spec.json index ebec7087274..5b9ece1480b 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/agg-on-text.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/agg-on-text.spec.json @@ -61,10 +61,6 @@ "messageEquals": "Numeric aggregation on a text field may return no value (null), because text is not stored as a number.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -91,10 +87,6 @@ "messageEquals": "Numeric aggregation on a text field may return no value (null), because text is not stored as a number.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -119,9 +111,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/command-suggestion.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/command-suggestion.spec.json deleted file mode 100644 index 021a37bb4b3..00000000000 --- a/integ-test/src/test/resources/ppl-lint/contracts/command-suggestion.spec.json +++ /dev/null @@ -1,149 +0,0 @@ -{ - "schemaVersion": 4, - "ruleId": "command-suggestion", - "channel": "syntax", - "grammarSurface": "runtime-bundle", - "schedule": "pr", - "wiring": { - "code": "UNKNOWN_COMMAND" - }, - "backendFixture": { - "indices": [ - "ACCOUNT" - ], - "clusterSettings": { - "calcite": true, - "calciteFallback": false - } - }, - "frontendContext": { - "isCalcite": true - }, - "index": "opensearch-sql_test_index_account", - "queries": { - "misspelled-command": { - "role": "trigger", - "query": "source={{index}} | wherre age > 1" - }, - "valid-command-control": { - "role": "control", - "query": "source={{index}} | where age > 1" - }, - "unrecognizable-command": { - "role": "suppression-control", - "query": "source={{index}} | zzzzzzzz" - }, - "incomplete-expression": { - "role": "suppression-control", - "query": "source={{index}} | where age >" - } - }, - "expectations": [ - { - "version": ">=0.0.0", - "queries": { - "misspelled-command": { - "frontend": { - "count": 1, - "code": "UNKNOWN_COMMAND", - "fixText": "where", - "matchMessage": "Unknown command \"wherre\". Did you mean \"where\"?", - "rawMessage": true, - "totalErrors": 1 - }, - "backends": { - "standard": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400 - } - }, - "analytics": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400 - } - } - } - }, - "valid-command-control": { - "frontend": { - "count": 0, - "code": "UNKNOWN_COMMAND", - "fixText": null, - "rawMessage": false, - "totalErrors": 0 - }, - "backends": { - "standard": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true - } - }, - "analytics": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true - } - } - } - }, - "unrecognizable-command": { - "frontend": { - "count": 0, - "code": "UNKNOWN_COMMAND", - "fixText": null, - "rawMessage": false, - "totalErrors": 1 - }, - "backends": { - "standard": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400 - } - }, - "analytics": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400 - } - } - } - }, - "incomplete-expression": { - "frontend": { - "count": 0, - "code": "UNKNOWN_COMMAND", - "fixText": null, - "rawMessage": false, - "totalErrors": 1 - }, - "backends": { - "standard": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400 - } - }, - "analytics": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400 - } - } - } - } - } - } - ] -} diff --git a/integ-test/src/test/resources/ppl-lint/contracts/division-by-zero.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/division-by-zero.spec.json index b643094471b..5bb88408554 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/division-by-zero.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/division-by-zero.spec.json @@ -56,10 +56,6 @@ "messageEquals": "Dividing by zero returns no value (null) instead of an error.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -86,10 +82,6 @@ "messageEquals": "Dividing by zero returns no value (null) instead of an error.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -114,9 +106,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { @@ -143,10 +132,6 @@ "messageEquals": "Dividing by zero returns no value (null) instead of an error.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/enabled-false-object.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/enabled-false-object.spec.json index 4e222982589..366a7f85bb1 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/enabled-false-object.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/enabled-false-object.spec.json @@ -63,10 +63,6 @@ "messageEquals": "This field is stored but not searchable, so PPL returns null for it.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -93,10 +89,6 @@ "messageEquals": "This field is stored but not searchable, so PPL returns null for it.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -121,9 +113,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/field-validation.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/field-validation.spec.json index 4094f0af7e0..f9c68833a02 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/field-validation.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/field-validation.spec.json @@ -68,10 +68,6 @@ "messageEquals": "Unknown field \"nonexistent_field\".", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -106,11 +102,7 @@ "endLine": 1, "endColumn": 63 }, - "expectedText": "field=firstname", "appliedQuery": "source=opensearch-sql_test_index_account | grok firstname \"%{WORD:w}\"" - }, - "aiAction": { - "offered": false } }, "backends": { @@ -135,9 +127,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { @@ -169,10 +158,6 @@ "messageEquals": "Unknown field \"nonexistent_field\".", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -215,11 +200,7 @@ "endLine": 1, "endColumn": 63 }, - "expectedText": "field=firstname", "appliedQuery": "source=opensearch-sql_test_index_account | grok firstname \"%{WORD:w}\"" - }, - "aiAction": { - "offered": false } }, "backends": { @@ -252,9 +233,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { @@ -286,10 +264,6 @@ "messageEquals": "Unknown field \"nonexistent_field\".", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -332,11 +306,7 @@ "endLine": 1, "endColumn": 63 }, - "expectedText": "field=firstname", "appliedQuery": "source=opensearch-sql_test_index_account | grok firstname \"%{WORD:w}\"" - }, - "aiAction": { - "offered": false } }, "backends": { @@ -369,9 +339,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/invalid-capture-group-name.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/invalid-capture-group-name.spec.json index b4ba3df32c8..b6a5779ff08 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/invalid-capture-group-name.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/invalid-capture-group-name.spec.json @@ -61,10 +61,6 @@ "messageEquals": "Capture group name \"user_name\" is invalid. Start with a letter and use only letters and numbers.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -99,10 +95,6 @@ "messageEquals": "Capture group name \"user-name\" is invalid. Start with a letter and use only letters and numbers.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -135,9 +127,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { @@ -170,10 +159,6 @@ "messageEquals": "Capture group name \"user_name\" is invalid. Start with a letter and use only letters and numbers.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -208,10 +193,6 @@ "messageEquals": "Capture group name \"user-name\" is invalid. Start with a letter and use only letters and numbers.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -244,9 +225,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/manifest.json b/integ-test/src/test/resources/ppl-lint/contracts/manifest.json index 20a6995acb8..fe720056ba0 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/manifest.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/manifest.json @@ -1,9 +1,8 @@ { "schemaVersion": 4, - "description": "Required PPL frontend/backend compatibility corpus for the approved 12 detector rules plus the command-suggestion syntax feature.", + "description": "PPL frontend/backend compatibility corpus for the approved 12 detector rules.", "contracts": [ "agg-on-text.spec.json", - "command-suggestion.spec.json", "division-by-zero.spec.json", "enabled-false-object.spec.json", "field-validation.spec.json", @@ -38,9 +37,7 @@ "union-min-datasets.spec.json", "unsupported-window-function-in-eventstats.spec.json" ], - "requiredSyntaxFeatures": [ - "command-suggestion.spec.json" - ], + "requiredSyntaxFeatures": [], "nonEnforcing": [ "agg-on-text.spec.json", "division-by-zero.spec.json", @@ -51,8 +48,8 @@ ], "notes": { "enforced": "Reviewed lint error contracts with deterministic backend behavior.", - "defaultError": "Exact approved six-rule detector error census. command-suggestion is an error-channel feature but is intentionally excluded because it is not a detector.", - "requiredSyntaxFeatures": "Syntax-channel features validated through the production runtime grammar listener.", + "defaultError": "Exact approved six-rule detector error census.", + "requiredSyntaxFeatures": "Reserved for future syntax-channel compatibility contracts; currently empty.", "nonEnforcing": "Oracle-quality classification for warning, info, advisory, and result-shape contracts; scheduling determines execution, not this list.", "dormantContracts": "Preserved default-off detector regression contracts. They do not count toward active shipping coverage and must force-enable their detector when run." } diff --git a/integ-test/src/test/resources/ppl-lint/contracts/multisearch-min-subsearch.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/multisearch-min-subsearch.spec.json index 106ed504f01..f13d9d4408c 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/multisearch-min-subsearch.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/multisearch-min-subsearch.spec.json @@ -58,10 +58,6 @@ "messageEquals": "The multisearch command requires at least two subsearches.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -96,10 +92,6 @@ "messageEquals": "The multisearch command requires at least two subsearches.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -132,9 +124,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/replace-wildcard-asymmetry.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/replace-wildcard-asymmetry.spec.json index c4b310bbac8..0ff54ce9fc3 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/replace-wildcard-asymmetry.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/replace-wildcard-asymmetry.spec.json @@ -59,10 +59,6 @@ "messageEquals": "The replace match and replacement have different numbers of \"*\" wildcards. The counts must match.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -97,10 +93,6 @@ "messageEquals": "The replace match and replacement have different numbers of \"*\" wildcards. The counts must match.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -133,9 +125,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { @@ -168,10 +157,6 @@ "messageEquals": "The replace match and replacement have different numbers of \"*\" wildcards. The counts must match.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -206,10 +191,6 @@ "messageEquals": "The replace match and replacement have different numbers of \"*\" wildcards. The counts must match.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -242,9 +223,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/rex-scan-cost.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/rex-scan-cost.spec.json index 5230aefe49f..43d02469f67 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/rex-scan-cost.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/rex-scan-cost.spec.json @@ -6,7 +6,7 @@ "schedule": "pr", "wiring": { "detector": "rex-scan-cost", - "enabled": true, + "enabled": false, "severity": "info", "runtimeOnly": false, "needsContext": true, @@ -25,6 +25,7 @@ }, "frontendContext": { "isCalcite": true, + "forceEnable": true, "deriveFromMapping": { "email": "text" } @@ -55,9 +56,6 @@ "messageEquals": "parse runs the pattern against every input row from text field \"email\", even when it finds no match.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { @@ -78,9 +76,6 @@ "messageEquals": "grok runs the pattern against every input row from text field \"email\", even when it finds no match.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { @@ -99,9 +94,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/type-mismatch-numeric.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/type-mismatch-numeric.spec.json index 5589bdb79f7..635f862a9d4 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/type-mismatch-numeric.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/type-mismatch-numeric.spec.json @@ -59,10 +59,6 @@ "messageEquals": "This field is numeric, but the compared value is not a number, so the comparison returns no rows.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -89,10 +85,6 @@ "messageEquals": "This field is numeric, but the compared value is not a number, so the comparison returns no rows.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -117,9 +109,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/union-min-datasets.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/union-min-datasets.spec.json index ca16823381a..60551229d9b 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/union-min-datasets.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/union-min-datasets.spec.json @@ -61,10 +61,6 @@ "messageEquals": "The union command requires at least two datasets.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -99,10 +95,6 @@ "messageEquals": "The union command requires at least two datasets.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -135,9 +127,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/unsupported-window-function-in-eventstats.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/unsupported-window-function-in-eventstats.spec.json index 6c017195e1a..478ed49c0be 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/unsupported-window-function-in-eventstats.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/unsupported-window-function-in-eventstats.spec.json @@ -55,10 +55,6 @@ "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -93,10 +89,6 @@ "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -129,9 +121,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { @@ -163,10 +152,6 @@ "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -201,10 +186,6 @@ "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -237,9 +218,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { @@ -271,10 +249,6 @@ "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -309,10 +283,6 @@ "messageEquals": "This window function is not supported in eventstats/streamstats. Only row_number is supported.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -345,9 +315,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/wildcard-source-zero-match.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/wildcard-source-zero-match.spec.json index daf9920cab7..43e94902d56 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/wildcard-source-zero-match.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/wildcard-source-zero-match.spec.json @@ -51,10 +51,6 @@ "messageEquals": "Wildcard source pattern matches no known index.", "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": true, - "commandId": "ppl.lint.aiFix" } }, "backends": { @@ -79,9 +75,6 @@ "count": 0, "deterministicFix": { "offered": false - }, - "aiAction": { - "offered": false } }, "backends": { diff --git a/scripts/ppl-lint/README.md b/scripts/ppl-lint/README.md index 62b7d157283..63cc7797409 100644 --- a/scripts/ppl-lint/README.md +++ b/scripts/ppl-lint/README.md @@ -50,10 +50,9 @@ backend-validation ──(target.json, ppl-grammar-bundle.json, backend-report.j | `workflow_dispatch` (`osd_ref`) | OSD-branch evidence | the given commit/branch | No — pre-merge evidence only | | `schedule` (nightly) | full corpus + coverage | `main` | No | -The required PR corpus contains 12 detector contracts plus the -`command-suggestion` syntax contract. All 13 declare `schedule: "pr"` and can -fail the required check. The nightly mode runs the same shipping corpus across -the supported version and execution-backend matrix. +The required PR corpus contains 12 detector contracts. The nightly mode runs +the same active corpus plus four dormant report-only contracts across the +supported version and execution-backend matrix. `workflow_dispatch` inputs: @@ -210,8 +209,8 @@ rule cannot be validated end to end. backend behavior. - `defaultError` — every rule that ships **enabled at error severity** in OSD's `rules_catalog.json`; it contains exactly six detector rules. -- `requiredSyntaxFeatures` — `command-suggestion` only. Syntax features never - appear in `defaultError` or the detector catalog. +- `requiredSyntaxFeatures` — reserved for future syntax compatibility contracts; + currently empty. - `nonEnforcing` — oracle-quality classification for warning, info, advisory, and result-shape contracts. Scheduling determines whether a contract runs. - `dormantContracts` — four preserved default-off detector contracts. They do @@ -363,13 +362,15 @@ rule with only one pinned trigger gets an explicit warning that a "fully relaxed verdict rests on a single observation. That is the gap the discovery corpus below closes. -Four guards keep the check from passing vacuously. Each exists because "we could -not check" must never render as "it is fine": +Three hard guards keep the check from passing vacuously. The shipping census is +also recorded, but remains report-only until the paired OSD default-alignment +change lands: -- A rule that is default-error in OSD's catalog but has no contract file fails the - run. The detector runner records the catalog's default-error census in - `detector-report.json`, and the aggregate step compares it against - `manifest.defaultError` — so a new error rule cannot land unvalidated. +- A rule that is default-error in OSD's catalog but has no contract file is + reported in the shipping census. The detector runner records the catalog's + default-error census in `detector-report.json`, and the aggregate step compares + it against `manifest.defaultError`. This becomes blocking when census + enforcement is enabled after OSD defaults are aligned. - A leg whose artifacts are missing is a hard failure, never a silently dropped version. The aggregate step also checks that every version the plan asked for produced a report, so a dead observe job cannot shrink the matrix into a green @@ -406,9 +407,14 @@ different places a developer looks: The required single-version lane follows the same rule: frontend and backend failures with a `[rule/query]` identity anchor on that contract's `ruleId`. -Shipping-census findings anchor on `manifest.json` and fail the required lane. -Artifact and job failures without a trustworthy -repository location remain file-less rather than pointing at a guessed line. +An individual detector/query execution error is recorded as an `error` row and +does not stop the remaining contracts from running or prevent +`detector-report.json` from being uploaded. The required check still fails after +the complete report is written, with the failing rule/query named directly. +Shipping-census findings anchor on `manifest.json` and remain report-only until +the paired OSD default-alignment change lands. Artifact and job failures without +a trustworthy repository location remain file-less rather than pointing at a +guessed line. Without the annotations the only thing above the summary is `Process completed with exit code 1`, so the natural next click lands in raw job logs rather than the @@ -483,7 +489,7 @@ harvest-queries.mjs ──▶ discovery-corpus.json ──┬──▶ run-front A query with no rule-owning ancestor is recorded unattributed and dropped rather than guessed at. Indices are rewritten onto the fixture index; JS string escapes are unescaped so the query matches what the test actually linted. The harvested - corpus is substantially larger than the curated 13-contract required corpus. + corpus is substantially larger than the curated 12-contract required corpus. Each file's **lint context** is harvested alongside its queries. Seven of the nineteen rules are `needsContext: true` and self-suppress without a `typeMap`, so diff --git a/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs b/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs index b42c825384d..df46ab6acd3 100644 --- a/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs +++ b/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs @@ -172,8 +172,6 @@ function writeLeg({ ...Object.fromEntries( [ 'deterministicFixMatched', - 'aiActionMatched', - 'actionDecisionMatched', 'fixMatched', 'rawMessageMatched', 'totalErrorsMatched', @@ -183,6 +181,8 @@ function writeLeg({ ), ...(c.assertions ? { assertions: c.assertions } : {}), ...(c.mismatches ? { mismatches: c.mismatches } : {}), + ...(c.detectorOutcome ? { outcome: c.detectorOutcome } : {}), + ...(c.detectorError ? { error: c.detectorError } : {}), ...(explicitIdentity ? { executionBackend } : {}), }); backend.push({ @@ -644,7 +644,7 @@ test('paired detector reports must be identical across execution backends', () = assert.match(stderr, /detector parity failed for union-min-datasets::trigger/); }); -test('identical exact-action mismatches across versions cannot aggregate green', () => { +test('identical deterministic-fix mismatches across versions cannot aggregate green', () => { const badCase = { detector: 1, rejected: true, @@ -689,6 +689,42 @@ test('identical exact-action mismatches across versions cannot aggregate green', assert.ok(report.matrix.every((row) => row.status === 'drift')); }); +test('a detector execution error is infrastructure evidence, not detector drift', () => { + const legs = { + '3.7.0': writeLeg({ + version: '3.7.0', + cases: { + trigger: { + detector: 0, + rejected: true, + detectorOutcome: 'error', + detectorError: 'detector crashed', + assertions: { execution: false }, + mismatches: [ + { + field: 'execution', + expected: 'completed', + actual: 'detector crashed', + }, + ], + }, + control: { detector: 0, rejected: false }, + }, + }), + }; + + const { status, report, stdout } = run({ contracts: writeContracts(), legs }); + assert.equal(status, 1); + assert.equal(report.drifts.length, 0); + assert.equal(report.result.enforcedInconclusive, 1); + assert.equal(report.matrix[0].status, 'inconclusive'); + assert.match( + report.inconclusive[0].reasons.join(' '), + /trigger \(frontend execution failed: detector crashed\)/ + ); + assert.doesNotMatch(stdout, /update-detector/); +}); + test('target and detector execution identities must match', () => { const dir = writeLeg({ version: '3.8.0', @@ -1159,6 +1195,27 @@ test('an enforced shipping census mismatch fails aggregation', () => { assert.match(stdout, /### Shipping census/); }); +test('a report-only shipping census mismatch remains green', () => { + const legs = { + '3.7.0': writeLeg({ + version: '3.7.0', + cases: { + trigger: { detector: 1, rejected: true }, + control: { detector: 0, rejected: false }, + }, + }), + }; + + const { status, report, stdout } = run({ contracts: writeContracts(), legs }); + assert.equal(status, 0); + assert.equal(report.result.passed, true); + assert.equal(report.result.blockingShippingCensusProblems, 0); + assert.ok(report.shippingCensus.problems.length > 0); + assert.equal(report.shippingCensus.blocking, false); + assert.match(stdout, /CENSUS REPORT-ONLY/); + assert.doesNotMatch(stdout, /CENSUS ENFORCED/); +}); + test('a schema-v2 detector report without a census fails closed', () => { const dir = writeLeg({ version: '3.8.0', diff --git a/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs b/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs index 7e2c1afa062..cb3b1497181 100644 --- a/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs +++ b/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs @@ -173,6 +173,42 @@ test('detector severity and message mismatches fail the manifest and summary', ( } }); +test('a detector execution error fails with its rule row instead of missing-artifact noise', () => { + const dir = makeRun(); + validArtifacts(dir); + const file = path.join(dir, 'artifacts', 'detector-report.json'); + const detector = JSON.parse(fs.readFileSync(file, 'utf8')); + detector.results[0] = { + ...detector.results[0], + expected: 0, + actual: 0, + assertions: { execution: false }, + mismatches: [ + { + field: 'execution', + expected: 'completed', + actual: 'detector crashed', + }, + ], + outcome: 'error', + error: 'detector crashed', + }; + detector.failures = [ + '[advisory-rule/trigger] frontend.execution failed: detector crashed', + ]; + fs.writeFileSync(file, JSON.stringify(detector)); + const summary = path.join(dir, 'summary.md'); + + const result = run(dir, { GITHUB_STEP_SUMMARY: summary }); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /advisory-rule::trigger execution failed: detector crashed/); + assert.doesNotMatch(result.stderr, /count mismatch/); + assert.doesNotMatch(result.stderr, /did not match its execution assertion/); + assert.doesNotMatch(result.stderr, /required artifact is missing/); + assert.doesNotMatch(result.stderr, /has no matching detector row/); + assert.match(fs.readFileSync(summary, 'utf8'), /advisory-rule.*Error.*Fail/); +}); + test('syntax-specific frontend mismatches fail artifact validation', () => { for (const field of ['fixMatched', 'rawMessageMatched', 'totalErrorsMatched']) { const dir = makeRun(); @@ -191,19 +227,19 @@ test('syntax-specific frontend mismatches fail artifact validation', () => { } }); -test('exact deterministic and AI action mismatches fail artifacts and summary rows', () => { - for (const field of ['deterministicFixMatched', 'aiActionMatched']) { +test('exact deterministic fix mismatches fail artifacts and summary rows', () => { + for (const field of ['deterministicFixMatched']) { const dir = makeRun(); validArtifacts(dir); const file = path.join(dir, 'artifacts', 'detector-report.json'); const detector = JSON.parse(fs.readFileSync(file, 'utf8')); detector.results[0][field] = false; detector.results[0].assertions = { - [field === 'deterministicFixMatched' ? 'deterministicFix' : 'aiAction']: false, + deterministicFix: false, }; detector.results[0].mismatches = [ { - field: field === 'deterministicFixMatched' ? 'deterministicFix' : 'aiAction', + field: 'deterministicFix', expected: { offered: false }, actual: { offered: true }, }, @@ -213,7 +249,7 @@ test('exact deterministic and AI action mismatches fail artifacts and summary ro const result = run(dir, { GITHUB_STEP_SUMMARY: summary }); assert.notEqual(result.status, 0); - assert.match(result.stderr, /did not match its (deterministic-fix|AI-action) assertion/); + assert.match(result.stderr, /did not match its deterministic-fix assertion/); assert.match(fs.readFileSync(summary, 'utf8'), /advisory-rule.*accepted.*Fail/); } }); diff --git a/scripts/ppl-lint/__tests__/contract-schema.test.mjs b/scripts/ppl-lint/__tests__/contract-schema.test.mjs index 5bb44957bf1..0aa55eeabb6 100644 --- a/scripts/ppl-lint/__tests__/contract-schema.test.mjs +++ b/scripts/ppl-lint/__tests__/contract-schema.test.mjs @@ -511,12 +511,11 @@ test('missing channel remains a backwards-compatible lint contract', () => { severity: 'warning', messageEquals: undefined, deterministicFix: undefined, - aiAction: undefined, } ); }); -test('schema-v4 lint frontend normalizes exact message, fix, and AI action oracles', () => { +test('schema-v4 lint frontend normalizes exact message and fix oracles', () => { const contract = spec(4); const frontend = normalizeFrontendOracle(contract, { frontend: { @@ -536,7 +535,6 @@ test('schema-v4 lint frontend normalizes exact message, fix, and AI action oracl expectedText: '0', appliedQuery: 'source=t | eval x = 1', }, - aiAction: { offered: false }, }, }); @@ -558,7 +556,6 @@ test('schema-v4 lint frontend normalizes exact message, fix, and AI action oracl expectedText: '0', appliedQuery: 'source=t | eval x = 1', }, - aiAction: { offered: false }, }); }); @@ -578,7 +575,7 @@ test('matchMessage remains available only to schema-v3 lint contracts', () => { ); }); -test('schema-v4 action payloads fail closed on partial or extra fields', () => { +test('schema-v4 deterministic-fix payloads fail closed on partial or extra fields', () => { const contract = spec(4); for (const [frontend, expected] of [ [ @@ -605,40 +602,34 @@ test('schema-v4 action payloads fail closed on partial or extra fields', () => { offered: true, title: 'Fix', text: 'x', - range: { startLine: 1, startColumn: 0, endLine: 1, endColumn: 1 }, + range: { startLine: 1, startColumn: 2, endLine: 1, endColumn: 1 }, + expectedText: 'y', appliedQuery: 'x', }, }, - /expectedText must be a string/, + /must end at or after its start/, ], - [ - { + ]) { + assert.throws(() => normalizeFrontendOracle(contract, { frontend }), expected); + } + + assert.doesNotThrow(() => + normalizeFrontendOracle(contract, { + frontend: { count: 1, deterministicFix: { offered: true, title: 'Fix', text: 'x', - range: { startLine: 1, startColumn: 2, endLine: 1, endColumn: 1 }, - expectedText: 'y', + range: { startLine: 1, startColumn: 0, endLine: 1, endColumn: 1 }, appliedQuery: 'x', }, }, - /must end at or after its start/, - ], - [ - { count: 1, aiAction: { offered: true } }, - /commandId must be a non-empty string/, - ], - [ - { count: 1, aiAction: { offered: false, commandId: 'ppl.lint.aiFix' } }, - /must contain only offered/, - ], - ]) { - assert.throws(() => normalizeFrontendOracle(contract, { frontend }), expected); - } + }) + ); }); -test('active lint contracts require exact messages and explicit exclusive action modes', () => { +test('active lint contracts require exact messages and deterministic-fix behavior', () => { const contract = spec(4); const expectation = structuredClone(contract.expectations[0]); expectation.queries.trigger = { @@ -647,7 +638,6 @@ test('active lint contracts require exact messages and explicit exclusive action severity: 'error', messageEquals: 'Exact diagnostic.', deterministicFix: { offered: false }, - aiAction: { offered: true, commandId: 'ppl.lint.aiFix' }, }, backends: { standard: { kind: 'rejection', httpStatus: 400, body: { status: 400 } }, @@ -657,7 +647,6 @@ test('active lint contracts require exact messages and explicit exclusive action expectation.queries.control.frontend = { count: 0, deterministicFix: { offered: false }, - aiAction: { offered: false }, }; delete expectation.queries.control.detectorCount; @@ -677,8 +666,8 @@ test('active lint contracts require exact messages and explicit exclusive action /severity is required/ ); - const simultaneousActions = structuredClone(expectation); - simultaneousActions.queries.trigger.frontend.deterministicFix = { + const fixWithoutFinding = structuredClone(expectation); + fixWithoutFinding.queries.control.frontend.deterministicFix = { offered: true, title: 'Fix', text: 'fixed', @@ -687,8 +676,8 @@ test('active lint contracts require exact messages and explicit exclusive action appliedQuery: 'fixed', }; assert.throws( - () => assertShippingFrontendOracles(contract, simultaneousActions), - /cannot offer deterministic and AI actions together/ + () => assertShippingFrontendOracles(contract, fixWithoutFinding), + /must not offer a deterministic fix/ ); }); diff --git a/scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs b/scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs index 6b482125211..ff0b48f9100 100644 --- a/scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs +++ b/scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs @@ -9,6 +9,7 @@ import { test } from 'node:test'; import { assertActiveShippingContracts, buildCensus, + buildFrontendExecutionError, evaluateFrontendAssertions, selectManifestContractNames, } from '../run-frontend-contract.mjs'; @@ -48,12 +49,7 @@ test('exact lint assertions materialize the effective deterministic edit', () => expectedText: 'bad', appliedQuery: 'source=t | good', }, - aiAction: { offered: false }, }, - decideAction: ({ hasDeterministicFix }) => ({ - kind: hasDeterministicFix ? 'deterministic' : 'ai', - commandId: 'ppl.lint.aiFix', - }), }); assert.deepEqual(result.mismatches, []); @@ -61,8 +57,6 @@ test('exact lint assertions materialize the effective deterministic edit', () => severity: true, message: true, deterministicFix: true, - aiAction: true, - actionDecision: true, }); assert.deepEqual(result.deterministicFixActual, { offered: true, @@ -74,43 +68,7 @@ test('exact lint assertions materialize the effective deterministic edit', () => }); }); -test('deterministic fixes require the production helper to choose the deterministic action', () => { - const result = evaluateFrontendAssertions({ - channel: 'lint', - query: 'source=t | bad', - matches: [ - { - severity: 'warning', - message: 'Replace bad.', - range: RANGE, - fix: { title: 'Replace bad', text: 'good', expectedText: 'bad' }, - }, - ], - frontendOracle: { - deterministicFix: { - offered: true, - title: 'Replace bad', - text: 'good', - range: RANGE, - expectedText: 'bad', - appliedQuery: 'source=t | good', - }, - aiAction: { offered: false }, - }, - decideAction: () => ({ kind: 'none' }), - }); - - assert.equal(result.deterministicFixMatched, true); - assert.equal(result.aiActionMatched, true); - assert.equal(result.actionDecisionMatched, false); - assert.equal( - result.mismatches.find(({ field }) => field === 'actionDecision')?.actual[0], - 'none' - ); -}); - -test('AI action identity and exact messages produce field-specific mismatches', () => { - let decisionInput; +test('exact message mismatches are field-specific', () => { const result = evaluateFrontendAssertions({ channel: 'lint', query: 'source=t | bad', @@ -125,26 +83,10 @@ test('AI action identity and exact messages produce field-specific mismatches', frontendOracle: { messageEquals: 'Expected message.', deterministicFix: { offered: false }, - aiAction: { offered: true, commandId: 'ppl.lint.aiFix' }, - }, - decideAction: (input) => { - decisionInput = input; - return { kind: 'ai', commandId: 'wrong.command' }; }, }); - assert.deepEqual( - result.mismatches.map(({ field }) => field), - ['message', 'aiAction'] - ); - assert.deepEqual(result.aiActionActual, { - offered: true, - commandId: 'wrong.command', - }); - assert.equal(decisionInput.enableAIFeatures, true); - assert.equal(decisionInput.hasAiFixHandler, true); - assert.equal(decisionInput.aiAgentAvailableForSource, true); - assert.equal(decisionInput.aiFixEligible, true); + assert.deepEqual(result.mismatches.map(({ field }) => field), ['message']); }); test('deterministic expectedText must match the source slice', () => { @@ -178,33 +120,45 @@ test('deterministic expectedText must match the source slice', () => { assert.equal(result.deterministicFixActual.expectedTextMatchesSource, false); }); -test('AI assertions fail closed when the production decision export is unavailable', () => { - const result = evaluateFrontendAssertions({ - channel: 'lint', - query: 'source=t', - matches: [], - frontendOracle: { aiAction: { offered: false } }, - }); - - assert.equal(result.aiActionMatched, false); - assert.equal(result.mismatches[0].field, 'aiAction'); - assert.equal(result.aiActionActual.unavailable, true); -}); - -test('AI assertions report production decision errors instead of passing absence', () => { - const result = evaluateFrontendAssertions({ - channel: 'lint', - query: 'source=t | bad', - matches: [{ message: 'Bad', range: RANGE }], - frontendOracle: { aiAction: { offered: false } }, - decideAction: () => { - throw new Error('decision failed'); - }, - }); - - assert.equal(result.aiActionMatched, false); - assert.match(result.aiActionActual.error, /decision failed/); - assert.equal(result.mismatches[0].field, 'aiAction'); +test('a frontend execution error remains a complete report row', () => { + assert.deepEqual( + buildFrontendExecutionError({ + ruleId: 'example-rule', + channel: 'lint', + queryName: 'trigger', + role: 'trigger', + query: 'source=t | bad', + expected: 0, + surface: 'runtime-bundle', + executionBackend: 'standard', + error: new Error('detector crashed'), + }), + { + ruleId: 'example-rule', + channel: 'lint', + queryName: 'trigger', + role: 'trigger', + query: 'source=t | bad', + expected: 0, + actual: 0, + severities: [], + severityMatched: true, + messageMatched: true, + assertions: { execution: false }, + mismatches: [ + { + field: 'execution', + expected: 'completed', + actual: 'detector crashed', + }, + ], + outcome: 'error', + error: 'detector crashed', + surface: 'runtime-bundle', + executionBackend: 'standard', + backendOracleStatus: 'error', + } + ); }); test('syntax suppression checks raw parser errors outside the suggestion code filter', () => { @@ -295,31 +249,26 @@ test('discovery mode accepts legacy generated specs without shipping oracles', ( ); }); -test('shipping census rejects duplicate detector IDs and syntax features in the catalog', () => { - const syntaxContract = { - file: 'command-suggestion.spec.json', +test('shipping census rejects duplicate detector IDs', () => { + const lintContract = { + file: 'example.spec.json', spec: { schemaVersion: 4, - ruleId: 'command-suggestion', - channel: 'syntax', + ruleId: 'example', }, }; const census = buildCensus( - [syntaxContract], + [lintContract], { - contracts: ['command-suggestion.spec.json'], + contracts: ['example.spec.json'], defaultError: [], - requiredSyntaxFeatures: ['command-suggestion.spec.json'], + requiredSyntaxFeatures: [], }, [ - { id: 'command-suggestion', enabled: false, severity: 'error' }, { id: 'duplicate-rule', enabled: false, severity: 'info' }, { id: 'duplicate-rule', enabled: false, severity: 'info' }, ] ); assert.ok(census.problems.some((problem) => /duplicate rule IDs/.test(problem))); - assert.ok( - census.problems.some((problem) => /outside the detector catalog/.test(problem)) - ); }); diff --git a/scripts/ppl-lint/aggregate-versions.mjs b/scripts/ppl-lint/aggregate-versions.mjs index 1a9abf03b0b..7ceb86ae6df 100644 --- a/scripts/ppl-lint/aggregate-versions.mjs +++ b/scripts/ppl-lint/aggregate-versions.mjs @@ -268,8 +268,6 @@ function normalizeDetectorReport(detector, target) { } for (const field of [ 'deterministicFixMatched', - 'aiActionMatched', - 'actionDecisionMatched', 'fixMatched', 'rawMessageMatched', 'totalErrorsMatched', @@ -613,12 +611,9 @@ function detectorParityValue(entry) { typeof entry.deterministicFixMatched === 'boolean' ? entry.deterministicFixMatched : undefined, - aiActionMatched: - typeof entry.aiActionMatched === 'boolean' ? entry.aiActionMatched : undefined, assertions: entry.assertions, mismatches: entry.mismatches, deterministicFix: entry.deterministicFix, - aiAction: entry.aiAction, code: entry.code, codes: entry.codes, totalErrors: entry.totalErrors, @@ -825,11 +820,11 @@ function auditShippingCensus(legs, specs, manifest) { if (activeLintRules.size !== 12) { problems.push(`expected 12 active lint contracts, found ${activeLintRules.size}`); } - if (requiredSyntaxRules.size !== 1) { - problems.push(`expected one required syntax feature, found ${requiredSyntaxRules.size}`); + if (requiredSyntaxRules.size !== 0) { + problems.push(`expected no required syntax features, found ${requiredSyntaxRules.size}`); } - if (activeRules.size !== 13) { - problems.push(`expected 13 active contracts, found ${activeRules.size}`); + if (activeRules.size !== 12) { + problems.push(`expected 12 active contracts, found ${activeRules.size}`); } if (!setsEqual(activeLintRules, enabledRules)) { problems.push( @@ -882,9 +877,10 @@ function auditShippingCensus(legs, specs, manifest) { function readBackendObservation(backendEntry, detectorResult) { const verdict = backendVerdict(backendEntry); const hasVerdict = typeof verdict.backendRejected === 'boolean'; + const detectorUsable = !!detectorResult && detectorResult.outcome !== 'error'; return { - usable: hasVerdict && !!detectorResult, + usable: hasVerdict && detectorUsable, observed: { detectorCount: detectorResult ? detectorResult.actual : 0, severities: detectorResult ? detectorResult.severities || [] : [], @@ -899,7 +895,6 @@ function readBackendObservation(backendEntry, detectorResult) { deterministicFixMatched: detectorResult ? detectorResult.deterministicFixMatched : undefined, - aiActionMatched: detectorResult ? detectorResult.aiActionMatched : undefined, fixMatched: detectorResult ? detectorResult.fixMatched : undefined, rawMessageMatched: detectorResult ? detectorResult.rawMessageMatched : undefined, totalErrorsMatched: detectorResult @@ -911,6 +906,20 @@ function readBackendObservation(backendEntry, detectorResult) { }; } +function unusableObservationReason(detectorResult) { + if (!detectorResult) { + return 'no detector result'; + } + if (detectorResult.outcome === 'error') { + const message = + typeof detectorResult.error === 'string' && detectorResult.error.length > 0 + ? detectorResult.error + : 'unknown frontend execution error'; + return `frontend execution failed: ${message}`; + } + return 'no engine verdict'; +} + function failedExtendedFrontendAssertions(entry, frontendOracle) { if (!entry) { return []; @@ -918,7 +927,6 @@ function failedExtendedFrontendAssertions(entry, frontendOracle) { const failures = new Set(); for (const field of [ 'deterministicFixMatched', - 'aiActionMatched', 'fixMatched', 'rawMessageMatched', 'totalErrorsMatched', @@ -972,9 +980,7 @@ function classifyOutOfScope({ spec, ruleId, leg, classify, divergentCases }) { const detectorResult = leg.detector.resultsByKey.get(rowKey); const { observed, usable } = readBackendObservation(backendEntry, detectorResult); if (!usable) { - unusable.push( - `${queryName} (${!detectorResult ? 'no detector result' : 'no engine verdict'})` - ); + unusable.push(`${queryName} (${unusableObservationReason(detectorResult)})`); continue; } observations.set(queryName, observed); @@ -1322,13 +1328,18 @@ function main() { ); } } + if (detectorResult && detectorResult.outcome === 'error') { + unusable.push(`${queryName} (${unusableObservationReason(detectorResult)})`); + if (role === 'trigger') { + unobservedTriggers.push(queryName); + } + continue; + } if (oracleSelection.status === 'coverage-missing') { const { usable } = readBackendObservation(backendEntry, detectorResult); if (!usable) { - unusable.push( - `${queryName} (${!detectorResult ? 'no detector result' : 'no engine verdict'})` - ); + unusable.push(`${queryName} (${unusableObservationReason(detectorResult)})`); if (role === 'trigger') { unobservedTriggers.push(queryName); } @@ -1457,9 +1468,7 @@ function main() { // verdict and no detector row looks exactly like "the detector went // silent", and the report would tell the engineer to go fix a detector // that is fine. Record it as not compared and move on. - unusable.push( - `${queryName} (${!detectorResult ? 'no detector result' : 'no engine verdict'})` - ); + unusable.push(`${queryName} (${unusableObservationReason(detectorResult)})`); if (role === 'trigger') { unobservedTriggers.push(queryName); } diff --git a/scripts/ppl-lint/assemble-run-manifest.mjs b/scripts/ppl-lint/assemble-run-manifest.mjs index 5d875e593bb..7047ffad28f 100644 --- a/scripts/ppl-lint/assemble-run-manifest.mjs +++ b/scripts/ppl-lint/assemble-run-manifest.mjs @@ -58,8 +58,6 @@ function failedFrontendAssertions(entry) { } for (const [field, label] of [ ['deterministicFixMatched', 'deterministic-fix'], - ['aiActionMatched', 'AI-action'], - ['actionDecisionMatched', 'action-decision'], ['fixMatched', 'syntax-fix'], ['rawMessageMatched', 'raw-parser-error'], ['totalErrorsMatched', 'total-error'], @@ -182,6 +180,14 @@ function main() { if (!backendByKey.has(key)) { artifactErrors.push(`detector row ${key} has no matching backend row`); } + if (entry.outcome === 'error') { + const message = + typeof entry.error === 'string' && entry.error.length > 0 + ? entry.error + : 'unknown frontend execution error'; + artifactErrors.push(`detector row ${key} execution failed: ${message}`); + continue; + } if (!Number.isInteger(entry.expected) || !Number.isInteger(entry.actual)) { artifactErrors.push(`detector row ${key} must contain integer expected/actual counts`); } else if (entry.actual !== entry.expected) { @@ -332,7 +338,12 @@ function writeSummary(manifest, detector, backend) { for (const r of detector.results || []) { const be = backendByKey.get(`${r.ruleId}::${r.queryName}`); - const detectorCell = `${r.actual}/${r.expected}${r.severities && r.severities.length ? ` (${r.severities.join(',')})` : ''}`; + const detectorCell = + r.outcome === 'error' + ? 'Error' + : `${r.actual}/${r.expected}${ + r.severities && r.severities.length ? ` (${r.severities.join(',')})` : '' + }`; const backendCell = !be ? '—' : typeof be.rejected !== 'boolean' @@ -343,7 +354,8 @@ function writeSummary(manifest, detector, backend) { const ok = r.reportOnly === true ? undefined - : r.actual === r.expected && + : r.outcome !== 'error' && + r.actual === r.expected && failedFrontendAssertions(r).length === 0 && !!be && be.outcome === 'pass'; diff --git a/scripts/ppl-lint/contract-schema.mjs b/scripts/ppl-lint/contract-schema.mjs index b1da53617fa..3b8e4bf68f9 100644 --- a/scripts/ppl-lint/contract-schema.mjs +++ b/scripts/ppl-lint/contract-schema.mjs @@ -14,7 +14,6 @@ const LINT_V4_FRONTEND_FIELDS = new Set([ 'severity', 'messageEquals', 'deterministicFix', - 'aiAction', ]); const SYNTAX_FRONTEND_FIELDS = new Set([ 'count', @@ -130,30 +129,14 @@ function normalizeDeterministicFix(value, label) { title: requireNonEmptyString(fix.title, `${label}.title`), text: requireString(fix.text, `${label}.text`), range: normalizeRange(fix.range, `${label}.range`), - expectedText: requireString(fix.expectedText, `${label}.expectedText`), + ...(fix.expectedText === undefined + ? {} + : { expectedText: requireString(fix.expectedText, `${label}.expectedText`) }), appliedQuery: requireString(fix.appliedQuery, `${label}.appliedQuery`), }; return normalized; } -function normalizeAiAction(value, label) { - const action = requireObject(value, label); - if (typeof action.offered !== 'boolean') { - throw new TypeError(`${label}.offered must be a boolean.`); - } - assertKnownKeys(action, new Set(['offered', 'commandId']), label); - if (!action.offered) { - if (Object.keys(action).length !== 1) { - throw new Error(`${label} must contain only offered when no AI action is expected.`); - } - return { offered: false }; - } - return { - offered: true, - commandId: requireNonEmptyString(action.commandId, `${label}.commandId`), - }; -} - export function contractChannel(spec) { requireObject(spec, 'contract'); const channel = spec.channel === undefined ? 'lint' : spec.channel; @@ -169,9 +152,8 @@ export function contractChannel(spec) { * Normalize legacy detector fields and channel-specific frontend assertions. * * Schema-v3 lint contracts retain substring messages. Schema-v4 lint contracts - * assert exact messages and exact deterministic/AI action availability. Syntax - * contracts assert stable parser error identity, quick-fix presence or absence, - * raw-message preservation, and the total syntax error census. + * assert exact messages and deterministic fixes. Syntax contracts retain their + * legacy parser error, quick-fix, and raw-message assertions. */ export function normalizeFrontendOracle(spec, queryExpectation) { const channel = contractChannel(spec); @@ -206,9 +188,6 @@ export function normalizeFrontendOracle(spec, queryExpectation) { ...(queryExpectation.deterministicFix !== undefined ? { deterministicFix: queryExpectation.deterministicFix } : {}), - ...(queryExpectation.aiAction !== undefined - ? { aiAction: queryExpectation.aiAction } - : {}), }; const allowed = spec.schemaVersion === 3 ? LINT_V3_FRONTEND_FIELDS : LINT_V4_FRONTEND_FIELDS; @@ -232,7 +211,7 @@ export function normalizeFrontendOracle(spec, queryExpectation) { } if ( hasFrontend && - ['severity', 'matchMessage', 'messageEquals', 'deterministicFix', 'aiAction'].some( + ['severity', 'matchMessage', 'messageEquals', 'deterministicFix'].some( (field) => queryExpectation[field] !== undefined ) ) { @@ -255,13 +234,6 @@ export function normalizeFrontendOracle(spec, queryExpectation) { frontend.deterministicFix, `[${spec.ruleId}] frontend.deterministicFix` ), - aiAction: - frontend.aiAction === undefined - ? undefined - : normalizeAiAction( - frontend.aiAction, - `[${spec.ruleId}] frontend.aiAction` - ), }), }; } @@ -322,8 +294,7 @@ export function normalizeFrontendOracle(spec, queryExpectation) { /** * Active shipping contracts are stricter than dormant compatibility contracts: - * every lint finding pins its exact message and action mode, every lint control - * pins action absence, and every syntax case pins fix/raw-error/error-count state. + * every lint finding pins its exact message and deterministic-fix behavior. */ export function assertShippingFrontendOracles(spec, expectation) { if (spec.schemaVersion !== 4) { @@ -352,9 +323,6 @@ export function assertShippingFrontendOracles(spec, expectation) { if (frontend.deterministicFix === undefined) { throw new Error(`${label}.deterministicFix must be explicitly asserted.`); } - if (frontend.aiAction === undefined) { - throw new Error(`${label}.aiAction must be explicitly asserted.`); - } if (frontend.count > 0) { if (frontend.severity === undefined) { throw new Error(`${label}.severity is required for a lint finding.`); @@ -362,11 +330,8 @@ export function assertShippingFrontendOracles(spec, expectation) { if (frontend.messageEquals === undefined) { throw new Error(`${label}.messageEquals is required for a lint finding.`); } - if (frontend.deterministicFix.offered && frontend.aiAction.offered) { - throw new Error(`${label} cannot offer deterministic and AI actions together.`); - } - } else if (frontend.deterministicFix.offered || frontend.aiAction.offered) { - throw new Error(`${label} must not offer actions when no finding is expected.`); + } else if (frontend.deterministicFix.offered) { + throw new Error(`${label} must not offer a deterministic fix when no finding is expected.`); } } } diff --git a/scripts/ppl-lint/run-frontend-contract.mjs b/scripts/ppl-lint/run-frontend-contract.mjs index 68438d4632e..0875f902338 100644 --- a/scripts/ppl-lint/run-frontend-contract.mjs +++ b/scripts/ppl-lint/run-frontend-contract.mjs @@ -44,7 +44,8 @@ * backend behavior for each query agrees with the observed detector output * — a trigger the detector flags is one the backend rejected; a control the * detector passes is one the backend accepted (design §3.2, §4.3). - * 4. Coverage (nightly only): every enabled catalog rule has a contract file. + * 4. Coverage census: records whether every enabled catalog rule has a + * contract file. Census drift is report-only unless explicitly enforced. * * ## Two grammar surfaces * @@ -107,8 +108,6 @@ const CATALOG_MODULE = 'packages/osd-monaco/ppl-lint'; // supports older checkouts (that is the coverage it adds), so fall back to the // source module, which `setup_node_env` transpiles on require anyway. const CATALOG_SOURCE_MODULE = 'packages/osd-monaco/src/ppl/lint/catalog'; -const ACTION_DECISION_MODULE = - 'packages/osd-monaco/src/ppl/lint/action_decision'; const DETECTOR_REGISTRY_MODULE = 'packages/osd-monaco/target/ppl/lint/detector_registry.js'; /** @@ -311,23 +310,6 @@ function loadContracts() { }; } -function resolveActionDecision(module) { - if (!module) { - return undefined; - } - for (const name of [ - 'decidePPLLintAction', - 'decidePPLDiagnosticAction', - 'getPPLDiagnosticActionDecision', - 'decideDiagnosticAction', - ]) { - if (typeof module[name] === 'function') { - return module[name]; - } - } - return undefined; -} - function loadOsd() { const osdRoot = process.cwd(); const require = createRequire(path.join(osdRoot, 'noop.js')); @@ -362,7 +344,6 @@ function loadOsd() { // so a checkout without the built export can still run the compiled surface. const catalogModule = resolveOsd(CATALOG_MODULE, { optional: true }) || resolveOsd(CATALOG_SOURCE_MODULE); - const actionDecisionModule = resolveOsd(ACTION_DECISION_MODULE, { optional: true }); const { getBundledCatalog } = catalogModule; const registry = resolveOsd(DETECTOR_REGISTRY_MODULE, { optional: true }); if (typeof getBundledCatalog !== 'function') { @@ -379,7 +360,6 @@ function loadOsd() { fatal(`PPLLanguageAnalyzer not found in ${ANALYZER_MODULE}.`); } const analyzer = new PPLLanguageAnalyzer(); - const headless = resolveOsd(HEADLESS_MODULE, { optional: true }); return { surface: SURFACE, // Same (query, grammar, context) shape as the bundle path so the main loop @@ -390,10 +370,6 @@ function loadOsd() { }, getBundledCatalog, getDetector, - decideAction: - resolveActionDecision(headless) || - resolveActionDecision(catalogModule) || - resolveActionDecision(actionDecisionModule), osdRoot, }; } @@ -418,10 +394,6 @@ function loadOsd() { deserializeBundleOrThrow, lintQuery: lintQueryWithBundle, validateSyntax, - decideAction: - resolveActionDecision(headless) || - resolveActionDecision(catalogModule) || - resolveActionDecision(actionDecisionModule), getBundledCatalog, getDetector, osdRoot, @@ -582,7 +554,7 @@ function selectExpectation(spec, version, isCalcite, failures, { allowMissing = failures.push(`[${spec.ruleId}] no version expectation matches backend version ${label}.`); } } else { - fatal( + failures.push( `[${spec.ruleId}] ${matches.length} expectations match backend version ${label} ` + '(exactly one required).' ); @@ -734,19 +706,6 @@ function materializeDeterministicFix(query, diagnostic) { }; } -function normalizeActionDecision(decision) { - if (typeof decision === 'string') { - return { kind: decision }; - } - if (!decision || typeof decision !== 'object' || Array.isArray(decision)) { - return { kind: 'invalid', value: decision }; - } - return { - kind: decision.kind || decision.type || decision.action, - commandId: decision.commandId || (decision.command && decision.command.id), - }; -} - function exactEqual(left, right) { return JSON.stringify(left) === JSON.stringify(right); } @@ -757,7 +716,6 @@ export function evaluateFrontendAssertions({ matches, allFrontendFindings = matches, frontendOracle, - decideAction, }) { const assertions = {}; const mismatches = []; @@ -826,90 +784,6 @@ export function evaluateFrontendAssertions({ ); } - let aiActionMatched = true; - let aiActionActual; - let actionDecisionMatched = true; - let actionDecisionActual; - if (frontendOracle.aiAction !== undefined) { - let decisions = []; - if (typeof decideAction !== 'function') { - aiActionActual = { - unavailable: true, - reason: 'production headless action-decision export is unavailable', - }; - actionDecisionActual = aiActionActual; - } else { - try { - for (const diagnostic of matches) { - decisions.push( - normalizeActionDecision( - decideAction({ - channel, - diagnostic, - hasDeterministicFix: !!diagnostic.fix, - aiFixEligible: diagnostic.aiFix?.eligible !== false, - enableAIFeatures: true, - hasAiFixHandler: true, - chatWired: true, - aiAgentAvailableForSource: true, - }) - ) - ); - } - } catch (error) { - aiActionActual = { - error: - `production action decision failed: ` + - `${error instanceof Error ? error.message : String(error)}`, - }; - actionDecisionActual = aiActionActual; - } - const invalidDecision = decisions.find( - (decision) => !['deterministic', 'ai', 'none'].includes(decision.kind) - ); - if (aiActionActual === undefined && invalidDecision) { - aiActionActual = { invalidDecision }; - } - if (aiActionActual === undefined) { - const actions = decisions - .filter((decision) => decision.kind === 'ai') - .map((decision) => ({ - offered: true, - ...(decision.commandId !== undefined - ? { commandId: decision.commandId } - : {}), - })); - aiActionActual = - actions.length === 0 - ? { offered: false } - : actions.length === 1 - ? actions[0] - : { offered: true, count: actions.length, actions }; - } - if (actionDecisionActual === undefined) { - actionDecisionActual = decisions.map((decision) => decision.kind); - } - } - aiActionMatched = record( - 'aiAction', - exactEqual(frontendOracle.aiAction, aiActionActual), - frontendOracle.aiAction, - aiActionActual - ); - const expectedDecisionKind = frontendOracle.deterministicFix?.offered - ? 'deterministic' - : frontendOracle.aiAction.offered - ? 'ai' - : 'none'; - const expectedDecisions = matches.map(() => expectedDecisionKind); - actionDecisionMatched = record( - 'actionDecision', - exactEqual(expectedDecisions, actionDecisionActual), - expectedDecisions, - actionDecisionActual - ); - } - let syntaxFixMatched = true; if (channel === 'syntax' && frontendOracle.fixText !== undefined) { const fixes = allFrontendFindings @@ -959,16 +833,53 @@ export function evaluateFrontendAssertions({ messageMatched, deterministicFixMatched, deterministicFixActual, - aiActionMatched, - aiActionActual, - actionDecisionMatched, - actionDecisionActual, syntaxFixMatched, rawMessageMatched, totalErrorsMatched, }; } +export function buildFrontendExecutionError({ + ruleId, + channel, + queryName, + role, + query, + expected = 0, + surface, + executionBackend, + error, + reportOnly = false, +}) { + const message = error instanceof Error ? error.message : String(error); + return { + ruleId, + channel, + queryName, + role, + query, + expected: Number.isInteger(expected) ? expected : 0, + actual: 0, + severities: [], + severityMatched: true, + messageMatched: true, + assertions: { execution: false }, + mismatches: [ + { + field: 'execution', + expected: 'completed', + actual: message, + }, + ], + outcome: 'error', + error: message, + surface, + executionBackend, + backendOracleStatus: 'error', + ...(reportOnly ? { reportOnly: true } : {}), + }; +} + function equalSets(left, right) { return left.size === right.size && [...left].every((value) => right.has(value)); } @@ -1050,19 +961,14 @@ export function buildCensus(contracts, manifest, catalog) { if (activeLintRules.length !== 12) { problems.push(`expected 12 active lint contracts, found ${activeLintRules.length}.`); } - if (requiredSyntaxFeatures.length !== 1) { + if (requiredSyntaxFeatures.length !== 0) { problems.push( - `expected one required syntax feature, found ${requiredSyntaxFeatures.length}.` - ); - } - if (!equalSets(new Set(requiredSyntaxFeatures), new Set(['command-suggestion']))) { - problems.push( - `required syntax features must equal ["command-suggestion"], found ` + + `required syntax features must be empty, found ` + `${JSON.stringify(requiredSyntaxFeatures)}.` ); } - if (activeContractRules.length !== 13) { - problems.push(`expected 13 active contracts, found ${activeContractRules.length}.`); + if (activeContractRules.length !== 12) { + problems.push(`expected 12 active contracts, found ${activeContractRules.length}.`); } if (!equalSets(new Set(activeSyntaxRules), new Set(requiredSyntaxFeatures))) { problems.push( @@ -1109,7 +1015,6 @@ function main() { getDetector, lintQuery, validateSyntax, - decideAction, osdRoot, surface, } = osd; @@ -1156,11 +1061,9 @@ function main() { includedDormant: process.env.PPL_LINT_INCLUDE_DORMANT === '1', reportOnlyFailures, // Census of the rules that ship enabled at ERROR severity, read from the OSD - // catalog this run linted with. The multi-version aggregator enforces its - // `defaultError` manifest set against this list, so a rule that becomes - // default-error in OSD without a contract file cannot slip through - // unvalidated — and the aggregator does not need its own OSD checkout to - // notice (design: default-error is the set users cannot opt out of). + // catalog this run linted with. The multi-version aggregator compares its + // `defaultError` manifest set against this list without needing its own OSD + // checkout. Drift remains report-only unless census enforcement is enabled. defaultErrorRules: catalog .filter((rule) => rule.enabled && rule.severity === 'error') .map((rule) => rule.id) @@ -1191,6 +1094,38 @@ function main() { `contracts=${contracts.length}` ); + const recordExecutionError = ({ + spec, + channel, + queryName, + queryDef, + expected, + error, + reportOnly, + scoringFailures, + }) => { + const query = (queryDef.query || '').split('{{index}}').join(spec.index); + const message = error instanceof Error ? error.message : String(error); + log(` FAIL ${spec.ruleId}/${queryName}: execution error — ${message}`); + scoringFailures.push( + `[${spec.ruleId}/${queryName}] frontend.execution failed: ${message}` + ); + report.results.push( + buildFrontendExecutionError({ + ruleId: spec.ruleId, + channel, + queryName, + role: queryDef.role || 'trigger', + query, + expected, + surface, + executionBackend, + error: message, + reportOnly, + }) + ); + }; + for (const { file, spec, reportOnly = false } of contracts) { const ruleId = spec.ruleId; const index = spec.index; @@ -1198,6 +1133,17 @@ function main() { const scoringFailures = reportOnly ? reportOnlyFailures : failures; const entry = checkWiring(spec, catalog, getDetector, scoringFailures); if (!entry) { + for (const [queryName, queryDef] of Object.entries(spec.queries || {})) { + recordExecutionError({ + spec, + channel, + queryName, + queryDef, + error: `OSD catalog entry "${ruleId}" is unavailable`, + reportOnly, + scoringFailures, + }); + } continue; } @@ -1247,6 +1193,17 @@ function main() { }); if (!expectation) { if (!observeOnly) { + for (const [queryName, queryDef] of Object.entries(spec.queries || {})) { + recordExecutionError({ + spec, + channel, + queryName, + queryDef, + error: `no unique expectation matches backend version ${engineVersion || 'unknown'}`, + reportOnly, + scoringFailures, + }); + } continue; } for (const [queryName, queryDef] of Object.entries(spec.queries || {})) { @@ -1267,37 +1224,48 @@ function main() { }); continue; } - if (channel === 'syntax' && typeof validateSyntax !== 'function') { - fatal( - `Syntax contract "${ruleId}" requires validateQueryWithBundle from ${SYNTAX_MODULE}. ` + - `Validate this SQL branch against the OSD headless-syntax PR.` - ); + try { + if (channel === 'syntax' && typeof validateSyntax !== 'function') { + throw new Error( + `syntax validation requires validateQueryWithBundle from ${SYNTAX_MODULE}` + ); + } + const result = + channel === 'syntax' + ? validateSyntax(query, grammar) + : lintQuery(query, grammar, context); + const matches = + channel === 'syntax' + ? result.errors || [] + : (result.diagnostics || []).filter((d) => d.ruleId === ruleId); + report.results.push({ + ruleId, + channel, + queryName, + role, + query, + surface, + executionBackend, + ...(reportOnly ? { reportOnly: true } : {}), + expected: 0, + actual: matches.length, + severities: matches.map((m) => m.severity), + severityMatched: true, + messageMatched: true, + backendOracleStatus: 'coverage-missing', + expectationStatus: 'coverage-missing', + }); + } catch (error) { + recordExecutionError({ + spec, + channel, + queryName, + queryDef, + error, + reportOnly, + scoringFailures, + }); } - const result = - channel === 'syntax' - ? validateSyntax(query, grammar) - : lintQuery(query, grammar, context); - const matches = - channel === 'syntax' - ? result.errors || [] - : (result.diagnostics || []).filter((d) => d.ruleId === ruleId); - report.results.push({ - ruleId, - channel, - queryName, - role, - query, - surface, - executionBackend, - ...(reportOnly ? { reportOnly: true } : {}), - expected: 0, - actual: matches.length, - severities: matches.map((m) => m.severity), - severityMatched: true, - messageMatched: true, - backendOracleStatus: 'coverage-missing', - expectationStatus: 'coverage-missing', - }); } continue; } @@ -1307,7 +1275,18 @@ function main() { try { assertExactQueryCoverage(spec, expectation); } catch (error) { - fatal(`Invalid contract ${file}: ${error.message}`); + for (const [queryName, queryDef] of Object.entries(queries)) { + recordExecutionError({ + spec, + channel, + queryName, + queryDef, + error: `invalid contract ${file}: ${error.message}`, + reportOnly, + scoringFailures, + }); + } + continue; } for (const queryName of Object.keys(queries)) { const queryDef = queries[queryName]; @@ -1318,7 +1297,16 @@ function main() { try { oracleSelection = resolveBackendOracle(spec, expected, executionBackend); } catch (error) { - fatal(`Invalid contract ${file} query "${queryName}": ${error.message}`); + recordExecutionError({ + spec, + channel, + queryName, + queryDef, + error: `invalid contract ${file} query "${queryName}": ${error.message}`, + reportOnly, + scoringFailures, + }); + continue; } const frontendOracle = oracleSelection.frontend; const expectedCount = frontendOracle.count; @@ -1349,206 +1337,191 @@ function main() { continue; } - if (channel === 'syntax' && typeof validateSyntax !== 'function') { - fatal( - `Syntax contract "${ruleId}" requires validateQueryWithBundle from ${SYNTAX_MODULE}. ` + - `Validate this SQL branch against the OSD headless-syntax PR.` - ); - } - const result = - channel === 'syntax' - ? validateSyntax(query, grammar) - : lintQuery(query, grammar, context); - const allFrontendFindings = - channel === 'syntax' ? result.errors || [] : result.diagnostics || []; - const matches = - channel === 'syntax' - ? allFrontendFindings.filter((finding) => finding.code === frontendOracle.code) - : allFrontendFindings.filter((finding) => finding.ruleId === ruleId); - const actual = matches.length; - const ok = actual === expectedCount; - - log( - ` ${ok ? 'PASS' : 'FAIL'} ${ruleId}/${queryName} (${role}): ` + - `expected ${expectedCount}, got ${actual} — ${query}` - ); + try { + let result; + if (channel === 'syntax' && typeof validateSyntax !== 'function') { + throw new Error( + `syntax validation requires validateQueryWithBundle from ${SYNTAX_MODULE}` + ); + } + result = + channel === 'syntax' + ? validateSyntax(query, grammar) + : lintQuery(query, grammar, context); + const allFrontendFindings = + channel === 'syntax' ? result.errors || [] : result.diagnostics || []; + const matches = + channel === 'syntax' + ? allFrontendFindings.filter((finding) => finding.code === frontendOracle.code) + : allFrontendFindings.filter((finding) => finding.ruleId === ruleId); + const actual = matches.length; + const ok = actual === expectedCount; - const evaluated = evaluateFrontendAssertions({ - channel, - query, - matches, - allFrontendFindings, - frontendOracle, - decideAction, - }); - const assertions = { count: ok, ...evaluated.assertions }; - const mismatches = [ - ...(ok - ? [] - : [ - { - field: 'count', - expected: expectedCount, - actual, - }, - ]), - ...evaluated.mismatches, - ]; - - const resultEntry = { - ruleId, - channel, - queryName, - role, - query, - expected: expectedCount, - actual, - severities: matches.map((m) => m.severity).filter(Boolean), - severityMatched: evaluated.severityMatched, - messageMatched: evaluated.messageMatched, - deterministicFixMatched: evaluated.deterministicFixMatched, - aiActionMatched: evaluated.aiActionMatched, - actionDecisionMatched: evaluated.actionDecisionMatched, - fixMatched: evaluated.syntaxFixMatched, - rawMessageMatched: evaluated.rawMessageMatched, - totalErrorsMatched: evaluated.totalErrorsMatched, - assertions, - mismatches, - ...(evaluated.deterministicFixActual !== undefined - ? { deterministicFix: evaluated.deterministicFixActual } - : {}), - ...(evaluated.aiActionActual !== undefined - ? { aiAction: evaluated.aiActionActual } - : {}), - ...(evaluated.actionDecisionActual !== undefined - ? { actionDecision: evaluated.actionDecisionActual } - : {}), - ...(channel === 'syntax' - ? { - code: frontendOracle.code, - codes: allFrontendFindings.map((finding) => finding.code).filter(Boolean), - totalErrors: allFrontendFindings.length, - } - : {}), - ...(reportOnly ? { reportOnly: true } : {}), - executionBackend, - backendOracleStatus: oracleSelection.status, - }; - - for (const mismatch of mismatches) { - scoringFailures.push( - `[${ruleId}/${queryName}] frontend.${mismatch.field} mismatch: ` + - `expected ${JSON.stringify(mismatch.expected)}, got ` + - `${JSON.stringify(mismatch.actual)} for: ${query}` + log( + ` ${ok ? 'PASS' : 'FAIL'} ${ruleId}/${queryName} (${role}): ` + + `expected ${expectedCount}, got ${actual} — ${query}` ); - } - if (oracleSelection.status === 'not-applicable') { - // Only the backend fixture is non-applicable. The detector still ran above and its - // count/severity/message assertions remain ordinary, comparable frontend evidence. - resultEntry.backendOracleReason = oracleSelection.reason; - } else if (oracleSelection.status === 'coverage-missing') { - resultEntry.outcome = 'coverage-missing'; - resultEntry.coverage = 'missing'; - resultEntry.reason = oracleSelection.reason; - resultEntry.coverageMissing = oracleSelection.reason; - if (!observeAnalytics) { + const evaluated = evaluateFrontendAssertions({ + channel, + query, + matches, + allFrontendFindings, + frontendOracle, + }); + const assertions = { count: ok, ...evaluated.assertions }; + const mismatches = [ + ...(ok + ? [] + : [ + { + field: 'count', + expected: expectedCount, + actual, + }, + ]), + ...evaluated.mismatches, + ]; + + const resultEntry = { + ruleId, + channel, + queryName, + role, + query, + expected: expectedCount, + actual, + severities: matches.map((m) => m.severity).filter(Boolean), + severityMatched: evaluated.severityMatched, + messageMatched: evaluated.messageMatched, + deterministicFixMatched: evaluated.deterministicFixMatched, + fixMatched: evaluated.syntaxFixMatched, + rawMessageMatched: evaluated.rawMessageMatched, + totalErrorsMatched: evaluated.totalErrorsMatched, + assertions, + mismatches, + ...(evaluated.deterministicFixActual !== undefined + ? { deterministicFix: evaluated.deterministicFixActual } + : {}), + ...(channel === 'syntax' + ? { + code: frontendOracle.code, + codes: allFrontendFindings.map((finding) => finding.code).filter(Boolean), + totalErrors: allFrontendFindings.length, + } + : {}), + ...(reportOnly ? { reportOnly: true } : {}), + executionBackend, + backendOracleStatus: oracleSelection.status, + }; + + for (const mismatch of mismatches) { scoringFailures.push( - `[${ruleId}/${queryName}] ${executionBackend} backend coverage missing: ${oracleSelection.reason}.` + `[${ruleId}/${queryName}] frontend.${mismatch.field} mismatch: ` + + `expected ${JSON.stringify(mismatch.expected)}, got ` + + `${JSON.stringify(mismatch.actual)} for: ${query}` ); } - } - // Differential: the observed backend behavior must agree with the observed - // detector output through the shared contract (design §3.2, §4.3). A - // rejection-kind query the backend rejected must be one the detector flags; - // a success/advisory query the backend accepted must be one the detector - // passes. This catches drift the two halves would otherwise hide by both - // pinning to the same JSON. - if (backendReport) { - const be = backendReport.get(`${ruleId}::${queryName}`); - if (!be) { - scoringFailures.push( - `[${ruleId}/${queryName}] no backend report entry (backend did not run this query).` - ); - } else { - const backendObservation = classifyBackendReportRow(be); - if (oracleSelection.status !== 'applicable') { - // A missing or non-applicable oracle is never an acceptance claim. Keep - // any backend observation visible, but do not coerce a missing verdict - // through `!!be.rejected` or score a differential against another route. - resultEntry.backendOutcome = backendObservation.status; - } else if (backendObservation.status !== 'observed') { - resultEntry.backendOutcome = backendObservation.status; + if (oracleSelection.status === 'not-applicable') { + // Only the backend fixture is non-applicable. The detector still ran above and its + // count/severity/message assertions remain ordinary, comparable frontend evidence. + resultEntry.backendOracleReason = oracleSelection.reason; + } else if (oracleSelection.status === 'coverage-missing') { + resultEntry.outcome = 'coverage-missing'; + resultEntry.coverage = 'missing'; + resultEntry.reason = oracleSelection.reason; + resultEntry.coverageMissing = oracleSelection.reason; + if (!observeAnalytics) { scoringFailures.push( - `[${ruleId}/${queryName}] backend report has no accepted/rejected verdict ` + - `(outcome=${JSON.stringify(backendObservation.status)}).` + `[${ruleId}/${queryName}] ${executionBackend} backend coverage missing: ${oracleSelection.reason}.` + ); + } + } + + // Differential: the observed backend behavior must agree with the observed + // detector output through the shared contract (design §3.2, §4.3). A + // rejection-kind query the backend rejected must be one the detector flags; + // a success/advisory query the backend accepted must be one the detector + // passes. This catches drift the two halves would otherwise hide by both + // pinning to the same JSON. + if (backendReport) { + const be = backendReport.get(`${ruleId}::${queryName}`); + if (!be) { + scoringFailures.push( + `[${ruleId}/${queryName}] no backend report entry (backend did not run this query).` ); } else { - const backendKind = oracleSelection.oracle.kind; - const expectRejected = backendKind === 'rejection'; - const backendRejected = backendObservation.rejected; - resultEntry.backendRejected = backendRejected; - if (backendRejected !== expectRejected) { - scoringFailures.push( - `[${ruleId}/${queryName}] differential: backend ${backendRejected ? 'rejected' : 'accepted'} ` + - `but the contract's backend.kind="${backendKind}" expects ${expectRejected ? 'rejection' : 'acceptance'} for: ${query}` - ); - } - // Trigger cross-check: a trigger the detector flags must be one the engine - // ALSO objects to — but only where the contract claims the engine objects - // at all. - // - // For a `rejection` rule the two coincide: detector flags <-> engine - // rejects, and a disagreement means one side drifted. That is the original - // check and it is unchanged. - // - // An ADVISORY rule is different by design. It flags a query the engine - // runs happily: `head-without-sort` marks non-determinism, - // `division-by-zero` marks a silent null, `dedup-consecutive` succeeds via - // the Calcite-to-v2 fallback. "Detector flagged, backend accepted" is that - // rule working, not drift — so pairing the detector against `be.rejected` - // failed every advisory trigger unconditionally. That, not runtime cost, - // is the structural reason those contracts could only run nightly. - // - // The contracts already carry the distinction in `backend.kind`, so this - // reads data that exists rather than adding a flag. Advisory triggers keep - // full coverage from the other two assertions: the backend-kind check above - // fires if the engine starts REJECTING a query pinned as accepted, and the - // `detectorCount` assertion fires if the detector stops flagging it. Only - // the pairing rule is scoped to the rules it makes sense for. - const detectorFlagged = actual > 0; - if (role === 'trigger' && expectRejected && detectorFlagged !== backendRejected) { - scoringFailures.push( - `[${ruleId}/${queryName}] differential: trigger detector ${detectorFlagged ? 'flagged' : 'passed'} ` + - `but backend ${backendRejected ? 'rejected' : 'accepted'} for: ${query}` - ); - } - // A control must pass on both sides regardless of kind: it is a valid - // query the rule has to stay quiet on. Unlike a trigger, that claim does - // not vary with `backend.kind`. - if (role === 'control' && (detectorFlagged || backendRejected)) { + const backendObservation = classifyBackendReportRow(be); + if (oracleSelection.status !== 'applicable') { + // A missing or non-applicable oracle is never an acceptance claim. Keep + // any backend observation visible, but do not coerce a missing verdict + // through `!!be.rejected` or score a differential against another route. + resultEntry.backendOutcome = backendObservation.status; + } else if (backendObservation.status !== 'observed') { + resultEntry.backendOutcome = backendObservation.status; scoringFailures.push( - `[${ruleId}/${queryName}] differential: control must pass on both sides but detector ${detectorFlagged ? 'flagged' : 'passed'} ` + - `and backend ${backendRejected ? 'rejected' : 'accepted'} for: ${query}` - ); - } - if ( - role === 'suppression-control' && - (detectorFlagged || !backendRejected) - ) { - scoringFailures.push( - `[${ruleId}/${queryName}] differential: suppression control must retain a backend ` + - `syntax rejection without a "${frontendOracle.code}" suggestion, but frontend ` + - `${detectorFlagged ? 'suggested a rewrite' : 'did not suggest a rewrite'} and ` + - `backend ${backendRejected ? 'rejected' : 'accepted'} for: ${query}` + `[${ruleId}/${queryName}] backend report has no accepted/rejected verdict ` + + `(outcome=${JSON.stringify(backendObservation.status)}).` ); + } else { + const backendKind = oracleSelection.oracle.kind; + const expectRejected = backendKind === 'rejection'; + const backendRejected = backendObservation.rejected; + resultEntry.backendRejected = backendRejected; + if (backendRejected !== expectRejected) { + scoringFailures.push( + `[${ruleId}/${queryName}] differential: backend ${backendRejected ? 'rejected' : 'accepted'} ` + + `but the contract's backend.kind="${backendKind}" expects ${expectRejected ? 'rejection' : 'acceptance'} for: ${query}` + ); + } + // Pair detector and backend rejection only for rejection rules. + // Advisory rules intentionally flag queries the backend accepts. + const detectorFlagged = actual > 0; + if ( + role === 'trigger' && + expectRejected && + detectorFlagged !== backendRejected + ) { + scoringFailures.push( + `[${ruleId}/${queryName}] differential: trigger detector ${detectorFlagged ? 'flagged' : 'passed'} ` + + `but backend ${backendRejected ? 'rejected' : 'accepted'} for: ${query}` + ); + } + if (role === 'control' && (detectorFlagged || backendRejected)) { + scoringFailures.push( + `[${ruleId}/${queryName}] differential: control must pass on both sides but detector ${detectorFlagged ? 'flagged' : 'passed'} ` + + `and backend ${backendRejected ? 'rejected' : 'accepted'} for: ${query}` + ); + } + if ( + role === 'suppression-control' && + (detectorFlagged || !backendRejected) + ) { + scoringFailures.push( + `[${ruleId}/${queryName}] differential: suppression control must retain a backend ` + + `syntax rejection without a "${frontendOracle.code}" suggestion, but frontend ` + + `${detectorFlagged ? 'suggested a rewrite' : 'did not suggest a rewrite'} and ` + + `backend ${backendRejected ? 'rejected' : 'accepted'} for: ${query}` + ); + } } } } - } - report.results.push(resultEntry); + report.results.push(resultEntry); + } catch (error) { + recordExecutionError({ + spec, + channel, + queryName, + queryDef, + expected: expectedCount, + error, + reportOnly, + scoringFailures, + }); + } } } From 91c102471815de8f1169608ee032b5624a49e90c Mon Sep 17 00:00:00 2001 From: Hanyu Wei Date: Tue, 4 Aug 2026 13:37:18 -0700 Subject: [PATCH 3/5] test(ci): prove PPL lint fault isolation Signed-off-by: Hanyu Wei --- .../__tests__/run-frontend-contract.test.mjs | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs b/scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs index ff0b48f9100..e8ffa5ad281 100644 --- a/scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs +++ b/scripts/ppl-lint/__tests__/run-frontend-contract.test.mjs @@ -4,7 +4,12 @@ */ import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; import { assertActiveShippingContracts, @@ -14,6 +19,8 @@ import { selectManifestContractNames, } from '../run-frontend-contract.mjs'; +const SCRIPT = fileURLToPath(new URL('../run-frontend-contract.mjs', import.meta.url)); + const RANGE = { startLine: 1, startColumn: 11, @@ -161,6 +168,133 @@ test('a frontend execution error remains a complete report row', () => { ); }); +test('the runner writes later rule rows after one frontend execution error', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ppl-lint-runner-')); + t.after(() => fs.rmSync(root, { recursive: true, force: true })); + const osdRoot = path.join(root, 'osd'); + const contractDir = path.join(root, 'contracts'); + const reportPath = path.join(root, 'detector-report.json'); + const grammarPath = path.join(root, 'ppl-grammar-bundle.json'); + const targetPath = path.join(root, 'target.json'); + fs.mkdirSync( + path.join(osdRoot, 'src/plugins/data/public/antlr/opensearch_ppl'), + { recursive: true } + ); + fs.mkdirSync(path.join(osdRoot, 'packages/osd-monaco'), { recursive: true }); + fs.mkdirSync(contractDir, { recursive: true }); + + const wiring = (ruleId) => ({ + detector: ruleId, + enabled: true, + severity: 'info', + runtimeOnly: false, + needsContext: false, + needsExplain: false, + sourceScoped: false, + appliesTo: {}, + }); + const contract = (ruleId, query) => ({ + schemaVersion: 4, + ruleId, + grammarSurface: 'runtime-bundle', + schedule: 'pr', + wiring: wiring(ruleId), + index: 'test-index', + queries: { + trigger: { role: 'trigger', query }, + }, + expectations: [ + { + version: '>=0.0.0', + queries: { + trigger: { + frontend: { + count: 0, + deterministicFix: { offered: false }, + }, + backends: { + standard: { kind: 'advisory', httpStatus: 200 }, + analytics: { kind: 'advisory', httpStatus: 200 }, + }, + }, + }, + }, + ], + }); + const files = ['first-rule.spec.json', 'second-rule.spec.json']; + fs.writeFileSync( + path.join(contractDir, files[0]), + JSON.stringify(contract('first-rule', 'source={{index}} | fail')) + ); + fs.writeFileSync( + path.join(contractDir, files[1]), + JSON.stringify(contract('second-rule', 'source={{index}} | pass')) + ); + fs.writeFileSync( + path.join(contractDir, 'manifest.json'), + JSON.stringify({ + schemaVersion: 4, + contracts: files, + defaultError: [], + requiredSyntaxFeatures: [], + }) + ); + fs.writeFileSync( + path.join(osdRoot, 'packages/osd-monaco/ppl-lint.js'), + `const wiring = (id) => ({ + id, detector: id, enabled: true, severity: 'info', runtimeOnly: false, + needsContext: false, needsExplain: false, sourceScoped: false, appliesTo: {} + }); + exports.getBundledCatalog = () => [wiring('first-rule'), wiring('second-rule')];` + ); + fs.writeFileSync( + path.join( + osdRoot, + 'src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint.js' + ), + `exports.deserializeBundleOrThrow = (bundle) => bundle; + exports.lintQueryWithBundle = (query) => { + if (query.includes('| fail')) throw new Error('detector crashed'); + return { diagnostics: [] }; + };` + ); + fs.writeFileSync(grammarPath, JSON.stringify({ grammarHash: 'sha256:test' })); + fs.writeFileSync( + targetPath, + JSON.stringify({ + schemaVersion: 2, + executionBackend: 'standard', + engineVersion: '3.8.0-SNAPSHOT', + grammarHash: 'sha256:test', + storage: 'lucene', + shardCount: 1, + }) + ); + + const result = spawnSync(process.execPath, [SCRIPT], { + cwd: osdRoot, + encoding: 'utf8', + env: { + ...process.env, + PPL_LINT_CONTRACT_DIR: contractDir, + PPL_LINT_SCHEDULE: 'pr', + PPL_LINT_GRAMMAR_BUNDLE: grammarPath, + PPL_LINT_TARGET_MANIFEST: targetPath, + PPL_LINT_REPORT: reportPath, + }, + }); + + assert.equal(result.status, 1); + assert.match(result.stderr, /first-rule\/trigger.*frontend\.execution failed/s); + assert.match(result.stdout, /PASS second-rule\/trigger/); + const report = JSON.parse(fs.readFileSync(reportPath, 'utf8')); + assert.equal(report.results.length, 2); + assert.equal(report.results[0].outcome, 'error'); + assert.equal(report.results[0].error, 'detector crashed'); + assert.equal(report.results[1].ruleId, 'second-rule'); + assert.equal(report.results[1].actual, 0); +}); + test('syntax suppression checks raw parser errors outside the suggestion code filter', () => { const parserErrors = [ { From 22553f5bf93037d4ed639ccc82ca5d8f8ba59077 Mon Sep 17 00:00:00 2001 From: Hanyu Wei Date: Tue, 4 Aug 2026 15:31:26 -0700 Subject: [PATCH 4/5] fix(ci): publish compatibility report before drift failure --- .../ppl-lint-multiversion-validation.yml | 561 +++--------------- ...ppl-lint-analytics-engine-ci-validation.md | 9 +- ...pl-lint-runtime-compatibility-ci-design.md | 188 ++++++ scripts/ppl-lint/README.md | 114 +--- .../__tests__/aggregate-versions.test.mjs | 177 ++++-- .../validate-pr-build-targets.test.mjs | 271 --------- scripts/ppl-lint/aggregate-versions.mjs | 239 +++----- .../ppl-lint/validate-pr-build-targets.mjs | 285 --------- 8 files changed, 504 insertions(+), 1340 deletions(-) create mode 100644 docs/dev/ppl-lint-runtime-compatibility-ci-design.md delete mode 100644 scripts/ppl-lint/__tests__/validate-pr-build-targets.test.mjs delete mode 100644 scripts/ppl-lint/validate-pr-build-targets.mjs diff --git a/.github/workflows/ppl-lint-multiversion-validation.yml b/.github/workflows/ppl-lint-multiversion-validation.yml index 294197584f4..ef169a78bb5 100644 --- a/.github/workflows/ppl-lint-multiversion-validation.yml +++ b/.github/workflows/ppl-lint-multiversion-validation.yml @@ -33,10 +33,9 @@ name: PPL lint multi-version validation # not export a candidate grammar bundle, so the detector half would have nothing # to lint against. Raise `ENGINE_VERSIONS` as older versions leave support. # -# Non-enforcing on purpose, for now: it reports and uploads, and the required -# check stays the sibling workflow's `validation-result`. Promoting this to -# required needs a green baseline across the whole matrix first (a rule that has -# quietly drifted on 3.6 would otherwise block every unrelated PR on day one). +# Compatibility differences are collected without interrupting the matrix. +# The aggregate writes the complete table and JSON artifact first, then the +# final step fails this job when a declared-supported rule has drifted. on: # Nightly is the primary schedule: the matrix pulls three engine images, so it @@ -67,10 +66,6 @@ on: description: 'JSON array of released engine versions to validate, e.g. ["3.6.0","3.7.0"]. The PR build is always added.' required: false type: string - compiled_versions: - description: 'JSON array of engine versions to validate on the compiled-simplified surface, e.g. ["2.19.0"]. Use "[]" to skip them.' - required: false - type: string permissions: contents: read @@ -87,23 +82,6 @@ env: # them when 3.6.1 / 3.7.1 publish, and never pin `.0` once a newer patch # exists — that would validate an engine no user runs). ENGINE_VERSIONS: '["3.6.0","3.7.0"]' - # Released engine versions to validate on the COMPILED-SIMPLIFIED surface. - # - # These engines cannot export a grammar bundle (GET /_plugins/_ppl/_grammar - # landed in 3.6), so the runtime surface cannot reach them at all. But the - # compiled surface has no such floor: it lints with OSD's own checked-in grammar, - # which is exactly what a user gets when no bundle is available — including every - # user on an engine below 3.6. Those legs still run the real contract queries - # against the real engine, so the backend half of the differential is genuine. - # - # Only contracts declaring `grammarSurface: "both"` are scored here; the rest are - # reported not-applicable. Nightly only — see the `compiled_versions` input to - # run one ad hoc. - # - # Always the LATEST PATCH of each line, never `.0`. A user on 2.19 is on - # 2.19.6, so validating 2.19.0 tests an engine nobody runs and attributes any - # bug fixed in between to the whole line. - COMPILED_ENGINE_VERSIONS: '["2.19.6","3.0.0","3.5.0"]' jobs: # Same reusable workflow + pinned SHA the sibling SQL workflows use, so a @@ -120,7 +98,6 @@ jobs: runs-on: ubuntu-latest outputs: released: ${{ steps.plan.outputs.released }} - compiled: ${{ steps.plan.outputs.compiled }} discovery_engine: ${{ steps.plan.outputs.discovery_engine }} osd_repo: ${{ steps.plan.outputs.osd_repo }} osd_ref: ${{ steps.plan.outputs.osd_ref }} @@ -130,9 +107,6 @@ jobs: env: REQUESTED_VERSIONS: ${{ inputs.engine_versions }} DEFAULT_VERSIONS: ${{ env.ENGINE_VERSIONS }} - REQUESTED_COMPILED: ${{ inputs.compiled_versions }} - DEFAULT_COMPILED: ${{ env.COMPILED_ENGINE_VERSIONS }} - EVENT_NAME: ${{ github.event_name }} REQUESTED_REPO: ${{ inputs.osd_repo }} REQUESTED_REF: ${{ inputs.osd_ref }} VAR_REPO: ${{ vars.OSD_REPO }} @@ -151,28 +125,6 @@ jobs: " echo "released=$released" >> "$GITHUB_OUTPUT" - # Compiled-surface legs add three more engine images, so they run on the - # nightly schedule (and on an explicit dispatch), not on every PR that - # touches the corpus. An explicit input always wins, including "[]". - if [ -n "${REQUESTED_COMPILED:-}" ]; then - compiled="$REQUESTED_COMPILED" - elif [ "$EVENT_NAME" = "pull_request" ]; then - compiled='[]' - else - compiled="$DEFAULT_COMPILED" - fi - # An EMPTY list is legitimate here (unlike engine_versions): it means "skip - # the compiled surface this run". Still reject a non-list. - echo "$compiled" | python3 -c " - import json,sys - v=json.load(sys.stdin) - assert isinstance(v,list), 'compiled_versions must be a JSON array' - for item in v: - assert isinstance(item,str), 'compiled_versions entries must be strings' - " - echo "compiled=$compiled" >> "$GITHUB_OUTPUT" - echo "Compiled-surface legs: $compiled" >> "$GITHUB_STEP_SUMMARY" - # Discovery runs against ONE engine — the newest released version in the # matrix. It is a lead-generator, not a version-drift check, so paying for # a full matrix would multiply cost without adding signal: a false positive @@ -294,191 +246,6 @@ jobs: name: ppl-lint-leg-${{ matrix.version }}-logs path: integ-test/build/reports/** - # Legs for engines BELOW the _grammar endpoint floor (3.6). These cannot export a - # grammar bundle, so they are observed for backend behavior only and their - # detector pass runs on the compiled-simplified surface — which is what a real - # user on such an engine gets, since no bundle can ever load there. - # - # Identical to observe-released except that `-Dppl.lint.grammar.bundle` is - # omitted: the IT skips the export when that property is unset, so no bundle - # fetch is attempted against an engine that has no such endpoint. - observe-compiled: - name: Observe engine ${{ matrix.version }} (compiled surface) - needs: plan - # An empty compiled list means "skip this surface" (the pull_request default). - if: ${{ needs.plan.outputs.compiled != '[]' }} - runs-on: ubuntu-latest - timeout-minutes: 30 - strategy: - fail-fast: false - matrix: - version: ${{ fromJSON(needs.plan.outputs.compiled) }} - services: - opensearch: - image: opensearchproject/opensearch:${{ matrix.version }} - env: - discovery.type: single-node - DISABLE_SECURITY_PLUGIN: 'true' - DISABLE_INSTALL_DEMO_CONFIG: 'true' - OPENSEARCH_JAVA_OPTS: -Xms1g -Xmx1g - ports: - - 9200:9200 - options: >- - --health-cmd "curl -sf http://localhost:9200/_cluster/health || exit 1" - --health-interval 15s - --health-timeout 10s - --health-retries 20 - --health-start-period 60s - steps: - - name: Checkout SQL pull request - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - - name: Wait for the engine and confirm its version - id: engine - run: | - set -euo pipefail - for i in $(seq 1 40); do - if curl -sf http://localhost:9200 > /tmp/root.json; then break; fi - echo "waiting for engine (${i}/40)..." - sleep 5 - done - cat /tmp/root.json - reported=$(python3 -c "import json;print(json.load(open('/tmp/root.json'))['version']['number'])") - case "$reported" in - ${{ matrix.version }}*) ;; - *) echo "::error::engine reported $reported but the matrix asked for ${{ matrix.version }}"; exit 1 ;; - esac - curl -sf http://localhost:9200/_cat/plugins | grep -i sql - - # `_cluster/health` goes GREEN before the bundled plugins finish creating - # their system indices, and the IT's first act is to wipe every non-system - # index. On 2.19 that DELETE landed while ML Commons was still initializing - # `.plugins-ml-config` and blocked until the client's 60s socket timeout, - # failing the leg before a single contract query ran. Wait for the plugin - # indices to stop appearing, so the wipe cannot race initialization. - - name: Wait for bundled plugin system indices to settle - run: | - set -euo pipefail - previous="" - stable=0 - for i in $(seq 1 30); do - current=$(curl -sf "http://localhost:9200/_cat/indices?h=index&expand_wildcards=all" \ - | sort | tr '\n' ',' || true) - if [ -n "$current" ] && [ "$current" = "$previous" ]; then - stable=$((stable + 1)) - # Three consecutive identical listings: no plugin is still creating - # indices. One match is not enough — initialization has gaps between - # an index being created and the next one starting. - if [ "$stable" -ge 3 ]; then - echo "index set stable after ${i} poll(s): $current" - exit 0 - fi - else - stable=0 - fi - previous="$current" - sleep 2 - done - # Not fatal: a slow-but-working engine should still be observed. The IT - # tolerates a wipe failure per index, and a genuinely unreachable cluster - # fails loudly in the next step anyway. - echo "::warning::plugin index set did not stabilize; continuing" - - # Probe the EXACT requests the test framework makes before any test runs. - # `OpenSearchRestTestCase.initClient` issues `GET _nodes/plugins`, and - # `wipeAllOpenSearchIndices` issues `GET _cat/indices?expand_wildcards=all`. - # A leg that dies with a bare socket timeout gives no clue which of those - # hung, so time them here where the output is readable. - - name: Probe the framework's own startup requests - run: | - set -uo pipefail - for path in "_nodes/plugins" "_cat/indices?format=json&expand_wildcards=all" "_cluster/health"; do - start=$(date +%s) - if curl -sS --max-time 30 -o /tmp/probe.out -w '%{http_code}' \ - "http://localhost:9200/${path}" > /tmp/probe.code 2>/tmp/probe.err; then - echo "OK $(($(date +%s) - start))s HTTP $(cat /tmp/probe.code) ${path} ($(wc -c < /tmp/probe.out) bytes)" - else - echo "::warning::SLOW/FAIL $(($(date +%s) - start))s ${path} $(cat /tmp/probe.err)" - fi - done - start=$(date +%s) - if curl -sS --http2 --max-time 30 -o /dev/null -w '%{http_version}' \ - "http://localhost:9200/_nodes/plugins" > /tmp/h2.out 2>/tmp/h2.err; then - echo "h2 probe: negotiated HTTP/$(cat /tmp/h2.out) in $(($(date +%s) - start))s" - else - echo "::warning::h2 probe FAILED after $(($(date +%s) - start))s: $(cat /tmp/h2.err)" - fi - # Response SIZE is the last untested difference. curl streams the body and - # does not care; the test framework calls entityAsMap on it, and - # _nodes/plugins on an engine with many bundled plugins is large. Record the - # sizes so a size-dependent hang is visible rather than inferred. - for path in "_nodes/plugins" "_nodes" "_cat/plugins"; do - bytes=$(curl -sS --max-time 30 "http://localhost:9200/${path}" | wc -c) - echo "size probe: ${path} -> ${bytes} bytes" - done - - - name: Set up JDK 21 - uses: actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 # v4 - with: - distribution: 'temurin' - java-version: 21 - - # The curl probes above reach this engine instantly, yet the contract IT's own - # client times out on the same endpoint before any test body runs. Everything - # curl can tell us has been exhausted, so run the probe from a JVM: raw TCP, - # then HttpURLConnection, then the real OpenSearch RestClient per endpoint. - # Whichever layer stops working is the answer. - # - # `continue-on-error` because this is a diagnostic: its findings must not be - # what decides the leg, and the contract step below is still the real check. - - name: Probe REST client connectivity from a JVM - continue-on-error: true - run: | - set -uo pipefail - ./gradlew :integ-test:integTestRemote \ - --tests 'org.opensearch.sql.calcite.remote.RestClientConnectivityProbeIT' \ - -Dtests.rest.cluster=localhost:9200 \ - -Dtests.cluster=localhost:9200 \ - -Dtests.clustername=docker-cluster \ - --info 2>&1 | grep -E 'rest-connectivity-probe|FAILED|BUILD|tests? completed|No tests found' || true - - - name: Run contract observation against engine ${{ matrix.version }} - run: | - set -euo pipefail - mkdir -p leg - # No -Dppl.lint.grammar.bundle: this engine predates the _grammar endpoint, - # and the IT correctly exports nothing when the property is unset. - ./gradlew :integ-test:integTestRemote \ - --tests org.opensearch.sql.calcite.remote.PplLintRuleValidationIT \ - -Dtests.rest.cluster=localhost:9200 \ - -Dtests.cluster=localhost:9200 \ - -Dtests.clustername=docker-cluster \ - -Dppl.lint.schedule=nightly \ - -Dppl.lint.observe.only=true \ - -Dppl.lint.execution_backend=standard \ - -Dppl.lint.sql_sha="${GITHUB_SHA}" \ - -Dppl.lint.report="$(pwd)/leg/backend-report.json" \ - -Dppl.lint.target="$(pwd)/leg/target.json" - # Mark the leg so the detect job knows to lint it on the compiled surface. - # A leg with no bundle would otherwise look like a failed export. - echo 'compiled-simplified' > leg/surface - - - name: Upload leg artifacts - if: ${{ always() }} - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 - with: - name: ppl-lint-leg-${{ matrix.version }}-compiled - path: leg - if-no-files-found: error - - - name: Upload failure logs - if: ${{ failure() }} - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 - continue-on-error: true - with: - name: ppl-lint-leg-${{ matrix.version }}-compiled-logs - path: integ-test/build/reports/** - # The PR's own engine build, so the newest point in the matrix is the code under # review rather than the last release. Same oracle as the released legs; the only # difference is a Gradle-managed cluster instead of a published image, which is @@ -541,179 +308,6 @@ jobs: integ-test/build/test-results/** integ-test/build/testclusters/*/logs/* - # The PR build through the full composite/Parquet + DataFusion stack. This is - # an observation leg: route/identity/infrastructure failures are fatal, while - # backend oracles are promoted only after their captured behavior is reviewed. - observe-pr-build-analytics: - name: Observe engine pr-build (analytics) - needs: Get-CI-Image-Tag - runs-on: ubuntu-latest - timeout-minutes: 30 - env: - ANALYTICS_FEATURE_BUILD_LATEST: https://ci.opensearch.org/ci/dbc/feature-build-opensearch/feature-datafusion/latest/linux/x64/tar/builds/opensearch - container: - image: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-version-linux }} - options: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-options }} - steps: - - name: Run start commands - run: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-command }} - - - name: Checkout SQL pull request - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - - name: Set up JDK 25 - uses: actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 # v4 - with: - distribution: 'temurin' - java-version: 25 - - - name: Resolve analytics feature build - id: analytics-build - run: | - set -euo pipefail - mkdir -p leg - requested_manifest="${ANALYTICS_FEATURE_BUILD_LATEST}/manifest.yml" - resolved_manifest=$(curl --fail --silent --show-error --location \ - --retry 3 --retry-all-errors \ - --output leg/analytics-feature-manifest.yml \ - --write-out '%{url_effective}' \ - "$requested_manifest") - artifact_root="${resolved_manifest%/manifest.yml}" - plugin_base="${artifact_root}/plugins" - native_url="${artifact_root}/dist/libopensearch_native.so" - { - echo "artifact_root=$artifact_root" - echo "plugin_base=$plugin_base" - echo "native_url=$native_url" - } >> "$GITHUB_OUTPUT" - ANALYTICS_ARTIFACT_ROOT="$artifact_root" \ - ANALYTICS_PLUGIN_BASE="$plugin_base" \ - ANALYTICS_NATIVE_URL="$native_url" \ - ANALYTICS_RESOLVED_MANIFEST="$resolved_manifest" \ - python3 - <<'PY' - import hashlib - import json - import os - from pathlib import Path - - manifest = Path("leg/analytics-feature-manifest.yml") - context = { - "schemaVersion": 1, - "stage": "feature-build-resolved", - "sqlSha": os.environ["GITHUB_SHA"], - "executionBackend": "analytics", - "storage": "composite-parquet", - "artifactRoot": os.environ["ANALYTICS_ARTIFACT_ROOT"], - "pluginBase": os.environ["ANALYTICS_PLUGIN_BASE"], - "nativeLibraryUrl": os.environ["ANALYTICS_NATIVE_URL"], - "resolvedManifestUrl": os.environ["ANALYTICS_RESOLVED_MANIFEST"], - "manifestSha256": "sha256:" + hashlib.sha256(manifest.read_bytes()).hexdigest(), - } - Path("leg/analytics-bootstrap.json").write_text( - json.dumps(context, indent=2) + "\n", encoding="utf-8" - ) - PY - - - name: Run analytics contract observation against the PR build - id: analytics-observation - env: - ANALYTICS_FEATURE_BUILD_BASE: ${{ steps.analytics-build.outputs.plugin_base }} - ANALYTICS_NATIVE_LIB_URL: ${{ steps.analytics-build.outputs.native_url }} - run: | - set -euo pipefail - chown -R 1000:1000 "$(pwd)" - su "$(id -un 1000)" -c "./gradlew :integ-test:analyticsEnginePplLintIT \ - -PanalyticsFeatureBuildBase=${ANALYTICS_FEATURE_BUILD_BASE} \ - -PanalyticsNativeLibUrl=${ANALYTICS_NATIVE_LIB_URL} \ - -Dppl.lint.schedule=nightly \ - -Dppl.lint.observe.only=true \ - -Dppl.lint.sql_sha=${GITHUB_SHA} \ - -Dppl.lint.report=$(pwd)/leg/backend-report.json \ - -Dppl.lint.grammar.bundle=$(pwd)/leg/ppl-grammar-bundle.json \ - -Dppl.lint.target=$(pwd)/leg/target.json" - - - name: Record analytics bootstrap provenance - if: ${{ always() }} - env: - OBSERVATION_OUTCOME: ${{ steps.analytics-observation.outcome }} - ANALYTICS_ARTIFACT_ROOT: ${{ steps.analytics-build.outputs.artifact_root }} - ANALYTICS_PLUGIN_BASE: ${{ steps.analytics-build.outputs.plugin_base }} - ANALYTICS_NATIVE_URL: ${{ steps.analytics-build.outputs.native_url }} - run: | - mkdir -p leg - python3 - <<'PY' - import hashlib - import json - import os - from pathlib import Path - - context_file = Path("leg/analytics-bootstrap.json") - if context_file.exists(): - context = json.loads(context_file.read_text(encoding="utf-8")) - else: - context = { - "schemaVersion": 1, - "sqlSha": os.environ["GITHUB_SHA"], - "executionBackend": "analytics", - "storage": "composite-parquet", - "artifactRoot": os.environ.get("ANALYTICS_ARTIFACT_ROOT") or None, - "pluginBase": os.environ.get("ANALYTICS_PLUGIN_BASE") or None, - "nativeLibraryUrl": os.environ.get("ANALYTICS_NATIVE_URL") or None, - } - - def describe(file): - digest = hashlib.sha256() - with file.open("rb") as stream: - for chunk in iter(lambda: stream.read(1024 * 1024), b""): - digest.update(chunk) - return { - "path": str(file), - "size": file.stat().st_size, - "sha256": "sha256:" + digest.hexdigest(), - } - - distributions = Path("integ-test/build/distributions") - native_libraries = sorted( - Path("integ-test/build/native").glob( - "*/release/libopensearch_native.so" - ) - ) - artifacts = ( - [describe(file) for file in sorted(distributions.glob("*.zip"))] - if distributions.is_dir() - else [] - ) - artifacts.extend(describe(file) for file in native_libraries) - context["stage"] = "observation-finished" - context["outcome"] = os.environ.get("OBSERVATION_OUTCOME") or "not-run" - context["effectiveJavaLibraryPaths"] = [ - str(file.parent) for file in native_libraries - ] - context["downloadedArtifacts"] = artifacts - context_file.write_text( - json.dumps(context, indent=2) + "\n", encoding="utf-8" - ) - PY - - - name: Upload analytics leg artifacts - if: ${{ always() }} - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 - with: - name: ppl-lint-leg-pr-build-analytics - path: leg - if-no-files-found: error - - - name: Upload analytics failure logs - if: ${{ failure() }} - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 - continue-on-error: true - with: - name: ppl-lint-leg-pr-build-analytics-logs - path: | - integ-test/build/reports/** - integ-test/build/test-results/** - integ-test/build/testclusters/*/logs/* - # Lint each engine's exported grammar with the OSD detectors. Separate from the # observation legs because OSD needs a newer Node/glibc than the engine image # provides, and because one bootstrap can serve every leg. @@ -722,13 +316,11 @@ jobs: # partial matrix must be visibly partial, not silently absent. The aggregate # step fails if NO leg produced a report. detect: - name: Detect on each engine grammar + name: Aggregate rule compatibility needs: - plan - observe-released - - observe-compiled - observe-pr-build - - observe-pr-build-analytics if: ${{ always() && needs.plan.result == 'success' }} runs-on: ubuntu-latest timeout-minutes: 40 @@ -744,16 +336,6 @@ jobs: pattern: ppl-lint-leg-* path: legs - - name: Validate PR-build target identity - run: | - node scripts/ppl-lint/validate-pr-build-targets.mjs \ - --standard legs/ppl-lint-leg-pr-build/target.json \ - --analytics legs/ppl-lint-leg-pr-build-analytics/target.json \ - --standard-report legs/ppl-lint-leg-pr-build/backend-report.json \ - --analytics-report legs/ppl-lint-leg-pr-build-analytics/backend-report.json \ - --contracts integ-test/src/test/resources/ppl-lint/contracts \ - --schedule nightly - - name: Checkout OpenSearch-Dashboards uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: @@ -814,26 +396,15 @@ jobs: fi for leg in "${legs[@]}"; do version=$(basename "$leg" | sed 's/^ppl-lint-leg-//') - # A leg is compiled-surface when its observe job said so. That marker is - # what distinguishes "this engine has no _grammar endpoint" from "the - # bundle export failed", which must stay a hard error. - if [ -f "$leg/surface" ] && [ "$(cat "$leg/surface")" = 'compiled-simplified' ]; then - surface_env=(PPL_LINT_SURFACE=compiled-simplified) - echo "=== detectors vs engine $version (compiled-simplified surface) ===" - elif [ -f "$leg/ppl-grammar-bundle.json" ]; then - surface_env=(PPL_LINT_SURFACE=runtime-bundle - PPL_LINT_GRAMMAR_BUNDLE="$leg/ppl-grammar-bundle.json") - echo "=== detectors vs engine $version (runtime-bundle surface) ===" - else + if [ ! -f "$leg/ppl-grammar-bundle.json" ]; then # Skip the log-only artifacts an observation failure may have uploaded. - echo "skipping $leg (no grammar bundle and no compiled-surface marker)" + echo "skipping $leg (no runtime grammar bundle)" continue fi - observe_env=(PPL_LINT_OBSERVE_ONLY=1) - if [ "$(jq -r '.executionBackend // empty' "$leg/target.json")" = 'analytics' ]; then - observe_env+=(PPL_LINT_OBSERVE_ANALYTICS=1) - fi - env "${surface_env[@]}" "${observe_env[@]}" \ + echo "=== detectors vs engine $version (runtime-bundle surface) ===" + env PPL_LINT_SURFACE=runtime-bundle \ + PPL_LINT_GRAMMAR_BUNDLE="$leg/ppl-grammar-bundle.json" \ + PPL_LINT_OBSERVE_ONLY=1 \ PPL_LINT_CONTRACT_DIR="$GITHUB_WORKSPACE/integ-test/src/test/resources/ppl-lint/contracts" \ PPL_LINT_SCHEDULE=nightly \ PPL_LINT_INCLUDE_DORMANT=1 \ @@ -860,7 +431,6 @@ jobs: id: aggregate env: RELEASED: ${{ needs.plan.outputs.released }} - COMPILED: ${{ needs.plan.outputs.compiled }} run: | set -euo pipefail shopt -s nullglob @@ -881,14 +451,7 @@ jobs: # a matrix that silently lost one — the exact vacuous pass this workflow # exists to prevent. A dead leg is a failure, not a smaller matrix. missing=() - # Compiled legs are labelled "-compiled" to match their artifact - # name, so they occupy their own column even when a runtime leg validated - # the same engine version. - compiled_wanted=$(echo "$COMPILED" | python3 -c " - import json,sys - print(' '.join(f'{v}-compiled' for v in json.load(sys.stdin))) - ") - for want in $(echo "$RELEASED" | python3 -c "import json,sys;print(' '.join(json.load(sys.stdin)))") $compiled_wanted pr-build pr-build-analytics; do + for want in $(echo "$RELEASED" | python3 -c "import json,sys;print(' '.join(json.load(sys.stdin)))") pr-build; do found=no for have in "${present[@]}"; do [ "$have" = "$want" ] && found=yes && break @@ -899,27 +462,74 @@ jobs: echo "::error::planned engine leg(s) produced no report: ${missing[*]}. Check those observe jobs; the matrix is incomplete so its result would be misleading." exit 1 fi + set +e node "$GITHUB_WORKSPACE/scripts/ppl-lint/aggregate-versions.mjs" \ --contracts "$GITHUB_WORKSPACE/integ-test/src/test/resources/ppl-lint/contracts" \ --out "$GITHUB_WORKSPACE/drift-report.json" \ --summary "$GITHUB_STEP_SUMMARY" \ --all-rules \ - --observe-analytics \ "${args[@]}" + aggregate_exit=$? + set -e + echo "exit_code=$aggregate_exit" >> "$GITHUB_OUTPUT" - name: Upload drift report if: ${{ always() }} uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 - continue-on-error: true with: name: ppl-lint-multiversion-drift + if-no-files-found: error + path: drift-report.json + + - name: Upload compatibility evidence + if: ${{ always() }} + continue-on-error: true + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: ppl-lint-multiversion-evidence + if-no-files-found: warn path: | - drift-report.json legs/**/detector-report.json legs/**/detector.log legs/**/target.json - legs/**/analytics-bootstrap.json - legs/**/analytics-feature-manifest.yml + + - name: Fail after publishing compatibility drift + if: ${{ always() }} + env: + AGGREGATE_EXIT: ${{ steps.aggregate.outputs.exit_code }} + RELEASED: ${{ needs.plan.outputs.released }} + run: | + set -euo pipefail + if [ -z "$AGGREGATE_EXIT" ]; then + echo "::error::aggregation did not complete" + exit 1 + fi + expected_legs=$(echo "$RELEASED" | jq 'length + 1') + expected_rules=$(jq -c \ + '[.contracts[] | sub("\\.spec\\.json$"; "")] | sort' \ + integ-test/src/test/resources/ppl-lint/contracts/manifest.json) + if [ ! -s drift-report.json ] || ! jq -e \ + --argjson expected_legs "$expected_legs" \ + --argjson expected_rules "$expected_rules" \ + ' + type == "object" and + ($expected_rules | length) == 12 and + ($expected_rules | index("command-suggestion") | not) and + (.legs | type == "array" and length == $expected_legs) and + (.matrix | type == "array") and + (.matrix | length) == (12 * $expected_legs) and + (.matrix | map(.ruleId) | unique | sort) == $expected_rules and + (.matrix | map(.legKey) | unique | length) == $expected_legs and + ([.matrix[] | [.ruleId, .legKey]] | unique | length) == + (12 * $expected_legs) + ' drift-report.json > /dev/null; then + echo "::error::aggregation did not produce the complete 12-rule compatibility matrix" + exit 1 + fi + if [ "$AGGREGATE_EXIT" -ne 0 ]; then + echo "::error::rule compatibility validation failed; see the table above and the ppl-lint-multiversion-drift artifact" + exit "$AGGREGATE_EXIT" + fi # Discovery: harvest queries from OSD's own lint tests, run both halves over them, # and report detector/engine disagreements as LEADS. @@ -1057,12 +667,8 @@ jobs: -H 'content-type: application/json' \ -d '{"account_number":1,"balance":39225,"age":32,"status":"ok","firstname":"Amber","lastname":"Duke","msg":"took 42ms","body":"INFO started"}' - # Export this engine's grammar bundle so the detector pass can run on the - # RUNTIME surface. That surface matters more than the compiled one here: the - # four `runtimeOnly` rules (union/multisearch/replace arity) are SKIPPED by - # lint_runner on the compiled grammar because the productions they walk do not - # exist there — so a compiled-only discovery run cannot observe them at all, - # and three of the four ship at error severity. + # Export this engine's grammar bundle. Discovery only runs against the runtime + # grammar surface, so an unavailable endpoint skips the detector pass. - name: Export the engine grammar bundle id: bundle run: | @@ -1086,23 +692,8 @@ jobs: " echo "surface=runtime-bundle" >> "$GITHUB_OUTPUT" else - # Not fatal. Discovery is best-effort, and the compiled surface still - # covers 12 of the rules — a lead-generator that produces nothing because - # one endpoint was unavailable is worse than one with narrower coverage. - # The surface is recorded in the report, so a reader can see which ran. - echo "::warning::_grammar export failed; falling back to the compiled surface (runtimeOnly rules will not be observed)." - echo "surface=compiled-simplified" >> "$GITHUB_OUTPUT" - python3 -c " - import json - json.dump({'schemaVersion': 2, - 'engineVersion': '${{ needs.plan.outputs.discovery_engine }}', - 'grammarHash': '', - 'grammarBundle': '', - 'executionBackend': 'standard', - 'storage': 'lucene', - 'shardCount': 1}, - open('$GITHUB_WORKSPACE/discovery-target.json','w')) - " + echo "::warning::_grammar export failed; skipping discovery detector pass." + echo "surface=unavailable" >> "$GITHUB_OUTPUT" fi - name: Run the detectors over the discovery corpus @@ -1111,21 +702,17 @@ jobs: SURFACE: ${{ steps.bundle.outputs.surface }} run: | set -uo pipefail - # Seeded with a harmless assignment rather than left empty: under `set -u`, - # expanding an empty array as "${a[@]}" is an unbound-variable error in bash - # before 4.4, which would crash the compiled-surface fallback — the very - # path that only runs when something else already went wrong. - extra=(PPL_LINT_DISCOVERY=1 - PPL_LINT_TARGET_MANIFEST="$GITHUB_WORKSPACE/discovery-target.json") - if [ "$SURFACE" = 'runtime-bundle' ]; then - extra+=(PPL_LINT_GRAMMAR_BUNDLE="$GITHUB_WORKSPACE/discovery-bundle.json" - ) + if [ "$SURFACE" != 'runtime-bundle' ]; then + echo "Discovery detector pass skipped: runtime grammar bundle unavailable." + exit 0 fi # A non-zero exit is EXPECTED and ignored: the generated specs carry # placeholder expectations, so the runner reports a "failure" for every # query whose real diagnostic count differs. Only the report is read. - env "${extra[@]}" \ - PPL_LINT_SURFACE="$SURFACE" \ + env PPL_LINT_DISCOVERY=1 \ + PPL_LINT_SURFACE=runtime-bundle \ + PPL_LINT_GRAMMAR_BUNDLE="$GITHUB_WORKSPACE/discovery-bundle.json" \ + PPL_LINT_TARGET_MANIFEST="$GITHUB_WORKSPACE/discovery-target.json" \ PPL_LINT_CONTRACT_DIR="$GITHUB_WORKSPACE/discovery-specs" \ PPL_LINT_SCHEDULE=nightly \ PPL_LINT_REPORT="$GITHUB_WORKSPACE/discovery-detector-report.json" \ diff --git a/docs/dev/ppl-lint-analytics-engine-ci-validation.md b/docs/dev/ppl-lint-analytics-engine-ci-validation.md index 3777f561dc7..ba7632b2d54 100644 --- a/docs/dev/ppl-lint-analytics-engine-ci-validation.md +++ b/docs/dev/ppl-lint-analytics-engine-ci-validation.md @@ -1,11 +1,18 @@ # Analytics Engine Coverage for PPL Lint CI Validation -- **Status:** Validated for phased implementation +- **Status:** Deferred; not part of PPL lint pull-request or multi-version CI - **Last updated:** 2026-07-28 - **Scope:** PPL lint contract validation in `.github/workflows/ppl-lint-rule-validation.yml` and `.github/workflows/ppl-lint-multiversion-validation.yml` +> **Decision update (2026-08-04):** Analytics-engine lint validation is +> deferred because the feature build and composite/Parquet fixture surface are +> not stable enough for this compatibility workflow. The active design is +> [PPL Lint Runtime Compatibility CI](ppl-lint-runtime-compatibility-ci-design.md), +> which covers standard runtime-bundle engines only. This document is retained +> as future design context and is not an implementation commitment. + ## 1. Summary The PPL lint CI contract currently compares OpenSearch Dashboards (OSD) diff --git a/docs/dev/ppl-lint-runtime-compatibility-ci-design.md b/docs/dev/ppl-lint-runtime-compatibility-ci-design.md new file mode 100644 index 00000000000..eb0c7242fbe --- /dev/null +++ b/docs/dev/ppl-lint-runtime-compatibility-ci-design.md @@ -0,0 +1,188 @@ +# PPL Lint Runtime Compatibility CI + +- **Status:** Proposed revision for the SQL PPL lint CI +- **Last updated:** 2026-08-04 +- **Scope:** `.github/workflows/ppl-lint-multiversion-validation.yml` + +## 1. Decision + +The multi-version workflow validates PPL lint compatibility only against +standard OpenSearch runtime grammar bundles: + +```text +OpenSearch 3.6 release ─┐ +OpenSearch 3.7 release ─┼─> Aggregate rule compatibility +SQL pull request build ─┘ +``` + +The workflow does not run: + +- the compiled-simplified grammar surface for pre-3.6 engines; +- the analytics engine or composite/Parquet storage; +- syntax-channel features; +- AI action tests. + +Analytics coverage is deferred until that engine and its fixtures provide a +stable CI contract. Pre-3.6 coverage is removed because those engines cannot +export the runtime grammar bundle consumed by the production lint path. + +The required single-version workflow remains responsible for proving that all +active shipping detectors agree with the standard SQL pull request build. The +multi-version workflow explains where each rule works and fails its final +aggregation job when a declared-supported version drifts. + +## 2. Rule Inventory + +The active inventory currently contains **12 detector rules**, not 13. +`command-suggestion` was removed from this effort and must not be silently +reintroduced as a lint rule. The final table is generated from +`manifest.json`. CI also asserts that the current inventory is exactly these 12 +rules, so adding a future reviewed rule requires an intentional guard and test +update. + +| Rule | Declared compatibility | +| --- | --- | +| `agg-on-text` | Calcite, OpenSearch >= 3.7 | +| `division-by-zero` | All runtime-bundle versions | +| `enabled-false-object` | Calcite, OpenSearch >= 3.7 | +| `field-validation` | All runtime-bundle versions | +| `invalid-capture-group-name` | OpenSearch >= 3.4 | +| `multisearch-min-subsearch` | OpenSearch >= 3.4 | +| `replace-wildcard-asymmetry` | Calcite, OpenSearch >= 3.4 | +| `rex-scan-cost` | All runtime-bundle versions | +| `type-mismatch-numeric` | Calcite, OpenSearch >= 3.7 | +| `union-min-datasets` | Calcite, OpenSearch >= 3.7 | +| `unsupported-window-function-in-eventstats` | OpenSearch >= 3.4 | +| `wildcard-source-zero-match` | All runtime-bundle versions | + +## 3. Workflow Shape + +### 3.1 Plan + +`Plan matrix` resolves: + +- released engines: `3.6.0` and `3.7.0`; +- the OSD repository and revision; +- the discovery engine, currently the newest released engine. + +There is no compiled-surface input or analytics target. + +### 3.2 Observe released engines + +One `Observe engine ` job runs per released engine. Each job: + +1. starts the official OpenSearch distribution containing its matching SQL + plugin; +2. runs the contract queries in observe-only mode; +3. exports that engine's runtime grammar bundle; +4. uploads `target.json`, `backend-report.json`, and + `ppl-grammar-bundle.json`. + +An expectation mismatch is observation data, not a job failure. + +### 3.3 Observe the pull request build + +`Observe engine pr-build` runs the same corpus against the standard Gradle test +cluster built from the pull request. It exports the same artifact shape as the +released legs. + +### 3.4 Aggregate rule compatibility + +`Aggregate rule compatibility` is the only fan-in job. It: + +1. waits for the released and pull request observation jobs; +2. downloads every `ppl-lint-leg-*` artifact; +3. bootstraps OSD once; +4. runs the production headless lint detector against each engine's runtime + grammar bundle; +5. compares declared compatibility with observed detector and backend results; +6. writes `drift-report.json`; +7. publishes the Markdown compatibility table in the GitHub step summary; +8. uploads the mandatory `ppl-lint-multiversion-drift` artifact and + supplemental `ppl-lint-multiversion-evidence` artifact; +9. fails if the aggregate result recorded supported-version drift. + +The job display name is intentionally explicit. A reader should not have to +infer that a job named "detect" is the final aggregation. + +## 4. Expected Versus Actual Compatibility + +The aggregate summary has one row per active rule: + +| Rule | Expected compatibility | 3.6 actual | 3.7 actual | PR build actual | +| --- | --- | --- | --- | --- | +| `agg-on-text` | Calcite, >= 3.7 | expected n/a | compatible | compatible | +| `division-by-zero` | all versions | compatible | compatible | compatible | + +Each actual cell uses one of these states: + +| State | Meaning | +| --- | --- | +| `compatible` | Detector output and backend behavior match the contract. | +| `expected n/a` | The engine is outside `wiring.appliesTo`, such as 3.6 for a rule with `minVersion: 3.7.0`. | +| `drift` | The engine is declared compatible but detector or backend behavior differs. | +| `inconclusive` | A fixture, query, or detector execution did not produce a trustworthy verdict. | + +`minVersion` is part of the expected result, not a workaround applied after +the fact. If a rule is intentionally unsupported on 3.6 and declares +`minVersion: 3.7.0`, the 3.6 cell is `expected n/a` and does not count as +drift. + +The JSON report retains query-level evidence and remediation details. The +Markdown table is the concise compatibility view, not a replacement for the +machine-readable report. + +## 5. Failure Semantics + +Compatibility aggregation is write-first and then enforcing: + +- observation jobs record detector and backend mismatches without failing; +- expected out-of-scope versions do not fail the workflow; +- an inconclusive rule produces an `inconclusive` cell and annotation; +- one rule cannot prevent results for the other rules; +- the fan-in writes the complete table and `drift-report.json`; +- the artifact upload runs even when the aggregate result is failing; +- only after those outputs exist does supported-version drift or an enforced + inconclusive result fail the final aggregation job. + +This ordering is required. A bare `Process completed with exit code 1` before +the table exists is not an actionable compatibility result. + +Structural failures remain errors because no truthful table can be produced: + +- a planned engine leg uploads no artifacts; +- JSON artifacts are malformed; +- target identity conflicts with report identity; +- the contract manifest is malformed; +- `drift-report.json` cannot be written. + +The artifact upload must require `drift-report.json`. An artifact named +`ppl-lint-multiversion-drift` that contains only raw target files is not a drift +report and must not be presented as one. + +## 6. Outputs + +Every run produces: + +- a GitHub step-summary table with expected and actual compatibility; +- `drift-report.json`; +- one detector report and detector log per engine leg; +- target manifests that identify the exact engine and grammar hash. + +The required PPL lint workflow remains the stable branch-protection signal. +The multi-version aggregation job is also red on declared-supported drift, with +the table and artifact serving as the evidence for adjusting `minVersion`, +narrowing a detector, or updating a backend oracle after review. + +## 7. Deferred Coverage + +Analytics-engine validation may return only after: + +- its feature build is immutable for the duration of a run; +- all required fixtures can be represented or explicitly scoped; +- route attestation is stable; +- a rule-specific analytics limitation cannot invalidate unrelated rules. + +Compiled-simplified coverage may return only if pre-3.6 support becomes a +shipping requirement. It must be a separate workflow because it tests OSD's +checked-in grammar rather than the SQL runtime grammar bundle. diff --git a/scripts/ppl-lint/README.md b/scripts/ppl-lint/README.md index 63cc7797409..660a88c09af 100644 --- a/scripts/ppl-lint/README.md +++ b/scripts/ppl-lint/README.md @@ -1,16 +1,16 @@ # PPL lint rule validation A cross-repository GitHub Actions check that proves the OpenSearch Dashboards -(OSD) PPL lint detectors and runtime syntax validation still agree with the SQL -backend on the **same candidate runtime grammar** built by a SQL pull request. +(OSD) PPL lint detectors still agree with the SQL backend on the **same +candidate runtime grammar** built by a SQL pull request. PPL language behavior lives in SQL; PPL lint detectors live in OSD. A SQL change can silently invalidate an OSD rule (a parser refactor stops a detector matching, or a semantic change makes a flagged query valid) without touching OSD. Neither repository's own unit tests catch that. This check does. -- **Design:** `ppl-lint-ci-validation-design.md` -- **Analytics rollout:** [`docs/dev/ppl-lint-analytics-engine-ci-validation.md`](../../docs/dev/ppl-lint-analytics-engine-ci-validation.md) +- **Multi-version design:** [`docs/dev/ppl-lint-runtime-compatibility-ci-design.md`](../../docs/dev/ppl-lint-runtime-compatibility-ci-design.md) +- **Deferred analytics design:** [`docs/dev/ppl-lint-analytics-engine-ci-validation.md`](../../docs/dev/ppl-lint-analytics-engine-ci-validation.md) - **Workflow:** [`.github/workflows/ppl-lint-rule-validation.yml`](../../.github/workflows/ppl-lint-rule-validation.yml) - **Contracts:** [`integ-test/src/test/resources/ppl-lint/contracts/`](../../integ-test/src/test/resources/ppl-lint/contracts) @@ -227,94 +227,36 @@ rule that is correct on `main` can be a false positive on 3.6 or a false negativ on 3.7, and the single-version check cannot see it. [`ppl-lint-multiversion-validation.yml`](../../.github/workflows/ppl-lint-multiversion-validation.yml) -validates every `defaultError` rule against several engine versions at once, and -reports **what to change in the linter** when one disagrees. +validates every active shipping detector against several engine versions at +once, and reports **what to change in the linter** when one disagrees. ``` -observe (matrix: released images + pr-build standard + pr-build analytics) +observe (matrix: released 3.6/3.7 images + standard pr-build) └── each leg exports the same 4 artifacts as the single-version check -detect (one OSD bootstrap, one detector pass per leg's grammar) +aggregate rule compatibility (one OSD bootstrap, one detector pass per runtime grammar) └── aggregate-versions.mjs → drift-report.json + remediation report ``` Released legs run the official `opensearchproject/opensearch:` image, which bundles the matching `opensearch-sql` plugin, so no old branch is built. The `pr-build` leg is the same Gradle test cluster the single-version check uses. The -`pr-build-analytics` leg installs the full Arrow, analytics, composite, Parquet, -Lucene-backend, and DataFusion-backend stack and fails unless fixture settings, -explain output, and a profiled canary attest the route. These legs -run the **same** contract oracle (`PplLintRuleValidationIT`) with +legs run the **same** contract oracle (`PplLintRuleValidationIT`) with `-Dppl.lint.observe.only=true`, which records real behavior instead of asserting against expectations — on an older engine a mismatch is the signal being collected, not a broken run. -**Engine floor: 3.6.0 — for the runtime-bundle surface.** +**Engine floor: 3.6.0.** `GET /_plugins/_ppl/_grammar` landed in #5162, which is an ancestor of 3.6 but not 3.5, so a 3.5 leg cannot export a grammar bundle for the detectors to lint against. -### The two grammar surfaces +This workflow intentionally excludes the compiled-simplified surface and +analytics engine. Those dimensions do not share the stable runtime-bundle +contract being compared here. -OSD ships lint on **two** surfaces, and a user gets whichever one their session -resolves to (`lintRuntimePPLQuery`): - -| Surface | When the product uses it | Engine floor | -| --- | --- | --- | -| `runtime-bundle` | the engine exported a grammar bundle and it has loaded | 3.6.0 | -| `compiled-simplified` | no bundle — no dataset selected, engine below 3.6, or bundle not yet loaded | none | - -The compiled surface is not a degraded copy of the runtime one: it runs detector -logic the runtime path does not (`field_validation`'s text-side pass keys off -`grammarSurface === 'compiled-simplified'`). It is also the surface with no engine -floor, so it is where old-engine coverage is possible at all. - -`PPL_LINT_SURFACE` selects which surface a detector run validates. It defaults to -`runtime-bundle`, so the required check is unchanged, and the compiled surface is -an **explicit opt-in** — never a silent fallback. A missing bundle on the runtime -surface stays a hard failure, because quietly linting OSD's own grammar instead of -the candidate would validate the wrong thing. - -**`runtimeOnly` rules do not run on the compiled surface.** `lint_runner` skips -them (the productions they walk are absent from the compiled grammar), so a -compiled leg reports them `not-applicable` rather than as zero diagnostics. This -distinction is load-bearing: counted as zero, a healthy rule would classify as -`detector-silent` drift and send someone to "fix" it. In the summary table those -cells read `n/a (surface)`, and a rule whose every case is inert is `n/a` — not -`agree` (it proved nothing) and not `inconclusive` (nothing went wrong, and there -is nothing to re-run). - -Two legs may share an engine version while validating different surfaces, so the -matrix is keyed on the **leg label**, grammar surface, and execution backend, not -the version alone. - -Each contract declares the surface(s) it was verified against, and a contract is -only scored on a matching leg — `"both"` opts into either. Judged on a surface it -never claimed, every verdict is meaningless: a runtime-bundle contract on a -compiled leg yields both `version-scope-too-narrow` ("the engine rejects but the -rule is scoped away") and a coverage hole, each about a surface the contract does -not describe. Contracts declaring `"both"` are what a pre-3.6 leg can actually -validate; the rest report `n/a (surface)`. - -**Compiled-surface legs run nightly** (`COMPILED_ENGINE_VERSIONS`, default -`2.19.0` / `3.0.0` / `3.5.0`) — three more engine images is too slow for every PR. -Dispatch with `compiled_versions` to run one ad hoc, or `[]` to skip. Their observe -job omits `-Dppl.lint.grammar.bundle` (the IT then exports nothing) and writes a -`surface` marker file, which is what tells the detect job to lint them on the -compiled surface rather than treating a missing bundle as a failed export. - -```bash -# A compiled-surface leg: no grammar bundle needed, so any engine version works. -PPL_LINT_SURFACE=compiled-simplified \ -PPL_LINT_CONTRACT_DIR= \ -PPL_LINT_TARGET_MANIFEST=/target.json \ -PPL_LINT_SCHEDULE=nightly \ -PPL_LINT_REPORT=/detector-report.json \ -node -r ./src/setup_node_env /scripts/ppl-lint/run-frontend-contract.mjs -``` - -This workflow is **non-enforcing for now**: it reports and uploads, while the -required check stays the single-version `validation-result`. Promoting it needs a -green baseline across the whole matrix first, so a rule that has already drifted -on 3.6 does not block every unrelated PR on day one. +Observation jobs do not fail on compatibility differences. The final +`Aggregate rule compatibility` job writes the complete expected-versus-actual +table and `drift-report.json`, uploads them, and then fails when a rule drifts on +a version declared by its `wiring.appliesTo` scope. ### What a drift report tells you @@ -422,9 +364,9 @@ remediation. Severity is not cosmetic: | Finding | Level | Why | | --- | --- | --- | -| enforced drift, coverage hole | `error` | a shipped default-error rule disagrees with a supported engine | +| enforced drift, coverage hole | `error` | an active shipping rule disagrees with a supported engine; aggregation writes the report and then fails | | non-enforced drift | `warning` | reported, but it does not block | -| `inconclusive` | `warning` | "we could not check" is a leg problem; the run is already red from the exit code, and rendering it as an error invites editing a rule because a leg timed out | +| `inconclusive` | `warning` | "we could not check" is a leg problem, not evidence that a rule is wrong | | unvalidated default-error rule | `error` (no file) | the edit goes in `manifest.json`, not a contract | A line number is emitted only when it is unambiguous. If a contract pins the same @@ -447,22 +389,20 @@ mkdir -p legs/3.7.0 -Dppl.lint.grammar.bundle=$PWD/legs/3.7.0/ppl-grammar-bundle.json \ -Dppl.lint.target=$PWD/legs/3.7.0/target.json -# Observe the PR build through composite/Parquet + DataFusion. -mkdir -p legs/pr-build-analytics -./gradlew :integ-test:analyticsEnginePplLintIT \ - -Dppl.lint.schedule=nightly -Dppl.lint.observe.only=true \ - -Dppl.lint.report=$PWD/legs/pr-build-analytics/backend-report.json \ - -Dppl.lint.grammar.bundle=$PWD/legs/pr-build-analytics/ppl-grammar-bundle.json \ - -Dppl.lint.target=$PWD/legs/pr-build-analytics/target.json - # Lint each leg's grammar from an OSD checkout (writes detector-report.json), -# then compare every version at once: +# then compare every standard runtime-bundle version at once. The aggregator +# writes the table and JSON report before returning a failing drift status. node scripts/ppl-lint/aggregate-versions.mjs \ --contracts integ-test/src/test/resources/ppl-lint/contracts \ - --leg 3.6.0=legs/3.6.0 --leg 3.7.0=legs/3.7.0 \ + --leg 3.6.0=legs/3.6.0 --leg 3.7.0=legs/3.7.0 --leg pr-build=legs/pr-build \ --out drift-report.json ``` +The step summary has one row per active detector. It prints the compatibility +declared by `wiring.appliesTo` next to the actual result for every engine leg. +For example, a rule with `minVersion: 3.7.0` renders `expected n/a` on 3.6 +instead of reporting drift. + The classifier is pure and has no cluster or OSD dependency, so its tests run anywhere: diff --git a/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs b/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs index df46ab6acd3..669b2d6bb54 100644 --- a/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs +++ b/scripts/ppl-lint/__tests__/aggregate-versions.test.mjs @@ -25,6 +25,18 @@ import { fileURLToPath } from 'node:url'; const HERE = path.dirname(fileURLToPath(import.meta.url)); const SCRIPT = path.join(HERE, '..', 'aggregate-versions.mjs'); +const REAL_CONTRACTS = path.resolve( + HERE, + '..', + '..', + '..', + 'integ-test', + 'src', + 'test', + 'resources', + 'ppl-lint', + 'contracts' +); /** Contract used by every case: a >=3.7 calcite-only rule with one trigger + one control. */ const SPEC = { @@ -225,11 +237,12 @@ function writeLeg({ return dir; } -/** Run the aggregator; returns { status, stdout, report }. */ +/** Run the aggregator; returns the process result plus its JSON and Markdown reports. */ function run({ contracts, legs, extraArgs = [] }) { const outDir = makeTmp('ppl-lint-out-'); const out = path.join(outDir, 'drift-report.json'); - const args = [SCRIPT, '--contracts', contracts, '--out', out]; + const summaryFile = path.join(outDir, 'summary.md'); + const args = [SCRIPT, '--contracts', contracts, '--out', out, '--summary', summaryFile]; const entries = Array.isArray(legs) ? legs : Object.entries(legs); for (const [version, dir] of entries) { args.push('--leg', `${version}=${dir}`); @@ -237,7 +250,14 @@ function run({ contracts, legs, extraArgs = [] }) { args.push(...extraArgs); const result = spawnSync(process.execPath, args, { encoding: 'utf8' }); const report = fs.existsSync(out) ? JSON.parse(fs.readFileSync(out, 'utf8')) : undefined; - return { status: result.status, stdout: result.stdout || '', stderr: result.stderr || '', report }; + const summary = fs.existsSync(summaryFile) ? fs.readFileSync(summaryFile, 'utf8') : ''; + return { + status: result.status, + stdout: result.stdout || '', + stderr: result.stderr || '', + report, + summary, + }; } /** The all-agree case, reused as the base for each drift scenario. */ @@ -291,17 +311,49 @@ function writeSchema4Contracts({ includeAnalytics = true } = {}) { }); } -test('all versions agreeing exits 0 and reports no drift', () => { - const { status, report, stdout } = run({ contracts: writeContracts(), legs: healthyLegs() }); +test('all versions agreeing exits 0 and reports expected versus actual compatibility', () => { + const { status, report, stdout, summary } = run({ + contracts: writeContracts(), + legs: healthyLegs(), + }); assert.equal(status, 0); assert.equal(report.result.passed, true); assert.equal(report.drifts.length, 0); assert.match(stdout, /agrees with all 2 engine version\(s\)/); + assert.match(summary, /\| Rule \| Expected compatibility \| `3\.7\.0` actual \| `3\.8\.0` actual \|/); + assert.match( + summary, + /\| `union-min-datasets` \| Calcite, >= 3\.7\.0 \| compatible \| compatible \|/ + ); // Every rule/version pair is accounted for in the matrix. assert.equal(report.matrix.length, 2); assert.ok(report.matrix.every((m) => m.status === 'agree')); }); +test('the compatibility table contains exactly the 12 active shipping detectors', () => { + const leg = writeLeg({ + version: '3.8.0', + cases: { + trigger: { detector: 1, rejected: true }, + control: { detector: 0, rejected: false }, + }, + }); + + const { status, summary } = run({ + contracts: REAL_CONTRACTS, + legs: [['pr-build', leg]], + extraArgs: ['--all-rules'], + }); + + assert.equal(status, 1, 'missing synthetic observations remain inconclusive'); + const ruleRows = summary + .split('\n') + .filter((line) => /^\| `[a-z0-9-]+` \|/.test(line)); + assert.equal(ruleRows.length, 12); + assert.match(summary, /\| `rex-scan-cost` \| all versions \|/); + assert.doesNotMatch(summary, /command-suggestion/); +}); + test('same-version standard and analytics verdicts are classified as backend divergence', () => { const grammarHash = 'sha256:shared-runtime-grammar'; const standard = writeLeg({ @@ -360,8 +412,10 @@ test('same-version standard and analytics verdicts are classified as backend div 0, 'route differences must not be rendered as product-version drift' ); - assert.match(stdout, /`3\.8\.0`
standard/); - assert.match(stdout, /`3\.8\.0`
analytics/); + const tableHeader = stdout.split('\n').find((line) => line.startsWith('| Rule |')); + assert.match(tableHeader, /Expected compatibility/); + assert.match(tableHeader, /`pr-build` actual/); + assert.doesNotMatch(tableHeader, /analytics/); }); test('schema-v3 analytics has explicit backend-oracle coverage holes', () => { @@ -878,6 +932,30 @@ test('a version where only one engine relaxed is red, and names just that versio assert.equal(report.matrix.find((m) => m.version === '3.7.0').status, 'agree'); }); +test('drift exits nonzero only after writing the JSON report and full Markdown table', () => { + const legs = healthyLegs(); + legs['3.8.0'] = writeLeg({ + version: '3.8.0', + cases: { + trigger: { detector: 0, rejected: true }, + control: { detector: 0, rejected: false }, + }, + }); + + const { status, report, summary } = run({ contracts: writeContracts(), legs }); + + assert.equal(status, 1); + assert.equal(report.result.passed, false); + assert.equal(report.result.enforcedDriftCount, 1); + assert.match(summary, /## PPL lint multi-version validation/); + assert.match(summary, /\| Rule \| Expected compatibility \|/); + assert.match( + summary, + /\| `union-min-datasets` \| Calcite, >= 3\.7\.0 \| compatible \| \*\*drift\*\* \|/ + ); + assert.match(summary, /### Remediation/); +}); + test('a changed rejection HTTP status is semantic drift, not agreement', () => { const leg = writeLeg({ version: '3.8.0', @@ -1060,23 +1138,29 @@ test('a rule out of scope on an older engine that accepts is not drift', () => { version: '3.6.0', cases: { trigger: { detector: 0, rejected: false }, control: { detector: 0, rejected: false } }, }); - const { status, report } = run({ contracts: writeContracts(), legs }); + const { status, report, summary } = run({ contracts: writeContracts(), legs }); assert.equal(status, 0); assert.equal(report.matrix.find((m) => m.version === '3.6.0').status, 'out-of-scope'); assert.equal(report.coverageHoles.length, 0); + const row = summary + .split('\n') + .find((line) => line.startsWith('| `union-min-datasets` |')); + assert.match(row, /Calcite, >= 3\.7\.0/); + assert.match(row, /expected n\/a/); + assert.equal((row.match(/compatible/g) || []).length, 2); }); -test('an out-of-scope engine that rejects is flagged as scoped too narrowly', () => { +test('an engine below minVersion is expected n/a even when its query rejects', () => { const legs = healthyLegs(); legs['3.6.0'] = writeLeg({ version: '3.6.0', cases: { trigger: { detector: 0, rejected: true }, control: { detector: 0, rejected: false } }, }); - const { status, report } = run({ contracts: writeContracts(), legs }); - assert.equal(status, 1); - const drift = report.drifts.find((d) => d.version === '3.6.0'); - assert.equal(drift.driftClass, 'version-scope-too-narrow'); - assert.equal(drift.remediation.action, 'version-scope-rule'); + const { status, report, summary } = run({ contracts: writeContracts(), legs }); + assert.equal(status, 0); + assert.equal(report.drifts.filter((drift) => drift.version === '3.6.0').length, 0); + assert.equal(report.matrix.find((row) => row.version === '3.6.0').status, 'out-of-scope'); + assert.match(summary, /expected n\/a/); }); test('an in-scope version with no expectation is a coverage hole, not silent success', () => { @@ -1406,18 +1490,19 @@ test('a reworded engine message does not mask a detector that went silent', () = assert.equal(drift.remediation.action, 'update-detector'); }); -test('a detector firing on a version its appliesTo excludes is reported', () => { - // OSD's version filter runs a rule when the cluster version is unknown, so an - // out-of-scope rule CAN reach users. Silence here would hide that false positive. +test('a detector observation below minVersion remains expected n/a', () => { const dir = writeLeg({ version: '3.6.0', // below the rule's 3.7 minVersion cases: { trigger: { detector: 1, rejected: false }, control: { detector: 0, rejected: false } }, }); - const { status, report } = run({ contracts: writeContracts(), legs: { '3.6.0': dir } }); - assert.equal(status, 1); - const drift = report.drifts.find((d) => d.version === '3.6.0'); - assert.equal(drift.driftClass, 'detector-noisy'); - assert.equal(drift.remediation.action, 'update-detector'); + const { status, report, summary } = run({ + contracts: writeContracts(), + legs: { '3.6.0': dir }, + }); + assert.equal(status, 0); + assert.equal(report.drifts.length, 0); + assert.equal(report.matrix[0].status, 'out-of-scope'); + assert.match(summary, /expected n\/a/); }); test('a calcite-scoped expectation is selected rather than counted twice', () => { @@ -1438,11 +1523,7 @@ test('a calcite-scoped expectation is selected rather than counted twice', () => ); }); -test('an errored trigger on an out-of-scope rule does not silently pass', () => { - // The out-of-scope path used to read `entry.rejected` directly. An errored - // observation has no such field, so it coerced to false, the - // version-scope-too-narrow check (which needs `=== true`) never fired, and a - // genuinely mis-scoped rule rendered as `out-of-scope` with exit 0. +test('an errored trigger below minVersion remains expected n/a', () => { const dir = writeLeg({ version: '3.6.0', // below the rule's 3.7 minVersion => out of scope cases: { trigger: { detector: 0, rejected: true }, control: { detector: 0, rejected: false } }, @@ -1473,21 +1554,13 @@ test('an errored trigger on an out-of-scope rule does not silently pass', () => contracts: writeContracts(), legs: { '3.6.0': dir }, }); - // The point is that an unobserved trigger yields no CLAIM either way: it must - // not be reported as a confident out-of-scope agreement... - assert.equal( - report.drifts.filter((d) => d.driftClass === 'version-scope-too-narrow').length, - 0, - 'an unobserved trigger cannot support a version-scope finding' - ); - // ...nor may it invent linter advice from a verdict that never arrived. + assert.equal(status, 0); assert.equal(report.drifts.length, 0); - assert.equal(status, 1); - assert.equal(report.matrix[0].status, 'inconclusive'); - assert.equal(report.result.enforcedInconclusive, 1); + assert.equal(report.matrix[0].status, 'out-of-scope'); + assert.equal(report.result.enforcedInconclusive, 0); }); -test('a missing detector and backend row is inconclusive even when the rule is out of scope', () => { +test('missing rows below minVersion remain expected n/a', () => { const dir = writeLeg({ version: '3.6.0', cases: { @@ -1510,16 +1583,12 @@ test('a missing detector and backend row is inconclusive even when the rule is o legs: { '3.6.0': dir }, }); - assert.equal(status, 1); - assert.equal(report.matrix[0].status, 'inconclusive'); - assert.match(report.inconclusive[0].reasons.join(' '), /trigger \(no detector result\)/); + assert.equal(status, 0); + assert.equal(report.matrix[0].status, 'out-of-scope'); + assert.equal(report.inconclusive.length, 0); }); -test('an errored control cannot fail open into "widen appliesTo" advice', () => { - // controlAlsoRejected suppresses the version-scope finding when the command - // itself is unsupported. Reading `entry.rejected` raw made that suppression fail - // OPEN on an errored control: the run would then advise lowering minVersion, - // shipping a precise-cause diagnostic for an unknown-command failure. +test('an errored control below minVersion remains expected n/a', () => { const dir = writeLeg({ version: '3.6.0', cases: { trigger: { detector: 0, rejected: true }, control: { detector: 0, rejected: true } }, @@ -1538,19 +1607,15 @@ test('an errored control cannot fail open into "widen appliesTo" advice', () => : { ...e, outcome: 'observed' } ); fs.writeFileSync(path.join(dir, 'backend-report.json'), JSON.stringify(backend)); - const { status, report, stdout } = run({ + const { status, report, stdout, summary } = run({ contracts: writeContracts(), legs: { '3.6.0': dir }, }); - const scoped = report.drifts.filter((d) => d.driftClass === 'version-scope-too-narrow'); - assert.equal( - scoped.length, - 0, - 'with the control unobserved there is no evidence the command is supported, so no widening advice' - ); + assert.equal(report.drifts.length, 0); assert.ok(!/Widen "/.test(stdout)); - assert.equal(status, 1); - assert.equal(report.matrix[0].status, 'inconclusive'); + assert.equal(status, 0); + assert.equal(report.matrix[0].status, 'out-of-scope'); + assert.match(summary, /expected n\/a/); }); test('a bad --leg argument is rejected', () => { diff --git a/scripts/ppl-lint/__tests__/validate-pr-build-targets.test.mjs b/scripts/ppl-lint/__tests__/validate-pr-build-targets.test.mjs deleted file mode 100644 index 430daf81797..00000000000 --- a/scripts/ppl-lint/__tests__/validate-pr-build-targets.test.mjs +++ /dev/null @@ -1,271 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import assert from 'node:assert/strict'; -import { spawnSync } from 'node:child_process'; -import fs from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; -import { after, test } from 'node:test'; -import { fileURLToPath } from 'node:url'; - -import { - validatePrBuildArtifacts, - validatePrBuildTargetPair, -} from '../validate-pr-build-targets.mjs'; - -const HERE = path.dirname(fileURLToPath(import.meta.url)); -const SCRIPT = path.join(HERE, '..', 'validate-pr-build-targets.mjs'); -const tmpDirs = []; - -function standardTarget(overrides = {}) { - return { - schemaVersion: 2, - sqlSha: 'candidate-sql-sha', - engineVersion: '3.8.0-SNAPSHOT', - grammarHash: 'sha256:candidate-grammar', - grammarBundle: 'ppl-grammar-bundle.json', - executionBackend: 'standard', - storage: 'lucene', - shardCount: 1, - ...overrides, - }; -} - -function analyticsTarget(overrides = {}) { - return { - schemaVersion: 2, - sqlSha: 'candidate-sql-sha', - engineVersion: '3.8.0-SNAPSHOT', - grammarHash: 'sha256:candidate-grammar', - grammarBundle: 'ppl-grammar-bundle.json', - executionBackend: 'analytics', - storage: 'composite-parquet', - shardCount: 1, - analyticsStack: { source: 'https://example.test/analytics-build' }, - routeAttestation: { - pluginsVerified: true, - clusterSettingsVerified: true, - fixtureIndicesVerified: true, - explainVerified: true, - profiledExecutionVerified: true, - }, - ...overrides, - }; -} - -function backendReport(executionBackend, queryNames = ['trigger', 'control']) { - return queryNames.map((queryName) => ({ - ruleId: 'test-rule', - queryName, - role: queryName === 'control' ? 'control' : 'trigger', - query: - queryName === 'control' - ? 'source=test-index | head 1' - : 'source=test-index | head 0', - executionBackend, - rejected: queryName !== 'control', - observed: { - httpStatus: queryName === 'control' ? 200 : 400, - rejected: queryName !== 'control', - }, - })); -} - -function writeContractCorpus() { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ppl-lint-target-contracts-')); - tmpDirs.push(dir); - fs.writeFileSync( - path.join(dir, 'manifest.json'), - JSON.stringify({ schemaVersion: 3, contracts: ['test-rule.spec.json'] }) - ); - fs.writeFileSync( - path.join(dir, 'test-rule.spec.json'), - JSON.stringify({ - schemaVersion: 3, - ruleId: 'test-rule', - schedule: 'pr', - queries: { - trigger: { role: 'trigger', query: 'source={{index}} | head 0' }, - control: { role: 'control', query: 'source={{index}} | head 1' }, - }, - }) - ); - return dir; -} - -after(() => { - for (const dir of tmpDirs) { - fs.rmSync(dir, { recursive: true, force: true }); - } -}); - -test('matching standard and analytics PR-build targets pass', () => { - const pair = validatePrBuildTargetPair(standardTarget(), analyticsTarget()); - assert.equal(pair.standard.executionBackend, 'standard'); - assert.equal(pair.analytics.executionBackend, 'analytics'); -}); - -test('the two PR-build target roles require their exact backend identities', () => { - assert.throws( - () => validatePrBuildTargetPair(analyticsTarget(), analyticsTarget()), - /standard PR-build target executionBackend must be "standard"/ - ); - assert.throws( - () => validatePrBuildTargetPair(standardTarget(), standardTarget()), - /analytics PR-build target executionBackend must be "analytics"/ - ); -}); - -test('both PR-build targets require the same non-empty SQL SHA', () => { - assert.throws( - () => validatePrBuildTargetPair(standardTarget({ sqlSha: '' }), analyticsTarget()), - /both report a non-empty SQL SHA/ - ); - assert.throws( - () => - validatePrBuildTargetPair( - standardTarget(), - analyticsTarget({ sqlSha: 'different-sql-sha' }) - ), - /report different values for SQL SHA/ - ); -}); - -test('both PR-build targets require the same engine version', () => { - assert.throws( - () => - validatePrBuildTargetPair( - standardTarget(), - analyticsTarget({ engineVersion: '3.9.0-SNAPSHOT' }) - ), - /report different values for engine version/ - ); -}); - -test('both PR-build targets require the same non-empty grammar hash', () => { - assert.throws( - () => validatePrBuildTargetPair(standardTarget(), analyticsTarget({ grammarHash: ' ' })), - /both report a non-empty grammar hash/ - ); - assert.throws( - () => - validatePrBuildTargetPair( - standardTarget(), - analyticsTarget({ grammarHash: 'sha256:different-grammar' }) - ), - /report different values for grammar hash/ - ); -}); - -test('target schema validation runs before pair identity comparison', () => { - assert.throws( - () => - validatePrBuildTargetPair( - standardTarget({ schemaVersion: 1 }), - analyticsTarget() - ), - /standard PR-build target is invalid: Unsupported target schemaVersion/ - ); -}); - -test('paired backend reports require exact, usable query coverage', () => { - const base = { - standardTarget: standardTarget(), - analyticsTarget: analyticsTarget(), - standardReport: backendReport('standard'), - analyticsReport: backendReport('analytics'), - contractsDir: writeContractCorpus(), - }; - const result = validatePrBuildArtifacts(base); - assert.equal(result.expectedRows, 2); - - assert.throws( - () => - validatePrBuildArtifacts({ - ...base, - analyticsReport: backendReport('analytics', ['trigger']), - }), - /analytics PR-build backend report query coverage is incomplete.*test-rule::control/ - ); - assert.throws( - () => - validatePrBuildArtifacts({ - ...base, - standardReport: [ - ...backendReport('standard'), - { ...backendReport('standard')[0] }, - ], - }), - /duplicate backend report key/ - ); - - const errored = backendReport('analytics'); - errored[0] = { ...errored[0], outcome: 'error' }; - assert.throws( - () => validatePrBuildArtifacts({ ...base, analyticsReport: errored }), - /contains rows without an engine verdict: test-rule::trigger/ - ); - - const unobservedCoverageGap = backendReport('analytics'); - unobservedCoverageGap[0] = { - ...unobservedCoverageGap[0], - outcome: 'coverage-missing', - }; - delete unobservedCoverageGap[0].rejected; - assert.throws( - () => - validatePrBuildArtifacts({ - ...base, - analyticsReport: unobservedCoverageGap, - }), - /contains rows without an engine verdict: test-rule::trigger/ - ); - - const changedQuery = backendReport('analytics'); - changedQuery[0] = { ...changedQuery[0], query: 'source=different-index | head 0' }; - assert.throws( - () => validatePrBuildArtifacts({ ...base, analyticsReport: changedQuery }), - /executed different query text for test-rule::trigger/ - ); -}); - -test('the CLI reads and validates both PR-build artifact sets', () => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ppl-lint-target-pair-')); - tmpDirs.push(dir); - const standardFile = path.join(dir, 'standard.json'); - const analyticsFile = path.join(dir, 'analytics.json'); - const standardReportFile = path.join(dir, 'standard-report.json'); - const analyticsReportFile = path.join(dir, 'analytics-report.json'); - fs.writeFileSync(standardFile, JSON.stringify(standardTarget())); - fs.writeFileSync(analyticsFile, JSON.stringify(analyticsTarget())); - fs.writeFileSync(standardReportFile, JSON.stringify(backendReport('standard'))); - fs.writeFileSync(analyticsReportFile, JSON.stringify(backendReport('analytics'))); - - const result = spawnSync( - process.execPath, - [ - SCRIPT, - '--standard', - standardFile, - '--analytics', - analyticsFile, - '--standard-report', - standardReportFile, - '--analytics-report', - analyticsReportFile, - '--contracts', - writeContractCorpus(), - '--schedule', - 'nightly', - ], - { encoding: 'utf8' } - ); - - assert.equal(result.status, 0, result.stderr); - assert.match(result.stdout, /verified engine=3\.8\.0-SNAPSHOT/); - assert.match(result.stdout, /backends=standard,analytics/); - assert.match(result.stdout, /backendRows=2/); -}); diff --git a/scripts/ppl-lint/aggregate-versions.mjs b/scripts/ppl-lint/aggregate-versions.mjs index 7ceb86ae6df..6f46f088a5e 100644 --- a/scripts/ppl-lint/aggregate-versions.mjs +++ b/scripts/ppl-lint/aggregate-versions.mjs @@ -959,75 +959,6 @@ function failedExtendedFrontendAssertions(entry, frontendOracle) { return [...failures].sort(); } -/** - * Check an out-of-scope rule for the one drift that still matters there: the - * engine rejects a trigger query, but the rule's `appliesTo` excludes this - * version, so users on it see no diagnostic for a real error. Everything else - * about an out-of-scope rule is intentional silence. - * - * The trigger queries come from the spec's own `queries` map (there is no - * expectation to read on this path), and the backend observation from this leg's - * report; `classifyDrift` decides, so the "too narrow" wording stays in one place. - */ -function classifyOutOfScope({ spec, ruleId, leg, classify, divergentCases }) { - const found = []; - const unusable = []; - const observations = new Map(); - - for (const [queryName] of Object.entries(spec.queries || {})) { - const rowKey = `${ruleId}::${queryName}`; - const backendEntry = leg.backend.get(rowKey); - const detectorResult = leg.detector.resultsByKey.get(rowKey); - const { observed, usable } = readBackendObservation(backendEntry, detectorResult); - if (!usable) { - unusable.push(`${queryName} (${unusableObservationReason(detectorResult)})`); - continue; - } - observations.set(queryName, observed); - } - - // What did this rule's CONTROL queries — valid uses of the same command — do on - // this engine? THREE states, not two, and the difference decides whether a - // rejected trigger means anything. - const controlVerdicts = Object.entries(spec.queries || {}) - .filter(([, def]) => (def.role || 'trigger') === 'control') - .map(([name]) => observations.get(name)?.backendRejected); - const controlAlsoRejected = controlVerdicts.some((v) => v === true); - // A rule with controls, none of which produced a verdict, cannot be judged here. - const controlUnknown = - controlVerdicts.length > 0 && !controlVerdicts.some((v) => typeof v === 'boolean'); - - for (const [queryName, queryDef] of Object.entries(spec.queries || {})) { - if ((queryDef.role || 'trigger') !== 'trigger') continue; - const rowKey = `${ruleId}::${queryName}`; - const outOfScopeObserved = observations.get(queryName); - if (!outOfScopeObserved) continue; - const pairedDivergence = divergentCases.has(`${leg.key}::${rowKey}`); - if (pairedDivergence && leg.executionBackend === 'analytics') continue; - const drift = classify({ - ruleId, - version: leg.version, - queryName, - role: 'trigger', - query: queryDef.query.split('{{index}}').join(spec.index), - // Out of scope means the rule is expected to stay silent here. - expected: { detectorCount: 0 }, - observed: outOfScopeObserved, - wiring: spec.wiring, - detectorPath: spec.detectorPath, - executionBackend: leg.executionBackend, - // An unknown control verdict is treated the same as a rejected one: both - // mean "we cannot claim this engine supports the command", and staying quiet - // is the only honest option. - controlAlsoRejected: controlAlsoRejected || controlUnknown, - // Deliberately no parser-rule check here: a grammar that lacks the rule is - // expected on an engine the command predates. - }); - if (drift) found.push(drift); - } - return { drifts: found, unusable }; -} - /** * Pick the contract expectation that applies to a version, reusing the same * "exactly one must match" rule as the two single-version halves. Returns @@ -1186,6 +1117,22 @@ function main() { } const inScope = versionInAppliesTo(appliesTo, leg.version); + if (!inScope) { + notApplicable.push({ + ruleId, + ...legFields(leg), + surface: legSurface, + kind: 'applies-to', + reason: `wiring.appliesTo excludes engine ${leg.version}`, + }); + matrix.push({ + ruleId, + ...legFields(leg), + status: 'out-of-scope', + drifts: 0, + }); + continue; + } // A parser rule that vanished from the grammar is one fact about this // rule on this engine, not one per query — raise it once and move on, so @@ -1209,42 +1156,6 @@ function main() { const expectation = selectExpectation(spec, leg.version, versionMatchesRange); if (!expectation) { - if (!inScope) { - // Deliberately out of scope on this engine. Still run the classifier - // for the one case that matters — an engine that rejects a trigger the - // rule has been scoped away from (a missed diagnostic). - const outOfScope = classifyOutOfScope({ - spec, - ruleId, - leg, - classify: classifyDrift, - divergentCases, - }); - for (const drift of outOfScope.drifts) { - addDrift(drift, leg, { enforced: isEnforced, contractFile: file }); - } - if (outOfScope.unusable.length > 0) { - inconclusive.push({ - ruleId, - file, - ...legFields(leg), - enforced: isEnforced, - reasons: outOfScope.unusable, - }); - } - matrix.push({ - ruleId, - ...legFields(leg), - status: - outOfScope.unusable.length > 0 - ? 'inconclusive' - : outOfScope.drifts.length > 0 - ? 'drift' - : 'out-of-scope', - drifts: outOfScope.drifts.length, - }); - continue; - } // In scope on this engine but nothing pins its behavior there. coverageHoles.push({ ruleId, @@ -1724,10 +1635,8 @@ function main() { } const enforcedDrifts = drifts.filter((d) => d.blocking); const enforcedHoles = coverageHoles.filter((h) => h.blocking); - // `--all-rules` widens observation to the whole corpus. Semantic drift remains - // enforced only for default-error rules, but a missing detector row or backend - // verdict is an infrastructure failure for every rule we asked the run to - // observe. + // `--all-rules` makes drift, missing coverage, and inconclusive observations + // blocking for every active shipping rule in the manifest. const enforcedInconclusive = inconclusive.filter((i) => i.enforced || args.allRules); const blockingMissingContracts = missingContracts.filter((entry) => entry.blocking); for (const row of matrix) { @@ -1807,7 +1716,7 @@ function main() { workspace: process.env.GITHUB_WORKSPACE, }); - const markdown = renderMarkdown(report, drifts, coverageHoles, legs); + const markdown = renderMarkdown(report, drifts, coverageHoles, legs, specs); // eslint-disable-next-line no-console console.log(markdown); if (args.summary) { @@ -1836,8 +1745,42 @@ function main() { ); } -/** Rule × version agreement matrix followed by the grouped remediation report. */ -function renderMarkdown(report, drifts, coverageHoles, legs) { +function expectedCompatibility(spec) { + const appliesTo = (spec.wiring && spec.wiring.appliesTo) || {}; + const scope = []; + if (appliesTo.engine) { + scope.push( + appliesTo.engine === 'calcite' + ? 'Calcite' + : appliesTo.engine.charAt(0).toUpperCase() + appliesTo.engine.slice(1) + ); + } + if (appliesTo.minVersion && appliesTo.maxVersion) { + scope.push(`>= ${appliesTo.minVersion}, <= ${appliesTo.maxVersion}`); + } else if (appliesTo.minVersion) { + scope.push(`>= ${appliesTo.minVersion}`); + } else if (appliesTo.maxVersion) { + scope.push(`<= ${appliesTo.maxVersion}`); + } else { + scope.push('all versions'); + } + return scope.join(', '); +} + +function actualCompatibility(row) { + if (!row) return 'not evaluated'; + if (row.status === 'agree') return 'compatible'; + if (row.status === 'out-of-scope') return 'expected n/a'; + if (row.status === 'drift') return row.drifts > 1 ? `**drift** (${row.drifts})` : '**drift**'; + if (row.status === 'uncovered' || row.status === 'inconclusive') { + return '**inconclusive**'; + } + if (row.status === 'not-applicable') return 'expected n/a'; + return `**${row.status}**`; +} + +/** Expected-vs-actual compatibility table followed by the detailed remediation report. */ +function renderMarkdown(report, drifts, coverageHoles, legs, specs) { const lines = []; lines.push('## PPL lint multi-version validation'); lines.push(''); @@ -1863,56 +1806,46 @@ function renderMarkdown(report, drifts, coverageHoles, legs) { `${report.result.blockingShippingCensusProblems} shipping census problem(s)` ); } + const compatibilityLegs = legs.filter( + (leg) => + leg.executionBackend === 'standard' && + (leg.surface || 'runtime-bundle') === 'runtime-bundle' + ); lines.push( - // Name the surface when a leg is not the default runtime-bundle one, so a - // reader knows a column speaks for OSD's compiled grammar rather than the - // engine's exported one — the two do not run the same set of rules. - `Engine versions: ${legs - .map((l) => { - const identity = - l.label === l.version ? `\`${l.version}\`` : `\`${l.label}\` → \`${l.version}\``; - return l.surface && l.surface !== 'runtime-bundle' - ? `${identity} (${l.executionBackend}, ${l.surface})` - : `${identity} (${l.executionBackend})`; - }) - .join(', ')} — ` + `**${report.result.passed ? 'PASS' : 'FAIL'}** (${reasons.join(', ')})` + `Standard runtime-bundle engines: ${ + compatibilityLegs + .map((leg) => + leg.label === leg.version + ? `\`${leg.version}\`` + : `\`${leg.label}\` → \`${leg.version}\`` + ) + .join(', ') || 'none' + } — **${report.result.passed ? 'PASS' : 'FAIL'}** (${reasons.join(', ')})` ); lines.push(''); - // Columns use the full leg key, including execution backend and grammar surface. - // A label or engine version alone is not unique once the same candidate runs - // through both standard and analytics. - const columns = legs.map((l) => ({ - key: l.key, + const columns = compatibilityLegs.map((leg) => ({ + key: leg.key, heading: - l.surface && l.surface !== 'runtime-bundle' - ? `\`${l.version}\`
${l.executionBackend}
${l.surface}` + - (l.label === l.version ? '' : `
${l.label}`) - : `\`${l.version}\`
${l.executionBackend}` + - (l.label === l.version ? '' : `
${l.label}`), + leg.label === leg.version + ? `\`${leg.version}\` actual` + : `\`${leg.label}\` actual
(\`${leg.version}\`)`, })); - const rules = [...new Set(report.matrix.map((m) => m.ruleId))].sort(); - lines.push(`| Rule | ${columns.map((c) => c.heading).join(' | ')} |`); - lines.push(`| ---- | ${columns.map(() => '----').join(' | ')} |`); - const cell = { - agree: 'agree', - drift: 'DRIFT', - uncovered: 'not covered', - 'out-of-scope': 'n/a (out of scope)', - 'not-applicable': 'n/a (surface)', - inconclusive: '**inconclusive**', - }; - for (const ruleId of rules) { + const rules = [...specs.entries()] + .filter(([, entry]) => contractChannel(entry.spec) === 'lint') + .sort(([left], [right]) => left.localeCompare(right)); + lines.push( + `| Rule | Expected compatibility | ${columns.map((column) => column.heading).join(' | ')} |` + ); + lines.push(`| ---- | ---- | ${columns.map(() => '----').join(' | ')} |`); + for (const [ruleId, { spec }] of rules) { const cells = columns.map((column) => { const row = report.matrix.find((m) => m.ruleId === ruleId && m.legKey === column.key); - if (!row) return '—'; - if (row.status === 'drift') return `**DRIFT** (${row.drifts})`; - // An unmapped status must still render as something visible. A blank cell - // reads as "nothing to see here", which is the opposite of what an - // unrecognized state means. - return cell[row.status] || `**${row.status}**`; + return actualCompatibility(row); }); - lines.push(`| \`${ruleId}\` | ${cells.join(' | ')} |`); + lines.push( + `| \`${ruleId}\` | ${expectedCompatibility(spec)} | ${cells.join(' | ')} |` + ); } lines.push(''); diff --git a/scripts/ppl-lint/validate-pr-build-targets.mjs b/scripts/ppl-lint/validate-pr-build-targets.mjs deleted file mode 100644 index b7944ed76d7..00000000000 --- a/scripts/ppl-lint/validate-pr-build-targets.mjs +++ /dev/null @@ -1,285 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import fs from 'node:fs'; -import path from 'node:path'; - -import { - assertContractSchema, - classifyBackendReportRow, - indexBackendReport, - normalizeTarget, -} from './contract-schema.mjs'; - -function normalizeLabeledTarget(target, label) { - try { - return normalizeTarget(target); - } catch (error) { - throw new Error(`${label} target is invalid: ${error.message}`); - } -} - -function requireMatchingNonEmptyField(standard, analytics, field, label) { - if ( - typeof standard[field] !== 'string' || - standard[field].trim().length === 0 || - typeof analytics[field] !== 'string' || - analytics[field].trim().length === 0 - ) { - throw new Error( - `standard and analytics targets must both report a non-empty ${label}: ` + - `standard=${JSON.stringify(standard[field])}, analytics=${JSON.stringify(analytics[field])}` - ); - } - if (standard[field] !== analytics[field]) { - throw new Error( - `standard and analytics targets report different values for ${label}: ` + - `standard=${JSON.stringify(standard[field])}, analytics=${JSON.stringify(analytics[field])}` - ); - } -} - -export function validatePrBuildTargetPair(standardRaw, analyticsRaw) { - const standard = normalizeLabeledTarget(standardRaw, 'standard PR-build'); - const analytics = normalizeLabeledTarget(analyticsRaw, 'analytics PR-build'); - - if (standard.executionBackend !== 'standard') { - throw new Error( - `standard PR-build target executionBackend must be "standard", got ` + - `${JSON.stringify(standard.executionBackend)}` - ); - } - if (analytics.executionBackend !== 'analytics') { - throw new Error( - `analytics PR-build target executionBackend must be "analytics", got ` + - `${JSON.stringify(analytics.executionBackend)}` - ); - } - - requireMatchingNonEmptyField(standard, analytics, 'engineVersion', 'engine version'); - requireMatchingNonEmptyField(standard, analytics, 'sqlSha', 'SQL SHA'); - requireMatchingNonEmptyField(standard, analytics, 'grammarHash', 'grammar hash'); - - return { standard, analytics }; -} - -function expectedBackendReportKeys(contractsDir, schedule) { - if (schedule !== 'pr' && schedule !== 'nightly') { - throw new Error(`schedule must be "pr" or "nightly", got ${JSON.stringify(schedule)}`); - } - const manifest = readJson(path.join(contractsDir, 'manifest.json'), 'contract manifest'); - if ( - manifest === null || - typeof manifest !== 'object' || - Array.isArray(manifest) || - !Array.isArray(manifest.contracts) - ) { - throw new TypeError('contract manifest.contracts must be a JSON array'); - } - - const files = new Set(); - const ruleIds = new Set(); - const expectedRows = new Map(); - for (const file of manifest.contracts) { - if (typeof file !== 'string' || file.length === 0) { - throw new TypeError('contract manifest entries must be non-empty strings'); - } - if (files.has(file)) { - throw new Error(`contract manifest contains duplicate file ${JSON.stringify(file)}`); - } - files.add(file); - - const spec = readJson(path.join(contractsDir, file), `contract ${file}`); - assertContractSchema(spec); - if (ruleIds.has(spec.ruleId)) { - throw new Error(`contract manifest contains duplicate ruleId ${JSON.stringify(spec.ruleId)}`); - } - ruleIds.add(spec.ruleId); - if (schedule === 'pr' && (spec.schedule || 'pr') !== 'pr') { - continue; - } - if ( - spec.queries === null || - typeof spec.queries !== 'object' || - Array.isArray(spec.queries) || - Object.keys(spec.queries).length === 0 - ) { - throw new TypeError(`[${spec.ruleId}] contract.queries must be a non-empty JSON object`); - } - for (const queryName of Object.keys(spec.queries)) { - const key = `${spec.ruleId}::${queryName}`; - if (expectedRows.has(key)) { - throw new Error(`contract corpus contains duplicate query key ${JSON.stringify(key)}`); - } - const query = spec.queries[queryName]; - if (query === null || typeof query !== 'object' || Array.isArray(query)) { - throw new TypeError(`[${spec.ruleId}] query ${JSON.stringify(queryName)} must be an object`); - } - expectedRows.set(key, { role: query.role || 'trigger' }); - } - } - if (expectedRows.size === 0) { - throw new Error(`contract corpus selected no queries for schedule ${JSON.stringify(schedule)}`); - } - return expectedRows; -} - -function validateBackendReport(raw, target, label, expectedRows) { - let rows; - try { - rows = indexBackendReport(raw, target); - } catch (error) { - throw new Error(`${label} backend report is invalid: ${error.message}`); - } - - const missing = [...expectedRows.keys()].filter((key) => !rows.has(key)).sort(); - const extra = [...rows.keys()].filter((key) => !expectedRows.has(key)).sort(); - if (missing.length > 0 || extra.length > 0) { - const details = []; - if (missing.length > 0) details.push(`missing: ${missing.join(', ')}`); - if (extra.length > 0) details.push(`unexpected: ${extra.join(', ')}`); - throw new Error(`${label} backend report query coverage is incomplete (${details.join('; ')})`); - } - - const unusable = []; - for (const [key, row] of rows) { - const expected = expectedRows.get(key); - if (row.role !== expected.role) { - throw new Error( - `${label} backend report row ${key}.role must be ${JSON.stringify(expected.role)}, ` + - `got ${JSON.stringify(row.role)}` - ); - } - if (typeof row.query !== 'string' || row.query.length === 0) { - throw new Error(`${label} backend report row ${key}.query must be a non-empty string`); - } - const status = classifyBackendReportRow(row).status; - if ( - status === 'error' || - (status === 'coverage-missing' && typeof row.rejected !== 'boolean') - ) { - unusable.push(key); - } - } - if (unusable.length > 0) { - throw new Error( - `${label} backend report contains rows without an engine verdict: ${unusable.sort().join(', ')}` - ); - } - return rows; -} - -export function validatePrBuildArtifacts({ - standardTarget, - analyticsTarget, - standardReport, - analyticsReport, - contractsDir, - schedule = 'nightly', -}) { - const pair = validatePrBuildTargetPair(standardTarget, analyticsTarget); - const expectedRows = expectedBackendReportKeys(contractsDir, schedule); - const standardRows = validateBackendReport( - standardReport, - pair.standard, - 'standard PR-build', - expectedRows - ); - const analyticsRows = validateBackendReport( - analyticsReport, - pair.analytics, - 'analytics PR-build', - expectedRows - ); - for (const key of expectedRows.keys()) { - const standard = standardRows.get(key); - const analytics = analyticsRows.get(key); - if (standard.query !== analytics.query) { - throw new Error( - `standard and analytics backend reports executed different query text for ${key}: ` + - `standard=${JSON.stringify(standard.query)}, analytics=${JSON.stringify(analytics.query)}` - ); - } - } - return { ...pair, expectedRows: expectedRows.size, standardRows, analyticsRows }; -} - -function parseArgs(argv) { - const options = new Map([ - ['--standard', 'standard'], - ['--analytics', 'analytics'], - ['--standard-report', 'standardReport'], - ['--analytics-report', 'analyticsReport'], - ['--contracts', 'contracts'], - ['--schedule', 'schedule'], - ]); - const args = { schedule: 'nightly' }; - const seen = new Set(); - for (let i = 0; i < argv.length; i++) { - const arg = argv[i]; - const key = options.get(arg); - if (!key) { - throw new Error(`unknown argument ${JSON.stringify(arg)}`); - } - const value = argv[++i]; - if (!value) { - throw new Error(`${arg} requires a value`); - } - if (seen.has(key)) { - throw new Error(`${arg} may be specified only once`); - } - seen.add(key); - args[key] = value; - } - for (const key of [ - 'standard', - 'analytics', - 'standardReport', - 'analyticsReport', - 'contracts', - ]) { - if (!args[key]) { - throw new Error(`${[...options].find(([, value]) => value === key)[0]} is required`); - } - } - return args; -} - -function readJson(file, label) { - try { - return JSON.parse(fs.readFileSync(file, 'utf8')); - } catch (error) { - throw new Error(`could not read ${label} ${file}: ${error.message}`); - } -} - -function main() { - const args = parseArgs(process.argv.slice(2)); - const { standard, analytics, expectedRows } = validatePrBuildArtifacts({ - standardTarget: readJson(args.standard, 'standard PR-build target'), - analyticsTarget: readJson(args.analytics, 'analytics PR-build target'), - standardReport: readJson(args.standardReport, 'standard PR-build backend report'), - analyticsReport: readJson(args.analyticsReport, 'analytics PR-build backend report'), - contractsDir: args.contracts, - schedule: args.schedule, - }); - // eslint-disable-next-line no-console - console.log( - `[ppl-lint-target-pair] verified engine=${standard.engineVersion} ` + - `sqlSha=${standard.sqlSha} grammarHash=${standard.grammarHash} ` + - `backends=${standard.executionBackend},${analytics.executionBackend} ` + - `backendRows=${expectedRows}` - ); -} - -if (process.argv[1] && process.argv[1].endsWith('validate-pr-build-targets.mjs')) { - try { - main(); - } catch (error) { - // eslint-disable-next-line no-console - console.error(`[ppl-lint-target-pair] FATAL: ${error.message}`); - process.exitCode = 2; - } -} From f15f0844aa1dc3c551b7370a47ea9b4210c0be2a Mon Sep 17 00:00:00 2001 From: Hanyu Wei Date: Tue, 4 Aug 2026 15:33:00 -0700 Subject: [PATCH 5/5] fix(ci): verify compatibility matrix leg identities --- .github/workflows/ppl-lint-multiversion-validation.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ppl-lint-multiversion-validation.yml b/.github/workflows/ppl-lint-multiversion-validation.yml index ef169a78bb5..af2aaac8264 100644 --- a/.github/workflows/ppl-lint-multiversion-validation.yml +++ b/.github/workflows/ppl-lint-multiversion-validation.yml @@ -520,6 +520,8 @@ jobs: (.matrix | length) == (12 * $expected_legs) and (.matrix | map(.ruleId) | unique | sort) == $expected_rules and (.matrix | map(.legKey) | unique | length) == $expected_legs and + (.matrix | map(.legKey) | unique | sort) == + (.legs | map(.key) | unique | sort) and ([.matrix[] | [.ruleId, .legKey]] | unique | length) == (12 * $expected_legs) ' drift-report.json > /dev/null; then