ID |
|
|---|---|
Status |
Backlog |
Bucket |
feature |
Priority |
13 |
Theme |
lsp |
Updated |
2026-08-06 |
LSP quick fixes for the @node/@nodeId migration, driven by shim facts
Pivoted 2026-07-14. This item was previously the sis-side migration tracker (phased manual schema edits driven by build-log WARN/ERROR diffing; see git history of sis-rewrite-migration.md). The pivot replaces the manual grind with tooling: surface every site where the @nodeId synthesis shims fire as an LSP diagnostic carrying a ready-made fix, so sis-graph developers walk the migration diagnostic-by-diagnostic with the correct directive text offered in-editor. The shims themselves already derive everything the fix needs; today they throw that information away into a console WARN.
The gap
The three synthesis-shim sites warn via SLF4J loggers, not via BuildContext.addWarning(BuildWarning):
-
Site A, output shim:
FieldBuilderPath-2 (bare scalarIDoutput field on aNodeTypeparent). Has in hand: parent type name, field name, the field’sSourceLocation(already a parameter), and the resolvedNodeType(table, key columns, typeId). -
Site B, input scalar shim:
BuildContext.classifyInputFieldInternal, NodeId-scalar arm (scalarIDinput on a@tableinput whose backing table carries__NODE_TYPE_ID/__NODE_KEY_COLUMNS). Has in hand: coordinates, table name,JooqCatalog.NodeIdMetadata(typeId, key columns);SourceLocationoneBuildContext.locationOf(field)call away. -
Site C, id-reference shim:
BuildContext.classifyInputFieldInternal, FK-qualifier arm. Already precomputes the exact canonical replacement string (@nodeId(typeName: "T"), plus@reference(path: [{key: "fk"}])when the qualifier is ambiguous).
Because only BuildWarning`s reach `ValidationReport.warnings(), and Diagnostics.validatorDiagnosticsForCurrent (graphitron-lsp) replays exactly that report at Warning severity, these WARNs produce no squiggle and nothing for a code action to anchor to. The LSP side is otherwise ready: GraphitronTextDocumentService.codeAction is wired, and LintQuickFixes.compute already projects a build-side BuildWarning.LintFinding carrying a LintFix into a rendered quick-fix TextEdit.
Shape of the fix
Follow the shipped LintQuickFixes pattern (R398; the same generator-computes/LSP-renders principle lsp-reference-path-authoring rung 3 takes from R233): the fix is computed generator-side from classifier authority and merely rendered by the LSP; the LSP never re-derives node facts.
-
Convert the three shim WARNs into
BuildContext.addWarning(new BuildWarning.LintFinding(...))with the field’sSourceLocationand aLintFixwhose edit inserts the canonical directive text:-
Site A: insert
@nodeId(bare form; the parent is the field’s own type, which is exactly where R473’s grammar keeps the bare form legal). Note the narrowing: this site no longer covers the field satisfying theNodeinterface, which is now a permanent carrier with no WARN and needs no fix offered. What remains here is the other bareIDfields on a node type. -
Site B: insert
@nodeId(typeName: "<T>")with the type name resolved from the node index rather than the raw typeId, per R473’s typeName-first direction. -
Site C: insert the already-computed canonical string.
-
-
The existing
LintQuickFixespath then renders these as per-diagnostic quick fixes with no LSP-side changes beyond tests. -
A companion diagnostic for the type level, narrower than it was, and with a gate it did not have. Metadata-carrying tables now promote on
implements Nodealone, so a hint offeringimplements Node @nodeis moot for that case:@nodewould be redundant. What survives is a hint offering bareimplements Nodeon a@tabletype whose backing class carries the metadata but which has not published the interface. This still replaces the judgment step the old Phase 1 asked authors to make by hand ("decide whether the parent should be a Node").The gate: *do not offer the hint where accepting it would collide.* Because the hint is now a one-word edit that promotes the type outright, and because step 4 plans workspace-scoped bulk application across ~250 sites, bulk-accepting it over a schema whose metadata-carrying tables share a `+__NODE_TYPE_ID+` mass-promotes exactly the population the retired promotion shim mass-failed on, through the tooling, in one click. The hint computation must therefore read the typeId-uniqueness reduction and not just the metadata probe: no hint where the resulting typeId would collide with an existing node or with a sibling the same sweep would promote. That stays inside this item's own "generator computes, LSP renders" discipline, since the reduction is generator-side.
A second exclusion from the same reasoning: no hint where the type's `+id+` field is `+@field+`-pinned to a non-key column. Promoting such a type yields a node whose `+Node.id+` is a raw column value, which cannot round-trip through `+Query.node(id:)+`. . Bulk application: with ~250 expected sites in sis, per-diagnostic clicking is not enough. Read step 3's gate before designing this tier; the bulk path is what makes an ungated type-level hint dangerous rather than merely noisy. Decide at Spec time between extending the finding-keyed path with file/workspace-scoped aggregation or hosting a detector-driven `+SdlAction+` for the bulk tier (the `+CodeActions+` dispatcher already has per-site / file-bulk / workspace-bulk activation for `+SdlAction+`s; mind the per-request re-parse noted in `+lsp-structural-consolidation+` if going that route).
Sequencing
-
The inserted grammar must be R473-conformant (
explicit-nodeid-grammar): bare@nodeIdonly on own-type output fields,typeName:everywhere else. Land this action before or together with R473 phase 2’s error flip so authors get fixes while the old forms still merely warn. -
The shims are already deleted: R473 (
explicit-nodeid-grammar) removed all three sites together with the grammar that replaces them, and R27 (retire-synthesis-shims) was discarded into it with an empty deletion set. So this item no longer unblocks anything; it is purely about migrating the ~250 sis declarations comfortably. The correctness question is settled (the user re-confirmed on 2026-08-09 that sis is the only touched subgraph), which means the quick fixes are an ergonomics deliverable rather than a gate, and the shapes they rewrite are the two the manual’s migration recipe names. -
The WARN-to-
BuildWarningconversion in step 1 is a prerequisite worth its own commit: it makes the shim findings visible in every consumer build report, LSP or not.
Out of scope
-
The sis-side execution itself (running the quick fixes over sis-graphql-spec); that happens in the sis repo once this ships.
-
The old plan’s Phase 2 (filter inputs missing
@table) and Phase 3 (author-error@node/@nodeIdcleanup): those already surface as ordinary validator errors with locations, so they are visible in-editor today; whether any deserve their own quick fixes is a separate question to file per finding kind if wanted. -
Deleting the shims and enforcing the grammar (both shipped in R473).
Re-spec target (2026-08-20)
The staleness audit of 2026-08-19/20 calls this item carried and self-contradictory, and it is
right: the "shim facts" driver is void. Re-confirmed independently here.
BuildContext.classifyInputFieldInternal survives but neither the NodeId-scalar arm (site B) nor
the FK-qualifier arm (site C) does, SkipMismatchedElement is absent from the tree, and site A’s
output shim went with them. So steps 1 and 2 of Shape of the fix name code that cannot be
edited, and the Sequencing section’s admission that "the shims are already deleted" contradicts
the deliverable above it rather than superseding it.
What that audit left open is what to re-derive the fix text from. There is now a better answer
than either option it named. intent_node_id_instruction, on trunk since 2026-08-20, carries a
basis column whose closed vocabulary is exactly which of the three forms of the instruction
carried it at that coordinate (EXPLICIT_TYPE_NAME, CONTAINING_NODE_TYPE,
TARGET_TABLE_NODE_TYPE, OWN_ID_FIELD, TARGET_ID_NAME), alongside node_type_name, the use
site, and a source position. A migration quick fix’s whole job is "this coordinate means a node
id, and here is the directive text that says so explicitly", which is basis plus
node_type_name plus the location. The three rows a fix would offer text for are the two
inferred bases and the two name-carried ones; the explicit basis needs no fix.
That also settles the generator-computes / LSP-renders discipline this item inherits from the
LintQuickFixes pattern without a BuildWarning conversion at all: the fact is a row both the
build report and the LSP read, which is what step 1 was reaching for by other means. Retitle off
"shim facts" and re-derive the deliverable onto the relation. Step 3’s type-level hint and its
typeId-collision gate are unaffected and stay as written; so is step 4’s bulk tier.
Detail: roadmap/audits/2026-08-20-nodeid-relation-impact-sweep.md, Finding 3.
Fact-base note (2026-08-06)
The three synthesis shims derive everything the quick-fix needs and throw it away, which is the fact-base thesis at the WARN grain. Once inferred claims carry join witnesses (R589), the quick-fix text is selectable from the claim row; re-anchor the deliverable on reading that relation rather than adding BuildWarning calls at the shim sites.
Context and the whole-board picture: roadmap/audits/2026-08-06-fact-base-impact-sweep.md.