Integrate the pyan3 file and script visualization layer (MOO-70)#4
Conversation
Server-side, Node-native tree-sitter parsing of a Python file's source into a flat symbol index (module/class/function/method), using web-tree-sitter loaded from disk rather than the browser client's CDN-fetching parser. Entries use the exact GraphIR SourceCoordinate field names (startLine/startColumn/endLine/endColumn, symbolPath, symbolKind) so the pyan3 join and GraphIR conversion in later commits need no reshaping.
Server-side execFile-only (no shell) wrapper around the pinned pyan3==2.6.2 Python package: stages already-fetched file content into the MOO-67 request workspace, runs pyan3 with an explicit file list (pyan3 has no directory/glob mode of its own) and --root pinned to the workspace dir, and categorizes failures into the shared ErrorCategory set — distinguishing pyan3's own SyntaxError crash (parser_failure, since pyan3 unlike the Commit 1 tree-sitter indexer is not syntax-error-tolerant) from other subprocess failures and timeouts. PYTHON_BIN/PYAN3_TIMEOUT_MS added to server config following the existing fail-fast env-var pattern, and CI now installs the pinned pyan3 via actions/setup-python. verifyPyan3Available is intentionally not wired into server startup yet, since no route depends on it until Commit 7's endpoint exists — that wiring is deferred to avoid breaking server boot for an unshipped feature.
Wraps ts-graphviz (chosen after a bounded maintenance check: it's the only actively-maintained DOT parser among graphlib-dot/dotparser/ @ts-graphviz/parser, all 3+ year abandoned single versions) to parse pyan3's raw DOT output, separating GraphViz layout attributes (fillcolor/fontcolor/style/group/rankdir) from the semantic content the Commit 4 join needs: each node's dotted qualified name, source path, line, and symbol kind, extracted from pyan3's tooltip attribute. Fixed a real parsing bug found via real-fixture testing: pyan3's tooltip uses a literal two-character "\n" as its line-break marker (Graphviz's own escape, not a real newline), which a naive global split corrupted whenever a staged file's absolute path happened to contain that same two-character sequence by coincidence (e.g. a path segment like "...\nested.py"). Fixed by taking only the first such occurrence for the qualifiedName boundary and anchoring kind/line extraction at the end of the string via regex instead of a second blind split. Edge-kind classification (defines vs uses, from pyan3's edge style/ color convention) is confirmed against a dedicated cross-method-call fixture, not just incidental containment edges.
Reconciles pyan3's relationship data with the tree-sitter symbol index, keyed on the dotted qualifiedName both analyzers derive from the same --root-relative module id (reliable, not coincidental, since pyan3Adapter.js always pins --root to the request workspace dir). Three distinct join outcomes, confirmed against real spike behavior rather than the illustrative file-pyan.json fixture: unresolved (zero symbol-index matches), ambiguous (more than one match -- defensively detected, shouldn't normally occur), and symbolOnly (tree-sitter has it, pyan3 doesn't -- e.g. pyan3 crashed on that file). Deliberately does not fall back to reverse-mangling a pyan3 node's "__" id into a dotted name, since that's ambiguous for any name already containing a literal double underscore (Python dunders). A real spike (importing an unstaged module, a third-party package, and calling an undefined name) confirmed pyan3 silently drops any cross-boundary reference entirely -- no placeholder node -- so a genuinely unresolved external call is invisible to both analyzers and is not synthesized here; it's a documented static-analysis limitation for Commit 9, not a graph node.
Adapts pyanSymbolJoin.js's output into a file-layer GraphIR, mirroring repositoryGraphAdapter.js's pattern exactly: a pure function producing a bare GraphIR via makeGraphIR, not an AdapterResult -- that envelope (partial/diagnostics/cache) is the Commit 7 service endpoint's job, same split the repository layer already uses between the adapter and server/routes/graph-repository.js. Matched/symbolOnly nodes get full authoritative coordinates from the tree-sitter symbol index; unresolved/ambiguous nodes get coordinate: null rather than a fabricated range from pyan3's tooltip text. Groups are derived from the resolved tree-sitter scope chain (parentScope), not pyan3's own DOT subgraph clusters, consistent with this ticket's "don't rely on presentation labels as source metadata" decision. symbolOnly nodes get metadata.noRelationshipData so a renderer can distinguish "known identity, no relationship data" from "unresolved identity" (unresolved/ambiguous). Fixed a real bug the ambiguous-duplicate scenario surfaced during testing: symbolOnly node ids are synthesized from path+qualifiedName, not qualifiedName alone, since two symbol-index entries can share a qualifiedName in the ambiguous case and both get left unclaimed -- qualifiedName alone would collide on node id.
Minimal, deliberately conservative depth policy for the file layer, matching the precedent set by the repository layer's own density work (MOO-69, docs/repository-layer-density.md), which built no clustering/node-budget/reduction logic and deferred all of that to the Garrison Step (MOO-44). MOO-70's ticket requires some depth policy to ship now, so this is the minimal version that satisfies it: a pure post-processing filter over the already-built full-detail GraphIR (Commits 1-5), keyed on the already-structural coordinate.symbolPath.length -- no clustering, no relevance scoring, no re-invoking pyan3 at a different depth. Four modes (modules/symbols/methods/full) map to symbolPath-length cutoffs. Package requests start at modules; file requests start at symbols or methods depending on an initial symbol-count estimate. Auto-increases toward full while the next mode's node/edge counts stay within an initial, documented-as-tunable budget; an explicit override is respected exactly with no auto-increase on top of it. Unresolved/ambiguous nodes stay visible in every mode since they're rare relationship warnings, not deep symbols. No committed fixture is large enough to exercise the "stops at a coarser mode because the budget was exceeded" path -- same limitation repository-layer-density.md notes for its own threshold -- so that path is tested with a hand-built synthetic graph, mirroring Commit 4's synthetic unresolved/ambiguous tests for the same reason.
Real-world verification against psf/requests' models.py (during MOO-70 Commit 7 work) surfaced a genuine bug: typing.overload stubs (multiple "def _encode_params(...)" signatures followed by one real implementation -- also hit via @Property getter/setter/deleter and conditional "if PY2: def x / else: def x" patterns) are common, idiomatic Python, not an edge case. Each overload stub is a distinct tree-sitter definition sharing one qualifiedName, which pyan3 collapses to a single node (matching Python's actual last-definition-wins name-shadowing semantics). The join previously treated any qualifiedName with more than one canonical match as "ambiguous" and left every candidate unclaimed, producing multiple symbolOnly entries with identical synthesized node ids -- a GraphIR schema violation ("duplicate node id"). Fixed by tie-breaking multiple canonical matches on startLine (highest wins), matching pyan3's own resolution and Python's real semantics. "ambiguous" is now reserved for a genuine tie in source position, which shouldn't occur for distinct definitions. Added a test reproducing the overload-stub scenario directly (tie-break to the last definition, zero ambiguous/symbolOnly output), alongside the existing genuine-tie test (same startLine) which still correctly asserts ambiguous.
New POST /api/graph/file, mirroring graph-repository.js's phase1 (network/subprocess, categorized failures) / phase2 (pure glue, AdapterResult) structure and reusing its buildRequestContext/ withTimeout/GraphAnalysisTimeoutError/RATE_LIMIT_PATTERN directly. Fetches only the requested file/package's blobs via github-analyzer-bridge.js's now-exported resolveRef/resolveCommitSha/ fetchTree/fetchAllContents, rather than analyzing the whole repository. Mode (file vs package) is inferred from the fetched Git tree rather than a client-supplied flag, so client and server can never disagree about what a path actually is. Revision pinning reuses the existing validated `ref` field (a drill-down client passes the parent graph's resolvedSha as ref) rather than adding a new field. pyan3 failing for a request degrades to a tree-sitter-only graph (Commit 5's symbolOnly path) instead of failing the request. verifyPyan3Available is now wired into server startup, since this is the first route that depends on it (deferred here intentionally back in Commit 2). Verified end-to-end against real GitHub (psf/requests) for both file and package-style requests, including a depth override -- this is what surfaced and got fixed as the prior commit's pyanSymbolJoin bug. No GITHUB_TOKEN is available in this environment, so full HTTP-path smoke-test coverage (mirroring tests/server-smoke.mjs's existing /api/graph/repository steps) is deferred/unverified; this commit adds full unit coverage of everything testable without network access, matching tests/server-graph-repository.test.mjs's own scope.
Adds the first real browser view of the file layer plus the first
client integration with the private server's auth-gated API. Real
research (not assumption) surfaced this scope: the browser client
never called any /api/graph/* endpoint before this -- repository
analysis runs entirely client-side against GitHub directly, with zero
existing fetch/auth plumbing for the server's own AUTH_TOKEN. Since
pyan3 is a Python subprocess, the file layer has no client-only
equivalent, so this had to happen now, scoped minimally per
confirmation: a small fetch wrapper (src/state/graphFileClient.js) and
an inline server-token prompt in the new panel, not a general
private-server-auth overhaul.
New src/render/fileRenderModel.js + src/render/fileGraph.js mirror the
repository layer's render-model/D3-renderer split but read GraphIR's
own hints.shape/hints.colorRole directly (repositoryGraph.js never
does, despite carrying them since MOO-68) -- a deliberately simpler,
separate renderer rather than bending the repository one to a
structurally different node schema (module/class/function/method vs
files/folders), matching the ticket's "specialized renderer fallback"
allowance. Convex-hull group visualization and full-text search/match-
cycling ("Raven patterns" with no existing analog anywhere in this
codebase) are explicitly deferred, mirroring
docs/repository-layer-density.md's own precedent of documenting rather
than speculatively building unvalidated UI.
Fixed a real reuse gap found while wiring drill-down: `tryCreateDrillDown`/
`findNodeByPath` key on coordinate.path, which uniquely identifies a
node at the repository layer (one node per file) but not at the file
layer (many functions/methods share one file's path). Added id-keyed
counterparts (findNodeById, tryCreateDrillDownById,
createSelectionEventForId, tryCreateOpenSourceEventById) to
src/state/repositoryDrillDown.js rather than misusing the path-keyed
originals against data they were never built to disambiguate.
Verified end-to-end in a real headless-Chromium browser session
(Playwright, already a project devDependency) against a real public
repo through the real server: repository graph render -> double-click
drill to file -> server-auth prompt -> real POST /api/graph/file ->
file-layer graph render with correct shapes/hover tooltips ->
double-click a function node -> correct MOO-71-pending state via the
existing breadcrumb/navigation contract. Zero console errors
throughout.
Server: server/routes/graph-file.js's phases are now individually diagnosed rather than falling into one generic catch-all -- source retrieval, workspace preparation, pyan3, and graph construction (symbol indexing + join + GraphIR conversion + depth selection) each get their own error handling and log message, satisfying the ticket's "diagnostic output identifies which stage failed" check. Logging now includes analyzer name/version and join match-rate stats (matched/unresolved/ambiguous/symbolOnly counts), not just node/edge counts. Extracted the pyan3-invocation-and-degrade logic into an exported, never-throwing runPyan3ForFile() specifically so the "keep [analysis] operational when pyan3 fails" resilience requirement is verified by a real test (forcing pyan3 to fail against syntax_error.py, no network needed) rather than only asserted in a comment. Documented why this failure is isolated from the repository layer: separate handler closures over the same read-only config, separate client render code paths -- architectural, not incidental. Client: FileLayerPanel gets a Retry button on fetch failure (the ticket's "user-facing retry" ask), wired via a retry-counter dependency that re-triggers the existing fetch effect. docs/file-layer-limitations.md documents the known static-analysis limitations flagged (but deliberately not fixed) throughout this ticket -- cross-boundary calls invisible to both analyzers, framework/ dynamic-dispatch reachability, last-definition-wins resolution for typing.overload/@Property redefinitions -- plus Garrison Step handoff notes for what Commit 8 deferred (convex hulls, search/match-cycling, within-mode node-budget reduction), mirroring docs/repository-layer-density.md's own precedent for this kind of documentation-only commit. This completes MOO-70's 9-commit checklist.
|
The join can silently attach the wrong canonical coordinate MOO-70 requires joining by path, qualified name, scope, and symbol kind, with conservative fallback behavior. The implementation currently indexes candidates only by qualifiedName, chooses the candidate with the greatest startLine, and then merely emits warnings if the path or kind differs. The node still receives matchState: "matched" and the selected symbol’s exact coordinate. That permits a path or kind mismatch to receive a high-confidence source coordinate—the precise failure the dual-analyzer design is meant to prevent. The join should: collect qualified-name candidates; Warnings are insufficient when the mismatched result is still used as authoritative identity. |
|
The symbol index misses definitions inside conditional and compound statements walkBody() only processes definitions that are direct children of the current module, class, or function body. Any non-definition child is skipped completely. A function declared inside an if, try, with, loop, or match branch will therefore never enter the tree-sitter index. That is particularly relevant because the PR explicitly cites conditional same-name definitions as a handled real-world case. The current tests cover direct nesting but contain no control-flow-wrapped definition fixture. Recursively traverse non-scope compound nodes using the existing scope chain and enclosing kind, while retaining the current special handling when an actual class or function introduces a new lexical scope. Add fixtures for definitions under if/else and try/except. |
|
The server-token prompt breaks after the first typed character FileLayerPanel renders the token input only while serverAuthToken is empty. The input’s onChange writes directly into that state. As soon as the first character is typed, the state becomes truthy, the input disappears, and a request starts with the one-character token. It only works when the entire token is pasted in one event. Use separate draft and committed token state: const [tokenDraft, setTokenDraft] = useState(''); Keep the input visible until explicit submission, then commit the complete token and begin the request. An authentication failure should also offer a way to replace the stored token rather than only retrying the same invalid one. |
|
Pyan3 availability currently takes down the entire application Per-request pyan3 failures correctly degrade to a tree-sitter-only graph. But the server performs a startup verification and exits the entire process if Python or pyan3 is unavailable. That also disables the repository layer and static application, contradicting MOO-70’s requirement that repository navigation remain operational when pyan3 fails. There is also no production Python dependency manifest in the PR. CI explicitly installs pyan3==2.6.2, but Railway is configured only to build through Railpack and execute npm start; package.json naturally contains only the JavaScript dependencies. Add a deployment-level pinned Python dependency declaration. Startup verification should record file-layer capability rather than terminate the server. A missing pyan3 installation can either: leave /api/graph/file operational in tree-sitter-only mode with a prominent warning; or |
|
The file endpoint still applies limits to the entire repository The endpoint claims to retrieve only the requested file or package, but it first calls fetchTree(), which applies MAX_REPO_FILES and MAX_REPO_BYTES to every analyzable file in the repository. Only afterward does it select the requested path. A tiny Python file in a large monorepo can therefore be rejected because unrelated repository content exceeds the repository-analysis limits. The recursive tree may be repository-wide, but file count and byte budgets for this endpoint should be applied after selecting the requested file/package. Package-specific limits can remain strict. |
Fixes 5 real issues found in review of MOO-70's PR: 1. Join could silently attach the wrong canonical coordinate. pyanSymbolJoin.js previously matched purely on qualifiedName and applied the same-name tie-break unconditionally, so a pyan3 node whose reported path or kind genuinely disagreed with its one candidate still got matchState:'matched' and that candidate's exact coordinate -- only a warning was logged. Fixed: path and kind/scope compatibility are now hard filters applied before any tie-break; a real mismatch now correctly falls through to 'unresolved'. staticmethod/classmethod are treated as compatible with our canonical 'method' kind (confirmed real pyan3 behavior), not a mismatch. This surfaced a related latent bug: pythonSymbolIndex.js never normalized its own `path` field, so the join's new path filter broke immediately on Windows (backslash vs pyan3's already-normalized forward-slash paths) -- fixed at the source. 2. The symbol index missed definitions inside conditional/compound statements. walkBody (renamed walkNode) only checked direct children of a body block, silently skipping any def nested inside if/elif/else, try/except/finally, for/while, with, or match -- exactly the `if PY2: def x / else: def x` pattern this same PR's join fix was written to handle, but which the indexer itself never surfaced as an entry. Now recurses into every non-scope-introducing node at the same lexical scope. Added a fixture covering all of these control-flow forms. 3. The server-token prompt broke after the first typed character. FileLayerPanel rendered the input only while the same state it wrote into via onChange was empty -- typing one character made it disappear. Separated draft/committed token state with an explicit submit action, plus a "Change token" affordance on any error (not just a detected 401), verified live by typing character-by-character rather than pasting. 4. pyan3 unavailability took down the entire application. server/index.js called process.exit(1) if the startup pyan3 check failed, disabling the repository layer and static app too -- contradicting this ticket's own "keep the repository layer operational when pyan3 fails" requirement. Now logs a warning and continues; each /api/graph/file request already degrades gracefully per-request, which turns out to be the same mechanism that covers a totally-missing install. Surfaced non-gating in /readyz. Added requirements.txt (pinned pyan3==2.6.2) and a best-effort postinstall script, since production had no Python dependency manifest at all -- documented as unverified against a live Railway deployment, since this repo has no way to exercise that here. 5. The file endpoint applied whole-repository size/count limits before selecting the requested file/package, so a tiny file in a large monorepo could be rejected for reasons unrelated to what was actually requested. Added fetchRawTree (no limits) + enforceFileRequestLimits (scoped to just the selected target set), replacing the whole-repo fetchTree/selectAnalyzableFiles call. All fixes verified against real repositories/binaries, not mocked: 369/369 tests passing, plus a real end-to-end browser re-verification of the token-input fix (typed character-by-character) and the degraded-server-still-serves-traffic behavior.
|
Addressed all 5 comments in 744b996:
All fixes verified against real repositories/binaries (not mocked): 369/369 tests passing, plus real end-to-end browser re-verification of the token-input fix and the degraded-server-stays-up behavior. |
|
/api/graph/file does not initialize the server-held GitHub token The route calls resolveRef, resolveCommitSha, fetchRawTree, and fetchAllContents directly. Those helpers ultimately use the shared GitHub object, whose token begins as an empty string. The token is currently assigned only inside analyzeGithubRepo(), which this route never calls. The public-repository browser test can therefore pass unauthenticated, while private repositories fail and public requests use GitHub’s much lower unauthenticated rate limit unless some unrelated repository-endpoint request happened to initialize the module first. Add an explicit bridge configuration function or pass config.githubToken through the request helpers. Do not rely on mutable initialization as a side effect of another route. |
|
Forked-PR drill-down uses the wrong allowlist identity The browser currently sends repositoryGraph.context.sourceOwner/sourceRepo to /api/graph/file. The server then applies its allowlist to those request fields. For a forked PR, that means it checks the contributor’s fork rather than the allowlisted base repository. It also discards the PR/base provenance that MOO-68 deliberately separated from the actual content-source repository. For PR contexts, send the base context.owner/context.repo plus prNumber; let resolveRef() recover the fork and head SHA. For ordinary branch/commit contexts, base and source remain identical. |
|
The raw recursive tree can still be truncated fetchRawTree() returns data.tree without checking GitHub’s truncated flag. GitHub caps recursive tree responses at 100,000 entries or 7 MB and instructs clients to traverse subtrees non-recursively when the response is truncated. That matters specifically to finding 5: a requested tiny file in a very large monorepo is no longer rejected by unrelated byte limits, but it can still be falsely reported as “not found” if its entry was omitted from the truncated response. At minimum, detect truncation and return a truthful categorized error. Preferably, traverse only the requested path’s subtrees so the file endpoint never needs a complete recursive repository tree. |
1. /api/graph/file never initialized the server-held GitHub token. resolveRef/resolveCommitSha/etc. use the shared GitHub client, whose token was only ever set inside analyzeGithubRepo() (the repository layer's entry point, never called here) -- meaning every GitHub call this route made ran unauthenticated unless some unrelated /api/graph/repository request happened to run first in the same process and set the shared token as a side effect. Added configureGithubClient(), called explicitly at the start of this route's fetch phase. Verified live: a completely fresh server process, hitting /api/graph/file as the literal first request, with no prior repository-endpoint call. 2. Forked-PR drill-down used the wrong allowlist identity. The client sent repositoryGraph.context.sourceOwner/sourceRepo (the resolved fork, for a forked PR) as the request's owner/repo, which the server applies its allowlist check to -- checking the contributor's fork instead of the allowlisted base repository, and losing the PR number resolveRef() needs to recover the fork/head sha itself. Now sends the base context.owner/context.repo plus `pr` for PR-mode graphs; branch/commit modes are unaffected (base and source are already identical there). Covered by a new unit test; not live-verified against an actual forked PR in this session (would need a real forked PR plus a matching allowlist setup) -- flagged rather than claimed. 3. The raw recursive tree could still be truncated. fetchRawTree() fetched the entire repository tree recursively and returned it unchecked; GitHub caps this at 100,000 entries/7MB and sets `truncated: true` rather than erroring, so a legitimately-existing small file could be silently missing from the response and falsely reported "not found" -- undermining the prior round's fix for large monorepos. Implemented the reviewer's preferred fix rather than the minimal one (which would have made comment 5's own fix pointless for truly large repos): resolvePathEntry walks non-recursively straight to the requested path (no full-tree fetch at all), and fetchSubtreeFiles recursively lists only the matched subtree for a package request -- both far below GitHub's truncation limits even when the repository as a whole is not. fetchTree (used by the repository layer) also now checks `truncated` for the same reason. Verified live: single-file, package/subtree, and not-found paths all resolved correctly against a real repository. 370/370 tests passing, build clean.
|
Addressed all 3 in b176fad:
370/370 tests passing, build clean. |
|
Forked-PR drill-down is no longer revision-pinned The fork/allowlist fix correctly sends the base repository and PR number. However, in PR mode the client now deliberately omits the parent graph’s resolvedSha; the test explicitly requires this behavior. The server then re-resolves the PR’s current head at file-drill-down time and uses that new SHA without comparing it to the repository graph’s pinned revision. Therefore: repository graph built at PR head A That violates MOO-70’s explicit requirement that repository-to-file requests cannot cross revisions. The focused fix is to send the PR number and an expectedResolvedSha from the parent context. After resolving the PR, the server must compare the current head against that expected SHA and reject a stale drill-down with a clear “PR changed; refresh the repository graph” response. Ideally, compare the expected source owner/repository as well, or reuse the existing context-propagation assertion. After that small fix and test, PR 4 is ready to merge and MOO-70 is ready to close as Done. |
A PR-mode file-layer request always re-resolves the PR's *current* head server-side (resolveRef ignores any `ref` once `pr` is set, by design -- that's what let the base-repository allowlist fix in round 2 work at all). Without telling the server what revision the parent repository graph actually pinned, a PR receiving a new commit between that graph loading and a file drill-down would silently analyze a different revision -- exactly what MOO-70's revision-pinning requirement exists to prevent: repository graph built at PR head A PR receives another commit user opens a file file graph silently built at PR head B Fixed per the reviewer's own suggested approach: the client now also sends expectedResolvedSha/expectedSourceOwner/expectedSourceRepo (PR mode only) from the parent graph's own AnalysisContext. validateFileRequest validates them; graph-file.js's new assertRevisionStillExpected reuses src/graph-ir/githubContext.js's assertContextPropagation (the same mechanism MOO-68 built for exactly this class of check) to compare the freshly-resolved revision against what was expected, before doing any of the real work -- rejecting a stale drill-down with a clear 409 "the pull request has changed... refresh the repository graph" message rather than silently proceeding. Verified live against a real forked PR (psf/requests#7586, forked from PEliet/requests): a request with no expectation still succeeds (backward compatible), a request with a deliberately wrong expectedResolvedSha is correctly rejected with the 409, and a request with the actual current head sha passes through correctly -- closing the loop on the one item flagged as unit-tested-only in the round 2 reply. 380/380 tests passing, build clean.
|
Fixed in 2143cee, exactly as suggested: the client now also sends Verified live against a real forked PR (
380/380 tests passing, build clean. |
1 participant
Summary
Implements MOO-70's full 9-commit checklist: the Python file-layer visualization, combining pyan3's relationship analysis with a tree-sitter-derived canonical symbol index into GraphIR the shared renderer/navigation model (MOO-68/MOO-69) can display, with real drill-down from the repository layer down through files and (eligibility-wise) into functions, ahead of MOO-71.
Commit 1 —
server/lib/pythonSymbolIndex.js: server-side, Node-native tree-sitter (web-tree-sitter+tree-sitter-wasms, loaded from disk rather than the client's CDN-fetching parser) indexes classes/functions/methods/module scope into a flat symbol list, using GraphIR's actual flatSourceRangeshape from the start so later commits need no reshaping layer. Syntax errors degrade to a partial index (parseErrors: true) rather than throwing, since pyan3 (Commit 2) is not equally tolerant.Commit 2 —
server/lib/pyan3Adapter.js: pinnedpyan3==2.6.2run viaexecFile(never a shell), staged inside MOO-67'sWorkspaceManager. Real-environment spikes drove the design: pyan3 takes an explicit file list (no directory/glob mode — enumerated in Node, not shell-globbed), needs--rootpinned explicitly to avoid module-name mis-inference, and is not syntax-error-tolerant (an uncaught Python traceback, categorizedparser_failuredistinct from other subprocess failures).PYTHON_BINadded to server config (defaultpython3) since Windows dev boxes commonly only havepython.Commit 3 —
server/lib/dotGraph.js:ts-graphviz@3.0.7chosen after a bounded maintenance check (the only actively-maintained DOT parser among the candidates —graphlib-dot/dotparser/@ts-graphviz/parserare all 3+ year abandoned). Separates GraphViz layout attributes from the semantic payload pyan3 packs intotooltip(qualified name/path/line/kind). Found and fixed a real parsing bug via real-fixture testing: pyan3's tooltip line-break marker is a literal two-character\n, which a naive global split corrupted whenever a staged file's Windows path coincidentally contained that same sequence (e.g. a path ending in\nested.py).Commits 4 & 5 —
server/lib/pyanSymbolJoin.js+src/adapters/fileGraphAdapter.js: planned together (join output feeds GraphIR conversion directly), implemented/committed separately. Three join outcomes —matched,unresolved(zero canonical matches),ambiguous(multiple matches),symbolOnly(tree-sitter has it, pyan3 doesn't, e.g. pyan3 crashed on that file). A real spike showed pyan3 silently drops any call/import to code outside the analyzed file set (no placeholder node) — confirmed with the reviewer that "unresolved" should only cover real join failures, not synthesized nodes for invisible cross-boundary calls. Real-world testing againstpsf/requestssurfaced and fixed a genuine bug (0f9a771):typing.overloadstubs /@propertygetter-setter-deleter / conditionalif PY2: def xpatterns are common Python where multiple tree-sitter definitions share one qualified name that pyan3 collapses to a single node — fixed via last-startLine-wins tie-break, matching Python's actual name-shadowing semantics, reservingambiguousfor a genuine position tie.Commit 6 —
src/graph-ir/depthPolicy.js: deliberately minimal, mirroringdocs/repository-layer-density.md's own precedent of building nothing speculative — a pure post-processing filter over the already-built full-detail graph (no clustering, no relevance scoring, no re-running pyan3 at a different depth). Four modes keyed onsymbolPath.length; package requests start atmodules, file requests start atsymbols/methodsby an initial symbol-count estimate; auto-advances towardfullwhile under a documented, initial (untuned) node/edge budget. An explicit override is respected exactly, no auto-increase on top of it.Commit 7 —
server/routes/graph-file.js: newPOST /api/graph/file, mirroringgraph-repository.js's phase1/phase2 structure and reusing its exported helpers directly. Fetches only the requested file/package's blobs (never the whole repo). Mode (file vs. package) is inferred from the fetched Git tree, not a client flag, so client/server can never disagree about what a path is. Revision pinning reuses the existing validatedreffield (a drill-down client passes the parent'sresolvedShaasref) rather than adding a new one. This is also whereverifyPyan3Availablefinally gets wired into server startup, since it's the first route that depends on it.Commit 8 —
src/render/fileGraph.js,src/render/fileRenderModel.js,src/state/graphFileClient.js: real research (not assumption) found the browser client never called any/api/graph/*endpoint before this — repository analysis runs entirely client-side against GitHub directly. Since pyan3 is a Python subprocess, the file layer has no client-only equivalent, making this also the first client integration with the private server's auth-gated API — scoped minimally (a small fetch wrapper + inline server-token prompt, not a general auth overhaul). Found and fixed a reuse gap:tryCreateDrillDown/findNodeByPathkey oncoordinate.path, unique at the repository layer but not at the file layer (many functions share one file's path) — added id-keyed counterparts instead of misusing the path-keyed originals. Verified end-to-end in a real headless-Chromium session (Playwright) against a real public repo through the real server: repository graph, drill into file, server-auth prompt, real fetch, file graph renders with correct shapes/hover, drill into function shows the correct MOO-71-pending state. Convex-hull group visualization and full-text search/match-cycling ("Raven patterns," no existing analog anywhere in this codebase) are explicitly deferred.Commit 9 —
docs/file-layer-limitations.md+server/routes/graph-file.jsdiagnostics: each failure phase (source retrieval, workspace prep, pyan3, graph construction) now gets its own diagnosis rather than one generic catch-all; logging includes analyzer version and join match-rate stats. The "keep [analysis] operational when pyan3 fails" resilience requirement is proven by a real test (runPyan3ForFile, forcing pyan3 to fail against a real syntax-error fixture) rather than only asserted in a comment. Added a client retry button. Documented known static-analysis limitations (cross-boundary calls invisible to both analyzers, framework/dynamic-dispatch reachability, last-definition-wins resolution) and Garrison Step handoff notes, mirroringdocs/repository-layer-density.md's own precedent.Notable real-world bugs found and fixed along the way
Two genuine bugs surfaced by testing against real repositories/fixtures rather than only curated test data, both fixed before merge:
typing.overload/@propertysame-name redefinitions, found againstpsf/requests(Commit 4, fix in 0f9a771).Test plan
node --test tests/*.test.mjs— 356/356 passingnpm run build— cleanpyan3==2.6.2) subprocess exercised directly, not mocked, throughout/api/graph/filefetch, file graph render, function-layer pending state — zero console errors🤖 Generated with Claude Code