feat(spring): build bean candidate inventory#2494
Conversation
|
@PieceOfFall is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
✨ PR AutofixFound fixable formatting / unused-import issues across 13 changed lines. Comment |
CI Report❌ Some checks failed Pipeline Status
Test Results
✅ All 14812 tests passed 62 test(s) skipped — expand for details
Code CoverageTests
📋 View full run · Generated by CI |
|
@magyargergo I’ve pushed the fix for the lint failure reported by CI. The requested implementation changes have already been addressed, and the PR is currently only blocked by the required checks. Could you please take another look or re-run/approve the workflows when convenient? The CI fix is in commit |
magyargergo
left a comment
magyargergo
left a comment
There was a problem hiding this comment.
Review: Spring bean candidate inventory
Reviewed at head d1b2d82d against merge-base c2dc9c11, in a dedicated worktree with a fresh --pdg GitNexus index. Evidence: full diff read; detect_changes (maps exactly the diff's 48 symbols); upstream impact on the persistence/MCP hubs; taint pass (clean — the new list writers are escaped or fed from the closed stereotype set, and the MCP lookup is parameterized); tsc --noEmit clean; all 6 PR test files pass locally; CI green. Four domain/cross-cutting expert passes followed, and the headline finding below was reproduced empirically with this PR's own built CLI.
The feature is well-architected: it reuses the established collectCaptureSideChannel/applyCaptureSideChannel/emitPostResolutionEdges seams exactly as C/C++/Kotlin do, keeps all Spring knowledge in languages/java/, keeps the three Class write paths (CREATE TABLE / COPY / MERGE) consistent, and the v7 INCREMENTAL_SCHEMA_VERSION bump correctly forces a full re-analyze on upgrade. The fixture matrix for the fail-closed cases is strong. The verdict is driven by two correctness gaps in the fail-closed guarantee itself:
Empirical repro for the HIGH finding (incremental staleness)
| Step | WildcardService.frameworkAnnotations |
|---|---|
Full analyze (import org.springframework.stereotype.*; + @Service) |
["org.springframework.stereotype.Service"] ✓ |
Add same-package public @interface Service {} → incremental analyze ("1 unchanged file rows preserved") |
["org.springframework.stereotype.Service"] ✗ stale false positive |
Same tree, analyze --force full rebuild |
[] ✓ |
Incremental and full rebuild diverge, and MCP context/impact serve a false-positive bean indefinitely — breaking #2411's criterion 4 (fail closed, no misleading results) and the incremental ≡ full-rebuild invariant. Details and remediation options inline.
Findings (inline)
- HIGH — incremental writeback persists stale bean attributions (
spring-bean-metadata.ts) - MEDIUM — wildcard gate fails open on three legal Java shadowing channels (
spring-bean-metadata.ts) - MEDIUM — new side-channel map lacks the per-pass clear its C/Kotlin precedents have (
capture-side-channel.ts) - MEDIUM — mid-test
returnsilently passes the batch-insert assertions on Windows (spring-bean-metadata-roundtrip.test.ts) - LOW — bean lookup runs as dead work on the deliberately-slimmed impact paths (
local-backend.ts) - LOW — CSV list items with commas silently split, diverging from the Cypher path (
csv-generator.ts)
Coverage and residual risk
Tested well: all six stereotypes incl. RestController's distinct package, eight negative shadow fixtures, real-DB roundtrip incl. two-item lists, MCP shapes for context+impact, version-gate assertions, and a route-discovery regression assertion in the same fixture. Missing: an incremental-analyze test that mutates annotation-relevant sibling files (HIGH finding), inherited-member / static-import / capped-package shadow cases (MEDIUM finding), and Windows coverage of the batch write path. Records/interfaces/enums are deliberately out of scope and pinned by a unit test — noted as a product boundary, not a defect (@Component on records is legal Spring 6, worth a follow-up).
Evidence is valid for head d1b2d82d; findings were verified in source (and the HIGH one end-to-end) rather than inferred from graph output alone.
|
@PieceOfFall Can you please resolve the conflicts? 🙏 |
|
@PieceOfFall could we please include kotlin in this implementation? 🙏 We need to share common functionalities with java. |
|
OK. I’ll add Kotlin support and share the common parts with Java. |
…eview/work/plan Eight incident-backed hardenings from running the full skill cycle (review -> plan -> work, 28-finding fix series) on PR abhigyanpatwari#2522: gitnexus-review: - Expert lenses execute the code under review on candidate failing shapes (empirical probe outranks source reading — every HIGH the language lenses found came from a probe, not a read). - Step 7 re-runs the exact CI check for refreshed baselines/fingerprints (a stale committed artifact is invisible in the diff; caught a red benchmarks arm). - Step 8 treats version/invalidation constants as review surface (INCREMENTAL_SCHEMA_VERSION class recurred verbatim from abhigyanpatwari#2494). gitnexus-work: - Step 4 proves regression tests discriminate against the pre-fix tree. - Step 5 rebuilds executed build output before every verification run (parse workers load dist/; a correct fix 'failed' until rebuilt). - Step 6 makes stage -> detect_changes -> commit one unbroken sequence. gitnexus-plan: - Phase 0 seeded-evidence mode: plan FROM a completed review's verified findings instead of re-running the graph ladder. - Template §7: fingerprint/golden-guarded output rebaselines once, at the series tip. All distribution copies resynced; shipped-skills-sync + skills-steering 24/24 locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks for the follow-up fixes. I rechecked the current head against current The earlier technical review findings look addressed in the Java path: the bean attribution now runs after scope resolution, stale incremental I would still keep this as changes-requested for the remaining maintainer scope: the implementation is still Java-only. The current diff adds the Spring bean metadata extraction under One useful next step would be to extract the stereotype/metadata derivation and post-resolution attachment shape into a JVM-shared helper, then wire both Java and Kotlin provider hooks with Kotlin fixtures covering |
|
I’m happy to add the Kotlin support and shared JVM logic. Since |
|
Would you be okay with bumping INCREMENTAL_SCHEMA_VERSION from 7 to 8? This PR does not change the physical database schema, but it changes the index completeness guarantee: v7 contains Java Spring Bean evidence, while this PR adds Kotlin evidence. A same-commit v7 index can hit alreadyUpToDate before parse-cache v14 invalidation runs. Kotlin frameworkAnnotations would then remain missing until a forced rebuild or relevant source change.v8 would trigger a one-time full re-analysis and mark the index complete for both Java and Kotlin. Alternatively, I can add a dedicated Spring Bean feature epoch, but that introduces additional metadata and migration logic. I recommend the v8 bump. Would that approach work for you? |
|
One additional thought before you decide: |
azizur100389
left a comment
azizur100389
left a comment
There was a problem hiding this comment.
Requesting changes for current-head readiness.
gitnexus/test/unit/spring-bean-schema.test.ts still expects PARSE_CACHE_VERSION to match ^13+, but current head has SCHEMA_BUMP = 17 in gitnexus/src/storage/parse-cache.ts, so the unit suite should fail after the latest merge-from-main.
The feature is also still Java-only. Since the current maintainer scope asks for Kotlin/shared JVM support, this is not ready unless the PR is explicitly narrowed to Java-only. If Kotlin support lands here, please also settle the durable version gate, either with INCREMENTAL_SCHEMA_VERSION = 8 or a dedicated Spring feature stamp checked before the up-to-date fast path.
Please rerun the required GitHub Actions checks after the fix; current head only shows the Vercel authorization failure.
|
@PieceOfFall Thanks, the latest fixes address all six findings and @azizur100389 confirmed the Java path independently, so the technical review is settled. Answers to your questions:
Once the Kotlin support is pushed I will approve the workflow runs and do the final review pass. |
|
Thanks, that is clear. I’ll proceed with v8, defer the dedicated Spring feature stamp, finish the Kotlin/shared JVM work, and update the schema assertions to minimum-version checks when syncing with main. |
# Conflicts: # gitnexus/src/core/ingestion/languages/java/captures.ts # gitnexus/src/core/ingestion/languages/kotlin/captures.ts # gitnexus/src/storage/parse-cache.ts # gitnexus/src/storage/repo-manager.ts # gitnexus/test/unit/call-summary-schema-version.test.ts
|
Thanks for the guidance. I followed the suggested direction and extracted the Spring Bean logic into shared framework modules. Shared Spring Bean ResolutionThe stereotype catalog, metadata derivation, annotation resolution, and post-resolution attachment are now shared by Java and Kotlin through thin language adapters. Resolution supports:
Kotlin SupportKotlin now participates in the same Bean inventory pipeline without requiring a second AST traversal. Annotation and package facts are captured during the existing scope traversal, persisted through the Kotlin The fixtures cover:
Interfaces, objects, enums, annotation classes, and composed annotations remain excluded. Java and Kotlin now produce the same canonical Shared JVM Package VisibilityI also extracted Java’s package-sibling handling into a shared JVM helper used by both languages. It distinguishes the default package from package parsing failures and marks visibility as incomplete when source content is missing, parsing fails, or the package-size limit is reached. Wildcard attribution therefore remains conservative. Incremental annotation drift detection now covers Cache and Index CompatibilityAfter syncing with the current
Missing, malformed, mismatched, or unknown capability stamps trigger a one-time full rebuild before the fast path can be used. The parse-cache schema is now v20. Upstream v19 is reserved for the Java enum-constant and anonymous-class identity changes, while v20 invalidates caches created before the new Java/Kotlin Spring side-channel data was available. Warm-Cache BehaviorKotlin also opts into a generic cached-source policy. Warm durable- Other resolvers retain their existing behavior, and the shared ingestion pipeline contains no language-specific checks. ValidationThe full test suite passes:
Existing Java Bean behavior, MCP context and impact queries, route and |
# Conflicts: # gitnexus/src/storage/parse-cache.ts
3 participants
Summary
Adds a Java-only Spring bean candidate inventory for the six common stereotype annotations. The inventory is persisted as canonical framework annotation evidence on existing
Classnodes and exposed as derived Bean metadata fromcontextandimpact.Motivation / context
Closes #2411.
Later Spring dependency-resolution work needs a reliable inventory of candidate classes without introducing framework-specific graph nodes or relationships. This change keeps Spring recognition in Java scope resolution while giving storage and MCP consumers a small, framework-neutral evidence field.
Areas touched
gitnexus/(CLI / core / MCP server)gitnexus-web/(Vite / React UI).github/(workflows, actions)eval/or other toolingAGENTS.md,CLAUDE.md,.cursor/,llms.txt, etc.)Scope & constraints
In scope
Component,Service,Repository,Controller,RestController, andConfigurationon Java class declarations.Class.frameworkAnnotationsand derive a stable{ framework, role, annotation }Bean shape for MCP output.HANDLES_ROUTEbehavior.Explicitly out of scope / not done here
@Beanmethods, Bean names, qualifiers,@Primary, conditions, profiles, or composed annotations.Implementation notes
Classgraph nodes only after resolution.Testing & verification
cd gitnexus && npm testcd gitnexus && npm run test:integrationcd gitnexus && npx tsc --noEmitcd gitnexus-web && npm test(not applicable)cd gitnexus-web && npx tsc -b --noEmit(not applicable)cd gitnexus && npm run test:unitcd gitnexus && node --import tsx bench/emit-persistence/measure.mjs --checkTargeted tests cover all six roles, explicit imports, FQNs, annotation arguments, nested classes, a single valid wildcard import, multiple-wildcard fail-closed behavior, lexical and cross-file same-package shadowing, route regression, persistence write paths, schema-version gates, and MCP
context/impactoutput.Risk & rollout
The MCP change is additive and does not alter impact traversal or risk calculation. Existing indexes require a full re-analysis because the
Classtable gains a nullable list column; schema-version gates already enforce that rebuild.Checklist
AGENTS.md/ overlays changed: not applicable; none are included in this PR