refactor: extract shared abstractions (leiden, config, builder, features) - #1786
refactor: extract shared abstractions (leiden, config, builder, features)#1786carlos-alm merged 9 commits into
Conversation
Sorry, something went wrong.
…docs check acknowledged) Impact: 1 functions changed, 2 affected
…n algorithm files docs check acknowledged: internal helper extraction only, no user-facing feature/language/architecture-table changes. Impact: 10 functions changed, 27 affected
Impact: 11 functions changed, 11 affected
…n readFileSafe readFileSafe's Atomics.wait busy-block froze the entire Node.js event loop (all I/O and timer callbacks) for up to 100ms per retry on the watch-mode hot path. Extracts journal.ts's existing sleepSync busy-spin helper into src/shared/sleep.ts so both readFileSafe and journal.ts's lock-retry loop share one implementation instead of duplicating it. docs check acknowledged: internal bug fix, no feature/language/architecture table changes warranted in README.md, CLAUDE.md, or ROADMAP.md. Impact: 2 functions changed, 31 affected
…complexity.ts Impact: 14 functions changed, 10 affected
Registers resolveSecrets' execFileSync timeout/maxBuffer in DEFAULTS.llm (apiKeyCommandTimeoutMs, apiKeyCommandMaxBufferBytes) and wires resolveSecrets to read them from config instead of hardcoding. Also adds three purely-additive @reserved DEFAULTS entries for constants hardcoded elsewhere in the codebase (build. largeCodebaseFileThreshold, db.busyTimeoutMs, community. capacityGrowthFactor) so their consumer files can be wired to them in follow-up commits. docs check acknowledged — no new feature/language/architecture change; docs/guides/configuration.md (the actual config reference) is already updated in this commit. Impact: 5 functions changed, 87 affected
Codegraph Impact Analysis42 functions changed → 165 callers affected across 72 files
|
Sorry, something went wrong.
Greptile SummaryThis PR extracts five clusters of duplicated logic into shared helpers: typed-array accessors and community-aggregate functions for the Leiden algorithm, a source-scanning name-map module for the benchmark scripts, a resolveFileTree helper shared by complexity.ts and dataflow.ts, a sleepSync module replacing Atomics.wait in readFileSafe, and four previously-hardcoded constants promoted into DEFAULTS/CodegraphConfig.
Confidence Score: 5/5Safe to merge — all five extraction clusters are behaviorally equivalent to the code they replace, and the two immediately-wired config fields are covered by updated tests. Every call site that changed argument order for accumulateNodeAggregates was updated consistently, TypeScript would catch any mismatch at compile time, and the DATAFLOW_RULES guard that was previously flagged is explicitly preserved. The resolveFileTree shared helper faithfully captures both callers' logic including the extension-set gate, the cached-tree fast path, and langId fallback. The @reserved config fields are clearly annotated and absent from user-facing docs. No files require special attention; the refactoring is mechanical and the call-site updates are complete across all five clusters. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Leiden
A[partition.ts] --> H1[aggregate-helpers.ts]
B[index.ts] --> H1
A --> H2[typed-array-helpers.ts]
B --> H2
C[adapter.ts] --> H2
end
subgraph Features
D[complexity.ts] --> R[resolve-file-tree.ts]
E[dataflow.ts] --> R
end
subgraph Scripts
F[compare-tools.mjs] --> N[scripts/lib/name-map.mjs]
G[import-jelly-micro.mjs] --> N
end
subgraph Infrastructure
K[builder/helpers.ts] --> S[shared/sleep.ts]
L[journal.ts] --> S
M[config.ts] --> DEFAULTS[DEFAULTS + CodegraphConfig]
DEFAULTS -->|wired| RS[resolveSecrets]
DEFAULTS -->|reserved| UW[largeCodebaseFileThreshold / busyTimeoutMs / capacityGrowthFactor]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
subgraph Leiden
A[partition.ts] --> H1[aggregate-helpers.ts]
B[index.ts] --> H1
A --> H2[typed-array-helpers.ts]
B --> H2
C[adapter.ts] --> H2
end
subgraph Features
D[complexity.ts] --> R[resolve-file-tree.ts]
E[dataflow.ts] --> R
end
subgraph Scripts
F[compare-tools.mjs] --> N[scripts/lib/name-map.mjs]
G[import-jelly-micro.mjs] --> N
end
subgraph Infrastructure
K[builder/helpers.ts] --> S[shared/sleep.ts]
L[journal.ts] --> S
M[config.ts] --> DEFAULTS[DEFAULTS + CodegraphConfig]
DEFAULTS -->|wired| RS[resolveSecrets]
DEFAULTS -->|reserved| UW[largeCodebaseFileThreshold / busyTimeoutMs / capacityGrowthFactor]
end
Reviews (4): Last reviewed commit: "Merge branch 'main' into refactor/titan-..." | Re-trigger Greptile |
Sorry, something went wrong.
| if (!resolved) return null; | ||
|
|
||
| return extractDataflow(resolved.tree, relPath, symbols.definitions, resolved.langId); |
There was a problem hiding this comment.
Missing DATAFLOW_RULES guard removed from call site
The original getDataflowForFile explicitly checked !DATAFLOW_RULES.has(langId) and returned null before calling extractDataflow. That guard is now gone. In its place, extractDataflow itself does if (!rules) return { parameters: [], … } — so the worst-case behavior shifts from returning null to returning an empty-but-non-null DataflowResult. Any caller that distinguishes null (skip this file) from an empty result (file was processed, just no edges found) could behave differently. Since DATAFLOW_EXTENSIONS = buildExtensionSet(DATAFLOW_RULES) the gate is equivalent in the no-cached-tree path, but the cached-tree path (symbols._tree != null, symbols._langId for a language removed from DATAFLOW_RULES) no longer short-circuits at this level.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed — restored the explicit DATAFLOW_RULES.has(resolved.langId) check in getDataflowForFile after the resolveFileTree call in src/features/dataflow.ts. resolveFileTree only gates on the extension set for the fresh-parse path and does not re-validate a cached tree's langId against DATAFLOW_RULES, so the cached-tree path could reach extractDataflow with an unsupported langId and get back an empty-but-non-null result instead of null. The guard now runs immediately after tree resolution, preserving the original null-for-unsupported contract for all callers.
Sorry, something went wrong.
…on (#1786) getDataflowForFile lost its DATAFLOW_RULES.has(langId) check on the cached-tree path when it was refactored to delegate to resolveFileTree, which only gates on file extension. For a cached tree whose langId was removed from DATAFLOW_RULES, extractDataflow's own internal guard now returns an empty DataflowResult instead of null, changing the skip-vs-processed contract for callers. Restore the explicit check after resolveFileTree resolves the tree. Impact: 1 functions changed, 6 affected
Sorry, something went wrong.
Summary
These five commits are grouped together because several later PRs in this stack build directly on the DEFAULTS extension in the last commit — splitting it out separately would break the dependent PRs' builds.
Titan Audit Context
Changes
Metrics Impact
Foundational extraction batch — enables the decomposition and quality-fix batches later in this stack (PRs #3-#7, #9) to reuse these helpers instead of duplicating logic.
Test plan