Improved schema completions and navigation#360
Open
holodorum wants to merge 15 commits intokson-org:mainfrom
Open
Improved schema completions and navigation#360holodorum wants to merge 15 commits intokson-org:mainfrom
holodorum wants to merge 15 commits intokson-org:mainfrom
Conversation
a16ecfd to
e752cdd
Compare
Schema navigation (go-to-definition, completions, hover) previously dead-ended at if/then/else constructs because navigateObjectProperty and navigateArrayItems only knew about direct properties, pattern properties, and combinators (allOf/anyOf/oneOf). Schemas where allOf contains if/then blocks that conditionally map property values to parameter $refs were invisible to the tooling. Add navigateThroughConditionals (paralleling navigateThroughCombinators) which traverses into then and else sub-schemas. Wire it into both navigateObjectProperty and navigateArrayItems, and expand conditionals in expandCombinators so completions also discover conditional branches. Introduce IF_THEN and IF_ELSE SchemaResolutionType variants so downstream code (e.g. SchemaFilteringService) can distinguish how a schema was reached. These new types fall into the unfiltered path, matching allOf/direct-property behavior — condition evaluation against the document is left as a future enhancement.
…ument Two-layer filtering for conditional schemas: 1. During navigation, evaluate the "if" condition against the document value at the current schema level. This is critical when the condition checks a sibling property that is not visible from the target property. When a document value is available, only the matching branch (then or else) is included. 2. During post-navigation filtering, validate IF_THEN/IF_ELSE schemas against the document the same way ANY_OF/ONE_OF are already filtered. This catches cases where the resolved schema itself is incompatible. Consolidate scattered ANY_OF/ONE_OF checks into a shared FILTERABLE_RESOLUTION_TYPES set and rename isCombinatorBranch to isFilterableBranch.
Several fixture schemas (DogParams/CatParams, long orchestra examples) were exploratory — useful while building if/then navigation, noisy now that the assertions are stable. Shrink them to minimal repros so the intent of each test is obvious at a glance. Also pass the parsed document through in SchemaFilteringServiceTest's navigate helper so it exercises if/then condition evaluation.
extractValueCompletions handled enum, boolean, and null but not const.
A schema like { const: "SNOWFLAKE" } produced zero completions. This
matters for if/then schemas that narrow a property to a single const
value based on a sibling — the navigation correctly resolves the const
schema but completions were silently empty.
The same helper was duplicated across SchemaCompletionLocationTest and follow-up completion tests. Move it to a shared interface so new tests can mix it in instead of recopying.
…tics The previous narrowing logic only kicked in when a reductive schema had a const value. This missed the common case where an if/then branch narrows a base property's enum to a subset. Replace the const-specific check with general intersection: when multiple reductive schemas (allOf, if/then, direct properties) provide value completions, only values present in ALL schemas survive. Falls back to union when the intersection is empty, which handles the case where no document value was available to evaluate if/then conditions.
When the cursor is at an empty value position (e.g. `key:`), KSON can't fully parse the document, so `ksonValue` is null. Without a document value, if/then conditions can't evaluate sibling properties and all branches are included — defeating the narrowing. Add `toPartialKsonValue()` which walks the AST and silently skips error nodes, preserving successfully-parsed siblings. Use this for schema navigation during completions so that sibling values are visible for if/then evaluation even when the cursor position has no value. The partial value is only used for navigation (if/then evaluation). Validation filtering and property dedup continue to use `ksonValue` to avoid side effects from partial document state.
The `schemaIdLookup` and `parsedDocument` fields on ResolvedSchemaContext were unused by callers once the partial-AST navigation was pulled inline into getCompletions. Drop the data class wrapper and make resolveAndFilterSchemas a plain function returning List<ResolvedRef>.
…tion Completions had its own inline schema resolution path to use partialKsonValue for if/then navigation while hover and go-to-definition used the shared resolveAndFilterSchemas with ksonValue (which is null when the document has parse errors). Change resolveAndFilterSchemas to take ToolingDocument directly and use partialKsonValue for navigation, ksonValue for validation filtering. All three features now share the same path, giving hover and go-to-definition the same broken-document resilience completions had.
When a schema uses allOf+oneOf to couple interdependent properties (e.g., integration determines which job values are valid), setting one property now narrows completions for the other. The mechanism: navigateThroughCombinators records which oneOf/anyOf branch each result came from (via the new parentBranch field on ResolvedRef). SchemaFilteringService uses this context in a dedicated first pass to check sibling property const/enum constraints against the document. A second pass does the existing leaf-level validation. The two passes use different document values: sibling filtering uses partialKsonValue (works even with parse errors at the cursor), while leaf validation uses the fully parsed ksonValue. Each pass has its own fallback so that completions degrade gracefully.
navigateByDocumentPointer always started with baseUri="" but schemas with $id store their root under that id in the lookup map. $ref resolution then failed because idMap[""] didn't exist. Use the root schema's $id as the starting base URI so refs resolve correctly.
The two classifications — which types contribute reductive (intersected) value completions, and which types need validation-based filtering — lived as setOf() constants in two unrelated files. A reader had to cross-reference to understand what an enum value meant in the system. Lift both into properties on SchemaResolutionType itself (isReductive, requiresValidationFiltering). Call sites become self-explanatory; the taxonomy is discoverable from the enum.
The function was expanded to handle if/then/else but the name still said combinators only. Rename to reflect what it actually does — flatten any branching construct (combinators + conditionals) into tagged ResolvedRefs.
Three separate comments scattered across getValidSchemas and filterByValidation
described individual "give up on filtering" cases. A reader had to piece the
overall pipeline together themselves.
Consolidate the four skip/fallback conditions into a single class-level KDoc
section. Also document the load-bearing nature of the containsKey("oneOf"/
"anyOf"/"if") check in requiresValidationFiltering — it catches preserved
parent schemas that would otherwise slip through filtering unchecked.
Navigation, expansion, and filter-time sibling checks each used to
implement their own view of "decompose a schema's branches," with
subtly different semantics. The worst effect: blind expansion at the
target level leaked else-branch properties into completions when if
matched (regression tests added at root and nested levels).
Introduce SchemaNavigator as an inner class of SchemaIdLookup, mirroring
TreeNavigator's shape from the walker package. It exposes one entry
point, navigate(pointer, docVal), built from two primitives:
- stepInto: structural per-token descent, no combinator logic
- flatten: doc-aware decomposition of top-level branches
(oneOf/anyOf/allOf unconditionally, if/then/else narrowed
by strict isValid)
Navigation calls flatten at every level, including the target, so the
previous post-nav expandBranches pass disappears entirely.
navigateThroughCombinators/Conditionals are absorbed into flatten.
SchemaFilteringService drops its expandBranches call site — navigate
already returns fully-flattened refs. Two existing SchemaNavigationTest
cases are updated to reflect the new contract (target schema is now the
head of the flattened result, followed by its expanded inner branches).
e752cdd to
4c98eb5
Compare
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.
if/then/elseconditional schemas — schema navigation (completions, hover,go-to-definition) now traverses
if/then/elseconstructs, evaluating conditions againstthe document to include only the matching branch
oneOf/anyOfwith sibling awareness — when a schema couples properties viaoneOf(e.g.,integrationdetermines validintegrationJobvalues), setting oneproperty narrows completions for the other by filtering branches whose sibling constraints
conflict with the document
(allOf, if/then) constrain a value, only completions present in ALL schemas survive,
replacing the previous const-only narrowing
error-tolerant partial AST values for schema navigation, so sibling property values are
visible for narrowing even when the cursor position has no value yet