refactor(import-report): findings-only summary, silent clean import#62
Conversation
Restructure the import report so each issue is a per-occurrence
ImportFinding ({ kind, command, pageIndex }) instead of three dedup
buckets. The parser emits findings with pageIndex=0; zplImportService
stamps the real page index when it merges the per-^XA block results,
which lets the modal show a 'Page N' badge next to each finding when
the import spans multiple pages.
Introduce describeFinding() in lib/importReport as the single source
of truth for finding wording. Both the UI list and the copy-as-text
formatter route through it so the two surfaces cannot drift. Replace
the kind→tone switch with a Record<ImportFindingKind, string> so the
compiler enforces exhaustiveness.
Skip the post-import result view entirely when there are no findings:
the canvas filling with the imported design is feedback enough, and
the pre-import disclaimer already covers the editable-reconstruction
caveat. Only stop on the result view when findings exist; that view
is now findings-only (no duplicated success banner).
Rename ImportReportModal.tsx to ImportSummary.tsx and drop the dead
ImportReportModal wrapper (only ImportSummaryBody was ever used).
Remove the dead notice/buildNotice path from zplImportService: the
notice string and its builder were only read by tests; the modal
surfaces import status through the findings list and its own error
copy. Drop the dimensionsDiffered tracking that only fed the notice.
There was a problem hiding this comment.
Code Review
This pull request refactors the ZPL import reporting system to support multi-page findings and provide a more detailed UI. It introduces a new findings array in the ImportReport interface that tracks the specific page index for each issue, and adds an ImportSummary component to display these findings per page. The logic in ZplImportModal was also updated to automatically close the modal if an import has no findings. Feedback includes a recommendation to restore deduplication for legacy report buckets (partial, browserLimit, unknown) to maintain consistency with existing documentation and behavior. Additionally, it is suggested to extract duplicated result-handling logic in ZplImportModal.tsx into a shared helper function to improve maintainability.
* Restore dedup on the legacy bucket views (partial / browserLimit / unknown) so they match the JSDoc contract on ImportReport. The per-occurrence model continues to live in `findings`. Wrapped the three near-identical filter/dedup expressions into a `dedupBy(kind)` closure. * Extract a `processImport` helper in ZplImportModal so the post-text path (parse, gate on supported content, hand off to store + modal finalisation) is no longer duplicated between the paste and file-pick handlers. The handlers now only carry source-specific validation; everything past that point lives in one place. * Re-export ImportFinding, ImportFindingKind and ImportReport from lib/importReport. UI components now reach the report domain types through that module exclusively, instead of importing from the parser directly. Keeps the parser as an implementation detail of the report surface.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the ZPL import reporting system to provide structured, per-page feedback. It introduces a new findings array in the ImportReport interface, centralizes finding descriptions, and updates the UI to display these issues with page indices. The previous notice string generation has been removed in favor of this more granular data structure. A review comment correctly pointed out that the JSDoc for the findings array incorrectly claimed 'encounter order' when the implementation actually groups them by kind, and suggested a documentation update to reflect this.
The previous wording claimed 'encounter order' but the parser concatenates kind-buckets (partial, browserLimit, unknown) when building the findings array. Encounter order only holds within each group, plus partials are deduplicated by command code. Clarify the contract so consumers know what to expect.
No description provided.