mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-20 00:11:37 +00:00
1ff324ca16
* feat(group): bridge.lbug storage + contract matching expansion Part 1 of 4 in the split of #606 (ticket: #791, closes #790 with a revised plan per @magyargergo's request). ## What changed Adds the LadybugDB-backed bridge storage infrastructure and extends the contract matching algorithm with wildcard support. All changes are additive: storage.ts, sync.ts, service.ts, cli/group.ts, mcp/tools.ts are left on their upstream main versions and will migrate to the new bridge in follow-up PRs (#792, #793, #794). ### Files **New (844 LOC prod):** - `gitnexus/src/core/group/bridge-db.ts` — atomic write-to-temp with `retryRename` for Windows EBUSY/EPERM, per-item write tolerance via `WriteBridgeReport`, `findContractNode` with three-tier symbol lookup (uid → filePath+name → filePath) - `gitnexus/src/core/group/bridge-schema.ts` — schema DDL - `gitnexus/src/core/group/normalization.ts` — contract ID canonicalization + `dedupeContracts` / `dedupeCrossLinks` helpers used by both matching and bridge write **Modified (+137 LOC prod):** - `gitnexus/src/core/group/matching.ts` — adds `runWildcardMatch` for `grpc::Service/*` wildcard consumers, `buildProviderIndex` helper, and canonical gRPC ID handling in `normalizeContractId` - `gitnexus/src/core/group/types.ts` — `MatchType` gains `'wildcard'`; new `BridgeHandle` and `BridgeMeta` interfaces **New tests (658 LOC):** - `gitnexus/test/unit/group/bridge-db.test.ts` — core write/read round trip, `WriteBridgeReport` shape, dropped-links counter, retryRename behavior on EBUSY/ENOENT/EPERM/EACCES - `gitnexus/test/unit/group/bridge-db-edge.test.ts` — edge cases (malformed meta, missing contract nodes, concurrent access) **Modified tests (+225 LOC):** - `gitnexus/test/unit/group/matching.test.ts` — wildcard consumer matching, gRPC canonical ID handling, same-service guard ### Self-review fixes folded in Carried forward from the original #606 self-review: - `writeBridge` try/finally handle lifecycle + `handleClosed` sentinel - `openBridgeDbReadOnly` partial-handle cleanup - `writeBridgeMeta` uses `retryRename` for Windows consistency - `retryRename` unit tests (was zero coverage) - Per-item try/catch around every CREATE loop so one malformed contract doesn't abort the whole write - Dropped cross-link counter (`linksDroppedMissingNode`) ### Why now magyargergo asked for the #606 PR to be split so we can iterate with confidence (https://github.com/abhigyanpatwari/GitNexus/pull/606#issuecomment-4229612271). This is the foundational layer — pure infra, no user-facing surface, no callers of the new APIs in this PR. Later PRs wire it in. ### How to verify - `cd gitnexus && npx tsc --noEmit` - `cd gitnexus && npx vitest run test/unit/group/bridge-db.test.ts --pool=forks` - `cd gitnexus && npx vitest run test/unit/group/bridge-db-edge.test.ts --pool=forks` - `cd gitnexus && npx vitest run test/unit/group/matching.test.ts --pool=forks` - Pre-commit hook runs clean ### Risk / rollback **Low.** All new code sits under `src/core/group/` in new files plus a minimal `+16/-1` diff to `types.ts` and a `+136/-0` diff to `matching.ts` (both purely additive). No existing callers reference the new APIs (bridge-db, openBridgeOrFallback, runWildcardMatch) — the PRs that wire them in come later in the split chain. Rollback = `git revert` of the merge commit; no state introduced, no schema migration triggered. ### Scope discipline (per GUARDRAILS.md) - Only the 8 files listed above are touched; no drive-by refactors - No CI/release/security config changes - No secrets, tokens, or machine-specific paths - Content is lifted from the #606 branch which already passed CI 11/11 green on `d15b8cb` (before the split) ### Dependencies - **Base:** `main` (no dependencies on other split PRs) - **Blocks:** extractor expansion (#792), sync pipeline (#793), cross-impact feature (#794) - **Related ticket:** #791 Co-authored-by: Claude <noreply@anthropic.com> * fix(group): address @claude review on #795 Addresses the findings from the automated review on PR #795 (https://github.com/abhigyanpatwari/GitNexus/pull/795#issuecomment-4229770000 — posted by @magyargergo / claude-code Action run). ### Medium severity (reviewer flagged as blockers) - **bridge-db.ts `openBridgeDbReadOnly` bak recovery** — the `.bak` recovery path used bare `fsp.rename(bakPath, dbPath)`, which is exactly the scenario most likely to hit Windows EBUSY/EPERM (an interrupted writer still holding the handle for a few ms). Switched to `retryRename` for consistency with the rest of the file's Windows-safe rename path. - **bridge-db.ts `ensureBridgeSchema` error detection** — the inline `msg.includes('already exists')` substring match has been lifted into a named constant `LBUG_ALREADY_EXISTS_MSG` with a comment documenting the coupling to LadybugDB's error message wording and why we can't use `IF NOT EXISTS` (LadybugDB DDL doesn't support it) or typed errors (LadybugDB's JS driver doesn't expose error codes). Also tightened the `catch (err: any)` to `catch (err: unknown)`. - **bridge-db.ts `findContractNode` — extracted out of writeBridge** — the 35-line async closure living inside `writeBridge` has been lifted to three module-level functions: `createContractLookupIndex`, `indexContract`, and `findContractNode`. `findContractNode` is now a pure synchronous function taking a prebuilt index instead of doing its own DB queries. The `writeBridge` cross-link loop is now ~25 lines instead of ~100. - **bridge-db.ts `findContractNode` — N+1 query elimination** — the old inner-closure version issued up to 6 DB round-trips per cross-link (2 endpoints × up to 3 tiers of fallback queries). For a group with 1000 cross-links, that's up to 6000 DB queries just to resolve endpoints. The new version consults an in-memory `ContractLookupIndex` built incrementally as contracts are inserted (`indexContract` called AFTER each successful insert so failed inserts don't poison the index). Cross-link resolution is now O(1) per link instead of O(3) DB queries per link, with zero DB round-trips during the cross-link loop. ### Minor severity - **bridge-db.ts `queryBridge` empty-array guard** — if LadybugDB ever returns an empty `QueryResult[]` at the top level (shouldn't happen with single-statement calls, but driver contract isn't explicit), the old code would call `.getAll()` on `undefined` and crash with a confusing stack. Added an `unwrapQueryResult` helper that throws an explicit `'empty QueryResult array'` error instead, making a potential driver regression visible immediately. - **normalization.ts `contractRichness` weights** — added a block-level comment documenting the weight ordering (+3 for symbolUid, +2 for each symbol-identifying field, +1 for service tag or non-manifest origin) and explicitly noting that the absolute numbers don't matter, only the relative ordering. Matches the "comment for contributors" suggestion in the review. - **bridge-schema.ts `BRIDGE_SCHEMA_VERSION` migration comment** — added a 4-point contract explaining what bumping the constant means ("discard and re-sync" strategy for V1, no in-place migration yet, new migration logic should live in a separate `bridge-migrations.ts` module when it becomes necessary). - **test/unit/group/fixtures.ts** — extracted the `makeContract` helper previously copy-pasted between `bridge-db.test.ts` and `bridge-db-edge.test.ts` into a shared fixtures module. Both test files now import from `./fixtures.js`. Kept the scope minimal: fixtures is NOT a general-purpose factory module, just the shared baseline contract builder. ### New tests Added 9 pure-function unit tests for the now-extracted `findContractNode` in `bridge-db.test.ts`: - returns null on empty index - tier 1 (symbolUid) match, including repo-scope and role-scope isolation - tier 2 (filePath + symbolName) fallback when symbolUid is empty or mismatches - tier 3 (filePath only) when exactly one contract lives in the file, and refusal when multiple do - priority ordering when multiple tiers could resolve These are fully isolated — no DB, no temp directories, no native LadybugDB binding — so they run in <10ms total and are immediately trustworthy as a regression safety net. ### Deliberately deferred (reviewer marked as "fine for now") - `BridgeHandle._db` / `._conn` typing to `unknown` with casts in `bridge-db.ts` — reviewer's note: "The typing is fine for now." - Batch inserts via `UNWIND` — needs LadybugDB support confirmation, tracked as a follow-up; the per-item pattern remains. - `queryBridge` prepared-statement lifecycle — the current pattern (prepare → execute → GC) relies on LadybugDB's internals, worth verifying against their docs in a separate audit. ### Scope discipline (per `GUARDRAILS.md`) - Only files touched by this PR (`bridge-db.ts`, `bridge-schema.ts`, `normalization.ts`, both bridge test files, new `fixtures.ts`) — no drive-by refactors - No CI/release/security config changes - No secrets ### Test + typecheck status - `npx tsc --noEmit` clean - `bridge-db.test.ts`: added 9 `findContractNode` tests, all pass in isolation. The full-file run still hits the pre-existing native LadybugDB cleanup segfault that flakes the reported count — same as every prior commit on this branch, not a regression. - `bridge-db-edge.test.ts`: 4/4 pass - `matching.test.ts`: 28/28 pass - `types.test.ts`: 5/5 pass - `retryRename` tests (4/4) and `findContractNode` tests (9/9) verified in isolation via `-t` filter Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|---|---|---|
| .. | ||
| call-routing | ||
| group | ||
| import-resolution | ||
| named-bindings | ||
| ai-context.test.ts | ||
| analyze-api.test.ts | ||
| analyze-job.test.ts | ||
| api-graph-streaming.test.ts | ||
| ast-cache.test.ts | ||
| binding-accumulator.test.ts | ||
| bm25-search.test.ts | ||
| call-form.test.ts | ||
| call-processor.test.ts | ||
| calltool-dispatch.test.ts | ||
| cli-commands.test.ts | ||
| cli-index-help.test.ts | ||
| cobol-copy-expander.test.ts | ||
| cobol-preprocessor.test.ts | ||
| cohesion-consistency.test.ts | ||
| community-processor.test.ts | ||
| compatible-stdio-transport.test.ts | ||
| cors.test.ts | ||
| csv-escaping.test.ts | ||
| dart-import-resolver.test.ts | ||
| dart-type-extractor.test.ts | ||
| embedder.test.ts | ||
| entry-point-scoring.test.ts | ||
| eval-formatters.test.ts | ||
| expo-routes.test.ts | ||
| extract-element-type-from-string.test.ts | ||
| extract-generic-type-args.test.ts | ||
| fetch-reason-parsing.test.ts | ||
| field-extraction.test.ts | ||
| framework-detection.test.ts | ||
| git-clone.test.ts | ||
| git-utils.test.ts | ||
| git.test.ts | ||
| graph.test.ts | ||
| has-method.test.ts | ||
| heritage-map.test.ts | ||
| heritage-processor.test.ts | ||
| hooks.test.ts | ||
| http-embedder.test.ts | ||
| hybrid-search.test.ts | ||
| ignore-service.test.ts | ||
| impact-batching-grouping.test.ts | ||
| impact-confidence.test.ts | ||
| import-processor.test.ts | ||
| index-repo-command.test.ts | ||
| ingestion-utils.test.ts | ||
| isWriteQuery.test.ts | ||
| jcl-parser.test.ts | ||
| language-skip.test.ts | ||
| lazy-action.test.ts | ||
| method-extraction.test.ts | ||
| method-props.test.ts | ||
| mro-processor.test.ts | ||
| noise-filter.test.ts | ||
| parse-diff-hunks.test.ts | ||
| parser-loader.test.ts | ||
| pipeline-exports.test.ts | ||
| process-processor.test.ts | ||
| receiver-extraction.test.ts | ||
| repo-manager.test.ts | ||
| resources.test.ts | ||
| route-tool-detection.test.ts | ||
| run-analyze.test.ts | ||
| schema.test.ts | ||
| security.test.ts | ||
| sequential-language-availability.test.ts | ||
| server.test.ts | ||
| setup-codex.test.ts | ||
| setup.test.ts | ||
| shape-check.test.ts | ||
| shared-type-extractors.test.ts | ||
| skill-gen.test.ts | ||
| skip-git-cli.test.ts | ||
| staleness.test.ts | ||
| stdout-silence.test.ts | ||
| structure-processor.test.ts | ||
| suffix-index-ambiguity.test.ts | ||
| symbol-resolver.test.ts | ||
| symbol-table.test.ts | ||
| tools.test.ts | ||
| topological-sort.test.ts | ||
| tree-sitter-queries.test.ts | ||
| type-env.test.ts | ||
| utils.test.ts | ||
| vue-sfc-extractor.test.ts | ||
| wiki-flags.test.ts | ||
| wiki-llm-client.test.ts | ||