GitNexus/gitnexus/test/unit/group/matching.test.ts
ivkond 1ff324ca16
feat(group): bridge.lbug storage + contract matching expansion (1/4 of #606 split) (#795)
* 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 &lt;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>
2026-04-11 19:46:12 +01:00

405 lines
14 KiB
TypeScript

import { describe, it, expect } from 'vitest';
import {
runExactMatch,
normalizeContractId,
buildProviderIndex,
runWildcardMatch,
} from '../../../src/core/group/matching.js';
import type { StoredContract } from '../../../src/core/group/types.js';
describe('normalizeContractId', () => {
it('lowercases HTTP method', () => {
expect(normalizeContractId('http::get::/api/users')).toBe('http::GET::/api/users');
});
it('strips trailing slash from HTTP path', () => {
expect(normalizeContractId('http::GET::/api/users/')).toBe('http::GET::/api/users');
});
it('lowercases gRPC package', () => {
expect(normalizeContractId('grpc::Hr.UserService/GetUser')).toBe(
'grpc::hr.userservice/GetUser',
);
});
it('preserves case for malformed gRPC id with leading slash (no full-string lowercasing)', () => {
expect(normalizeContractId('grpc::/MyPkg/DoThing')).toBe('grpc::/MyPkg/DoThing');
});
it('handles malformed grpc with leading slash and no package', () => {
// grpc::/Method — leading slash, no package
expect(normalizeContractId('grpc::/Method')).toBe('grpc::/Method');
});
it('handles grpc with no slash at all', () => {
// grpc::ServiceName — no slash, ambiguous; MVP: lowercase entire token
expect(normalizeContractId('grpc::ServiceName')).toBe('grpc::servicename');
});
it('trims and lowercases topic', () => {
expect(normalizeContractId('topic:: Employee.Hired ')).toBe('topic::employee.hired');
});
it('lowercases lib package coordinates', () => {
expect(normalizeContractId('lib::@Hr/Common::UserDTO')).toBe('lib::@hr/common::userdto');
});
});
describe('runExactMatch', () => {
const makeContract = (
id: string,
role: 'provider' | 'consumer',
repo: string,
): StoredContract => ({
contractId: id,
type: 'http',
role,
symbolUid: `uid-${repo}-${id}`,
symbolRef: { filePath: `src/${repo}.ts`, name: `fn-${id}` },
symbolName: `fn-${id}`,
confidence: 0.8,
meta: {},
repo,
});
it('matches provider and consumer with same contract ID', () => {
const contracts: StoredContract[] = [
makeContract('http::GET::/api/users', 'provider', 'backend'),
makeContract('http::GET::/api/users', 'consumer', 'frontend'),
];
const { matched, unmatched } = runExactMatch(contracts);
expect(matched).toHaveLength(1);
expect(matched[0].contractId).toBe('http::GET::/api/users');
expect(matched[0].matchType).toBe('exact');
expect(matched[0].confidence).toBe(1.0);
expect(matched[0].from.repo).toBe('frontend');
expect(matched[0].to.repo).toBe('backend');
expect(unmatched).toHaveLength(0);
});
it('handles multiple consumers for one provider', () => {
const contracts: StoredContract[] = [
makeContract('http::GET::/api/users', 'provider', 'backend'),
makeContract('http::GET::/api/users', 'consumer', 'frontend'),
makeContract('http::GET::/api/users', 'consumer', 'bff'),
];
const { matched } = runExactMatch(contracts);
expect(matched).toHaveLength(2);
});
it('reports unmatched contracts', () => {
const contracts: StoredContract[] = [
makeContract('http::GET::/api/users', 'provider', 'backend'),
makeContract('http::GET::/api/orphan', 'consumer', 'frontend'),
];
const { matched, unmatched } = runExactMatch(contracts);
expect(matched).toHaveLength(0);
expect(unmatched).toHaveLength(2);
});
it('normalizes contract IDs before matching', () => {
const contracts: StoredContract[] = [
makeContract('http::GET::/api/users/', 'provider', 'backend'),
makeContract('http::get::/api/users', 'consumer', 'frontend'),
];
const { matched } = runExactMatch(contracts);
expect(matched).toHaveLength(1);
});
it('does not match contracts within the same repo', () => {
const contracts: StoredContract[] = [
makeContract('http::GET::/api/users', 'provider', 'backend'),
makeContract('http::GET::/api/users', 'consumer', 'backend'),
];
const { matched } = runExactMatch(contracts);
expect(matched).toHaveLength(0);
});
it('matches same-repo contracts with different service boundaries', () => {
const contracts: StoredContract[] = [
{
...makeContract('http::GET::/api/users', 'provider', 'monorepo'),
service: 'services/auth',
},
{
...makeContract('http::GET::/api/users', 'consumer', 'monorepo'),
service: 'services/gateway',
},
];
const { matched } = runExactMatch(contracts);
expect(matched).toHaveLength(1);
expect(matched[0].from.repo).toBe('monorepo');
expect(matched[0].to.repo).toBe('monorepo');
expect(matched[0].from.service).toBe('services/gateway');
expect(matched[0].to.service).toBe('services/auth');
});
it('does not match same-repo contracts with same service', () => {
const contracts: StoredContract[] = [
{
...makeContract('http::GET::/api/users', 'provider', 'monorepo'),
service: 'services/auth',
},
{
...makeContract('http::GET::/api/users', 'consumer', 'monorepo'),
service: 'services/auth',
},
];
const { matched } = runExactMatch(contracts);
expect(matched).toHaveLength(0);
});
it('does not match same-repo when only one has service', () => {
const contracts: StoredContract[] = [
{
...makeContract('http::GET::/api/users', 'provider', 'monorepo'),
service: 'services/auth',
},
makeContract('http::GET::/api/users', 'consumer', 'monorepo'),
];
const { matched } = runExactMatch(contracts);
expect(matched).toHaveLength(0);
});
it('cross-repo matching works regardless of service field', () => {
const contracts: StoredContract[] = [
{ ...makeContract('http::GET::/api/users', 'provider', 'backend'), service: 'services/auth' },
{ ...makeContract('http::GET::/api/users', 'consumer', 'frontend'), service: 'services/web' },
];
const { matched } = runExactMatch(contracts);
expect(matched).toHaveLength(1);
expect(matched[0].from.service).toBe('services/web');
expect(matched[0].to.service).toBe('services/auth');
});
it('matches consumer http::*::path to a concrete provider method on that path', () => {
const contracts: StoredContract[] = [
makeContract('http::POST::/api/users', 'provider', 'backend'),
makeContract('http::*::/api/users', 'consumer', 'frontend'),
];
const { matched, unmatched } = runExactMatch(contracts);
expect(matched).toHaveLength(1);
expect(matched[0].contractId).toBe('http::*::/api/users');
expect(matched[0].to.repo).toBe('backend');
expect(unmatched).toHaveLength(0);
});
});
// ---------------------------------------------------------------------------
// Helpers for Task 6 tests
// ---------------------------------------------------------------------------
function makeGrpcContract(
id: string,
role: 'provider' | 'consumer',
repo: string,
overrides: Partial<StoredContract> = {},
): StoredContract {
return {
contractId: id,
type: 'grpc',
role,
symbolUid: `uid-${repo}-${id}`,
symbolRef: { filePath: `src/${repo}.ts`, name: `fn-${id}` },
symbolName: `fn-${id}`,
confidence: 0.9,
meta: {},
repo,
...overrides,
};
}
// ---------------------------------------------------------------------------
// buildProviderIndex
// ---------------------------------------------------------------------------
describe('buildProviderIndex', () => {
it('test_buildProviderIndex_creates_normalized_keys', () => {
const contracts: StoredContract[] = [
makeGrpcContract('grpc::Com.Example.UserService/GetUser', 'provider', 'backend'),
makeGrpcContract('grpc::Com.Example.UserService/GetUser', 'consumer', 'frontend'),
];
const index = buildProviderIndex(contracts);
// Only providers should be in the index
expect(index.size).toBe(1);
// Key should be normalized (lowercased package)
expect(index.has('grpc::com.example.userservice/GetUser')).toBe(true);
expect(index.get('grpc::com.example.userservice/GetUser')).toHaveLength(1);
expect(index.get('grpc::com.example.userservice/GetUser')![0].role).toBe('provider');
});
});
// ---------------------------------------------------------------------------
// runExactMatch — gRPC wildcard skip
// ---------------------------------------------------------------------------
describe('runExactMatch — gRPC wildcard handling', () => {
it('test_runExactMatch_skips_grpc_wildcard_contracts', () => {
const contracts: StoredContract[] = [
makeGrpcContract('grpc::com.example.UserService/*', 'consumer', 'frontend'),
makeGrpcContract('grpc::com.example.UserService/*', 'provider', 'backend'),
];
const { matched, unmatched } = runExactMatch(contracts);
// gRPC wildcards should NOT be matched in exact pass
expect(matched).toHaveLength(0);
// Both should appear in unmatched
expect(unmatched).toHaveLength(2);
});
it('test_runExactMatch_does_not_skip_http_wildcards', () => {
const contracts: StoredContract[] = [
{
contractId: 'http::GET::/api/users',
type: 'http',
role: 'provider',
symbolUid: 'uid-backend-http',
symbolRef: { filePath: 'src/backend.ts', name: 'fn-http' },
symbolName: 'fn-http',
confidence: 0.9,
meta: {},
repo: 'backend',
},
{
contractId: 'http::*::/api/users',
type: 'http',
role: 'consumer',
symbolUid: 'uid-frontend-http',
symbolRef: { filePath: 'src/frontend.ts', name: 'fn-http' },
symbolName: 'fn-http',
confidence: 0.9,
meta: {},
repo: 'frontend',
},
];
const { matched } = runExactMatch(contracts);
// HTTP wildcard should still match via findMatchingKeys
expect(matched).toHaveLength(1);
expect(matched[0].contractId).toBe('http::*::/api/users');
});
});
// ---------------------------------------------------------------------------
// runWildcardMatch
// ---------------------------------------------------------------------------
describe('runWildcardMatch', () => {
it('test_runWildcardMatch_fq_service_match', () => {
const consumer = makeGrpcContract('grpc::com.example.UserService/*', 'consumer', 'frontend');
const provider = makeGrpcContract(
'grpc::com.example.UserService/GetUser',
'provider',
'backend',
);
const providerIndex = buildProviderIndex([provider]);
const { matched } = runWildcardMatch([consumer], providerIndex);
expect(matched).toHaveLength(1);
expect(matched[0].from.repo).toBe('frontend');
expect(matched[0].to.repo).toBe('backend');
});
it('test_runWildcardMatch_bare_name_match', () => {
const consumer = makeGrpcContract('grpc::UserService/*', 'consumer', 'frontend');
const provider = makeGrpcContract(
'grpc::com.example.UserService/GetUser',
'provider',
'backend',
);
const providerIndex = buildProviderIndex([provider]);
const { matched } = runWildcardMatch([consumer], providerIndex);
expect(matched).toHaveLength(1);
expect(matched[0].from.repo).toBe('frontend');
expect(matched[0].to.repo).toBe('backend');
});
it('test_runWildcardMatch_no_match_different_service', () => {
const consumer = makeGrpcContract('grpc::UserService/*', 'consumer', 'frontend');
const provider = makeGrpcContract(
'grpc::com.example.OtherService/GetUser',
'provider',
'backend',
);
const providerIndex = buildProviderIndex([provider]);
const { matched, remaining } = runWildcardMatch([consumer], providerIndex);
expect(matched).toHaveLength(0);
expect(remaining).toContainEqual(consumer);
});
it('test_runWildcardMatch_skips_wildcard_providers', () => {
const consumer = makeGrpcContract('grpc::com.example.UserService/*', 'consumer', 'frontend');
const provider = makeGrpcContract('grpc::com.example.UserService/*', 'provider', 'backend');
const providerIndex = buildProviderIndex([provider]);
const { matched } = runWildcardMatch([consumer], providerIndex);
// Wildcard provider key ends with /*, so it should be skipped
expect(matched).toHaveLength(0);
});
it('test_runWildcardMatch_confidence_min', () => {
const consumer = makeGrpcContract('grpc::com.example.UserService/*', 'consumer', 'frontend', {
confidence: 0.7,
});
const provider = makeGrpcContract(
'grpc::com.example.UserService/GetUser',
'provider',
'backend',
{
confidence: 0.5,
},
);
const providerIndex = buildProviderIndex([provider]);
const { matched } = runWildcardMatch([consumer], providerIndex);
expect(matched).toHaveLength(1);
expect(matched[0].confidence).toBe(0.5);
});
it('test_runWildcardMatch_matchType_wildcard', () => {
const consumer = makeGrpcContract('grpc::com.example.UserService/*', 'consumer', 'frontend');
const provider = makeGrpcContract(
'grpc::com.example.UserService/GetUser',
'provider',
'backend',
);
const providerIndex = buildProviderIndex([provider]);
const { matched } = runWildcardMatch([consumer], providerIndex);
expect(matched).toHaveLength(1);
expect(matched[0].matchType).toBe('wildcard');
});
it('test_runWildcardMatch_contractId_is_consumers', () => {
const consumer = makeGrpcContract('grpc::com.example.UserService/*', 'consumer', 'frontend');
const provider = makeGrpcContract(
'grpc::com.example.UserService/GetUser',
'provider',
'backend',
);
const providerIndex = buildProviderIndex([provider]);
const { matched } = runWildcardMatch([consumer], providerIndex);
expect(matched).toHaveLength(1);
expect(matched[0].contractId).toBe('grpc::com.example.UserService/*');
});
});