feat: GoThirdPartyCombinedLoader — CDN-first + local fallback routing#648
Open
shivasurya wants to merge 10 commits intoshiva/pr-07-golang-thirdpartyfrom
Open
feat: GoThirdPartyCombinedLoader — CDN-first + local fallback routing#648shivasurya wants to merge 10 commits intoshiva/pr-07-golang-thirdpartyfrom
shivasurya wants to merge 10 commits intoshiva/pr-07-golang-thirdpartyfrom
Conversation
…ting - Add GoThirdPartyCombinedLoader wrapping CDN and local GoThirdPartyLoader instances with CDN-first resolution and authoritative-miss semantics - CDN transient errors (non-nil err) fall through to local; authoritative miss (nil, nil) stops resolution to avoid masking CDN extraction bugs - InitGoThirdPartyLoader creates combined loader when CDN manifest loads successfully; falls back to local-only on CDN failure or absent logger - CPF_CDN_URL env var overrides the CDN base URL at runtime Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
shivasurya
added
enhancement
shivasurya
added
enhancement
SafeDep Report Summary
Package Details
This report is generated by SafeDep Github App |
Code Pathfinder Security Scan
No security issues detected.
Powered by Code Pathfinder |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## shiva/pr-07-golang-thirdparty #648 +/- ##
=================================================================
+ Coverage 84.45% 84.59% +0.13%
=================================================================
Files 163 164 +1
Lines 23594 23915 +321
=================================================================
+ Hits 19926 20230 +304
- Misses 2920 2937 +17
Partials 748 748 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…tion Three targeted fixes to the Go call graph builder hot path: 1. buildParentMap once: findContainingGoFunction and findParentGoFunction previously rebuilt the full child→parent reverse-edge map on every call node (225k nodes × 65k calls ≈ 14.7B iterations on Ollama). Both now accept a pre-built parentMap passed from their call sites. 2. buildPkgVarIndex once: Source 3 in resolveGoCallTarget previously did a full O(N) scan of all CodeGraph nodes per unresolved method call to find package-level variables. Replaced with a map[dir::varName]*Node index built once before Pass 4, making each lookup O(1). 3. isBuiltin package-level var: promoted the builtin name map from a per-call allocation to a package-level var. All existing tests pass unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- resolveGoCallTarget now returns resolveSource as 4th value: "thirdparty_local" for Pattern 1a import calls and Check 2.5 method calls into third-party packages; "" for all other paths - Outer resolution loop propagates resolveSource to CallSite.TypeSource so resolution-report stats correctly count third-party resolutions instead of mislabeling them as user-code - Remove unused codeGraph parameter from resolveGoCallTarget - Add built-in type names (int, float64, string, rune, byte, etc.) to goBuiltins so T(x) type-conversion expressions resolve to builtin.T instead of being counted as unresolved function calls - Add TestThirdPartyResolution_TypeSourceLabeling verifying both Pattern 1a and Check 2.5 paths emit TypeSource=thirdparty_local Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three new call resolution improvements: 1. Receiver variable binding (go_variables.go): - Extracts receiver name (e.g. `s` in `func (s *Store) Method()`) - Adds typed GoVariableBinding to the method scope - Fixes `s.db.Query()`, `s.sched.Run()` etc. inside method bodies - New helper: extractReceiverName() 2. var_declaration support (go_variables.go): - Handles `var sb strings.Builder`, `var mu sync.Mutex` etc. - Uses explicit type annotation (confidence 0.9) when present - Falls back to RHS inference for `var x = someFunc()` - New functions: processVarDeclaration(), processVarSpec() 3. Struct field index + Source 4 (go_builder.go, core/types.go): - Builds GoStructFieldIndex: "PkgType.Field" → resolved field type FQN - Source 4 splits ObjectName="a.KNorm" → looks up root type then field - Resolves `a.KNorm.Forward()`, `m.TextEncoder.Encode()`, `db.conn.QueryRow()` - No signature changes: index stored in CallGraph.GoStructFieldIndex Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The two [debug-1b] Fprintf(os.Stderr) calls in resolveGoCallTarget fired unconditionally for every method call where scope lookup missed — tens of thousands of syscalls on large projects like Ollama. - Add logger *output.Logger parameter to BuildGoCallGraph and resolveGoCallTarget (nil-safe: tests pass nil, cmd callers pass logger) - Replace both fmt.Fprintf calls with logger.Debug(), guarded by logger.IsDebug() — only fires when --debug / VerbosityDebug is active - All four cmd entry points (scan, ci, serve, resolution-report) thread the existing logger through All tests pass unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t field index - go_variables_vardecl_test.go: 11 tests covering processVarDeclaration / processVarSpec (stdlib qualified, bytes.Buffer, sync.Mutex, net/url alias, multi-name, grouped var block, unqualified same-pkg, method body scope, RHS value fallback) and extractReceiverName (pointer + value receivers) - go_builder_structfield_test.go: 6 tests covering buildStructFieldIndex (basic mapping, same-package field, non-struct skip, embedded field skip) and Source 4 end-to-end (Attention.KNorm.Forward, Store.db.QueryRow) Also fixes processVarSpec condition: typeFQN != typeStr was incorrectly preventing bindings for stdlib packages where alias == import path (e.g. strings→strings, bytes→bytes). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ply pattern Split the sequential Pass 4 resolution loop into two stages: - Stage 1: N workers each resolve a shard of callsites independently, collecting (callerFQN, targetFQN, callSite) results into per-shard local slices. All reads in resolveGoCallTarget are immutable-by-Pass-4 or RWMutex-protected; no shared writes occur during this stage. - Stage 2: Single goroutine applies shard results sequentially via AddEdge/AddCallSite, eliminating the need for any mutex on the call graph write path. Progress is tracked via atomic.Int64 with \r overwrite matching the existing Pass 2b display pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add go_builder_pass4_test.go with 23 targeted tests covering all previously uncovered branches in the parallel Pass 4 code and the helper functions it depends on: Pass 4 worker body: - TestPass4_UnresolvedCallSiteRecorded — unresolved else branch (lines 289-303) - TestPass4_Source2EnrichmentStripsPointerPrefix — * stripping in Source 2 enrichment (lines 251-253) Debug logger paths: - TestResolveGoCallTarget_DebugLoggerScopeNoBinding (lines 587-589) - TestResolveGoCallTarget_DebugLoggerNoScope (lines 590-592) S4 struct field chained resolution: - TestResolveGoCallTarget_S4Source1FunctionParam (lines 624-630) - TestResolveGoCallTarget_S4Source3PkgVar (lines 644-648) Stdlib/ThirdParty loader paths: - TestResolveGoCallTarget_StdlibLoaderMethodFound (lines 674-676) - TestResolveGoCallTarget_ThirdPartyLoaderFound (lines 669-676) - TestResolveGoCallTarget_PromotedMethodViaCheck3 (lines 702-704) Pattern 4 unresolved: - TestResolveGoCallTarget_Pattern4Unresolved (line 740) buildStructFieldIndex edge cases: - TestBuildStructFieldIndex_DirNotInRegistry (lines 849-850) - TestBuildStructFieldIndex_EmptyTypeAfterPointerStrip (lines 863-864) Parent-map multilevel walk: - TestFindContainingGoFunction_MultilevelWalk (line 901) - TestFindParentGoFunction_MultilevelWalk (line 923) resolvePromotedMethod with StdlibLoader (lines 1040-1050): - TestResolvePromotedMethod_StdlibLoaderTypeNotFound - TestResolvePromotedMethod_StdlibLoaderInvalidFQN - TestResolvePromotedMethod_StdlibLoaderCallsFromFields resolvePromotedMethodFromFields method found (lines 1071-1076): - TestResolvePromotedMethodFromFields_MethodFoundInEmbedded All 29 packages pass; builder coverage 80.9% → 83.2%. Remaining 3 uncovered ranges are pre-existing progress-print paths requiring 5000+ nodes and an import-extraction error path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…olution - Add GetPackage to GoStdlibLoader interface for package-wide type scanning - Check 2b: scan package interfaces for promoted methods not listed on the concrete type in CDN data (e.g. testing.T.Fatalf → testing.TB.Fatalf) - S4-Source4b: lazy CDN lookup for stdlib struct fields (net/http.Request.Header → net/http.Header) when user-code struct index has no entry - Expand short-qualified CDN field types (url.URL) using calling file's importMap (url → net/url) before Check 4 rejects them as incomplete FQNs - Remove GoStructFieldIndex guard on Source 4 so S4-Source4b fires even for projects with no user-defined struct fields - Add gopls-based ground-truth validation tool under tools/validate_go_resolution - Improve Go call resolution: 79.1% → 88.4% on ollama, 94.8% on sast-engine Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ion_report Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GoThirdPartyCombinedLoaderwrapping CDN and localGoThirdPartyLoaderinstances with CDN-first resolutionerr) fall through to local; authoritative miss (nil, nil) stops resolution to avoid masking CDN data bugsInitGoThirdPartyLoadercreates a combined loader when CDN manifest loads; falls back to local-only on CDN failure or absent loggerCPF_CDN_URLenv var overrides the CDN base URL at runtime; package-levelthirdPartyRegistryBaseURLis test-overridableTest plan
go test ./graph/callgraph/registry/ ./graph/callgraph/builder/ -count=1— all passgolangci-lint run ./graph/callgraph/... ./tools/...— 0 issuesnilCDN or local loader degrades gracefully (skipped silently)🤖 Generated with Claude Code