fix(schema): admit L4's compound body-node ids so an -a 4 payload validates - #208
Merged
Merged
Conversation
…idates The localId pattern was written before L4 and admitted only a bare line:col or an @-tagged synthetic vertex. SdgVertices keys a call site's actuals off the site's own local id — <line:col>/actual_in:<i> and <line:col>/actual_out — so every one of them was rejected and an -a 4 document failed this repository's own conformance gate: 2936 errors on daytrader8, all of them these two forms. The pattern now enumerates the two compound forms rather than loosening its character class, so ids no builder emits are still rejected: an actual_in missing its ordinal, an actual_out carrying one, an unknown suffix, a base that is not a position, and a bare trailing slash all remain invalid. The emitted ids were correct throughout; only the schema describing them was stale. -a 1, -a 2 and -a 3 documents already validated, which also settles the question the issue raised about whether lower levels shared the gap. L4GateTest now validates the real -a 4 document it already builds, and L4SchemaOracleTest covers the accept and reject cases directly. Closes #207
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
localIdinsrc/test/resources/schema/analysis.v2.schema.jsonwas written before L4 and admitted only a bareline:color an@-tagged synthetic vertex.SdgVerticeskeys a call site's actuals off the site's own local id —<line:col>/actual_in:<i>and<line:col>/actual_out— so none of them matched.An
-a 4document therefore failed this repository's own conformance gate. Measured on thedaytrader8fixture: 2936 validation errors, every one of them these two forms (1943 ×N:N/actual_in:N, 993 ×N:N/actual_out) and nothing else.Surfaced while running the release gates for 3.0.0 — the release that introduces L4.
Fix
The pattern now enumerates the two compound forms rather than loosening its character class, per the issue's caveat that a pattern relaxed to accept anything containing a slash would gate nothing:
Still rejected: an
actual_inmissing its ordinal, anactual_outcarrying one, an unknown suffix, a base that is not a position, and a bare trailing slash.The emitted ids were correct throughout — only the schema describing them was stale. No id changes, no graph-projection changes, no revisiting of the id grammar.
Answering the issue's open question
The issue flagged that the gate "has apparently only ever run at L1, so its coverage of L2 and L3 ids is also unproven." Checked directly:
-a 1,-a 2and-a 3documents all validate clean against the unmodified schema. L4 was the only gap.Tests
L4GateTest.theLevel4DocumentValidatesAgainstTheV2Schema— validates the real-a 4document the test already builds from thel4-sdg-testfixture. Fails before this change, passes after.L4SchemaOracleTest— 9 hand-authored cases: the two compound forms and the pre-existing forms accepted, five malformed shapes rejected.Suite: 487 tests, 1 failure, 4 skips — the failure is the pre-existing Docker-dependent
CodeAnalyzerIntegrationTest > initializationError, unrelated and unchanged.Closes #207