Skip to content

Clarify diff layout, geometry, and bounds naming#152

Merged
benvinegar merged 1 commit intomainfrom
pi/rename-diff-geometry-terms
Mar 31, 2026
Merged

Clarify diff layout, geometry, and bounds naming#152
benvinegar merged 1 commit intomainfrom
pi/rename-diff-geometry-terms

Conversation

@benvinegar
Copy link
Copy Markdown
Member

Summary

  • rename the diff section helper module and exported symbols from metrics to geometry and bounds where they describe measured spatial data
  • rename the file-section helper exports from *Metric(s) to *Layout(s) and simplify the header-top lookup naming
  • add concise AGENTS naming guidance so future helpers use layout, geometry, and bounds consistently

Testing

  • bun run typecheck
  • bun test

This PR description was generated by Pi using OpenAI o3

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 31, 2026

Greptile Summary

This is a pure rename/refactor PR that clarifies the naming vocabulary across the diff layout and geometry subsystems. It renames sectionHeights.tsdiffSectionGeometry.ts, section-heights.test.tsdiff-section-geometry.test.ts, and propagates consistent layout / geometry / bounds terminology throughout DiffPane.tsx, fileSectionLayout.ts, plannedReviewRows.ts, benchmarks, and all test files. It also codifies the new naming conventions in AGENTS.md.

Key changes:

  • DiffSectionMetricsDiffSectionGeometry, DiffSectionRowMetricDiffSectionRowBounds (and the offset field on row bounds renamed to top)
  • FileSectionLayoutMetricFileSectionLayout; buildFileSectionLayoutMetricsbuildFileSectionLayouts; resolveFileSectionHeaderTopgetFileSectionHeaderTop
  • measurePlannedHunkBoundsmeasurePlannedSectionGeometry; new PlannedSectionGeometry return type added with explicit annotation
  • estimateDiffBodyRowsestimateDiffSectionBodyRows; estimateHunkAnchorRowestimateHunkAnchorBodyRow
  • Benchmark METRIC output key renamed from section_metrics_ms to section_geometry_ms — worth noting if any external tooling parses that key
  • One incomplete rename remains: the local variable previousSectionMetrics in DiffPane.tsx (lines 503–515) was not updated to previousSectionGeometry even though the ref it is read from was renamed to previousSectionGeometryRef

Confidence Score: 5/5

Safe to merge — purely a rename/refactor with no logic changes; all old identifiers have been consistently replaced across source and tests.

No P0 or P1 issues found. Every renamed export was verified to have zero remaining callers using the old name. The sole finding is a single unconverted local variable name (previousSectionMetrics → previousSectionGeometry) in DiffPane.tsx, which is a P2 style inconsistency with no runtime impact. All tests were updated, type-check is expected to pass, and no breakage was found in imports or logic.

src/ui/components/panes/DiffPane.tsx — one local variable (previousSectionMetrics, lines 503–515) was not renamed to previousSectionGeometry.

Important Files Changed

Filename Overview
src/ui/components/panes/DiffPane.tsx Comprehensive rename of metrics → geometry/bounds throughout the component; one local variable (previousSectionMetrics) was not renamed to match the new convention.
src/ui/lib/diffSectionGeometry.ts Renamed from sectionHeights.ts; all exported types and functions consistently updated (DiffSectionRowBounds, DiffSectionGeometry, measureDiffSectionGeometry, estimateDiffSectionBodyRows, estimateHunkAnchorBodyRow); offset→top property rename is correct throughout.
src/ui/lib/fileSectionLayout.ts FileSectionLayoutMetric → FileSectionLayout, buildFileSectionLayoutMetrics → buildFileSectionLayouts, resolveFileSectionHeaderTop → getFileSectionHeaderTop — all cleanly renamed with no missed call sites.
src/ui/diff/plannedReviewRows.ts Renamed measurePlannedHunkBounds → measurePlannedSectionGeometry and introduced the new PlannedSectionGeometry return type; adds an explicit return type annotation that was previously missing.
test/diff-section-geometry.test.ts Renamed from section-heights.test.ts; all test descriptions and variable names updated to match new geometry/bounds vocabulary; test assertions unchanged and still correct.
AGENTS.md Adds a new naming section documenting the layout/geometry/bounds vocabulary distinctions to guide future contributors.
benchmarks/large-stream-profile.ts Updated import path and function call to measureDiffSectionGeometry; renames benchmark variable and METRIC output key from section_metrics_ms to section_geometry_ms.
test/ui-lib.test.ts Import and call sites updated for estimateDiffSectionBodyRows and measureDiffSectionGeometry; test descriptions updated to match new names.
test/ui-components.test.tsx Import updated from sectionHeights to diffSectionGeometry and call site renamed; no functional changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    DP["DiffPane.tsx"]
    DSG["diffSectionGeometry.ts\nmeasureDiffSectionGeometry\nDiffSectionGeometry\nDiffSectionRowBounds"]
    FSL["fileSectionLayout.ts\nbuildFileSectionLayouts\nFileSectionLayout\ngetFileSectionHeaderTop"]
    PRR["plannedReviewRows.ts\nmeasurePlannedSectionGeometry\nPlannedSectionGeometry\nPlannedHunkBounds"]

    DP -->|"imports measureDiffSectionGeometry\nDiffSectionGeometry, DiffSectionRowBounds"| DSG
    DP -->|"imports buildFileSectionLayouts\nfindHeaderOwningFileSection\ngetFileSectionHeaderTop"| FSL
    DSG -->|"imports PlannedHunkBounds"| PRR

    subgraph "Renamed symbols (old → new)"
        A["sectionHeights.ts → diffSectionGeometry.ts"]
        B["DiffSectionMetrics → DiffSectionGeometry"]
        C["DiffSectionRowMetric.offset → DiffSectionRowBounds.top"]
        D["FileSectionLayoutMetric → FileSectionLayout"]
        E["resolveFileSectionHeaderTop → getFileSectionHeaderTop"]
        F["measurePlannedHunkBounds → measurePlannedSectionGeometry"]
    end
Loading

Comments Outside Diff (1)

  1. src/ui/components/panes/DiffPane.tsx, line 503-515 (link)

    P2 Incomplete local variable rename

    The ref was renamed to previousSectionGeometryRef, but the local variable extracted from it on line 503 was left as previousSectionMetrics. This is inconsistent with the rest of the PR's naming convention and is the only remaining use of the old …Metrics vocabulary in this scope.

    Then lines 506 and 515 would also need updating:

        if (wrapChanged && previousSectionGeometry && previousFiles.length > 0) {
          // ...
          const anchor = findViewportRowAnchor(
            previousFiles,
            previousSectionGeometry,
            previousScrollTop,
          );

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "refactor(ui): clarify layout, geometry, ..." | Re-trigger Greptile

@benvinegar benvinegar force-pushed the pi/rename-diff-geometry-terms branch from a85b11d to 89a6db6 Compare March 31, 2026 15:11
@benvinegar benvinegar merged commit 11379c8 into main Mar 31, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant