diff --git a/.github/workflows/ppl-lint-multiversion-validation.yml b/.github/workflows/ppl-lint-multiversion-validation.yml index de047b07569..8e31ab55918 100644 --- a/.github/workflows/ppl-lint-multiversion-validation.yml +++ b/.github/workflows/ppl-lint-multiversion-validation.yml @@ -9,18 +9,11 @@ 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 DEFAULT-ERROR rule (enabled: true + severity: -# error in OSD's rules_catalog.json) against SEVERAL released engine versions -# plus the PR build, and — when a rule disagrees with any of them — says what to -# change in the linter rather than only that a count was wrong. -# -# Why default-error only: an error-severity rule is one the user cannot opt out -# of and which marks their query as broken. A wrong error is the most expensive -# possible lint defect, so that set gets the multi-version treatment first. -# Warning/info rules stay on the single-version check. The set is not hand-copied: -# the detector run records the catalog's default-error census, and the aggregate -# step fails if a rule in that census has no contract file (see manifest.json's -# `defaultError` note). +# 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. # # Shape — a per-version matrix of observation legs, then one aggregation: # diff --git a/.github/workflows/ppl-lint-rule-validation.yml b/.github/workflows/ppl-lint-rule-validation.yml index 078fd285494..6f83c60a60b 100644 --- a/.github/workflows/ppl-lint-rule-validation.yml +++ b/.github/workflows/ppl-lint-rule-validation.yml @@ -370,6 +370,7 @@ jobs: # green (design §4.4). A workflow_dispatch run is pre-merge evidence and is # intentionally not what repo admins pin to branch protection. - name: Require both validation jobs to have succeeded + if: ${{ always() }} env: BACKEND_RESULT: ${{ needs.backend-validation.result }} DETECTOR_RESULT: ${{ needs.detector-validation.result }} 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 e8dbafc42a4..c6a503fddaf 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 @@ -32,7 +32,7 @@ import org.opensearch.sql.ppl.PPLIntegTestCase; /** - * Backend half of the schema-v3/schema-v4 PPL lint rule validation contract. + * Backend half of the schema-v3/schema-v4 PPL frontend validation contract. * *

This test drives the live {@code POST /_plugins/_ppl} endpoint on the SQL plugin built from * the current checkout. For every contract (see {@code @@ -50,9 +50,9 @@ * confirmed by a single run, e.g. head nondeterminism, fallback warnings). * * - *

The contract files are shared verbatim with the SQL-owned OSD detector runner ({@code + *

The contract files are shared verbatim with the SQL-owned OSD frontend runner ({@code * scripts/ppl-lint/run-frontend-contract.mjs}) so the same reviewed cases pin both the OSD analyzer - * diagnostic count and the SQL backend behavior; neither side can drift without a red build. The + * output and the SQL backend behavior; neither side can drift without a red build. The * rejection-body parsing mirrors {@link * org.opensearch.sql.calcite.remote.CalciteErrorReportStageIT}; the Calcite setup follows {@link * org.opensearch.sql.calcite.remote.CalcitePPLEventstatsIT}. @@ -60,15 +60,14 @@ *

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 API, 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 and syntax APIs, 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 the full corpus. Every - * contract in the corpus currently declares {@code schedule: "pr"}, so the two are equivalent - * today; the filter stays because it is the only mechanism for holding a new contract back from PR - * runs while its oracle is still settling. + * 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 + * analytics oracles are still settling. * *

Note that a contract which RUNS also ASSERTS. This class does not consult the manifest's * {@code enforced} list — that list records oracle quality and review status, not blocking diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index 38e37c41d31..97db1640a5e 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -877,6 +877,11 @@ public enum Index { "flat_object", getFlatObjectIndexMapping(), "src/test/resources/flat_object.json"), + PPL_LINT_DISABLED_OBJECT( + TestsConstants.TEST_INDEX_PPL_LINT_DISABLED_OBJECT, + "ppl_lint_disabled_object", + getPplLintDisabledObjectIndexMapping(), + "src/test/resources/ppl_lint_disabled_object.json"), DUPLICATION_NULLABLE( TestsConstants.TEST_INDEX_DUPLICATION_NULLABLE, "duplication_nullable", diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java index 25385201e19..1d92f807249 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java @@ -587,6 +587,11 @@ public static String getFlatObjectIndexMapping() { return getMappingFile(mappingFile); } + public static String getPplLintDisabledObjectIndexMapping() { + String mappingFile = "ppl_lint_disabled_object_index_mapping.json"; + return getMappingFile(mappingFile); + } + public static String getPhraseIndexMapping() { String mappingFile = "phrase_index_mapping.json"; return getMappingFile(mappingFile); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java index 957ff0108d6..357b5d37ef4 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java @@ -77,6 +77,8 @@ public class TestsConstants { public static final String TEST_INDEX_ALIAS = TEST_INDEX + "_alias"; public static final String TEST_INDEX_FLATTENED_VALUE = TEST_INDEX + "_flattened_value"; public static final String TEST_INDEX_FLAT_OBJECT = TEST_INDEX + "_flat_object"; + public static final String TEST_INDEX_PPL_LINT_DISABLED_OBJECT = + TEST_INDEX + "_ppl_lint_disabled_object"; public static final String TEST_INDEX_GEOIP = TEST_INDEX + "_geoip"; public static final String DATASOURCES = ".ql-datasources"; public static final String TEST_INDEX_STATE_COUNTRY = TEST_INDEX + "_state_country"; diff --git a/integ-test/src/test/resources/indexDefinitions/ppl_lint_disabled_object_index_mapping.json b/integ-test/src/test/resources/indexDefinitions/ppl_lint_disabled_object_index_mapping.json new file mode 100644 index 00000000000..6893eceb163 --- /dev/null +++ b/integ-test/src/test/resources/indexDefinitions/ppl_lint_disabled_object_index_mapping.json @@ -0,0 +1,13 @@ +{ + "mappings": { + "properties": { + "session": { + "type": "object", + "enabled": false + }, + "status": { + "type": "keyword" + } + } + } +} 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 new file mode 100644 index 00000000000..4c574aaa86c --- /dev/null +++ b/integ-test/src/test/resources/ppl-lint/contracts/agg-on-text.spec.json @@ -0,0 +1,127 @@ +{ + "schemaVersion": 4, + "ruleId": "agg-on-text", + "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", + "wiring": { + "detector": "agg-on-text", + "enabled": true, + "severity": "warning", + "runtimeOnly": false, + "needsContext": true, + "needsExplain": false, + "sourceScoped": true, + "appliesTo": { + "minVersion": "3.7.0", + "engine": "calcite" + } + }, + "backendFixture": { + "indices": [ + "ACCOUNT" + ], + "clusterSettings": { + "calcite": true, + "calciteFallback": false + } + }, + "frontendContext": { + "isCalcite": true, + "deriveFromMapping": { + "firstname": "text", + "balance": "long" + } + }, + "index": "opensearch-sql_test_index_account", + "queries": { + "avg-text-field": { + "role": "trigger", + "query": "source={{index}} | stats avg(firstname) as avg_firstname" + }, + "sum-text-field": { + "role": "trigger", + "query": "source={{index}} | stats sum(firstname) as sum_firstname" + }, + "avg-numeric-control": { + "role": "control", + "query": "source={{index}} | stats avg(balance) as avg_balance" + } + }, + "expectations": [ + { + "version": ">=3.7.0", + "engine": "calcite", + "queries": { + "avg-text-field": { + "frontend": { + "count": 1, + "severity": "warning", + "matchMessage": "text field" + }, + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "columnAllNull": "avg_firstname" + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "columnAllNull": "avg_firstname" + } + } + } + }, + "sum-text-field": { + "frontend": { + "count": 1, + "severity": "warning", + "matchMessage": "text field" + }, + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + } + } + }, + "avg-numeric-control": { + "frontend": { + "count": 0 + }, + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + } + } + } + } + } + ] +} 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 new file mode 100644 index 00000000000..e1bb53ffb3b --- /dev/null +++ b/integ-test/src/test/resources/ppl-lint/contracts/command-suggestion.spec.json @@ -0,0 +1,142 @@ +{ + "schemaVersion": 4, + "ruleId": "command-suggestion", + "channel": "syntax", + "grammarSurface": "runtime-bundle", + "schedule": "nightly", + "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": "where", + "rawMessage": true + }, + "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", + "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", + "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", + "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/dedup-consecutive-unsupported.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/dedup-consecutive-unsupported.spec.json index fffdce393dd..b1dbcda115a 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/dedup-consecutive-unsupported.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/dedup-consecutive-unsupported.spec.json @@ -5,11 +5,12 @@ "schedule": "pr", "wiring": { "detector": "dedup-consecutive-unsupported", - "enabled": true, + "enabled": false, "severity": "warning", "runtimeOnly": false, "needsContext": false, "needsExplain": false, + "sourceScoped": false, "appliesTo": { "minVersion": "3.3.0", "engine": "calcite" @@ -25,7 +26,8 @@ } }, "frontendContext": { - "isCalcite": true + "isCalcite": true, + "forceEnable": true }, "index": "opensearch-sql_test_index_account", "queries": { diff --git a/integ-test/src/test/resources/ppl-lint/contracts/disabled-join-type.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/disabled-join-type.spec.json index d2d5df24260..ebfe44389ac 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/disabled-join-type.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/disabled-join-type.spec.json @@ -5,11 +5,12 @@ "schedule": "pr", "wiring": { "detector": "disabled-join-type", - "enabled": true, + "enabled": false, "severity": "warning", "runtimeOnly": false, "needsContext": false, "needsExplain": false, + "sourceScoped": false, "appliesTo": {} }, "backendFixture": { @@ -23,7 +24,8 @@ } }, "frontendContext": { - "isCalcite": true + "isCalcite": true, + "forceEnable": true }, "index": "opensearch-sql_test_index_account", "queries": { 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 bc30f5ecb77..88c4226fd8e 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 @@ -1,5 +1,5 @@ { - "schemaVersion": 3, + "schemaVersion": 4, "ruleId": "division-by-zero", "note": "The detector flags both division and modulo by a literal zero because both operations return null silently. The backend result-shape oracle verifies that behavior independently for each operator.", "grammarSurface": "both", @@ -11,7 +11,8 @@ "runtimeOnly": false, "needsContext": false, "needsExplain": false, - "appliesTo": {} + "appliesTo": {}, + "sourceScoped": false }, "backendFixture": { "indices": [ @@ -51,43 +52,79 @@ "divide-by-zero-literal": { "detectorCount": 1, "severity": "warning", - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "columnAllNull": "ratio" + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "columnAllNull": "ratio" + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "columnAllNull": "ratio" + } } } }, "divide-by-decimal-zero-literal": { "detectorCount": 1, "severity": "warning", - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "columnAllNull": "ratio" + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "columnAllNull": "ratio" + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "columnAllNull": "ratio" + } } } }, "divide-by-nonzero-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } }, "modulo-by-zero-literal": { "detectorCount": 1, "severity": "warning", - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "columnAllNull": "m" + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "columnAllNull": "m" + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "columnAllNull": "m" + } } } } 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 new file mode 100644 index 00000000000..b9ebc0eef91 --- /dev/null +++ b/integ-test/src/test/resources/ppl-lint/contracts/enabled-false-object.spec.json @@ -0,0 +1,129 @@ +{ + "schemaVersion": 4, + "ruleId": "enabled-false-object", + "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", + "wiring": { + "detector": "enabled-false-object", + "enabled": true, + "severity": "warning", + "runtimeOnly": false, + "needsContext": true, + "needsExplain": false, + "sourceScoped": true, + "appliesTo": { + "minVersion": "3.7.0", + "engine": "calcite" + } + }, + "backendFixture": { + "indices": [ + "PPL_LINT_DISABLED_OBJECT" + ], + "clusterSettings": { + "calcite": true, + "calciteFallback": false + } + }, + "frontendContext": { + "isCalcite": true, + "deriveFromMapping": { + "status": "keyword" + }, + "disabledObjectFields": [ + "session" + ] + }, + "index": "opensearch-sql_test_index_ppl_lint_disabled_object", + "queries": { + "disabled-object-field": { + "role": "trigger", + "query": "source={{index}} | fields session.id | head 1" + }, + "disabled-object-filter": { + "role": "trigger", + "query": "source={{index}} | where session.id = 'abc' | fields status" + }, + "indexed-field-control": { + "role": "control", + "query": "source={{index}} | where status = 'ok' | fields status | head 1" + } + }, + "expectations": [ + { + "version": ">=3.7.0", + "engine": "calcite", + "queries": { + "disabled-object-field": { + "frontend": { + "count": 1, + "severity": "warning", + "matchMessage": "not searchable" + }, + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + } + } + }, + "disabled-object-filter": { + "frontend": { + "count": 1, + "severity": "warning", + "matchMessage": "not searchable" + }, + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsCount": 1 + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsCount": 1 + } + } + } + }, + "indexed-field-control": { + "frontend": { + "count": 0 + }, + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + } + } + } + } + } + ] +} 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 5814d837249..6225eaed87b 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 @@ -1,5 +1,5 @@ { - "schemaVersion": 3, + "schemaVersion": 4, "ruleId": "field-validation", "grammarSurface": "both", "schedule": "pr", @@ -10,7 +10,8 @@ "runtimeOnly": false, "needsContext": false, "needsExplain": false, - "appliesTo": {} + "appliesTo": {}, + "sourceScoped": true }, "backendFixture": { "indices": [ @@ -63,32 +64,59 @@ "unknown-field-existence": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400 + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400 + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400 + } } } }, "grok-field-slot-shape-typo": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400 + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400 + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400 + } } } }, "known-field-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } @@ -100,14 +128,27 @@ "unknown-field-existence": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Invalid Query" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } } } } @@ -115,25 +156,47 @@ "grok-field-slot-shape-typo": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Invalid Query" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } } } } }, "known-field-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } @@ -146,14 +209,27 @@ "detectorCount": 1, "severity": "error", "matchMessage": "nonexistent_field", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Field [nonexistent_field] not found." + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Field [nonexistent_field] not found." + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Field [nonexistent_field] not found." + } } } } @@ -161,25 +237,47 @@ "grok-field-slot-shape-typo": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Field [field] not found." + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Field [field] not found." + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Field [field] not found." + } } } } }, "known-field-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } diff --git a/integ-test/src/test/resources/ppl-lint/contracts/flat-object-subfield.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/flat-object-subfield.spec.json index 0ba5c95be35..70006fafbdf 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/flat-object-subfield.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/flat-object-subfield.spec.json @@ -10,12 +10,16 @@ "notes": "Live-verified on OpenSearch 3.8 with Calcite on: a flat_object field cannot be referenced by PPL at all. BOTH a dotted subfield (`fields attributes.service`) AND the bare root (`fields attributes`) fail with IllegalArgumentException 'Field [...] not found.', and the same holds in a where clause. NOTE the rejection reason is byte-identical to the one field-validation produces for a genuinely absent field, so the backend reason alone cannot attribute a diagnostic to a rule — attribution comes from the detector's ruleId, which is why every case here pins detectorCount for THIS ruleId only. The detector self-suppresses without a typeMap, hence the deriveFromMapping block below (needsContext: true). The analytics feature build cannot create flat_object in composite/Parquet storage, so every analytics backend oracle is explicitly non-applicable while the frontend detector assertions still run.", "wiring": { "detector": "flat-object-subfield", - "enabled": true, + "enabled": false, "severity": "error", "runtimeOnly": false, "needsContext": true, "needsExplain": false, - "appliesTo": {} + "sourceScoped": true, + "appliesTo": { + "minVersion": "3.8.0", + "engine": "calcite" + } }, "backendFixture": { "indices": [ @@ -28,6 +32,7 @@ }, "frontendContext": { "isCalcite": true, + "forceEnable": true, "deriveFromMapping": { "name": "keyword", "status": "integer", diff --git a/integ-test/src/test/resources/ppl-lint/contracts/head-without-sort.spec.json b/integ-test/src/test/resources/ppl-lint/contracts/head-without-sort.spec.json index 56c272abc9c..cb1d65825d0 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/head-without-sort.spec.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/head-without-sort.spec.json @@ -5,11 +5,12 @@ "schedule": "pr", "wiring": { "detector": "head-without-sort", - "enabled": true, + "enabled": false, "severity": "info", "runtimeOnly": false, "needsContext": false, "needsExplain": false, + "sourceScoped": false, "appliesTo": {} }, "backendFixture": { @@ -22,7 +23,8 @@ } }, "frontendContext": { - "isCalcite": true + "isCalcite": true, + "forceEnable": true }, "index": "opensearch-sql_test_index_account", "queries": { 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 d96142d62e0..4c2cfc36c6f 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 @@ -1,5 +1,5 @@ { - "schemaVersion": 3, + "schemaVersion": 4, "ruleId": "invalid-capture-group-name", "grammarSurface": "runtime-bundle", "schedule": "pr", @@ -14,7 +14,10 @@ "runtimeOnly": false, "needsContext": false, "needsExplain": false, - "appliesTo": {} + "appliesTo": { + "minVersion": "3.4.0" + }, + "sourceScoped": false }, "backendFixture": { "indices": [ @@ -54,14 +57,27 @@ "rex-capture-name-underscore": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Invalid Query" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } } } } @@ -69,25 +85,47 @@ "rex-capture-name-hyphen": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Invalid Query" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } } } } }, "rex-capture-name-alphanumeric-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } @@ -100,14 +138,27 @@ "rex-capture-name-underscore": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Invalid capture group name 'user_name'." + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid capture group name 'user_name'." + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid capture group name 'user_name'." + } } } } @@ -115,25 +166,47 @@ "rex-capture-name-hyphen": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Invalid capture group name 'user-name'." + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid capture group name 'user-name'." + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid capture group name 'user-name'." + } } } } }, "rex-capture-name-alphanumeric-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } 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 d316c1e8de1..3e83f493e0c 100644 --- a/integ-test/src/test/resources/ppl-lint/contracts/manifest.json +++ b/integ-test/src/test/resources/ppl-lint/contracts/manifest.json @@ -1,46 +1,68 @@ { - "schemaVersion": 3, - "description": "Index of PPL lint rule validation contracts. Each entry pins one OSD analyzer rule to live SQL /_plugins/_ppl behavior. The detector runner (scripts/ppl-lint/run-frontend-contract.mjs) and backend IT (PplLintRuleValidationIT) both read these files. `contracts` is the full corpus. EVERY contract now declares `schedule: \"pr\"`, so every contract runs \u2014 and asserts \u2014 on every pull request: neither reader consults `enforced`, so any contract that runs is a hard assertion. The `enforced` / `nonEnforcing` lists below therefore describe oracle QUALITY and review status, not whether a mismatch blocks (design \u00a75.1, \u00a75.2). They are what a reviewer should read when judging how much to trust a red result.", + "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.", "contracts": [ - "invalid-capture-group-name.spec.json", - "unsupported-window-function-in-eventstats.spec.json", + "agg-on-text.spec.json", + "command-suggestion.spec.json", "division-by-zero.spec.json", - "head-without-sort.spec.json", - "disabled-join-type.spec.json", + "enabled-false-object.spec.json", "field-validation.spec.json", - "flat-object-subfield.spec.json", - "dedup-consecutive-unsupported.spec.json", + "invalid-capture-group-name.spec.json", "multisearch-min-subsearch.spec.json", + "replace-wildcard-asymmetry.spec.json", + "rex-scan-cost.spec.json", + "type-mismatch-numeric.spec.json", "union-min-datasets.spec.json", - "replace-wildcard-asymmetry.spec.json" + "unsupported-window-function-in-eventstats.spec.json", + "wildcard-source-zero-match.spec.json" + ], + "dormantContracts": [ + "dedup-consecutive-unsupported.spec.json", + "disabled-join-type.spec.json", + "flat-object-subfield.spec.json", + "head-without-sort.spec.json" ], "enforced": [ + "field-validation.spec.json", "invalid-capture-group-name.spec.json", - "unsupported-window-function-in-eventstats.spec.json", "multisearch-min-subsearch.spec.json", + "replace-wildcard-asymmetry.spec.json", "union-min-datasets.spec.json", - "replace-wildcard-asymmetry.spec.json" + "unsupported-window-function-in-eventstats.spec.json" ], "defaultError": [ + "field-validation.spec.json", "invalid-capture-group-name.spec.json", - "unsupported-window-function-in-eventstats.spec.json", "multisearch-min-subsearch.spec.json", - "union-min-datasets.spec.json", "replace-wildcard-asymmetry.spec.json", - "field-validation.spec.json", - "flat-object-subfield.spec.json" + "union-min-datasets.spec.json", + "unsupported-window-function-in-eventstats.spec.json" + ], + "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" ], - "pendingReview": [], "nonEnforcing": [ + "agg-on-text.spec.json", "division-by-zero.spec.json", - "head-without-sort.spec.json", - "disabled-join-type.spec.json", - "dedup-consecutive-unsupported.spec.json" + "enabled-false-object.spec.json", + "rex-scan-cost.spec.json", + "type-mismatch-numeric.spec.json", + "wildcard-source-zero-match.spec.json" ], "notes": { - "enforced": "Reviewed error rules with a deterministic backend rejection and a valid negative control. The most trustworthy oracles in the corpus \u2014 a mismatch here is almost certainly a real drift.", - "defaultError": "Every rule that ships enabled at ERROR severity in the OSD catalog \u2014 the set the MULTI-VERSION check enforces (scripts/ppl-lint/aggregate-versions.mjs). A default-error rule is what users cannot opt out of and what blocks a query in the editor, so it is exactly the set that must agree with every supported engine version. Kept in sync with the catalog by the coverage assertion in the aggregate step: a rules_catalog.json entry with enabled:true + severity:error and no contract file here fails the check.", - "pendingReview": "Error rules awaiting Peng/Chen usefulness + false-positive review (design \u00a75.2) before joining `enforced`. Empty now that field-validation and flat-object-subfield are pinned across versions by the multi-version check; they remain outside single-version `enforced` because their backend oracle is a semantic 'Field [...] not found.' rejection shared with each other rather than a rule-unique grammar rejection.", - "nonEnforcing": "Warning / info / advisory / result-shape rules. Their oracle is weaker than a clean rejection (an advisory rule's query SUCCEEDS, so the contract asserts a result shape or mere acceptance), which makes them likelier to move for reasons unrelated to the lint rule \u2014 dedup-consecutive, for instance, depends on the Calcite-to-v2 fallback staying enabled. They ran nightly-only until every contract moved to the PR schedule so the multi-version rollup sees a full trigger census on each PR; they now block like any other contract, and a red result here warrants checking the oracle before editing a rule." + "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 345742aa9d8..21cdae63845 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 @@ -1,5 +1,5 @@ { - "schemaVersion": 3, + "schemaVersion": 4, "ruleId": "multisearch-min-subsearch", "grammarSurface": "runtime-bundle", "schedule": "pr", @@ -7,7 +7,7 @@ "multisearchCommand", "subSearch" ], - "notes": "Query-initial (no leading pipe) on purpose \u2014 see the note on union-min-datasets. OSD's runtime lint prepends a synthetic 'source=t ' prefix to pipe-first queries, which would change the effective parse relative to what the backend receives. A query-initial 'multisearch [...]' is sent byte-identically to both halves.", + "notes": "Query-initial (no leading pipe) on purpose — see the note on union-min-datasets. OSD's runtime lint prepends a synthetic 'source=t ' prefix to pipe-first queries, which would change the effective parse relative to what the backend receives. A query-initial 'multisearch [...]' is sent byte-identically to both halves.", "wiring": { "detector": "multisearch-min-subsearch", "enabled": true, @@ -17,7 +17,8 @@ "needsExplain": false, "appliesTo": { "minVersion": "3.4.0" - } + }, + "sourceScoped": false }, "backendFixture": { "indices": [ @@ -53,14 +54,27 @@ "multisearch-single-subsearch": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "SyntaxCheckException", - "reason": "Invalid Query" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "SyntaxCheckException", + "reason": "Invalid Query" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "SyntaxCheckException", + "reason": "Invalid Query" + } } } } @@ -68,25 +82,47 @@ "multisearch-single-subsearch-with-where": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "SyntaxCheckException", - "reason": "Invalid Query" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "SyntaxCheckException", + "reason": "Invalid Query" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "SyntaxCheckException", + "reason": "Invalid Query" + } } } } }, "multisearch-two-subsearches-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } 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 c6fa5b162b5..29def9f3b8a 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 @@ -1,5 +1,5 @@ { - "schemaVersion": 3, + "schemaVersion": 4, "ruleId": "replace-wildcard-asymmetry", "grammarSurface": "runtime-bundle", "schedule": "pr", @@ -17,7 +17,8 @@ "appliesTo": { "minVersion": "3.4.0", "engine": "calcite" - } + }, + "sourceScoped": false }, "backendFixture": { "indices": [ @@ -54,14 +55,27 @@ "replace-wildcard-count-mismatch": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Invalid Query" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } } } } @@ -69,25 +83,47 @@ "replace-wildcard-count-mismatch-reverse": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Invalid Query" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Invalid Query" + } } } } }, "replace-symmetric-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } @@ -100,14 +136,27 @@ "replace-wildcard-count-mismatch": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Error in 'replace' command: Wildcard count mismatch - pattern has 1 wildcard(s), replacement has 2. Replacement must have same number of wildcards or none." + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Error in 'replace' command: Wildcard count mismatch - pattern has 1 wildcard(s), replacement has 2. Replacement must have same number of wildcards or none." + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Error in 'replace' command: Wildcard count mismatch - pattern has 1 wildcard(s), replacement has 2. Replacement must have same number of wildcards or none." + } } } } @@ -115,25 +164,47 @@ "replace-wildcard-count-mismatch-reverse": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Error in 'replace' command: Wildcard count mismatch - pattern has 2 wildcard(s), replacement has 1. Replacement must have same number of wildcards or none." + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Error in 'replace' command: Wildcard count mismatch - pattern has 2 wildcard(s), replacement has 1. Replacement must have same number of wildcards or none." + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Error in 'replace' command: Wildcard count mismatch - pattern has 2 wildcard(s), replacement has 1. Replacement must have same number of wildcards or none." + } } } } }, "replace-symmetric-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } 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 new file mode 100644 index 00000000000..918eaacb0cd --- /dev/null +++ b/integ-test/src/test/resources/ppl-lint/contracts/rex-scan-cost.spec.json @@ -0,0 +1,103 @@ +{ + "schemaVersion": 4, + "ruleId": "rex-scan-cost", + "channel": "lint", + "grammarSurface": "both", + "schedule": "nightly", + "wiring": { + "detector": "rex-scan-cost", + "enabled": true, + "severity": "info", + "runtimeOnly": false, + "needsContext": true, + "needsExplain": false, + "sourceScoped": true, + "appliesTo": {} + }, + "backendFixture": { + "indices": [ + "ACCOUNT" + ], + "clusterSettings": { + "calcite": true, + "calciteFallback": false + } + }, + "frontendContext": { + "isCalcite": true, + "deriveFromMapping": { + "email": "text" + } + }, + "index": "opensearch-sql_test_index_account", + "queries": { + "parse-text-field": { + "role": "trigger", + "query": "source={{index}} | parse email '.+@(?.+)' | fields email, host | head 1" + }, + "grok-text-field": { + "role": "trigger", + "query": "source={{index}} | grok email '.+@%{HOSTNAME:grok_host}' | fields email, grok_host | head 1" + }, + "plain-field-control": { + "role": "control", + "query": "source={{index}} | fields email | head 1" + } + }, + "expectations": [ + { + "version": ">=0.0.0", + "queries": { + "parse-text-field": { + "frontend": { + "count": 1, + "severity": "info", + "matchMessage": "every input row" + }, + "backends": { + "standard": { + "kind": "advisory", + "httpStatus": 200 + }, + "analytics": { + "kind": "advisory", + "httpStatus": 200 + } + } + }, + "grok-text-field": { + "frontend": { + "count": 1, + "severity": "info", + "matchMessage": "every input row" + }, + "backends": { + "standard": { + "kind": "advisory", + "httpStatus": 200 + }, + "analytics": { + "kind": "advisory", + "httpStatus": 200 + } + } + }, + "plain-field-control": { + "frontend": { + "count": 0 + }, + "backends": { + "standard": { + "kind": "advisory", + "httpStatus": 200 + }, + "analytics": { + "kind": "advisory", + "httpStatus": 200 + } + } + } + } + } + ] +} 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 new file mode 100644 index 00000000000..0f3e14791f5 --- /dev/null +++ b/integ-test/src/test/resources/ppl-lint/contracts/type-mismatch-numeric.spec.json @@ -0,0 +1,125 @@ +{ + "schemaVersion": 4, + "ruleId": "type-mismatch-numeric", + "channel": "lint", + "grammarSurface": "both", + "schedule": "nightly", + "wiring": { + "detector": "type-mismatch-numeric", + "enabled": true, + "severity": "warning", + "runtimeOnly": false, + "needsContext": true, + "needsExplain": false, + "sourceScoped": true, + "appliesTo": { + "minVersion": "3.7.0", + "engine": "calcite" + } + }, + "backendFixture": { + "indices": [ + "ACCOUNT" + ], + "clusterSettings": { + "calcite": true, + "calciteFallback": false + } + }, + "frontendContext": { + "isCalcite": true, + "deriveFromMapping": { + "age": "long" + } + }, + "index": "opensearch-sql_test_index_account", + "queries": { + "numeric-field-string-value": { + "role": "trigger", + "query": "source={{index}} | where age = \"thirty\" | fields age" + }, + "string-value-numeric-field": { + "role": "trigger", + "query": "source={{index}} | where \"thirty\" = age | fields age" + }, + "numeric-string-control": { + "role": "control", + "query": "source={{index}} | where age = \"32\" | fields age | head 1" + } + }, + "expectations": [ + { + "version": ">=3.7.0", + "engine": "calcite", + "queries": { + "numeric-field-string-value": { + "frontend": { + "count": 1, + "severity": "warning", + "matchMessage": "not a number" + }, + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsCount": 0 + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsCount": 0 + } + } + } + }, + "string-value-numeric-field": { + "frontend": { + "count": 1, + "severity": "warning", + "matchMessage": "not a number" + }, + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsCount": 0 + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsCount": 0 + } + } + } + }, + "numeric-string-control": { + "frontend": { + "count": 0 + }, + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + } + } + } + } + } + ] +} 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 d5eadcc25f5..6de9db9a7d6 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 @@ -1,5 +1,5 @@ { - "schemaVersion": 3, + "schemaVersion": 4, "ruleId": "union-min-datasets", "grammarSurface": "runtime-bundle", "schedule": "pr", @@ -8,7 +8,7 @@ "unionDataset", "pplCommands" ], - "notes": "Query-initial (no leading pipe) on purpose. OSD's runtime lint prepends a synthetic 'source=t ' prefix to pipe-first queries, so linting '| union [...]' actually parses 'source=t | union [...]' \u2014 a valid MID-pipeline union (implicit upstream dataset) that the detector deliberately does not flag. The backend, receiving the raw pipe-first query, would still reject it, so a pipe-first trigger makes the two halves test different effective queries (violating the design's 'same queries' rule). A query-initial 'union [...]' is sent byte-identically to both sides and keeps the differential sound.", + "notes": "Query-initial (no leading pipe) on purpose. OSD's runtime lint prepends a synthetic 'source=t ' prefix to pipe-first queries, so linting '| union [...]' actually parses 'source=t | union [...]' — a valid MID-pipeline union (implicit upstream dataset) that the detector deliberately does not flag. The backend, receiving the raw pipe-first query, would still reject it, so a pipe-first trigger makes the two halves test different effective queries (violating the design's 'same queries' rule). A query-initial 'union [...]' is sent byte-identically to both sides and keeps the differential sound.", "wiring": { "detector": "union-min-datasets", "enabled": true, @@ -19,7 +19,8 @@ "appliesTo": { "minVersion": "3.7.0", "engine": "calcite" - } + }, + "sourceScoped": false }, "backendFixture": { "indices": [ @@ -56,14 +57,27 @@ "union-single-dataset": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Union command requires at least two datasets. Provided: 1" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Union command requires at least two datasets. Provided: 1" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Union command requires at least two datasets. Provided: 1" + } } } } @@ -71,25 +85,47 @@ "union-single-dataset-with-fields": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "IllegalArgumentException", - "reason": "Union command requires at least two datasets. Provided: 1" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Union command requires at least two datasets. Provided: 1" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "IllegalArgumentException", + "reason": "Union command requires at least two datasets. Provided: 1" + } } } } }, "union-two-datasets-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } 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 75e45299c0f..fda4d273fb9 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 @@ -1,5 +1,5 @@ { - "schemaVersion": 3, + "schemaVersion": 4, "ruleId": "unsupported-window-function-in-eventstats", "detectorPath": "packages/osd-monaco/src/ppl/lint/rules/unsupported_window_function.ts", "grammarSurface": "both", @@ -13,7 +13,8 @@ "needsExplain": false, "appliesTo": { "minVersion": "3.4.0" - } + }, + "sourceScoped": false }, "backendFixture": { "indices": [ @@ -50,14 +51,27 @@ "eventstats-rank": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 500, - "body": { - "status": 500, - "error": { - "type": "UnsupportedOperationException", - "reason": "There was internal problem at backend" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 500, + "body": { + "status": 500, + "error": { + "type": "UnsupportedOperationException", + "reason": "There was internal problem at backend" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 500, + "body": { + "status": 500, + "error": { + "type": "UnsupportedOperationException", + "reason": "There was internal problem at backend" + } } } } @@ -65,25 +79,47 @@ "eventstats-dense-rank": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 500, - "body": { - "status": 500, - "error": { - "type": "UnsupportedOperationException", - "reason": "There was internal problem at backend" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 500, + "body": { + "status": 500, + "error": { + "type": "UnsupportedOperationException", + "reason": "There was internal problem at backend" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 500, + "body": { + "status": 500, + "error": { + "type": "UnsupportedOperationException", + "reason": "There was internal problem at backend" + } } } } }, "eventstats-avg-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } @@ -95,14 +131,27 @@ "eventstats-rank": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 500, - "body": { - "status": 500, - "error": { - "type": "UnsupportedOperationException", - "reason": "Unexpected window function: rank" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 500, + "body": { + "status": 500, + "error": { + "type": "UnsupportedOperationException", + "reason": "Unexpected window function: rank" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 500, + "body": { + "status": 500, + "error": { + "type": "UnsupportedOperationException", + "reason": "Unexpected window function: rank" + } } } } @@ -110,25 +159,47 @@ "eventstats-dense-rank": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 500, - "body": { - "status": 500, - "error": { - "type": "UnsupportedOperationException", - "reason": "Unexpected window function: dense_rank" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 500, + "body": { + "status": 500, + "error": { + "type": "UnsupportedOperationException", + "reason": "Unexpected window function: dense_rank" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 500, + "body": { + "status": 500, + "error": { + "type": "UnsupportedOperationException", + "reason": "Unexpected window function: dense_rank" + } } } } }, "eventstats-avg-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } @@ -140,14 +211,27 @@ "eventstats-rank": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "CalciteUnsupportedException", - "reason": "Unexpected window function: rank" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "CalciteUnsupportedException", + "reason": "Unexpected window function: rank" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "CalciteUnsupportedException", + "reason": "Unexpected window function: rank" + } } } } @@ -155,25 +239,47 @@ "eventstats-dense-rank": { "detectorCount": 1, "severity": "error", - "backend": { - "kind": "rejection", - "httpStatus": 400, - "body": { - "status": 400, - "error": { - "type": "CalciteUnsupportedException", - "reason": "Unexpected window function: dense_rank" + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "CalciteUnsupportedException", + "reason": "Unexpected window function: dense_rank" + } + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 400, + "body": { + "status": 400, + "error": { + "type": "CalciteUnsupportedException", + "reason": "Unexpected window function: dense_rank" + } } } } }, "eventstats-avg-control": { "detectorCount": 0, - "backend": { - "kind": "result-shape", - "httpStatus": 200, - "expect": { - "datarowsNonEmpty": true + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } } } } 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 new file mode 100644 index 00000000000..df01303aefb --- /dev/null +++ b/integ-test/src/test/resources/ppl-lint/contracts/wildcard-source-zero-match.spec.json @@ -0,0 +1,94 @@ +{ + "schemaVersion": 4, + "ruleId": "wildcard-source-zero-match", + "channel": "lint", + "grammarSurface": "both", + "schedule": "nightly", + "wiring": { + "detector": "wildcard-source-zero-match", + "enabled": true, + "severity": "info", + "runtimeOnly": false, + "needsContext": true, + "needsExplain": false, + "sourceScoped": false, + "appliesTo": {} + }, + "backendFixture": { + "indices": [ + "ACCOUNT" + ], + "clusterSettings": { + "calcite": true, + "calciteFallback": false + } + }, + "frontendContext": { + "isCalcite": true, + "visibleIndices": [ + "{{index}}" + ] + }, + "index": "opensearch-sql_test_index_account", + "queries": { + "missing-wildcard-source": { + "role": "trigger", + "query": "source={{index}}-definitely-missing-* | head 1" + }, + "matching-wildcard-control": { + "role": "control", + "query": "source={{index}}* | head 1" + } + }, + "expectations": [ + { + "version": ">=0.0.0", + "queries": { + "missing-wildcard-source": { + "frontend": { + "count": 1, + "severity": "info", + "matchMessage": "matches no known index" + }, + "backends": { + "standard": { + "kind": "rejection", + "httpStatus": 404, + "body": { + "status": 404 + } + }, + "analytics": { + "kind": "rejection", + "httpStatus": 404, + "body": { + "status": 404 + } + } + } + }, + "matching-wildcard-control": { + "frontend": { + "count": 0 + }, + "backends": { + "standard": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + }, + "analytics": { + "kind": "result-shape", + "httpStatus": 200, + "expect": { + "datarowsNonEmpty": true + } + } + } + } + } + } + ] +} diff --git a/integ-test/src/test/resources/ppl_lint_disabled_object.json b/integ-test/src/test/resources/ppl_lint_disabled_object.json new file mode 100644 index 00000000000..30759d7a585 --- /dev/null +++ b/integ-test/src/test/resources/ppl_lint_disabled_object.json @@ -0,0 +1,4 @@ +{"index":{"_id":"1"}} +{"session":{"id":"abc","raw":"not-indexed"},"status":"ok"} +{"index":{"_id":"2"}} +{"session":{"id":"def","raw":"not-indexed"},"status":"error"} diff --git a/scripts/ppl-lint/README.md b/scripts/ppl-lint/README.md index 79bd5d8f1dc..f79a3052513 100644 --- a/scripts/ppl-lint/README.md +++ b/scripts/ppl-lint/README.md @@ -1,8 +1,8 @@ # PPL lint rule validation -A required, cross-repository GitHub Actions check that proves the OpenSearch -Dashboards (OSD) PPL lint detectors and the SQL backend still agree — on the -**same candidate runtime grammar** built by a SQL pull request. +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. 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, @@ -32,10 +32,10 @@ backend-validation ──(target.json, ppl-grammar-bundle.json, backend-report.j 2. **detector-validation** (`ubuntu-latest`). Checks out and bootstraps OSD as a Node code dependency (no OSD server, no Monaco, no browser), then runs [`run-frontend-contract.mjs`](run-frontend-contract.mjs). That runner - deserializes the candidate bundle through OSD's production headless lint API - (`src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint`) and lints - each query with the **real** detectors on the **candidate** grammar. It then - asserts the detector-vs-backend differential. + deserializes the candidate bundle through OSD's production headless APIs and + runs each query with either the real lint detectors or the shared runtime + syntax listener on the **candidate** grammar. It then asserts the + frontend-vs-backend differential. 3. **validation-result**. `if: always()`, `needs: [backend-validation, detector-validation]`. Fails unless both succeeded — so a skipped detector (because the backend failed first) still reds the check instead of looking @@ -50,11 +50,12 @@ 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 | -Every contract declares `schedule: "pr"`, so a PR run exercises the **whole corpus** -— 11 rules, 35 queries. A contract that runs also asserts: neither the IT nor the -detector runner consults the manifest's `enforced` list, so any contract on the PR -schedule can fail the required check. Keep that in mind when adding one; a new -contract whose oracle has not settled should say `schedule: "nightly"` until it has. +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. `workflow_dispatch` inputs: @@ -65,7 +66,7 @@ contract whose oracle has not settled should say `schedule: "nightly"` until it an immutable commit SHA and recorded in the run manifest. A manual run **cannot** satisfy branch protection; merge the OSD change first, then rerun the required `pull_request` check against OSD `main`. -- `schedule` — `pr` (fast blocking subset) or `nightly` (full corpus). +- `schedule` — `pr` (reviewed blocking contracts) or `nightly` (all active contracts). To validate an OSD change that is not yet merged, push it to a branch on your OSD fork and dispatch with `osd_repo=/OpenSearch-Dashboards` and @@ -124,15 +125,17 @@ writes `detector-report.json`. ## Contract format (schema v3 and v4) -One JSON file per rule under `contracts/`, listed in `manifest.json`. Each file -has a top-level `queries` map (each `{ role: "trigger"|"control", query }`) and a -version-scoped `expectations[]`. Exactly one expectation must match the candidate -backend version (zero or more than one fails before any query runs). +One JSON file per rule or syntax feature under `contracts/`, listed in +`manifest.json`. Each file has `channel: "lint"|"syntax"` (missing defaults to +`lint`), a top-level `queries` map, and version-scoped `expectations[]`. +`suppression-control` is syntax-only: the frontend must retain a raw syntax error +without producing the contracted friendly rewrite. ```jsonc { "schemaVersion": 4, "ruleId": "union-min-datasets", + "channel": "lint", "grammarSurface": "runtime-bundle", "schedule": "pr", "wiring": { "detector": "union-min-datasets", "enabled": true, "severity": "error", ... }, @@ -149,14 +152,14 @@ backend version (zero or more than one fails before any query runs). "engine": "calcite", "queries": { "union-single-dataset": { - "detectorCount": 1, "severity": "error", + "frontend": { "count": 1, "severity": "error" }, "backends": { "standard": { "kind": "rejection", "httpStatus": 400, "body": { "status": 400, "error": { "type": "IllegalArgumentException" } } }, "analytics": { "kind": "rejection", "httpStatus": 400, "body": { "status": 400, "error": { "type": "IllegalArgumentException" } } } } }, "union-two-datasets-control": { - "detectorCount": 0, + "frontend": { "count": 0 }, "backends": { "standard": { "kind": "result-shape", "httpStatus": 200, "expect": { "datarowsNonEmpty": true } }, "analytics": { "kind": "result-shape", "httpStatus": 200, "expect": { "datarowsNonEmpty": true } } @@ -168,6 +171,10 @@ backend version (zero or more than one fails before any query runs). } ``` +Legacy lint expectations using `detectorCount`, `severity`, and `matchMessage` +normalize to the same internal frontend oracle. Syntax expectations use +`frontend.code`, `fixText`, `rawMessage`, and `totalErrors`. + Schema v3's `backend` is read only as `backends.standard`; it is never an implicit analytics oracle. Schema v4's `backends` selects the configured `standard` or `analytics` execution backend. Missing analytics oracles are @@ -201,32 +208,21 @@ rule cannot be validated end to end. `manifest.json` partitions the corpus: -- `enforced` — reviewed error rules with a deterministic backend rejection and a - valid negative control. These block `validation-result` on the single-version - check: `invalid-capture-group-name`, - `unsupported-window-function-in-eventstats`, `multisearch-min-subsearch`, - `union-min-datasets`, `replace-wildcard-asymmetry`. +- `enforced` — the six reviewed detector error contracts with deterministic + backend behavior. - `defaultError` — every rule that ships **enabled at error severity** in OSD's - `rules_catalog.json`. This is the set the **multi-version** check enforces (see - below). It is a superset of `enforced`, adding `field-validation` and - `flat-object-subfield`. -- `pendingReview` — error rules awaiting Peng/Chen usefulness review before - joining `enforced`. Empty: `field-validation` and `flat-object-subfield` are now - pinned across versions by the multi-version check, but stay out of the - single-version `enforced` set because their backend oracle is a semantic - `Field [...] not found.` rejection they share with each other rather than a - rule-unique grammar rejection. -- `nonEnforcing` — warning/info/advisory/result-shape rules. Their oracle is weaker - than a clean rejection: an advisory rule's query *succeeds*, so the contract can - only assert a result shape or plain acceptance, which is likelier to move for - reasons unrelated to the lint rule (`dedup-consecutive` depends on the - Calcite-to-v2 fallback staying on). These ran nightly-only until every contract - moved to the PR schedule, so they now block like any other. A red result here is - worth checking against the oracle before editing a rule. - -The `enforced` / `nonEnforcing` split therefore describes **oracle quality and review -status, not blocking behavior** — it tells a reviewer how much to trust a red result, -not whether one can occur. + `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 + not count as active shipping coverage and explicitly force-enable their rule. + +The `enforced` / `nonEnforcing` split describes **oracle quality and review +status, not blocking behavior**. ## Multi-version validation @@ -412,6 +408,12 @@ different places a developer looks: 2. **The job summary** — the rule × version table plus the full grouped remediation report, which stays the authoritative account. +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 +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 remediation. Severity is not cosmetic: diff --git a/scripts/ppl-lint/__tests__/annotate.test.mjs b/scripts/ppl-lint/__tests__/annotate.test.mjs index 357b046d47c..1209c1b0637 100644 --- a/scripts/ppl-lint/__tests__/annotate.test.mjs +++ b/scripts/ppl-lint/__tests__/annotate.test.mjs @@ -8,6 +8,7 @@ import test from 'node:test'; import { buildAnnotations, + buildRequiredAnnotations, contractRepoPath, findExpectationLine, findRuleIdLine, @@ -37,6 +38,22 @@ const CONTRACT = `{ `; const readStub = (text) => () => text; +const REQUIRED_MANIFEST = `{ + "schemaVersion": 4, + "contracts": [ + "invalid-capture-group-name.spec.json" + ], + "defaultError": [ + "invalid-capture-group-name.spec.json" + ] +} +`; +const readRequired = (_dir, file) => + file === 'manifest.json' + ? REQUIRED_MANIFEST + : file === 'invalid-capture-group-name.spec.json' + ? CONTRACT + : undefined; test('anchors on the expectation entry that drifted, not the first one', () => { assert.equal(findExpectationLine(CONTRACT, '>=3.7.0'), 14); @@ -211,6 +228,69 @@ test('an unreadable contract still produces a file-less annotation', () => { assert.equal(annotations[0].file, 'c/gone.spec.json'); }); +test('required-lane failures anchor to their contract and census findings to the manifest', () => { + const annotations = buildRequiredAnnotations( + { + detectorFailures: [ + '[invalid-capture-group-name/trigger] expected 1 diagnostic, got 0', + ], + censusProblems: ['active lint contracts do not equal enabled catalog rules'], + censusEnforced: false, + }, + { + contractsDir: '/w/integ-test/resources/contracts', + workspace: '/w', + readFile: readRequired, + } + ); + + assert.equal(annotations.length, 2); + assert.deepEqual( + { + level: annotations[0].level, + file: annotations[0].file, + line: annotations[0].line, + }, + { + level: 'error', + file: 'integ-test/resources/contracts/invalid-capture-group-name.spec.json', + line: 3, + } + ); + assert.equal(annotations[1].level, 'warning'); + assert.equal(annotations[1].file, 'integ-test/resources/contracts/manifest.json'); + assert.match(annotations[1].message, /REPORT ONLY/); +}); + +test('required artifact row failures recover the rule identity for an inline annotation', () => { + const annotations = buildRequiredAnnotations( + { + artifactErrors: [ + 'backend row invalid-capture-group-name::trigger did not pass its oracle (outcome="fail")', + ], + }, + { + contractsDir: '/w/contracts', + workspace: '/w', + readFile: readRequired, + } + ); + + assert.equal(annotations.length, 1); + assert.equal(annotations[0].file, 'contracts/invalid-capture-group-name.spec.json'); + assert.match(annotations[0].title, /invalid-capture-group-name\/trigger/); +}); + +test('required job failures without a contract identity remain file-less', () => { + const annotations = buildRequiredAnnotations( + { backendResult: 'failure', detectorResult: 'skipped' }, + { contractsDir: '/w/contracts', workspace: '/w', readFile: readRequired } + ); + assert.equal(annotations.length, 2); + assert.ok(annotations.every((annotation) => annotation.file === undefined)); +}); + test('a clean report emits nothing', () => { assert.deepEqual(buildAnnotations({}, { contractsDir: '/w/c', workspace: '/w' }), []); + assert.deepEqual(buildRequiredAnnotations({}, { contractsDir: '/w/c', workspace: '/w' }), []); }); diff --git a/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs b/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs index 1c8e1f4b21d..a3ee6f10acd 100644 --- a/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs +++ b/scripts/ppl-lint/__tests__/assemble-run-manifest.test.mjs @@ -172,3 +172,18 @@ test('detector severity and message mismatches fail the manifest and summary', ( assert.match(fs.readFileSync(summary, 'utf8'), /advisory-rule.*accepted.*Fail/); } }); + +test('syntax-specific frontend mismatches fail artifact validation', () => { + for (const field of ['fixMatched', 'rawMessageMatched', 'totalErrorsMatched']) { + 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; + fs.writeFileSync(file, JSON.stringify(detector)); + + const result = run(dir); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /did not match its (fix|raw-message|total-error) assertion/); + } +}); diff --git a/scripts/ppl-lint/__tests__/contract-schema.test.mjs b/scripts/ppl-lint/__tests__/contract-schema.test.mjs index 1f4516ca83b..914dd31569f 100644 --- a/scripts/ppl-lint/__tests__/contract-schema.test.mjs +++ b/scripts/ppl-lint/__tests__/contract-schema.test.mjs @@ -10,7 +10,10 @@ import { assertContractSchema, assertExactQueryCoverage, classifyBackendReportRow, + contractChannel, indexBackendReport, + normalizeFrontendOracle, + normalizeLintWiring, normalizeTarget, resolveBackendOracle, } from '../contract-schema.mjs'; @@ -489,3 +492,151 @@ test('schema-v2 standard backend rows cannot omit identity', () => { /does not match target "standard"/ ); }); + +test('missing channel remains a backwards-compatible lint contract', () => { + const contract = spec(4, { + detectorCount: 1, + severity: 'warning', + backends: { + standard: { kind: 'advisory', httpStatus: 200 }, + }, + }); + assert.equal(contractChannel(contract), 'lint'); + assert.deepEqual( + normalizeFrontendOracle(contract, contract.expectations[0].queries.trigger), + { + channel: 'lint', + count: 1, + severity: 'warning', + matchMessage: undefined, + } + ); +}); + +test('syntax frontend assertions normalize stable code, fix, raw message, and error census', () => { + const contract = { + schemaVersion: 4, + ruleId: 'command-suggestion', + channel: 'syntax', + wiring: { code: 'UNKNOWN_COMMAND' }, + queries: { + trigger: { role: 'trigger', query: 'source=t | wherre a > 1' }, + }, + }; + const frontend = normalizeFrontendOracle(contract, { + frontend: { + count: 1, + code: 'UNKNOWN_COMMAND', + fixText: 'where', + matchMessage: 'where', + rawMessage: true, + totalErrors: 1, + }, + }); + assert.deepEqual(frontend, { + channel: 'syntax', + count: 1, + code: 'UNKNOWN_COMMAND', + fixText: 'where', + matchMessage: 'where', + rawMessage: true, + totalErrors: 1, + }); + assert.equal(assertContractSchema(contract), 4); +}); + +test('lint and syntax frontend fields cannot cross channels', () => { + assert.throws( + () => + normalizeFrontendOracle(spec(4), { + frontend: { count: 1, code: 'UNKNOWN_COMMAND' }, + }), + /code is not valid/ + ); + const syntax = { + schemaVersion: 4, + ruleId: 'command-suggestion', + channel: 'syntax', + wiring: { code: 'UNKNOWN_COMMAND' }, + queries: { + trigger: { role: 'trigger', query: 'source=t | wherre a > 1' }, + }, + }; + assert.throws( + () => normalizeFrontendOracle(syntax, { detectorCount: 1 }), + /must use frontend/ + ); + assert.throws( + () => + assertContractSchema({ + ...syntax, + wiring: { code: 'UNKNOWN_COMMAND', detector: 'command-suggestion' }, + }), + /must contain only/ + ); +}); + +test('suppression-control is syntax-only', () => { + assert.throws( + () => + assertContractSchema({ + ...spec(4), + queries: { + suppressed: { + role: 'suppression-control', + query: 'source=t | zzzzzzzz', + }, + }, + }), + /valid only for syntax/ + ); + assert.doesNotThrow(() => + assertContractSchema({ + schemaVersion: 4, + ruleId: 'command-suggestion', + channel: 'syntax', + wiring: { code: 'UNKNOWN_COMMAND' }, + queries: { + suppressed: { + role: 'suppression-control', + query: 'source=t | zzzzzzzz', + }, + }, + }) + ); +}); + +test('normalized wiring exposes omitted version, engine, and source scope gates', () => { + const catalog = normalizeLintWiring('example-rule', { + detector: 'example-rule', + enabled: true, + severity: 'warning', + appliesTo: { minVersion: '3.7.0', engine: 'calcite' }, + sourceScoped: true, + }); + const omittedVersion = normalizeLintWiring('example-rule', { + detector: 'example-rule', + enabled: true, + severity: 'warning', + appliesTo: { engine: 'calcite' }, + sourceScoped: true, + }); + const omittedEngine = normalizeLintWiring('example-rule', { + detector: 'example-rule', + enabled: true, + severity: 'warning', + appliesTo: { minVersion: '3.7.0' }, + sourceScoped: true, + }); + const omittedSourceScope = normalizeLintWiring('example-rule', { + detector: 'example-rule', + enabled: true, + severity: 'warning', + appliesTo: { minVersion: '3.7.0', engine: 'calcite' }, + }); + + assert.notDeepEqual(omittedVersion, catalog); + assert.notDeepEqual(omittedEngine, catalog); + assert.notDeepEqual(omittedSourceScope, catalog); + assert.equal(omittedSourceScope.sourceScoped, false); +}); diff --git a/scripts/ppl-lint/__tests__/harvest-queries.test.mjs b/scripts/ppl-lint/__tests__/harvest-queries.test.mjs index 42539f4529e..4d50c56155d 100644 --- a/scripts/ppl-lint/__tests__/harvest-queries.test.mjs +++ b/scripts/ppl-lint/__tests__/harvest-queries.test.mjs @@ -17,9 +17,13 @@ */ import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import { test } from 'node:test'; import { + findTestFiles, harvestContext, harvestFile, referencedIdentifiers, @@ -36,6 +40,38 @@ const RULES = [ 'division-by-zero', ]; +test('test discovery recurses through current rule locations and excludes generated data', () => { + const osd = fs.mkdtempSync(path.join(os.tmpdir(), 'ppl-lint-harvest-')); + const lintRoot = path.join(osd, 'packages/osd-monaco/src/ppl/lint'); + const files = [ + 'rules/inline_rule.test.ts', + 'rules/__tests__/nested_rule.test.tsx', + '__tests__/catalog.test.ts', + 'rules/__fixtures__/fixture.test.ts', + 'rules/__snapshots__/snapshot.test.ts', + 'generated/generated.test.ts', + 'rules/slow.bench.test.ts', + 'rules/manual.verify.test.ts', + ]; + try { + for (const file of files) { + const absolute = path.join(lintRoot, file); + fs.mkdirSync(path.dirname(absolute), { recursive: true }); + fs.writeFileSync(absolute, ''); + } + assert.deepEqual( + findTestFiles(osd).map((file) => path.relative(lintRoot, file)), + [ + '__tests__/catalog.test.ts', + 'rules/__tests__/nested_rule.test.tsx', + 'rules/inline_rule.test.ts', + ] + ); + } finally { + fs.rmSync(osd, { recursive: true, force: true }); + } +}); + // --- attribution ------------------------------------------------------------- test('an exact describe title names its rule', () => { diff --git a/scripts/ppl-lint/aggregate-versions.mjs b/scripts/ppl-lint/aggregate-versions.mjs index 973b1f8728f..a8c29560fb1 100644 --- a/scripts/ppl-lint/aggregate-versions.mjs +++ b/scripts/ppl-lint/aggregate-versions.mjs @@ -39,6 +39,7 @@ import { assertExactQueryCoverage, assertExecutionBackend, classifyBackendReportRow, + contractChannel, indexBackendReport, normalizeTarget, resolveBackendOracle, @@ -214,6 +215,22 @@ function normalizeDetectorReport(detector, target) { if (!Array.isArray(detector.defaultErrorRules)) { throw new TypeError('detector report.defaultErrorRules must be a JSON array'); } + for (const field of ['enabledRules', 'requiredSyntaxFeatures', 'activeContractRules']) { + if (detector[field] === undefined) continue; + if (!Array.isArray(detector[field])) { + throw new TypeError(`detector report.${field} must be a JSON array`); + } + const values = new Set(); + for (const value of detector[field]) { + if (typeof value !== 'string' || value.length === 0) { + throw new TypeError(`detector report.${field} entries must be non-empty strings`); + } + if (values.has(value)) { + throw new Error(`detector report.${field} contains duplicate rule "${value}"`); + } + values.add(value); + } + } const census = new Set(); for (const ruleId of detector.defaultErrorRules) { if (typeof ruleId !== 'string' || ruleId.length === 0) { @@ -301,7 +318,14 @@ function reportItemKey(item, kind) { function loadContracts(dir) { const manifest = readJson(path.join(dir, 'manifest.json')); const specs = new Map(); - for (const name of manifest.contracts || []) { + const contractNames = manifest.contracts || []; + if (!Array.isArray(contractNames)) { + fatal(`${path.join(dir, 'manifest.json')} contracts must be an array`); + } + if (new Set(contractNames).size !== contractNames.length) { + fatal(`${path.join(dir, 'manifest.json')} contracts contains duplicate file names`); + } + for (const name of contractNames) { const spec = readJson(path.join(dir, name)); try { assertContractSchema(spec); @@ -318,12 +342,36 @@ function loadContracts(dir) { } catch (error) { artifactFatal(path.join(dir, name), error); } + if (specs.has(spec.ruleId)) { + fatal(`contract manifest contains duplicate ruleId "${spec.ruleId}"`); + } specs.set(spec.ruleId, { spec, file: name }); } // `defaultError` is the multi-version enforced set: every rule that ships // enabled at error severity. Fall back to `enforced` for older manifests so // this script still runs against an un-migrated corpus. const enforcedFiles = new Set(manifest.defaultError || manifest.enforced || []); + for (const file of enforcedFiles) { + if (!contractNames.includes(file)) { + fatal(`manifest.defaultError references inactive or missing contract "${file}"`); + } + } + const requiredSyntaxFiles = manifest.requiredSyntaxFeatures || []; + if (!Array.isArray(requiredSyntaxFiles)) { + fatal('manifest.requiredSyntaxFeatures must be an array'); + } + if (new Set(requiredSyntaxFiles).size !== requiredSyntaxFiles.length) { + fatal('manifest.requiredSyntaxFeatures contains duplicate file names'); + } + for (const file of requiredSyntaxFiles) { + const entry = [...specs.values()].find((candidate) => candidate.file === file); + if (!entry) { + fatal(`manifest.requiredSyntaxFeatures references inactive or missing contract "${file}"`); + } + if (contractChannel(entry.spec) !== 'syntax') { + fatal(`manifest.requiredSyntaxFeatures entry "${file}" is not a syntax contract`); + } + } const enforcedRules = new Set(); for (const [ruleId, { file }] of specs) { if (enforcedFiles.has(file)) enforcedRules.add(ruleId); @@ -515,6 +563,7 @@ function pairBackendLegs(legs) { function detectorParityValue(entry) { return { + channel: entry.channel || 'lint', role: entry.role || 'trigger', query: entry.query || '', expected: entry.expected, @@ -524,6 +573,14 @@ function detectorParityValue(entry) { typeof entry.severityMatched === 'boolean' ? entry.severityMatched : undefined, messageMatched: typeof entry.messageMatched === 'boolean' ? entry.messageMatched : undefined, + fixMatched: typeof entry.fixMatched === 'boolean' ? entry.fixMatched : undefined, + rawMessageMatched: + typeof entry.rawMessageMatched === 'boolean' ? entry.rawMessageMatched : undefined, + totalErrorsMatched: + typeof entry.totalErrorsMatched === 'boolean' ? entry.totalErrorsMatched : undefined, + code: entry.code, + codes: entry.codes, + totalErrors: entry.totalErrors, }; } @@ -660,9 +717,109 @@ function auditDefaultErrorCensus(legs, specs, enforcedRules) { }); } } + for (const ruleId of [...enforcedRules].sort()) { + if (!census.has(ruleId)) { + missing.push({ + ruleId, + reason: 'listed under manifest.defaultError but not enabled at error severity in OSD', + }); + } + } return missing; } +function setsEqual(left, right) { + return left.size === right.size && [...left].every((value) => right.has(value)); +} + +function auditShippingCensus(legs, specs, manifest) { + const reports = legs + .map((leg) => leg.detector) + .filter( + (detector) => + Array.isArray(detector.enabledRules) && + Array.isArray(detector.activeContractRules) && + Array.isArray(detector.requiredSyntaxFeatures) + ); + if (reports.length === 0) { + return { + available: false, + enforced: false, + passed: false, + problems: [ + 'detector reports predate the active shipping census; rerun with the channel-aware frontend runner', + ], + }; + } + + const activeRules = new Set(specs.keys()); + const activeLintRules = new Set( + [...specs.entries()] + .filter(([, { spec }]) => contractChannel(spec) === 'lint') + .map(([ruleId]) => ruleId) + ); + const activeSyntaxRules = new Set( + [...specs.entries()] + .filter(([, { spec }]) => contractChannel(spec) === 'syntax') + .map(([ruleId]) => ruleId) + ); + const requiredSyntaxRules = new Set( + (manifest.requiredSyntaxFeatures || []) + .map((file) => [...specs.entries()].find(([, entry]) => entry.file === file)) + .filter(Boolean) + .map(([ruleId]) => ruleId) + ); + const enabledRules = new Set(reports.flatMap((report) => report.enabledRules)); + const reportedActiveRules = new Set( + reports.flatMap((report) => report.activeContractRules) + ); + const reportedSyntaxRules = new Set( + reports.flatMap((report) => report.requiredSyntaxFeatures) + ); + const problems = []; + + 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 (activeRules.size !== 13) { + problems.push(`expected 13 active contracts, found ${activeRules.size}`); + } + if (!setsEqual(activeLintRules, enabledRules)) { + problems.push( + `active lint rules ${JSON.stringify([...activeLintRules].sort())} do not equal enabled OSD ` + + `rules ${JSON.stringify([...enabledRules].sort())}` + ); + } + if (!setsEqual(activeSyntaxRules, requiredSyntaxRules)) { + problems.push( + `active syntax rules ${JSON.stringify([...activeSyntaxRules].sort())} do not equal manifest ` + + `required syntax features ${JSON.stringify([...requiredSyntaxRules].sort())}` + ); + } + if (!setsEqual(activeRules, reportedActiveRules)) { + problems.push('detector report activeContractRules does not match this manifest'); + } + if (!setsEqual(requiredSyntaxRules, reportedSyntaxRules)) { + problems.push('detector report requiredSyntaxFeatures does not match this manifest'); + } + + return { + available: true, + enforced: reports.some( + (report) => report.census && report.census.enforced === true + ), + enabledRules: [...enabledRules].sort(), + activeContractRules: [...activeRules].sort(), + activeLintRules: [...activeLintRules].sort(), + requiredSyntaxFeatures: [...requiredSyntaxRules].sort(), + passed: problems.length === 0, + problems, + }; +} + /** * Read one backend report entry into an observation, distinguishing "the engine * accepted this" from "we never got an answer". @@ -877,6 +1034,16 @@ function main() { // the census each detector leg recorded from the OSD catalog it linted with, so // a new default-error rule cannot land unvalidated. const missingContracts = auditDefaultErrorCensus(legs, specs, enforcedRules); + const shippingCensus = auditShippingCensus(legs, specs, manifest); + const blockCensusDrift = !shippingCensus.available || shippingCensus.enforced; + for (const entry of missingContracts) { + entry.blocking = blockCensusDrift; + } + if (!shippingCensus.passed) { + for (const problem of shippingCensus.problems) { + log(`CENSUS REPORT-ONLY: ${problem}`); + } + } for (const [ruleId, { spec, file }] of specs) { const isEnforced = enforcedRules.has(ruleId); @@ -1366,6 +1533,26 @@ function main() { } } + for (const collection of [drifts, coverageHoles, inconclusive, notApplicable, matrix]) { + for (const entry of collection) { + const contract = specs.get(entry.ruleId); + entry.channel = contract ? contractChannel(contract.spec) : 'lint'; + } + } + for (const drift of drifts) { + if (drift.channel !== 'syntax') continue; + drift.remediation = { + action: 'review-syntax-validation', + target: + 'OSD runtime_validation_core and the syntax contract expectation', + detail: + `Reproduce "${drift.ruleId}" with the candidate runtime grammar. Update the shared OSD ` + + `parser/listener core if UNKNOWN_COMMAND identity, suppression, or quick-fix behavior ` + + `regressed; update this contract only after confirming an intentional syntax UX change. ` + + `Do not change detector catalog appliesTo metadata for a syntax-channel failure.`, + }; + } + const isObservedAnalyticsFinding = (entry) => args.observeAnalytics && (entry.executionBackend === 'analytics' || @@ -1375,10 +1562,12 @@ function main() { entry.driftClass === DRIFT_CLASSES.BACKEND_ORACLE_MISMATCH || entry.kind === 'backend-oracle'); for (const drift of drifts) { - drift.blocking = !!drift.enforced && !isObservedAnalyticsFinding(drift); + drift.blocking = + (!!drift.enforced || args.allRules) && !isObservedAnalyticsFinding(drift); } for (const hole of coverageHoles) { - hole.blocking = !!hole.enforced && !isObservedAnalyticsFinding(hole); + hole.blocking = + (!!hole.enforced || args.allRules) && !isObservedAnalyticsFinding(hole); } const enforcedDrifts = drifts.filter((d) => d.blocking); const enforcedHoles = coverageHoles.filter((h) => h.blocking); @@ -1387,6 +1576,7 @@ function main() { // verdict is an infrastructure failure for every rule we asked the run to // observe. const enforcedInconclusive = inconclusive.filter((i) => i.enforced || args.allRules); + const blockingMissingContracts = missingContracts.filter((entry) => entry.blocking); for (const row of matrix) { row.key = reportItemKey(row, 'matrix'); } @@ -1421,6 +1611,7 @@ function main() { })), enforcedRules: [...enforcedRules].sort(), missingContracts, + shippingCensus, manifestDescription: manifest.description || '', matrix, drifts, @@ -1435,13 +1626,14 @@ function main() { drifts.filter((d) => d.enforced && !d.blocking).length + coverageHoles.filter((h) => h.enforced && !h.blocking).length, missingContractCount: missingContracts.length, + blockingMissingContractCount: blockingMissingContracts.length, enforcedInconclusive: enforcedInconclusive.length, // An inconclusive default-error rule fails too: "we could not check" must // never render as "it is fine". passed: enforcedDrifts.length === 0 && enforcedHoles.length === 0 && - missingContracts.length === 0 && + blockingMissingContracts.length === 0 && enforcedInconclusive.length === 0, }, }; @@ -1475,13 +1667,14 @@ function main() { // eslint-disable-next-line no-console console.error( `[ppl-lint-multiversion] FAIL: ${enforcedDrifts.length} drift(s), ` + - `${enforcedHoles.length} coverage hole(s), ${missingContracts.length} unvalidated ` + + `${enforcedHoles.length} coverage hole(s), ${blockingMissingContracts.length} unvalidated ` + `default-error rule(s) and ${enforcedInconclusive.length} inconclusive rule/version pair(s).` ); process.exit(1); } log( - `PASS: every default-error rule agrees with all ${legs.length} engine version(s)` + + `PASS: every ${args.allRules ? 'active shipping' : 'default-error'} rule agrees with all ` + + `${legs.length} engine version(s)` + (drifts.length > 0 ? ` (${drifts.length} non-enforced finding(s) reported)` : '') + '.' ); diff --git a/scripts/ppl-lint/annotate.mjs b/scripts/ppl-lint/annotate.mjs index 422177ce204..9c9d6bf2c16 100644 --- a/scripts/ppl-lint/annotate.mjs +++ b/scripts/ppl-lint/annotate.mjs @@ -4,7 +4,7 @@ */ /** - * GitHub Actions annotations for the PPL lint multi-version check. + * GitHub Actions annotations for the PPL lint required and multi-version checks. * * The drift report and the job summary already say exactly what to change. The * problem is WHERE a developer looks first: GitHub renders workflow-command @@ -88,6 +88,16 @@ export function findRuleIdLine(contractText) { return undefined; } +function findJsonKeyLine(jsonText, key) { + if (!jsonText || !key) return undefined; + const lines = jsonText.split('\n'); + const pattern = new RegExp(`^\\s*${JSON.stringify(key)}\\s*:`); + for (let i = 0; i < lines.length; i++) { + if (pattern.test(lines[i])) return i + 1; + } + return undefined; +} + /** * Repo-relative path of a contract file, for `file=`. * @@ -189,7 +199,7 @@ export function buildAnnotations(report, { contractsDir, workspace, readFile = r for (const missing of report.missingContracts || []) { const ruleId = missing.ruleId || missing; annotations.push({ - level: 'error', + level: missing.blocking === false ? 'warning' : 'error', // A rule with no contract has no file to point at; the manifest is where the // reader's edit goes. file: undefined, @@ -198,13 +208,125 @@ export function buildAnnotations(report, { contractsDir, workspace, readFile = r `"${ruleId}" ships enabled at error severity in OSD's rules_catalog.json but ` + `${missing.reason || 'has no contract in this corpus'}. A default-error rule with no ` + `contract is invisible to this check. Add a contract file and list it under ` + - `manifest.defaultError, or lower the rule's severity in OSD.`, + `manifest.defaultError, or lower the rule's severity in OSD.` + + (missing.blocking === false + ? ' This compatibility phase reports the census mismatch without blocking until the paired OSD default-alignment change lands.' + : ''), }); } return annotations; } +function ruleIdentity(message) { + const bracketed = String(message).match(/^\[([A-Za-z0-9._-]+)(?:\/([A-Za-z0-9._-]+))?\]/); + if (bracketed && !['census', 'contracts', 'grammar-export', 'report'].includes(bracketed[1])) { + return { ruleId: bracketed[1], queryName: bracketed[2] }; + } + const row = String(message).match( + /\b(?:backend|detector) row ([A-Za-z0-9._-]+)::([A-Za-z0-9._-]+)\b/ + ); + return row ? { ruleId: row[1], queryName: row[2] } : {}; +} + +function loadContractFiles(contractsDir, readFile) { + const files = new Map(); + const manifestText = readFile(contractsDir, 'manifest.json'); + if (!manifestText) return { files, manifestText }; + try { + const manifest = JSON.parse(manifestText); + const names = [...(manifest.contracts || []), ...(manifest.dormantContracts || [])]; + for (const name of names) { + const text = readFile(contractsDir, name); + if (!text) continue; + try { + const spec = JSON.parse(text); + if (spec.ruleId) files.set(spec.ruleId, { name, text }); + } catch { + // Malformed contracts are reported by the schema/runner. Keep this helper + // best-effort so annotation generation never hides the original failure. + } + } + } catch { + // The manifest parse failure is itself annotated below without a line anchor. + } + return { files, manifestText }; +} + +function manifestKeyFor(message) { + if (/defaultError/.test(message)) return 'defaultError'; + if (/requiredSyntaxFeatures/.test(message)) return 'requiredSyntaxFeatures'; + if (/dormantContracts/.test(message)) return 'dormantContracts'; + return 'contracts'; +} + +/** + * Build annotations for the required single-version lane. + * + * Detector failures use their `[rule/query]` prefix to land on the owning + * contract. Census findings land on manifest.json. Job and artifact failures + * without a trustworthy repository location remain file-less. + */ +export function buildRequiredAnnotations( + report, + { contractsDir, workspace, readFile = readContract } = {} +) { + const annotations = []; + const { files, manifestText } = loadContractFiles(contractsDir, readFile); + const seen = new Set(); + + const addFailure = (message, source) => { + const text = String(message); + const { ruleId, queryName } = ruleIdentity(text); + const contract = ruleId ? files.get(ruleId) : undefined; + const census = source === 'census' || /^\[census\]/.test(text); + const file = census + ? contractRepoPath(contractsDir, 'manifest.json', workspace) + : contract + ? contractRepoPath(contractsDir, contract.name, workspace) + : undefined; + const line = census + ? findJsonKeyLine(manifestText, manifestKeyFor(text)) + : findRuleIdLine(contract?.text); + const level = census && report.censusEnforced !== true ? 'warning' : 'error'; + const title = census + ? 'PPL lint shipping census mismatch' + : ruleId + ? `PPL lint required validation: ${ruleId}${queryName ? `/${queryName}` : ''}` + : `PPL lint required validation: ${source}`; + const key = `${level}\0${file || ''}\0${line || ''}\0${title}\0${text}`; + if (seen.has(key)) return; + seen.add(key); + annotations.push({ + level, + file, + line, + title, + message: + census && report.censusEnforced !== true + ? `${text}\nREPORT ONLY: align the active SQL manifest with the approved OSD shipping catalog before enabling census enforcement.` + : text, + }); + }; + + for (const message of report.detectorFailures || []) addFailure(message, 'frontend'); + for (const message of report.artifactErrors || []) addFailure(message, 'artifact'); + for (const message of report.censusProblems || []) addFailure(message, 'census'); + + for (const [job, result] of [ + ['backend-validation', report.backendResult], + ['detector-validation', report.detectorResult], + ]) { + if (result && result !== 'success') { + addFailure( + `${job} finished with result "${result}". See that job's logs and uploaded artifacts for the underlying failure.`, + job + ); + } + } + return annotations; +} + function readContract(contractsDir, fileName) { try { return fs.readFileSync(path.join(contractsDir, fileName), 'utf8'); @@ -238,3 +360,15 @@ export function emitAnnotations(report, options = {}) { } return annotations; } + +/** Emit required-lane annotations under the same Actions-only policy. */ +export function emitRequiredAnnotations(report, options = {}) { + const enabled = options.force || process.env.GITHUB_ACTIONS === 'true'; + if (!enabled) return []; + const annotations = buildRequiredAnnotations(report, options); + for (const annotation of annotations) { + // eslint-disable-next-line no-console + console.log(formatAnnotation(annotation)); + } + return annotations; +} diff --git a/scripts/ppl-lint/assemble-run-manifest.mjs b/scripts/ppl-lint/assemble-run-manifest.mjs index faf21f078b1..4d0cd183503 100644 --- a/scripts/ppl-lint/assemble-run-manifest.mjs +++ b/scripts/ppl-lint/assemble-run-manifest.mjs @@ -27,8 +27,10 @@ import { indexBackendReport, normalizeTarget, } from './contract-schema.mjs'; +import { emitRequiredAnnotations } from './annotate.mjs'; const ARTIFACTS = 'artifacts'; +const CONTRACTS = path.resolve('integ-test/src/test/resources/ppl-lint/contracts'); function readJson(file, errors) { try { @@ -147,6 +149,15 @@ function main() { if (entry.messageMatched !== true) { artifactErrors.push(`detector row ${key} did not match its message assertion`); } + 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 [key, entry] of backendByKey) { if (!detectorKeys.has(key)) { @@ -213,6 +224,21 @@ function main() { writeSummary(manifest, detector, backend); + emitRequiredAnnotations( + { + artifactErrors, + detectorFailures: Array.isArray(detector.failures) ? detector.failures : [], + censusProblems: Array.isArray(detector.census?.problems) ? detector.census.problems : [], + censusEnforced: detector.census?.enforced === true, + backendResult, + detectorResult, + }, + { + contractsDir: CONTRACTS, + workspace: process.env.GITHUB_WORKSPACE || process.cwd(), + } + ); + if (artifactErrors.length > 0) { throw new Error(`invalid PPL lint artifacts:\n- ${artifactErrors.join('\n- ')}`); } diff --git a/scripts/ppl-lint/contract-schema.mjs b/scripts/ppl-lint/contract-schema.mjs index 291f2f9c405..ca3c5a809c2 100644 --- a/scripts/ppl-lint/contract-schema.mjs +++ b/scripts/ppl-lint/contract-schema.mjs @@ -6,6 +6,17 @@ const EXECUTION_BACKENDS = new Set(['standard', 'analytics']); 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 SYNTAX_FRONTEND_FIELDS = new Set([ + 'count', + 'code', + 'fixText', + 'matchMessage', + 'rawMessage', + 'totalErrors', +]); function isObject(value) { return value !== null && typeof value === 'object' && !Array.isArray(value); @@ -42,6 +53,164 @@ function assertOptionalString(value, label) { } } +function assertKnownKeys(value, allowed, label) { + for (const key of Object.keys(value)) { + if (!allowed.has(key)) { + throw new Error(`${label}.${key} is not valid for this contract channel.`); + } + } +} + +export function contractChannel(spec) { + requireObject(spec, 'contract'); + const channel = spec.channel === undefined ? 'lint' : spec.channel; + if (!CONTRACT_CHANNELS.has(channel)) { + throw new Error( + `contract.channel must be "lint" or "syntax", got ${describe(channel)}.` + ); + } + return channel; +} + +/** + * 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. + */ +export function normalizeFrontendOracle(spec, queryExpectation) { + const channel = contractChannel(spec); + requireObject(queryExpectation, `[${spec.ruleId}] query expectation`); + + const hasLegacy = Object.prototype.hasOwnProperty.call( + queryExpectation, + 'detectorCount' + ); + const hasFrontend = Object.prototype.hasOwnProperty.call( + queryExpectation, + 'frontend' + ); + if (hasLegacy && hasFrontend) { + throw new Error( + `[${spec.ruleId}] query expectation must use either detectorCount or frontend, not both.` + ); + } + + if (channel === 'lint') { + const frontend = hasFrontend + ? requireObject(queryExpectation.frontend, `[${spec.ruleId}] frontend`) + : { + count: queryExpectation.detectorCount, + severity: queryExpectation.severity, + matchMessage: queryExpectation.matchMessage, + }; + assertKnownKeys(frontend, LINT_FRONTEND_FIELDS, `[${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 ( + hasFrontend && + (queryExpectation.severity !== undefined || + queryExpectation.matchMessage !== undefined) + ) { + throw new Error( + `[${spec.ruleId}] severity and matchMessage must be nested under frontend when frontend is present.` + ); + } + return { + channel, + count: frontend.count, + severity: frontend.severity, + matchMessage: frontend.matchMessage, + }; + } + + if (hasLegacy) { + throw new Error( + `[${spec.ruleId}] syntax contracts must use frontend instead of detectorCount.` + ); + } + const frontend = requireObject( + queryExpectation.frontend, + `[${spec.ruleId}] frontend` + ); + 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.matchMessage !== undefined && + typeof frontend.matchMessage !== 'string' + ) { + throw new TypeError( + `[${spec.ruleId}] frontend.matchMessage must be a string when present.` + ); + } + if (frontend.rawMessage !== undefined && typeof frontend.rawMessage !== 'boolean') { + throw new TypeError(`[${spec.ruleId}] frontend.rawMessage must be a boolean.`); + } + if (frontend.totalErrors !== undefined) { + requireNonNegativeInteger( + frontend.totalErrors, + `[${spec.ruleId}] frontend.totalErrors` + ); + } + for (const field of ['severity', 'matchMessage']) { + if (Object.prototype.hasOwnProperty.call(queryExpectation, field)) { + throw new Error( + `[${spec.ruleId}] syntax ${field} must be nested under frontend.` + ); + } + } + return { channel, ...frontend }; +} + +export function normalizeLintWiring(ruleId, wiring, label = 'wiring') { + requireNonEmptyString(ruleId, `${label}.id`); + requireObject(wiring, label); + const appliesTo = + wiring.appliesTo === undefined ? {} : requireObject(wiring.appliesTo, `${label}.appliesTo`); + const normalizedAppliesTo = {}; + for (const key of ['minVersion', 'maxVersion', 'engine']) { + assertOptionalString(appliesTo[key], `${label}.appliesTo.${key}`); + if (appliesTo[key] !== undefined) { + normalizedAppliesTo[key] = appliesTo[key]; + } + } + for (const key of [ + 'runtimeOnly', + 'needsContext', + 'needsExplain', + 'sourceScoped', + ]) { + if (wiring[key] !== undefined && typeof wiring[key] !== 'boolean') { + throw new TypeError(`${label}.${key} must be a boolean when present.`); + } + } + if (typeof wiring.enabled !== 'boolean') { + throw new TypeError(`${label}.enabled must be a boolean.`); + } + return { + id: ruleId, + detector: requireNonEmptyString(wiring.detector, `${label}.detector`), + enabled: wiring.enabled, + severity: requireNonEmptyString(wiring.severity, `${label}.severity`), + appliesTo: normalizedAppliesTo, + runtimeOnly: wiring.runtimeOnly === true, + needsContext: wiring.needsContext === true, + needsExplain: wiring.needsExplain === true, + sourceScoped: wiring.sourceScoped === true, + }; +} + function assertBackendOracle(oracle, ruleId, executionBackend) { const label = `[${ruleId}] ${executionBackend} backend oracle`; requireObject(oracle, label); @@ -206,6 +375,38 @@ export function assertContractSchema(spec) { ); } requireNonEmptyString(spec.ruleId, 'contract.ruleId'); + const channel = contractChannel(spec); + if (spec.wiring !== undefined) { + const wiring = requireObject(spec.wiring, `[${spec.ruleId}] contract.wiring`); + if (channel === 'syntax') { + const keys = Object.keys(wiring); + if (keys.length !== 1 || keys[0] !== 'code') { + throw new Error( + `[${spec.ruleId}] syntax wiring must contain only the stable error code.` + ); + } + requireNonEmptyString(wiring.code, `[${spec.ruleId}] contract.wiring.code`); + } else if (Object.prototype.hasOwnProperty.call(wiring, 'code')) { + throw new Error(`[${spec.ruleId}] lint wiring must not contain syntax code.`); + } + } + if (spec.queries !== undefined) { + requireObject(spec.queries, `[${spec.ruleId}] contract.queries`); + for (const [queryName, query] of Object.entries(spec.queries)) { + requireObject(query, `[${spec.ruleId}] contract.queries.${queryName}`); + const role = query.role === undefined ? 'trigger' : query.role; + if (!QUERY_ROLES.has(role)) { + throw new Error( + `[${spec.ruleId}] query "${queryName}" has invalid role ${describe(role)}.` + ); + } + if (role === 'suppression-control' && channel !== 'syntax') { + throw new Error( + `[${spec.ruleId}] suppression-control is valid only for syntax contracts.` + ); + } + } + } return spec.schemaVersion; } @@ -251,32 +452,11 @@ export function assertExactQueryCoverage(spec, expectation) { export function resolveBackendOracle(spec, queryExpectation, executionBackend) { const schemaVersion = assertContractSchema(spec); assertExecutionBackend(executionBackend); - requireObject(queryExpectation, `[${spec.ruleId}] query expectation`); - - requireNonNegativeInteger( - queryExpectation.detectorCount, - `[${spec.ruleId}] detectorCount` - ); - if ( - Object.prototype.hasOwnProperty.call(queryExpectation, 'severity') && - (typeof queryExpectation.severity !== 'string' || - queryExpectation.severity.length === 0) - ) { - throw new TypeError( - `[${spec.ruleId}] severity must be a non-empty string when present.` - ); - } - if ( - Object.prototype.hasOwnProperty.call(queryExpectation, 'matchMessage') && - typeof queryExpectation.matchMessage !== 'string' - ) { - throw new TypeError(`[${spec.ruleId}] matchMessage must be a string when present.`); - } - + const frontend = normalizeFrontendOracle(spec, queryExpectation); const detector = { - count: queryExpectation.detectorCount, - severity: queryExpectation.severity, - matchMessage: queryExpectation.matchMessage, + count: frontend.count, + severity: frontend.severity, + matchMessage: frontend.matchMessage, }; let oracle; @@ -308,6 +488,7 @@ export function resolveBackendOracle(spec, queryExpectation, executionBackend) { status: 'coverage-missing', executionBackend, detector, + frontend, oracle: undefined, reason: missingReason, }; @@ -319,6 +500,7 @@ export function resolveBackendOracle(spec, queryExpectation, executionBackend) { status: 'not-applicable', executionBackend, detector, + frontend, oracle, reason: oracle.reason, }; @@ -328,6 +510,7 @@ export function resolveBackendOracle(spec, queryExpectation, executionBackend) { status: 'applicable', executionBackend, detector, + frontend, oracle, reason: undefined, }; diff --git a/scripts/ppl-lint/harvest-queries.mjs b/scripts/ppl-lint/harvest-queries.mjs index 82023a8b187..4a19a6b9b50 100644 --- a/scripts/ppl-lint/harvest-queries.mjs +++ b/scripts/ppl-lint/harvest-queries.mjs @@ -66,18 +66,7 @@ function fatal(message) { process.exit(2); } -/** - * Directories under an OSD checkout that hold PPL lint tests. Kept explicit - * rather than globbing the whole repo: a wide sweep would pull in queries from - * autocomplete/highlighting suites that were never written as lint trigger or - * control cases, and a query harvested from the wrong intent produces a - * "disagreement" that is really just a query nobody claimed anything about. - */ -const LINT_TEST_DIRS = [ - 'packages/osd-monaco/src/ppl/lint/__tests__', - 'packages/osd-monaco/src/ppl/lint/hover/__tests__', - 'packages/osd-monaco/src/ppl/lint/explain/__tests__', -]; +const LINT_TEST_ROOT = 'packages/osd-monaco/src/ppl/lint'; /** * Benchmarks and repro captures are excluded. Bench files hold deliberately @@ -85,6 +74,13 @@ const LINT_TEST_DIRS = [ * they would dominate the corpus with near-duplicates. */ const EXCLUDED_FILE_PATTERNS = [/\.bench\.test\.ts$/, /\.verify\.test\.ts$/]; +const EXCLUDED_DIRS = new Set([ + '__fixtures__', + '__snapshots__', + 'fixtures', + 'generated', + 'target', +]); function parseArgs(argv) { const args = { @@ -142,18 +138,25 @@ function readRuleList(value) { .filter((s) => s && !s.startsWith('#')); } -/** Every lint test file under the OSD checkout, excluding benches. */ -function findTestFiles(osdRoot) { +/** Every lint test file under the lint package, excluding generated test data. */ +export function findTestFiles(osdRoot) { const files = []; - for (const dir of LINT_TEST_DIRS) { - const abs = path.join(osdRoot, dir); - if (!fs.existsSync(abs)) continue; - for (const name of fs.readdirSync(abs)) { + const root = path.join(osdRoot, LINT_TEST_ROOT); + const visit = (dir) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + if (entry.isDirectory()) { + if (!EXCLUDED_DIRS.has(entry.name)) { + visit(path.join(dir, entry.name)); + } + continue; + } + const name = entry.name; if (!name.endsWith('.test.ts') && !name.endsWith('.test.tsx')) continue; if (EXCLUDED_FILE_PATTERNS.some((re) => re.test(name))) continue; - files.push(path.join(abs, name)); + files.push(path.join(dir, name)); } - } + }; + if (fs.existsSync(root)) visit(root); return files.sort(); } @@ -621,6 +624,20 @@ function main() { 'detector output; there are no pinned expectations and this corpus must never fail a build.', index: args.index || null, sourceFiles: files.map((f) => path.relative(args.osd, f)), + ruleCoverage: args.catalogRules + .map((ruleId) => { + const entries = kept.filter((entry) => entry.ruleId === ruleId); + return { + ruleId, + filesScanned: [ + ...new Set(entries.map((entry) => entry.source.split(':')[0])), + ].sort(), + ownedQueryCount: entries.length, + explicitException: null, + }; + }) + .sort((a, b) => a.ruleId.localeCompare(b.ruleId)), + exceptions: [], queries: kept, unowned: args.keepUnowned ? unowned : [], stats: { diff --git a/scripts/ppl-lint/label-discovery.mjs b/scripts/ppl-lint/label-discovery.mjs index 3ee6c318860..487975c2fa3 100644 --- a/scripts/ppl-lint/label-discovery.mjs +++ b/scripts/ppl-lint/label-discovery.mjs @@ -306,11 +306,29 @@ function main() { const findings = labelled.filter((l) => l.finding); const byRule = new Map(); + for (const coverage of corpus.ruleCoverage || []) { + byRule.set(coverage.ruleId, { + triggers: [], + controls: [], + unknown: [], + suppressed: 0, + files: new Set(coverage.filesScanned || []), + explicitException: coverage.explicitException || null, + }); + } for (const row of labelled) { if (!byRule.has(row.ruleId)) { - byRule.set(row.ruleId, { triggers: [], controls: [], unknown: [], suppressed: 0 }); + byRule.set(row.ruleId, { + triggers: [], + controls: [], + unknown: [], + suppressed: 0, + files: new Set(), + explicitException: null, + }); } const bucket = byRule.get(row.ruleId); + if (row.source) bucket.files.add(row.source.split(':')[0]); if (row.suppressed) bucket.suppressed++; if (row.role === ROLES.TRIGGER) bucket.triggers.push(row.queryName); else if (row.role === ROLES.CONTROL) bucket.controls.push(row.queryName); @@ -347,10 +365,16 @@ function main() { triggerCoverage: [...byRule] .map(([ruleId, b]) => ({ ruleId, + filesScanned: [...b.files].sort(), triggers: b.triggers.length, controls: b.controls.length, unknown: b.unknown.length, suppressed: b.suppressed, + unattributedQueryCount: (corpus.stats && corpus.stats.unowned) || 0, + explicitException: b.explicitException, + coverageSatisfied: + b.triggers.length + b.controls.length + b.unknown.length > 0 || + !!b.explicitException, // Below two triggers, "every trigger relaxed" is a single observation and // cannot support a version-scoping decision. Flagged so the gap is visible // rather than implied by a number nobody reads. diff --git a/scripts/ppl-lint/run-frontend-contract.mjs b/scripts/ppl-lint/run-frontend-contract.mjs index 7bd7de2ce61..305f55fdbb6 100644 --- a/scripts/ppl-lint/run-frontend-contract.mjs +++ b/scripts/ppl-lint/run-frontend-contract.mjs @@ -82,7 +82,9 @@ import { assertContractSchema, assertExactQueryCoverage, classifyBackendReportRow, + contractChannel, indexBackendReport, + normalizeLintWiring, normalizeTarget, resolveBackendOracle, } from './contract-schema.mjs'; @@ -90,6 +92,8 @@ import { // OSD's Node-safe headless lint API (design §4.3). Deep-path module; resolved // against the OSD checkout root, not this script's SQL-repo location. const HEADLESS_MODULE = 'src/plugins/data/public/antlr/opensearch_ppl/headless_ppl_lint'; +const SYNTAX_MODULE = + 'src/plugins/data/public/antlr/opensearch_ppl/runtime_validation_core'; // The COMPILED-simplified surface: OSD's own checked-in grammar, used when the // engine cannot export a runtime bundle. See `PPL_LINT_SURFACE` below. const ANALYZER_MODULE = 'packages/osd-monaco/src/ppl/ppl_language_analyzer'; @@ -174,7 +178,12 @@ function loadContracts() { if (!fs.existsSync(single)) { fatal(`Contract file not found: ${single}`); } - return [loadContractFile(single)]; + const contract = loadContractFile(single); + return { + contracts: [contract], + manifest: { contracts: [path.basename(single)] }, + manifestPath: '', + }; } if (!dir) { @@ -186,8 +195,8 @@ function loadContracts() { const manifestPath = path.join(dir, 'manifest.json'); let files; + let manifest; if (fs.existsSync(manifestPath)) { - let manifest; try { manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); } catch (error) { @@ -196,6 +205,9 @@ function loadContracts() { 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.`); + } files = manifest.contracts.map((name) => path.join(dir, name)); } else { files = fs @@ -205,12 +217,13 @@ function loadContracts() { .map((f) => path.join(dir, f)); } - return files.map((file) => { + const contracts = files.map((file) => { if (!fs.existsSync(file)) { fatal(`Contract referenced by manifest not found: ${file}`); } return loadContractFile(file); }); + return { contracts, manifest: manifest || { contracts: files.map(path.basename) }, manifestPath }; } function loadOsd() { @@ -286,11 +299,17 @@ function loadOsd() { `Is the OSD checkout on a branch that ships the headless API (design §4.3)?` ); } + const syntaxModule = resolveOsd(SYNTAX_MODULE, { optional: true }); + const validateSyntax = + syntaxModule && typeof syntaxModule.validateQueryWithBundle === 'function' + ? syntaxModule.validateQueryWithBundle + : undefined; return { surface: SURFACE, deserializeBundleOrThrow, lintQuery: lintQueryWithBundle, + validateSyntax, getBundledCatalog, getDetector, osdRoot, @@ -466,6 +485,9 @@ function selectExpectation(spec, version, isCalcite, failures, { allowMissing = */ function checkWiring(spec, catalog, getDetector, failures) { const { ruleId, wiring } = spec; + if (contractChannel(spec) === 'syntax') { + return { id: ruleId, syntaxCode: wiring && wiring.code }; + } const entry = catalog.find((c) => c.id === ruleId); if (!entry) { failures.push(`[${ruleId}] not present in the OSD bundled catalog.`); @@ -475,31 +497,20 @@ function checkWiring(spec, catalog, getDetector, failures) { return entry; // no wiring block to assert } - const checks = [ - ['detector', wiring.detector, entry.detector], - ['enabled', wiring.enabled, entry.enabled], - ['severity', wiring.severity, entry.severity], - ['runtimeOnly', !!wiring.runtimeOnly, !!entry.runtimeOnly], - ['needsContext', !!wiring.needsContext, !!entry.needsContext], - ['needsExplain', !!wiring.needsExplain, !!entry.needsExplain], - ]; - for (const [name, expected, actual] of checks) { - if (expected !== undefined && expected !== actual) { - failures.push( - `[${ruleId}] wiring.${name} expected ${JSON.stringify(expected)} but catalog has ${JSON.stringify(actual)}.` - ); - } + let expected; + let actual; + try { + expected = normalizeLintWiring(ruleId, wiring, `[${ruleId}] contract.wiring`); + actual = normalizeLintWiring(ruleId, entry, `[${ruleId}] catalog`); + } catch (error) { + failures.push(error.message); + return entry; } - - if (wiring.appliesTo) { - const a = entry.appliesTo || {}; - for (const key of ['minVersion', 'maxVersion', 'engine']) { - if (wiring.appliesTo[key] !== undefined && wiring.appliesTo[key] !== a[key]) { - failures.push( - `[${ruleId}] wiring.appliesTo.${key} expected ${JSON.stringify(wiring.appliesTo[key])} but catalog has ${JSON.stringify(a[key])}.` - ); - } - } + if (JSON.stringify(expected) !== JSON.stringify(actual)) { + failures.push( + `[${ruleId}] normalized wiring mismatch: contract=${JSON.stringify(expected)} ` + + `catalog=${JSON.stringify(actual)}.` + ); } if (wiring.detector && typeof getDetector === 'function' && typeof getDetector(wiring.detector) !== 'function') { @@ -553,16 +564,124 @@ function buildContext(spec, engineVersion) { return context; } +function equalSets(left, right) { + return left.size === right.size && [...left].every((value) => right.has(value)); +} + +function buildCensus(contracts, manifest, catalog) { + const problems = []; + const byFile = new Map( + contracts.map(({ file, spec }) => [path.basename(file), spec]) + ); + const resolveManifestRules = (field) => { + const names = manifest[field] || []; + if (!Array.isArray(names)) { + problems.push(`manifest.${field} must be an array.`); + return []; + } + if (new Set(names).size !== names.length) { + problems.push(`manifest.${field} contains duplicate file names.`); + } + const rules = []; + for (const name of names) { + const spec = byFile.get(name); + if (!spec) { + problems.push(`manifest.${field} references inactive or missing contract "${name}".`); + } else { + rules.push(spec.ruleId); + } + } + return rules; + }; + + const activeContractRules = contracts.map(({ spec }) => spec.ruleId).sort(); + const duplicateRuleIds = activeContractRules.filter( + (ruleId, index) => activeContractRules.indexOf(ruleId) !== index + ); + if (duplicateRuleIds.length > 0) { + problems.push(`active contracts contain duplicate rule IDs: ${duplicateRuleIds.join(', ')}.`); + } + + const activeLintRules = contracts + .filter(({ spec }) => contractChannel(spec) === 'lint') + .map(({ spec }) => spec.ruleId) + .sort(); + const activeSyntaxRules = contracts + .filter(({ spec }) => contractChannel(spec) === 'syntax') + .map(({ spec }) => spec.ruleId) + .sort(); + const enabledRules = catalog + .filter((rule) => rule.enabled) + .map((rule) => rule.id) + .sort(); + const defaultErrorRules = catalog + .filter((rule) => rule.enabled && rule.severity === 'error') + .map((rule) => rule.id) + .sort(); + const manifestDefaultErrorRules = resolveManifestRules('defaultError').sort(); + const requiredSyntaxFeatures = resolveManifestRules('requiredSyntaxFeatures').sort(); + + if (activeLintRules.length !== 12) { + problems.push(`expected 12 active lint contracts, found ${activeLintRules.length}.`); + } + if (requiredSyntaxFeatures.length !== 1) { + problems.push( + `expected one required syntax feature, found ${requiredSyntaxFeatures.length}.` + ); + } + if (activeContractRules.length !== 13) { + problems.push(`expected 13 active contracts, found ${activeContractRules.length}.`); + } + if (!equalSets(new Set(activeSyntaxRules), new Set(requiredSyntaxFeatures))) { + problems.push( + `active syntax contracts ${JSON.stringify(activeSyntaxRules)} do not equal ` + + `manifest.requiredSyntaxFeatures ${JSON.stringify(requiredSyntaxFeatures)}.` + ); + } + if (!equalSets(new Set(activeLintRules), new Set(enabledRules))) { + problems.push( + `active lint contracts ${JSON.stringify(activeLintRules)} do not equal enabled catalog ` + + `rules ${JSON.stringify(enabledRules)}.` + ); + } + if (!equalSets(new Set(manifestDefaultErrorRules), new Set(defaultErrorRules))) { + problems.push( + `manifest.defaultError rules ${JSON.stringify(manifestDefaultErrorRules)} do not equal ` + + `enabled error catalog rules ${JSON.stringify(defaultErrorRules)}.` + ); + } + + return { + enabledRules, + defaultErrorRules, + requiredSyntaxFeatures, + activeContractRules, + activeLintRules, + activeSyntaxRules, + manifestDefaultErrorRules, + passed: problems.length === 0, + problems, + }; +} + function main() { const schedule = process.env.PPL_LINT_SCHEDULE || 'pr'; const reportPath = process.env.PPL_LINT_REPORT; const target = loadTarget(); const backendReport = loadBackendReport(target); - const contracts = loadContracts(); + const { contracts, manifest, manifestPath } = loadContracts(); const osd = loadOsd(); - const { getBundledCatalog, getDetector, lintQuery, osdRoot, surface } = osd; + const { + getBundledCatalog, + getDetector, + lintQuery, + validateSyntax, + osdRoot, + surface, + } = osd; const catalog = getBundledCatalog(); + const census = buildCensus(contracts, 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 @@ -610,8 +729,24 @@ function main() { .filter((rule) => rule.enabled && rule.severity === 'error') .map((rule) => rule.id) .sort(), + enabledRules: census.enabledRules, + requiredSyntaxFeatures: census.requiredSyntaxFeatures, + activeContractRules: census.activeContractRules, + census: { + enforced: process.env.PPL_LINT_ENFORCE_CENSUS === '1', + manifest: manifestPath, + ...census, + }, results: [], }; + if (!census.passed) { + for (const problem of census.problems) { + log(`CENSUS REPORT-ONLY: ${problem}`); + } + if (process.env.PPL_LINT_ENFORCE_CENSUS === '1') { + failures.push(...census.problems.map((problem) => `[census] ${problem}`)); + } + } log(`OSD root: ${osdRoot}`); log( @@ -623,6 +758,11 @@ function main() { for (const { file, spec } of contracts) { const ruleId = spec.ruleId; const index = spec.index; + const channel = contractChannel(spec); + const entry = checkWiring(spec, catalog, getDetector, failures); + if (!entry) { + continue; + } // A contract runs on PR only when scheduled for PR; nightly runs everything. const contractSchedule = spec.schedule || 'pr'; @@ -650,6 +790,7 @@ function main() { for (const [queryName, queryDef] of Object.entries(spec.queries || {})) { report.results.push({ ruleId, + channel, queryName, role: queryDef.role || 'trigger', query: (queryDef.query || '').split('{{index}}').join(index), @@ -662,11 +803,6 @@ function main() { continue; } - const entry = checkWiring(spec, catalog, getDetector, failures); - if (!entry) { - continue; - } - const context = buildContext(spec, engineVersion); const expectation = selectExpectation(spec, engineVersion, context.isCalcite, failures, { allowMissing: observeOnly, @@ -681,6 +817,7 @@ function main() { if (surface === 'compiled-simplified' && entry.runtimeOnly) { report.results.push({ ruleId, + channel, queryName, role, query, @@ -691,10 +828,23 @@ function main() { }); continue; } - const result = lintQuery(query, grammar, context); - const matches = (result.diagnostics || []).filter((d) => d.ruleId === ruleId); + 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 matches = + channel === 'syntax' + ? result.errors || [] + : (result.diagnostics || []).filter((d) => d.ruleId === ruleId); report.results.push({ ruleId, + channel, queryName, role, query, @@ -730,9 +880,10 @@ function main() { } catch (error) { fatal(`Invalid contract ${file} query "${queryName}": ${error.message}`); } - const expectedCount = oracleSelection.detector.count; - const expectedSeverity = oracleSelection.detector.severity; - const expectedMessage = oracleSelection.detector.matchMessage; + 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 @@ -747,6 +898,7 @@ function main() { ); report.results.push({ ruleId, + channel, queryName, role, query, @@ -758,8 +910,22 @@ function main() { continue; } - const result = lintQuery(query, grammar, context); - const matches = (result.diagnostics || []).filter((d) => d.ruleId === ruleId); + 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; @@ -769,23 +935,51 @@ function main() { ); 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 resultEntry = { ruleId, + channel, queryName, role, query, expected: expectedCount, actual, - severities: matches.map((m) => m.severity), + severities: matches.map((m) => m.severity).filter(Boolean), severityMatched: severityOk, messageMatched: messageOk, + fixMatched: fixOk, + rawMessageMatched: rawMessageOk, + totalErrorsMatched: totalErrorsOk, + ...(channel === 'syntax' + ? { + code: frontendOracle.code, + codes: allFrontendFindings.map((finding) => finding.code).filter(Boolean), + totalErrors: allFrontendFindings.length, + } + : {}), executionBackend, backendOracleStatus: oracleSelection.status, }; @@ -805,6 +999,22 @@ function main() { `[${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}` + ); + } if (oracleSelection.status === 'not-applicable') { // Only the backend fixture is non-applicable. The detector still ran above and its @@ -894,6 +1104,17 @@ function main() { `and backend ${backendRejected ? 'rejected' : 'accepted'} for: ${query}` ); } + if ( + role === 'suppression-control' && + (detectorFlagged || !backendRejected) + ) { + failures.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}` + ); + } } } } @@ -902,16 +1123,6 @@ function main() { } } - // Nightly-only coverage: every enabled catalog rule must have a contract file. - if (schedule === 'nightly') { - const covered = new Set(contracts.map(({ spec }) => spec.ruleId)); - for (const rule of catalog) { - if (rule.enabled && !covered.has(rule.id)) { - failures.push(`[coverage] enabled catalog rule "${rule.id}" has no contract file.`); - } - } - } - if (reportPath) { report.failures = failures; try {