feat(kotlin): extract visibility and modifiers, and land decorators on Interface/Object - #1620
Conversation
Follows CodeGraphContext#1596, which added `decorators`. The same `modifiers` node also carries visibility (public/private/internal/protected) and the class kind (data/sealed/value/annotation), plus abstract/open/override and suspend/inline -- none of which reached the graph. Adds two properties on Function and Class, and completes the two columns CodeGraphContext#1596 deferred on Interface and Object: Function/Class visibility STRING, modifiers STRING[] Interface/Object visibility STRING, modifiers STRING[], decorators STRING[] Each in all three required places -- node-table declaration, SCHEMA_MAP allow-list, and simple_migrations so pre-existing databases get them via ALTER TABLE. Without the last, CREATE NODE TABLE throws "already exists" on an existing database and is swallowed, so the columns never arrive. Two grammar details worth knowing: `enum` is not a modifier. `enum class C` produces no `modifiers` node at all -- the keyword is a direct child of class_declaration, exactly like `interface`. It is derived with the pattern _parse_classes already uses for interface detection, so `modifiers` is the single place to ask what kind of class this is. visibility defaults to the string "public" rather than null, matching Kotlin's own default, so consumers need no null handling. Since CodeGraphContext#1596 gated `decorators` behind `category == "classes"` only because Interface/Object had no column, that gate is removed here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@rrodriguesNutrium is attempting to deploy a commit to the shashankss1205's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Android/Kotlin series — merge order Each PR targets Happy to split, reorder, or squash any of these differently if it suits review better. |
Conflict was additive on both sides of tests/unit/core/test_database_kuzu_kotlin_metadata.py: this branch's four visibility/modifiers tests vs main's three write_inheritance_links probe tests (CodeGraphContext#1617). Kept all seven, plus the module-level 'import kuzu' this branch's migration tests need. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Shashankss1205
left a comment
There was a problem hiding this comment.
Approving. The schema discipline here is exactly right and worth calling out, because it's the step that's easy to miss: new columns plus migration entries plus SCHEMA_MAP allow-list entries. Kùzu drops unknown properties silently, so any one of the three missing would have produced a feature that appears to work and stores nothing — I hit precisely that trap on #1613.
Adding decorators to Interface and Object also closes the gap #1596 deliberately left open (it gated on category == "classes" because those tables had no such column).
I resolved the conflict with main and pushed to your branch. It was additive on both sides of test_database_kuzu_kotlin_metadata.py — your four visibility/modifiers tests against main's three write_inheritance_links probe tests from #1617 — so all seven are kept. Git had split the hunks mid-function rather than at function boundaries, so I rebuilt the union from the two full versions by AST rather than splicing the markers; also restored the module-level import kuzu your migration tests need, which main's version didn't carry.
test_database_kuzu_kotlin_metadata.py 13 passed
tests/unit/ 1219 passed
tests/integration/ 45 passed
The migration tests that build a pre-existing database and then assert the columns appear are the right shape for this — that's the failure mode that would otherwise only surface on someone's months-old local graph.
Conflict in tests/unit/core/test_database_kuzu_kotlin_metadata.py: main is a strict superset (this branch's tests all landed via CodeGraphContext#1620, plus main has CodeGraphContext#1617's three probe tests), so main's version is taken whole. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follows #1596. That PR added
decoratorsto Kotlin, but gated the assignment behindcategory == "classes"— becauseInterfaceandObjecthad nodecoratorscolumn to write into. This PR adds the missing columns and removes the gate.Why this is a prerequisite, not cosmetics
The title undersells it. On
maintoday:Kotlin DI modules are conventionally
objects.@Module object NetworkModuleis therefore invisible to the graph, and any consumer that looks for annotations on an interface or object silently sees nothing — not[], butNone. Anything built on Hilt/Dagger module detection cannot work until this lands.I hit this concretely: a
@Providesresolution test passes for a module declared asabstract classand fails for the identical module declared asobject. Same annotation, same grammar path, different category gate.What it adds
The same
modifiersnode that carries annotations also carries visibility and the class kind. None of it reached the graph.Each in all three required places — node-table declaration,
SCHEMA_MAPallow-list, andsimple_migrations. All three are load-bearing: on an existing databaseCREATE NODE TABLEthrows "already exists" and is swallowed, so without the migration entry the column never arrives; andSCHEMA_MAPis an allow-list, so a property missing from it is dropped silently at write time (database_embedded_kuzu.py,if allowed_props and k not in allowed_props: continue).That silent-drop path is why the columns are added in triplicate rather than just declared. A property that reaches Kuzu but is absent from the allow-list simply disappears, while schemaless backends keep it — the two backends diverge with no error anywhere.
Two grammar details worth knowing
enumis not a modifier.enum class Cproduces nomodifiersnode at all — the keyword is a direct child ofclass_declaration, exactly likeinterface. It's derived with the pattern_parse_classesalready uses for interface detection, somodifiersstays the single place to ask what kind of class this is.visibilitydefaults to the string"public", not null — matching Kotlin's own default, so consumers need no null handling.Scope and verification
One source file (
kotlin.py, +57/-6) plus schema declarations and tests.pytest tests/unit -q→12 failed, 1186 passed, 19 skipped.Same 12 failures on an unmodified
origin/main(12 failed, 1176 passed, 19 skipped) — pre-existing, unrelated (MCP SSE disconnect, Kotlin overload resolution, SCIP pipeline).First of a five-PR series adding Android/Kotlin coverage. GitHub can't host a stacked PR base across a fork boundary, so each PR targets
mainand carries its prerequisites as earlier commits — review only the last commit on each. This one is the base of the stack.