mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-07 02:58:02 +00:00
5 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Dinh Huy
|
dbd4e1c9fb
|
feat(group): Support Django route extraction for multi-repo (#1836)
* [+] Add django route discovery to create cross-link for multi-repo * [+] Update ingestion * [~] Fix bugs and abstraction violation * feat(python-http): add keyword url= and variable propagation for consumer detection - Add REQUESTS_KEYWORD_URL_PATTERNS for requests.get(url='...') keyword args - Add WRAPPER_URI_PATTERNS for generic wrapper.fetch(uri='...') calls - Add WRAPPER_URI_VAR_PATTERNS + buildLocalStringMap for uri=variable propagation - Add LOCAL_STRING_ASSIGNMENTS to track uri='...' assignments - Wire both direct-string and variable-propagation loops in scan() - Add normalizeConsumerPath() helper Note: Automatic cross-link detection remains limited for runtime-computed URLs (URLs built via .format(), string concat, or module constants). Manual manifest links needed for known cross-repo contracts. * [+] add extract uri and url keywork pattern for request http * feat(python-http): add variable propagation for uri=/url= consumer patterns Re-add LOCAL_STRING_ASSIGNMENTS, WRAPPER_URI_VAR_PATTERNS, buildLocalStringMap(), and normalizeConsumerPath() lost during cherry-pick merge of upstream keyword-URL commit. Together with the upstream WRAPPER_URI_PATTERNS and REQUESTS_KEYWORD_URL_PATTERNS, we now detect: - requests.get(url='literal') keyword args - wrapper.fetch(uri='literal') keyword args - wrapper.fetch(uri=variable) where variable was assigned a string literal * fix(group): discover Django roots relative to manage.py dir + multi-project (#1836 R1) A Django project not at the repo root (e.g. backend/manage.py) discovered zero routes: the settings module path was resolved repo-root-relative only, so backend/myproj/settings.py was never found and discovery returned null. Resolve settings, star-imported base settings, ROOT_URLCONF, and the root urls.py against the manage.py's own directory first, then the repo root (resolvedSettingsPath is now project-dir-aware so relative imports anchor correctly). Iterate every manage.py so a monorepo with several Django projects yields each project's root — the provider hook becomes plural (discoverRootRouteFiles → string[]) and the main-thread pass loops over all roots (inner-scoped continues, parser hoisted once per language). Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(group): remove dead code in Django root discovery (#1836 R9) - Collapse the identical if/else in extractStarImports to one push. - Drop the unreachable baseModule.startsWith('.') branch (baseModule is always a resolved slash-path or a bare absolute module — never dot-prefixed). - Import DjangoFileReader from django.ts instead of re-declaring the type. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): walk Django includes once per prefix, not per file (#1836 R2) The include() recursion guard was keyed on file path alone and shared across the whole walk, so a urlconf included under two prefixes (a "diamond" — the same app mounted at /v1/ and /v2/) emitted routes for only the first mount. Key the guard on (resolvedFilePath, accumulatedPrefix) at all three sites (function entry, path()-wrapped include, bare include) so a file reached under two distinct prefixes is walked once per prefix while a genuine cycle (same file + same prefix) still terminates — null/'' prefixes collapse to one key so a no-prefix re-entry is treated as a cycle. MAX_INCLUDE_DEPTH remains the backstop. Adds diamond + self-include-cycle tests. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): extract Django routes from non-list urlpatterns (#1836 R3) findUrlpatternsLists only accepted a list-literal RHS, so common shapes yielded zero routes: concatenation (urlpatterns = a + b), wrapper calls (format_suffix_patterns([...]), i18n_patterns, staticfiles_urlpatterns), and tuples. Add collectUrlpatternContainers to descend binary_operator operands, known wrapper-call list arguments, and tuples. Inherently-dynamic forms (DRF router.urls, comprehensions, bare names) still yield nothing but now emit a debug log so the silent-zero case is observable rather than mysterious. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(group): thread Django parser explicitly, drop module singleton (#1836 R4) extractDjangoRoutes relied on a module-level _djangoParser set via setDjangoParser before each call — hidden state that would break if a second language ever used the include re-parse path, and an easy-to-forget contract. Pass the tree-sitter parser as an explicit parameter of extractDjangoRoutes (the extractRoutes provider hook already receives it) and delete the global plus its setter. The Python provider wires it directly; tests pass the parser in place of the removed setDjangoParser() call. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ingestion): isolate a throwing extractRoutes in the cross-file route pass (#1836 R5) The main-thread cross-file route pass called provider.extractRoutes without a guard, so a throw (e.g. a future grammar edge case in the include() walk) would propagate out of the parse phase and abort the entire analyze — unlike the worker, which isolates per-file failures. Wrap the per-root extractRoutes call in try/catch that logs a warning and continues to the next root. Export extractCrossFileRoutes and add a unit test driving a stub provider whose extractRoutes throws, asserting the pass returns [] and does not propagate. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(ingestion): bucket only route-capable languages in cross-file pass (#1836 R6) extractCrossFileRoutes runs in the deferred band on every analyze (incl. warm all-cache-hit runs). It now derives the set of languages whose provider exposes the cross-file route hooks once, returns early if none do, and buckets only those languages' paths — so a non-framework repo no longer pays to bucket the languages it doesn't use here. Route results are intentionally not persisted across runs, so a Django repo still re-derives its routes each analyze; documented inline that cross-run route caching is a deliberate follow-up rather than implemented here. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(group): prettier-format http-patterns/python.ts (#1836 R7) The file was not formatted to the root .prettierrc (the consumer-path normalizer used single-line try/catch and method chains), so the CI quality/format check (`prettier --check .`) failed. Reflow only — no logic change (`git diff -w` confines the change to normalizeConsumerPath's layout). Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): dedup Python URI detections by byte offset, not line arithmetic (#1836 R8) The wrapper-URI dedup key was lineNum*1000+methodRow, which can collide for distinct calls in files over 1000 lines (carry into the row term) and can fail to dedup a genuine duplicate when a node straddles a line boundary. Key on node byte offsets (`${pathNode.startIndex}:${methodNode.startIndex}`), matching the sibling seenVarDetections dedup a few lines below. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ingestion): end-to-end Django cross-file route extraction (#1836 R10) Adds an integration test that runs runPipelineFromRepo against a Django fixture whose project lives under backend/, asserting the resulting Route graph nodes (/health, /api/items, /api/items/<int:pk>). This exercises the previously-untested main-thread orchestration glue (discovery → parse → extractRoutes → allExtractedRoutes → Route nodes) and, because the project is in a subdirectory, regresses the subdir-discovery fix (R1) — a repo-root-only resolver would discover nothing and emit zero Route nodes. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): anchor Django include() resolution at the project root (#1836 review F1) resolveIncludedFile tried the bare repo-root candidate (app/urls.py) before the project-relative one, so in a monorepo with both a repo-root app/ and a backend/ Django project that also has an app/, include('app.urls') from the backend project resolved to the WRONG service's routes. Probe up-tree from the root urls.py for the nearest manage.py (the Django project root / sys.path entry) and try that-anchored candidate first. Absolute module paths like `app.urls` now resolve to <projectRoot>/app/urls.py unambiguously. When no manage.py is reachable (e.g. unit tests with a urls-only reader) the prior strategy order is preserved. Adds a monorepo wrong-app test. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): drop bogus Django provider source-scan, use graph routes (#1836 review F2) The DJANGO_PATH_PATTERNS / DJANGO_URL_PATTERNS source scan emitted an HTTP provider contract for every path()/re_path()/url() string literal, without checking it was inside urlpatterns, without skipping include() mount points, and without composing the include() prefix across files. For `path('api/', include('app.urls'))` + child `path('items/', view)` it emitted providers for `/api` (a mount, not a route) and `/items` (un-prefixed) — which survived the exact-contract-ID dedup alongside the correct graph route `/api/items`, polluting cross-repo matching with false providers. Remove the Django provider patterns and their scan blocks. Django provider contracts come from the graph Route nodes, which the ingestion route extractor builds with includes already composed (and now correctly, per the other fixes). Python HTTP *consumer* patterns (requests/wrapper) are unaffected. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): match method-agnostic Django providers to any-method consumers (#1836 review F3) Django function views are method-agnostic, so extractDjangoRoutes emits httpMethod '*'. That '*' was dropped by normalizeRouteMethod and then defaulted to GET by the contract extractor, while the matcher only expanded wildcard *consumers* — so a `POST /api/items` consumer never matched the Django provider that was silently narrowed to GET. - routes.ts: preserve '*' as a method-agnostic marker on the Route node, so the contract layer emits a wildcard provider (http::*::path) instead of GET. - matching.ts: make findMatchingKeys symmetric — a specific-method consumer now matches an exact-method provider OR a wildcard (http::*::) provider on the same path, mirroring the existing wildcard-consumer expansion. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Dinh Huy <huynd86@fpt.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
1PLee
|
05ca80ea30
|
feat(ingestion): add thrift contracts impl (#1234) | ||
|
Ivan Uzun
|
46586a8319
|
fix(group): add configurable cross-link path exclusions to reduce false positives (#1093)
* fix(group): add configurable cross-link path exclusions to reduce false positives Add matching.exclude_links_paths and matching.exclude_links_param_only_paths to group.yaml config. These filter out noisy HTTP contracts (health checks, param-only catch-all routes) from cross-link matching while preserving them in the contract registry for documentation purposes. Defaults are empty/false for backward compatibility — no behavior change unless the operator explicitly configures exclusions. * fix(group): address review findings — filter unmatched, normalize trailing slash, add tests - Excluded contracts no longer inflate SyncResult.unmatched (isNoisy guard) - pathPart in buildNoisyContractFilter strips trailing slashes before comparison - 8 new unit tests for buildNoisyContractFilter covering all code paths - Config-parser test asserts defaults for new matching fields * fix(group): normalize configured exclusion paths and add root-path test - Strip trailing slashes from configured exclude_links_paths at Set-build time so root path '/' (which normalizes to '') matches correctly - Add test: exclude_links_paths: ['/'] suppresses http::GET::/ contracts - Add new matching fields as commented examples in fixture group.yaml (DoD §2.4) * docs(group): document exclude_links_paths and exclude_links_param_only_paths config fields Add JSDoc to MatchingConfig interface, update the microservices guide YAML example and field notes, and scaffold the new fields (commented out) in the group create template. |
||
|
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 <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> |
||
|
ivkond
|
52277247fe |
feat(group): add group infrastructure and contract matching
Core foundation for repository group analysis: - Type system: ContractType, ExtractedContract, StoredContract, CrossLink with optional `service` field for intra-repo matching - Config parser for group.yaml (repos, detection flags, matching thresholds) - Contract registry storage with atomic writes - Exact matching engine with per-type normalization (HTTP, gRPC, topic) and intra-repo support (different services within same repo can match) - Extract LadybugDB pool-adapter from MCP backend for reuse by sync pipeline - Git staleness checker for group status reporting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |