fix(cross-repo): emit HTTP_CALLS for unindexed client libs and normalize URLs for route matching (#523)#536
Conversation
|
Built this branch on macOS (arm64) and traced resolve_single_call to see exactly what happens. The two halves behave differently: pass_cross_repo.c (URL normalize + template match): works. Holding an HTTP_CALLS edge constant (url_path = the full URL http://order-service:8080/v2/orders/123), main links nothing and this branch links it to the templated route /v2/orders/{id}. Clean before/after. pass_calls.c (emit without the client lib indexed): doesn't fire for a genuinely external requests. The call reaches resolve_single_call and first_string_arg holds the URL, so that part is fine. But with requests not installed or vendored anywhere indexable, the import map is empty (imp_count=0), so cbm_registry_resolve returns an empty qualified_name and the function returns at The moment requests is locally resolvable (a vendored stub or an installed venv), imp_count=1, res_qn becomes '...requests.get', svc=1, and it emits, but that is the resolved path, which main emits on too. So the emit-without-target path seems to help only when the callee resolves to a QN that has no node, not when the external client resolves to nothing. Was requests pip-installed in your WSL repro? If so, cross_http_calls: 1 is the matcher fix plus a resolvable call, and the index-just-my-service case (the #523 scenario) is still 0. Happy to share the repro. |
|
Good catch you were exactly right. The emit sat after the empty-QN early return, so a genuinely external Fixed in the latest commit: the detection now lives in the empty-QN branch and classifies from the raw callee name (
One caveat worth flagging separately: a single-file provider still returns 0, but for an unrelated reason FastAPI route extraction ( thanks for tracing this on your end ;) |
|
Re-validated 0a8a44f on macOS (arm64), with a genuinely external requests (no stub, no install, consumer indexes only its own service):
So the empty-QN path is fixed. Confirmed on my end. On the single-file caveat: I'm not reproducing it here. My provider is a single app.py with two routes (@app.get + @app.post), both Route nodes extracted fine, which is why the end-to-end run above links. So the no-route-on-single-file behavior may be platform-specific or a narrower trigger than file count, rather than a general <50-file thing. Didn't block the cross-repo case for me, but happy to share details if you open a separate issue for it. Nice work turning this around so fast. |
|
Thanks for re-validating ;) You're right to push back on the single-file theory if your single Appreciate the thorough trace throughout it made both fixes tighter. |
|
Thanks @RithvikReddy0-0 — the unindexed-client + URL-normalization direction is right. Two things before this can land:
Also, |
…DeusData#523) Addresses review on DeusData#536. insert_cross_edge now skips insertion when an identical (source_id, target_id, type) edge already exists. The pass reaches the same caller/route pair from both directions and emit_cross_route_bidirectional writes both DBs, so without this guard the same CROSS_HTTP_CALLS pair was re-emitted and inflated http_edges. Verified idempotent: repeated runs and runs from either project side both yield cross_http_calls: 1 with exactly one edge per DB. Documented why emit_http_async_edge is called with source_node as both source and target in the unindexed-external-client path. Signed-off-by: RithvikReddy0-0 <rithvikreddymukkara@gmail.com>
|
Thanks for the review. Addressed all three in 4817d79. 1. Duplicate edges. Fixed via an idempotency guard in
One thing worth raising on the "single direction" suggestion: I don't think dropping the reverse Tradeoff I want to flag: the guard adds a 3. Self-pass comment. Added documents that the external client has no graph node, so 2. Test. This is the one I'd like a pointer on. |
|
Thanks @RithvikReddy0-0 — the two-pronged diagnosis (emit HTTP_CALLS for unindexed clients + bidirectional route matching) is right, and this is close. A few things before it can land:
Solid work overall — looking forward to the revision. |
4817d79 to
55fa3e1
Compare
|
Rebased onto latest main and adapted to the new
On the rebase: main's For the regression test — |
|
Thank you for the deep dive on #523 — half of this PR is still exactly what's needed. Status after verifying against current main: the pass_calls.c root cause was real, but main landed a broader variant in beaa776 two days after your last push (it also covers the parallel path), so that hunk is now superseded — dropping it resolves your merge conflict entirely. The pass_cross_repo.c half is NOT superseded and is genuinely still broken on main: no scheme/host stripping, no concrete-path-vs-template matching, forward-direction only. That's the valuable part. Could you: (1) rebase dropping the pass_calls.c hunk (cite beaa776 in the commit if you like); (2) add reproduce-first tests for the three cross-repo behaviors (scheme-stripped URL match, template fuzzy match, provider-side reverse run + dedup idempotence)? One perf note for the rework: the fuzzy fallback currently enumerates every Route node per unmatched edge — fine for now, but worth a comment acknowledging the O(calls×routes) bound. Thanks — the cross-repo matching fixes are wanted. Update: to keep momentum on the bug backlog, we're going to carry the changes above over the line ourselves shortly — a distilled follow-up on current main implementing the notes in this thread, with you credited as |
55fa3e1 to
6e577c8
Compare
…repo edges (DeusData#523) cross-repo-intelligence returned 0 HTTP edges because match_http_routes could not link a consumer's HTTP_CALLS to a provider's Route: - Consumer url_path carried a full URL (scheme+host+port) while the provider Route is a bare path. cr_url_path() strips scheme+authority before the route-QN lookup. main's cbm_route_canon_path normalizes placeholder syntax but not scheme/host. - Concrete paths (/v2/orders/123) never matched templated routes (/v2/orders/{}). cr_path_matches_template() + find_route_handler_fuzzy() do segment-wise matching as a fallback when the exact QN lookup misses. - match_http_routes only searched HTTP_CALLS in the src project; a provider-initiated run has no outbound calls. Added the reverse direction so both orientations are covered. - insert_cross_edge / emit_cross_route_bidirectional are idempotent and return whether a row was inserted; match loops count real inserts, so a pair matched from both directions doesn't duplicate or inflate counts. The external-client HTTP_CALLS emission originally in this PR is dropped: main's beaa776 landed a broader variant (covers registry mis-resolution and both pipeline paths) that supersedes it. Tests: extend tests/repro/repro_issue523.c with three reproduce-first cases for the matcher behaviors — scheme/host-stripped URL match, concrete-path vs templated-route fuzzy match, and provider-side reverse run + dedup idempotence (asserts exactly one CROSS_HTTP_CALLS row after two runs). All four DeusData#523 repro cases pass against the fix. Signed-off-by: RithvikReddy0-0 <rithvikreddymukkara@gmail.com>
6e577c8 to
7736348
Compare
|
Thanks I saw the offer and pushed the update just now (7736348), so no need to carry it yourselves. All three notes are addressed:
Rebased on current main, review requested. Happy to adjust anything, thanks for the detailed steer , this project is really interesting ;) and for the co-author offer either way. |
|
Thank you for the deep dive on #523 — half of this PR was still exactly what was needed. The pass_calls.c half had been superseded by beaa776 (which also covers the parallel path), but the pass_cross_repo.c half was genuinely still broken on main: no scheme/host stripping, no concrete-vs-template matching, forward-direction only. We carried that half over the line as 4cd8d3e (PR #810) with you credited as co-author, plus the reproduce-first tests the review asked for — and one correction: the pre-insert dedup guard turned out unnecessary (the store's upsert already prevents duplicates) and its row-counting semantics would have re-reported the exact 0-edges symptom on provider-side re-runs, so idempotence is asserted differently. Refs #523 — the reporter should see cross-repo edges now. Closing in favor of the distill — thanks again! |
Distills the pass_cross_repo.c half of DeusData#536; the pass_calls.c half landed separately as beaa776 (broader: covers the parallel path too). match_http_routes could not match a stored HTTP_CALLS url_path that was a full URL (scheme://host:port/path is stored raw from the call's first string argument), never matched a concrete client path against a templated route QN, and only ran consumer->provider, so a provider-side run found nothing - and worse, destroyed previously recorded links: delete_cross_edges wiped the provider's reverse edges and nothing recreated them. - cr_url_path(): strip scheme://host[:port] before route-QN construction; a bare base URL maps to the root route path. - cr_path_matches_template() + find_route_handler_fuzzy(): segment-wise fallback so /v2/orders/123 matches /v2/orders/{} (method-gated, only for edges the exact lookup missed; O(calls x routes) per project pair, bounded by CR_MAX_EDGES). - Reverse-direction match_http_routes run so provider-side invocations find the consumer's HTTP_CALLS and re-create the wiped reverse edges. Row dedup needs no insert guard: the edges table upserts on its (source_id, target_id, type) UNIQUE key. Four reproduce-first tests in tests/repro/repro_issue523.c (scheme-strip, template fuzzy match, provider-side run, bidirectional convergence), all red on the previous code, green with the fix. Refs DeusData#523 Co-authored-by: RithvikReddy0-0 <rithvikreddymukkara@gmail.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Fixes two root causes of cross-repo-intelligence returning 0 edges (#523).
pass_calls.c
HTTP client calls (requests, httpx, axios, etc.) were silently dropped when
the client library wasn't indexed (external pip/npm dep). The callee resolved
to a QN but cbm_gbuf_find_by_qn returned NULL, so the call was discarded
before HTTP classification.
Fix: detect known HTTP/async patterns via cbm_service_pattern_match and emit
the edge even without a target node in the graph.
pass_cross_repo.c
Three issues in match_http_routes:
Consumer url_path carried full URL (scheme+host+port); provider Route has
bare path. Added cr_url_path() to strip scheme+authority before QN lookup.
Concrete paths (/v2/orders/123) never matched templated routes
(/v2/orders/{id}). Added cr_path_matches_template() and
find_route_handler_fuzzy() for segment-level template matching.
match_http_routes only searched HTTP_CALLS in the src project. When
cross-repo is run from the provider side, HTTP_CALLS live in the consumer
DB. Added reverse direction call so both orientations are covered.
Repro
Verified manually: FastAPI provider + requests consumer, cross-repo-intelligence
now returns cross_http_calls: 1 where it previously returned 0.
Checklist
git commit -s) — required, CI rejectsunsigned commits (DCO, see CONTRIBUTING.md)
make -f Makefile.cbm test)make -f Makefile.cbm lint-ci)