Skip to content

Split graph.rs (1744 LOC) and symbol_analyzer.rs (1609 LOC) to clear the 1000-LOC gate #473

Description

@zackees

Background

The 1000-LOC gate (PR #467, workflow .github/workflows/loc-gate.yml) currently fails on every PR because two files in the bloat-analysis tree are over the threshold:

File LOC Was added by
`crates/fbuild-core/src/symbol_analysis/graph.rs` 1744 #463 (back-ref graph) + #470 (bidirectional + dual-rank callees)
`crates/fbuild-build/src/symbol_analyzer.rs` 1609 #424 (initial analyzer) + #460 (cref) + #463 (graph integration) + #470 (markdown bidirectional + dual sub-table)

Both grew past the cap in #470 (forward-refs + dual-rank). The gate now flags every new PR red, even ones (like #471) that don't touch these files at all.

Proposed splits

`graph.rs` (1744 LOC) → 3-4 modules

  • `graph/mod.rs` — public re-exports + `GraphNode` / `GraphEdge` / `NodeKind` / `EdgeDirection` / `Direction` types.
  • `graph/config.rs` — `GraphConfig` + `GraphDepth` knobs.
  • `graph/backward.rs` — the existing `BackrefGraph::build_with_index` BFS walker.
  • `graph/forward.rs` — `walk_forward` + `CalleeCandidate` + `rank_callees_dual` (added feat(bloat): per-symbol forward refs + bidirectional graph + dual-rank sub-table #470).
  • `graph/render.rs` — `to_dot`, `node_color`, `node_width`, `format_*_label`, escape/sanitize.
  • (tests stay alongside the unit they exercise.)

`symbol_analyzer.rs` (1609 LOC) → 3-4 modules

  • `symbol_analyzer/mod.rs` — re-exports + `AnalyzeConfig` + `analyze_elf` driver.
  • `symbol_analyzer/objdump.rs` — `run_objdump_and_attribute` (feat(bloat): per-symbol forward refs + bidirectional graph + dual-rank sub-table #470) + the objdump subprocess wiring.
  • `symbol_analyzer/markdown.rs` — `format_text_report`, `format_markdown_report`, `format_markdown_report_with_graphs`, `emit_backref_graph_section`, `emit_dual_callees_subtable`.
  • `symbol_analyzer/sidecar.rs` — `write_sidecar_dot_files` + `SidecarOptions`.

Acceptance

  • Both files under 1000 LOC.
  • `fbuild bloat` / `fbuild symbols` / `fbuild bloat-diff` / `fbuild bloat graph` all still work (existing tests pass unchanged).
  • Public API is unchanged — only internal module layout shifts; downstream callers (CLI handlers in `fbuild-cli/src/cli/{bloat_cmd,bloat_diff_cmd,symbols_cmd,graph_cmd}.rs`) keep their existing `use` paths via re-exports from the new `mod.rs`.

Why not in PR #471

PR #471 is a docs + regression-test PR for #417. Folding a 700-line refactor into it would balloon the diff for no review benefit. Splitting cleanly is its own follow-up.

🤖 Filed by Claude Code while triaging #471's CI.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status
    Triage

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions