Skip to content

Simplify the merged FileTree implementation #166

Open

Description

@Razberrry

Context

PR #142 replaced FileTreePage.jsx (1,120 lines) with a features/fileTree/ tree of ~30 files / ~4,000 added lines. It ships as-is for now; this issue records the problems for a later pass.

Net effect: the rewrite is less readable than the file it replaced — 3.5× the code, 5+ layers between a click and its state change, and three disagreeing implementations of the same data parsing. The individual pieces look tidy; the whole is a maze.

Suggested direction: adopt a headless tree library (e.g. headless-tree or react-arborist) — see §2 and "Remedy sketch" at the bottom. That dissolves the tree/interaction half of the problem (§§1–2). The data-layer half (§§3, 6–8) needs a single canonical parse regardless of any library.

1. Abstraction explosion for a single page

Following one interaction takes 5+ file hops: FileTreeView → FileTreeContent → FileTreeGroupRow/FileTreeFileRow → FileTreeIndentedRow → useFileTreeState. The state hook returns a 26-property object mixing URL-derived state, local Set state, and callbacks, so it is hard to tell what drives what. Several files exist to hold almost nothing (FileTreeIndentedRow.tsx — 19 lines, PartitionPathIcon.tsx — 12 lines).

2. Hand-rolled tree machinery a headless tree library already provides

model/partitionModel.ts (162 lines), model/fileTreeRows.ts (110), FileTreeVirtualList.tsx, and most of useFileTreeState hand-build folder-tree grouping, row flattening, expansion state, checkbox selection, and virtualization. Every one of these is a built-in feature of a headless tree library (headless-tree / react-arborist): expand/collapse state, flattened virtual rows, multi-selection, search/filtering, and keyboard navigation come out of the box, headless so the existing Tailwind row styling stays. Adopting one would delete fileTreeRows.ts, FileTreeVirtualList.tsx, FileTreeIndentedRow.tsx, and roughly half of the 26-property state hook, leaving only the IceGraph-specific part: mapping partitions/files into tree items.

3. Two competing type-normalization layers

schemas.ts normalizes at the boundary with dense Zod chains (z.union([z.string(), z.number(), z.bigint()]).transform(String); the .nullish().transform(v => v ?? undefined).optional() pattern repeated per field flavor) — yet consumers still re-narrow everything with scattered typeof ceremony: aggregateReadableMetrics.ts:41-83 branches pairwise on typeof bigint/number/string/boolean for every comparison, with more in graphIndex.ts and PanelContent.tsx:120-131. If the boundary parse were authoritative, one canonical type would flow through and the typeof branching would disappear.

4. Duplicated side-panel layer

New SidePanelFrame.tsx (174 lines) lands while ResizableSidePanel.jsx remains in use by Graph/Timeline — two coexisting panel shells. PanelContent was likewise split (PanelSubtitle, ReadableMetricsSummary) with responsibilities mixed between old and new.

5. URL/state contract

Five new filetree_* search params, with filetree_branch decoding a three-way sentinel: undefined → "main", "" → null, otherwise the value (useFileTreeState.ts). A sentinel-in-URL makes links unreadable and the decode logic brittle.

6. The metric-parsing machinery exists three times — and the copies already disagree

Three independent normalization layers now answer "what is a metric value":

  1. features/fileTree/schemas.ts — Zod: numericValueSchema coerces strings through Number() (silently losing precision above 2^53), while identifierSchema coerces bigint → string.
  2. features/fileTree/model/aggregateReadableMetrics.ts — its own typeof bigint / number / digit-string branching, summing in BigInt to preserve the precision layer 1 throws away.
  3. utils/readableMetricsStatistics.ts (314 lines) — a third parser set (parseMetricInteger, parseSignedInteger, parseFiniteNumber, parseDecimal) plus a bespoke comparison engine (compareBigInts, compareDecimals, compareMetricRatios, compareSourceIds) feeding the table sort.

Why this is a real defect and not a style nit — the duplicates have already diverged inside this one PR: getFileSizeBytes is written twice, once in model/partitionModel.ts (accepts any finite number: Number.isFinite(x) && x >= 0) and once as a fresh plain-JS copy added to GraphPage.jsx (requires a safe integer: Number.isSafeInteger(x) && x >= 0, after its own typeof gate). The same file can therefore report a size in the FileTree and null in the Graph panel. This is the inevitable end-state of three parsing layers: every future metrics change must be made in three places, and any place missed becomes a cross-page inconsistency exactly like this one.

7. Whole-payload re-validation in page render

pages/FileTreePage.tsx runs fileTreeContextSchema.safeParse() over the entire graph payload — which TableLayout already fetched and parsed — synchronously on every render of the page. For a large table this is a multi-MB structural validation on the render path, duplicating work the data layer already did. PHILOSOPHY.md puts Zod at the runtime boundary (the API layer), precisely so pages receive already-trusted data; validating per-page means every page pays the parse cost again and each page can disagree about what "valid" means.

8. New code placed in the legacy layout it is migrating away from

readableMetricsStatistics.ts — 314 brand-new TypeScript lines — landed in the pre-migration utils/ folder rather than the features/<feature>/ or shared/lib/ structure this PR's own migration is supposed to establish. The migration PR itself not following the target layout means the "legacy vs. migrated" boundary is no longer a reliable signal, and the file will have to be moved (and its importers touched) a second time.

Remedy sketch

  • Tree/interaction half (§§1–2, part of §5): replace the hand-rolled model/row/virtualization stack with a headless tree library; keep only the partition → tree-item mapping. The 26-property hook shrinks to the library's instance plus the few URL-backed params.
  • Data half (§§3, 6–8): one authoritative parse at the API boundary producing one canonical metrics type (pick one numeric representation and keep it); delete the other two parser sets and the duplicated getFileSizeBytes; move what remains into the target folder structure.

(Note: a tree library is a new dependency and needs the usual approval step per CLAUDE.md.)

Activity

  1. changed the title [-]FileTree TS rewrite (PR #142): structure and data-layer complexity issues[/-] [+]Simplify the merged FileTree implementation[/+] on Aug 27, 2026
  2. moved this from Todo to In Progress in IceGraph Roadmapon Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions