Skip to content

feat: implement BlockView component for financial data rendering#122

Merged
jfrench9 merged 6 commits intomainfrom
feature/block-viewers
Apr 26, 2026
Merged

feat: implement BlockView component for financial data rendering#122
jfrench9 merged 6 commits intomainfrom
feature/block-viewers

Conversation

@jfrench9
Copy link
Copy Markdown
Member

Summary

Introduces the BlockView component system — a composable, projection-based architecture for rendering financial data blocks (statements, schedules, and fact tables) within the ledger close workflow. This PR also refactors existing panel components and the report viewer to leverage the new BlockView abstraction, significantly reducing code duplication and improving maintainability.

Changes

New Components

  • BlockView — Core component that accepts a typed envelope and delegates rendering to the appropriate projection based on the data shape.
  • StatementRendering — Projection for rendering financial statement blocks with line items, sections, and totals.
  • ScheduleRendering — Projection for rendering schedule-type blocks with tabular detail data.
  • FactTable — Projection for rendering simple key/value fact tables.
  • blockview/types.ts — Shared TypeScript types for block envelopes, projections, and rendering contracts.
  • blockview/index.ts — Public barrel export for clean imports.

Refactors

  • SchedulePanel & StatementPanel — Streamlined to remove inline rendering logic; now delegate to BlockView projections, reducing ~200 lines of duplicated code.
  • ReportViewerContent (reports/[id]/content.tsx) — Refactored to utilize ReportPackage and integrate BlockView for item rendering, reducing the file from ~350+ lines of mixed concerns to a leaner orchestration layer.
  • StructureSidebar — Updated prop interface to accept structureId instead of reportId + structureType, simplifying the API and aligning with the domain model.
  • content.tsx (ledger/close) — Updated to pass structureId to sidebar and child components.

Dependencies

  • Bumped @robosystems/client from previous version to 0.3.14 (likely includes types/API surface needed for block envelopes).

Key UI/UX Improvements

  • No user-facing visual changes expected — this is a structural refactor that preserves existing rendering behavior while establishing a scalable pattern for future block types.
  • The projection-based architecture makes it straightforward to add new financial data visualizations without modifying existing components.

Breaking Changes

  • StructureSidebar props changed: reportId and structureType props replaced with structureId. Any consumers of this component outside this PR will need to update their prop usage.
  • SchedulePanel and StatementPanel interfaces may have changed as part of the simplification — reviewers should verify any external consumers.

Testing

Automated Tests Added

  • BlockView.test.tsx — Unit tests for envelope dispatching to correct projections, edge cases for unknown types.
  • FactTable.test.tsx — Rendering tests for key/value fact table projection.
  • ScheduleRendering.test.tsx — Rendering tests for schedule block projection with mock envelope data.
  • StatementRendering.test.tsx — Rendering tests for statement block projection with sections, line items, and totals.
  • _envelope-fixtures.ts — Shared mock data fixtures used across all BlockView test suites.
  • Existing tests for StructureSidebar and content updated to reflect the structureId prop change.

Manual Testing Notes for Reviewers

  1. Navigate to Ledger → Close and verify statements and schedules render identically to main.
  2. Open a report via Reports → [any report] and confirm the report viewer renders all items correctly.
  3. Verify sidebar navigation still works after the structureId refactor.
  4. Check that loading, empty, and error states are preserved in both panel types.

Browser Compatibility

  • No new browser APIs introduced. All changes use standard React component patterns and TypeScript.
  • No CSS changes that would affect cross-browser rendering.
  • Should be compatible with all browsers currently supported by the application (modern evergreen browsers).

🤖 Generated with Claude Code

Branch Info:

  • Source: feature/block-viewers
  • Target: main
  • Type: feature

Co-Authored-By: Claude noreply@anthropic.com

@jfrench9
Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude
Copy link
Copy Markdown

claude Bot commented Apr 26, 2026

Claude finished @jfrench9's task in 2m 50s —— View job


PR Review: feat: implement BlockView component for financial data rendering

Tasks:

  • Gather context and understand the request
  • Read new BlockView components and types
  • Read refactored panel components (SchedulePanel, StatementPanel)
  • Read ReportViewerContent and StructureSidebar changes
  • Read test files
  • Read content.tsx changes
  • Analyze and compile review feedback
  • Post final review

Overall Assessment

This is a well-executed structural refactor. The projection-based dispatch pattern cleanly separates data-fetching (panel shells), routing (BlockView), and rendering (projections), and the test coverage is thorough. Below are issues found during review, from highest to lowest priority.


Issues

🔴 Medium — Potentially non-unique row key in StatementRendering

File: src/app/(app)/ledger/close/components/blockview/projections/StatementRendering.tsx:137

key={`${row.elementId}-${idx}`}

If elementId is not guaranteed to be unique across rows (e.g. the same element appears as both a line item and a subtotal roll-up), this key can collide — two rows with elementId=elem_rev at indices 0 and 5 are fine, but if two rows share elementId and idx in a re-render, React's reconciler will produce unexpected DOM updates. Since idx is already unique within the map, key={idx} alone is both simpler and safer here. If row identity across data refreshes matters, a truly stable key (e.g. ${row.elementId}-${row.depth}-${row.isSubtotal}) would be better.

Similarly in StatementRendering.tsx:121:

{periods.map((period: EnvelopeRenderingPeriod, i: number) => (
  <TableHeadCell key={i} ...>

Index keys are harmless here only because periods don't reorder in practice, but period.end or period.label would be more correct.

Fix this →


🔴 Medium — createClosingEntry receives periodEnd as both 3rd and 5th arguments

File: src/app/(app)/ledger/close/components/SchedulePanel.tsx:57-63

await clients.ledger.createClosingEntry(
  graphId,
  structureId,
  periodEnd,
  periodStart,
  periodEnd   // ← same value as argument 3
)

The 5th positional argument duplicates periodEnd. If the SDK signature is (graphId, structureId, periodEnd, periodStart, reportingEnd) and these are genuinely the same date, this should be documented; if it should be a different value (fiscal year-end, close date, etc.), this is a silent bug. Please verify the SDK signature and either remove the duplicate or add a comment explaining the intent.


🟡 Low — Silent period_close fallback for unknown item types

File: src/app/(app)/ledger/close/components/StructureSidebar.tsx:46-49

default:
  return { type: 'period_close' }

When the API introduces a new itemType (e.g. 'note'), clicking that sidebar item will silently navigate to Period Close instead of showing an appropriate state or logging the unhandled type. Consider adding a console.warn at minimum, or an explicit exhaustive check.

Fix this →


🟡 Low — Unnecessary optional chaining on envelope in handlePresetClick

File: src/app/(app)/ledger/close/components/StatementPanel.tsx:221

const reportName = envelope?.displayName ?? envelope?.name ?? 'Report'

handlePresetClick is only callable when mappingId is set, and the component returns an empty state when envelope is null (line 258-263), so handlePresetClick can only fire when envelope is non-null. The optional chaining on envelope is misleading — it signals to the reader that envelope could be null here, when it can't be. Use envelope.displayName ?? envelope.name ?? 'Report'.

Fix this →


🟡 Low — No 'schedule' constant in types.ts (inconsistent with STATEMENT_BLOCK_TYPES)

File: src/app/(app)/ledger/close/components/blockview/BlockView.tsx:65 and types.ts

STATEMENT_BLOCK_TYPES is exported as a typed constant for block-type discrimination, but schedule is checked via an inline string literal in BlockView.tsx:

if (envelope.blockType === 'schedule') {

If schedule is added to the registry or renamed server-side, the constant would be the right place to catch it. A SCHEDULE_BLOCK_TYPES constant (or including 'schedule' in a broader registry constant) would keep the pattern consistent and make the registry comment in types.ts accurate.


🟡 Low — Inline ack.result type cast in ReportViewerContent

File: src/app/(app)/reports/[id]/content.tsx:107-118

const shareResults =
  (
    ack.result as {
      results?: Array<{ target_graph_id: string; status: string; error: string | null; fact_count?: number }>
    } | null
  )?.results ?? []

This inline cast will silently mishandle any future change in the API response shape. If ack.result is typed as unknown or any in the SDK, the right fix is to add a typed response interface and a runtime guard, or to update the SDK type. At a minimum, move the shape assertion to a named type so it's easier to find and update.


🟢 Nit — TrialBalancePanel not mocked in content.test.tsx

File: src/app/(app)/ledger/close/__tests__/content.test.tsx

All panel components are mocked except TrialBalancePanel. Since none of the tests exercise the trial_balance branch, this doesn't cause test failures today, but if a test is added for that path it will pull in the real panel and all its dependencies. Add a mock for consistency.

Fix this →


Positive Highlights

  • Clean dispatch patternBlockView is a textbook dispatcher: thin, single-responsibility, easy to extend.
  • Fixture design_envelope-fixtures.ts with makeElement/makeFact/makeRendering/makeEnvelope overrides is exactly right for this kind of type-heavy domain. Deliberately partial shapes keep the tests decoupled from codegen churn.
  • useMemo usage — The elementsById Map and period grouping in the projections are appropriately memoized on the correct dependency arrays.
  • Error / empty / loading state coverage — All three states are handled in both panel shells and the projections themselves.
  • content.tsx (StructureSidebar) — The itemToSelected helper with a switch on itemType is a good pattern; isActive matching symmetrically on the SelectedItem discriminant means adding a new item type only requires one update per function.

@jfrench9 jfrench9 merged commit 6e2d0d2 into main Apr 26, 2026
3 checks passed
@jfrench9 jfrench9 deleted the feature/block-viewers branch April 26, 2026 06:17
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