diff --git a/openspec/changes/export-report/design.md b/openspec/changes/export-report/design.md index f5a87b11..50288a1e 100644 --- a/openspec/changes/export-report/design.md +++ b/openspec/changes/export-report/design.md @@ -43,9 +43,12 @@ fragile regex (`output.match(/^✓.*- (\d+) /gm)`), which breaks on any CLI outp | New ExportReport struct | Clean separation, tailored | Small duplication of Stats fields | | Generic Report wrapper | Maximum reuse | Over-engineering for two report types | -**Decision**: New `ExportReport` struct. Reuses `ExportStats` for counts. BatchReport stays private to `Batch.swift`, -ExportReport is internal to ExFigCLI. Different shapes: BatchReport has `results: [ConfigReport]` array, ExportReport -has flat `stats` + `manifest` for a single command. +**Decision**: New `ExportReport` struct with its own `ReportStats: Encodable` containing count fields only +(colors, icons, images, typography). `ExportStats` from `BatchResult.swift` is NOT Codable and contains +batch-only fields (`computedNodeHashes`, `granularCacheStats`, `fileVersions`), so direct reuse is not viable. +This matches the pattern already established by `BatchReport.Stats` which manually maps the same count fields. +BatchReport stays private to `Batch.swift`, ExportReport is internal to ExFigCLI. Different shapes: BatchReport +has `results: [ConfigReport]` array, ExportReport has flat `stats` + `manifest` for a single command. ### Decision 2: `--report ` Flag @@ -80,17 +83,21 @@ the option set grows. ### Decision 4: Warning Collection -**What**: Expose warnings collected by TerminalUI in the report. +**What**: Collect warnings emitted during export for inclusion in the report. **Options considered**: -| Option | Pros | Cons | -| -------------------------- | ----------------- | --------------------------- | -| Expose TerminalUI warnings | Already collected | Couples report to UI layer | -| Separate warning collector | Clean separation | Duplicates collection logic | +| Option | Pros | Cons | +| ----------------------------- | ---------------------------------------- | --------------------------------------- | +| Add collection to TerminalUI | Single point of interception | Adds state to stateless class | +| Separate WarningCollector | Clean separation, follows actor pattern | Must wire into export commands | +| Intercept at queueLogMessage | No TerminalUI changes | Only works in batch mode | -**Decision**: TerminalUI already collects warnings via its internal list. Expose collected warnings as `[String]` -for inclusion in the report. No new collection mechanism needed. +**Decision**: Create new `WarningCollector` actor following `SharedThemeAttributesCollector` pattern +(see `Sources/ExFigCLI/Batch/SharedThemeAttributes.swift`). TerminalUI currently does NOT store warnings — +it only prints/queues them. The collector is injected into export commands when `--report` is specified, +and `TerminalUI.warning()` is extended to also forward to the active collector. Collected as `[String]` +(formatted message strings) for inclusion in the report. ### Decision 5: Asset Manifest with FileWriter Tracking @@ -121,13 +128,23 @@ file write with its action and checksum. Tracking is opt-in -- zero overhead whe Detection uses content hash comparison before write. For `deleted`, the system compares against a previous report file at the same path (if it exists). This is the only action that requires a previous report. -### Decision 7: SHA256 Checksum +### Decision 7: Content Checksum -**What**: Compute SHA256 hex digest for each file in the manifest. +**What**: Compute a content hash for each file in the manifest. -**Rationale**: Enables downstream tools (exfig-action, CI scripts) to detect changes without reading file -contents. SHA256 is computed from data already in memory during write -- no additional I/O. The checksum -is only computed when `--report` is specified. +**Options considered**: + +| Option | Pros | Cons | +| ----------------------- | --------------------------------------- | --------------------------------------------------------- | +| SHA256 (CryptoKit) | Industry standard, 64-char hex | macOS-only; Linux needs `swift-crypto` dependency | +| SHA256 (swift-crypto) | Cross-platform, industry standard | New dependency | +| FNV-1a (already in use) | No new deps, fast (~2 GB/s), in codebase | Non-cryptographic, 16-char hex, collision-prone at scale | + +**Decision**: Use `FNV1aHasher.hashToHex()` already available in the codebase (see +`Sources/ExFigCLI/Cache/FNV1aHasher.swift`). The checksum purpose is change detection, not security — +same use case as granular cache node hashing. Produces 16-character lowercase hex string. If downstream +tools later require SHA256, `swift-crypto` can be added as a future enhancement. The checksum is computed +from data already in memory during write — no additional I/O. Only computed when `--report` is specified. ### Decision 8: Report Write Failure Isolation @@ -137,12 +154,33 @@ is only computed when `--report` is specified. disk full, invalid path), the export results are still valid. The system logs a warning and continues. This matches batch mode behavior where report write failure is caught and logged. +### Decision 9: Report Version Field + +**What**: Include a `version` integer field in `ExportReport` for forward compatibility. + +**Rationale**: As the report schema evolves (e.g., adding manifest in Phase 2), downstream consumers +(exfig-action) need to know which fields to expect. Starting at `version: 1` for the initial report +(timing, stats, warnings). Increment when adding breaking changes to the schema structure. + +### Decision 10: Export Command Result Capture + +**What**: Modify single export commands' `run()` methods to capture export results instead of discarding them. + +**Current state**: All four export commands discard results: `_ = try await performExport(...)`. The +`performExportWithResult()` method exists but is only called in batch mode. + +**Decision**: In `run()`, call `performExportWithResult()` (or `performExport()` and capture count) +to obtain the stats needed for the report. Wrap the export in a do/catch to capture both success +and failure states. Capture `startTime` before export and `endTime` after. + ## Risks / Trade-offs -| Risk | Impact | Mitigation | -| --------------------------------- | ------ | ------------------------------------------------- | -| FileWriter tracking overhead | Low | Only active when `--report` specified | -| SHA256 computation cost | Low | Computed from in-memory data during write | -| exfig-action Phase 3 dependency | Medium | Action requires CLI release with `--report` first | -| Report format evolution | Low | Add `version` field to report for forward compat | -| `deleted` detection needs history | Low | Optional -- requires previous report at same path | +| Risk | Impact | Mitigation | +| --------------------------------- | ------ | ----------------------------------------------------- | +| FileWriter tracking overhead | Low | Only active when `--report` specified | +| FNV-1a collision risk | Low | Non-cryptographic but sufficient for change detection | +| exfig-action Phase 3 dependency | Medium | Action requires CLI release with `--report` first | +| Report format evolution | Low | `version` field in report for forward compat | +| `deleted` detection needs history | Low | Optional -- requires previous report at same path | +| Warning collector wiring | Low | Follow SharedThemeAttributesCollector actor pattern | +| run() refactor for result capture | Low | performExportWithResult() already exists | diff --git a/openspec/changes/export-report/proposal.md b/openspec/changes/export-report/proposal.md index c6cfa291..dc117e04 100644 --- a/openspec/changes/export-report/proposal.md +++ b/openspec/changes/export-report/proposal.md @@ -7,12 +7,13 @@ **Phase 1 — `--report` for single export commands:** - Add `--report ` option to `ExportColors`, `ExportIcons`, `ExportImages`, `ExportTypography` -- New `ExportReport` struct (analogous to `BatchReport` but for a single command): command name, config path, timing, success/error, stats, collected warnings -- Reuse existing `ExportStats` from `BatchResult.swift` and `JSONCodec.encodePrettySorted()` from swift-yyjson +- New `ExportReport` struct (analogous to `BatchReport` but for a single command): version, command name, config path, timing, success/error, stats, collected warnings +- New `ReportStats: Encodable` struct with count fields only (colors, icons, images, typography) — analogous to `BatchReport.Stats`, NOT reusing `ExportStats` directly (it contains batch-only fields like `computedNodeHashes`, `granularCacheStats`) +- Serialize via `JSONCodec.encodePrettySorted()` from swift-yyjson **Phase 2 — Asset Manifest:** -- New `AssetManifest` struct tracking every generated file: path, action (`created`/`modified`/`unchanged`/`deleted`), optional SHA256 checksum, asset type +- New `AssetManifest` struct tracking every generated file: path, action (`created`/`modified`/`unchanged`/`deleted`), FNV-1a content checksum, asset type - Track file write status in `FileWriter` and attach manifest to `ExportReport` - Enables: precise change tracking, PR diff comments, design drift detection @@ -38,6 +39,7 @@ _(none — batch report behavior is unchanged; single commands currently have no - `Sources/ExFigCLI/Subcommands/ExportIcons.swift` — add `--report` option - `Sources/ExFigCLI/Subcommands/ExportImages.swift` — add `--report` option - `Sources/ExFigCLI/Subcommands/ExportTypography.swift` — add `--report` option -- `Sources/ExFigCLI/Batch/BatchResult.swift` — reuse `ExportStats` +- `Sources/ExFigCLI/Batch/BatchResult.swift` — reference for `ExportStats` count fields - `Sources/ExFigCLI/Output/FileWriter.swift` — track write status for manifest +- `Sources/ExFigCLI/TerminalUI/TerminalUI.swift` — add warning collection mechanism (currently only prints, does not store) - External: `alexey1312/exfig-action` repo (Phase 3) diff --git a/openspec/changes/export-report/specs/export-report/spec.md b/openspec/changes/export-report/specs/export-report/spec.md index 42a821fc..a8519b7e 100644 --- a/openspec/changes/export-report/specs/export-report/spec.md +++ b/openspec/changes/export-report/specs/export-report/spec.md @@ -42,14 +42,15 @@ ExportColors, ExportIcons, ExportImages, and ExportTypography SHALL accept a `-- ### Requirement: ExportReport SHALL contain structured JSON with timing and metadata -The report SHALL contain: `command` (string), `config` (string path to PKL config), `startTime` (ISO8601 string), `endTime` (ISO8601 string), `duration` (number, seconds), `success` (boolean), `error` (string or null on success), `stats` (object), and `warnings` (string array). +The report SHALL contain: `version` (integer, starting at 1), `command` (string: `"colors"`, `"icons"`, `"images"`, or `"typography"`), `config` (string path to PKL config), `startTime` (ISO8601 string), `endTime` (ISO8601 string), `duration` (number, seconds), `success` (boolean), `error` (string or null on success), `stats` (object), and `warnings` (string array). #### Scenario: Successful export produces complete report - **GIVEN** a valid PKL config with iOS colors entries - **WHEN** running `exfig colors -i exfig.pkl --report results.json` - **AND** the export completes successfully -- **THEN** the report JSON SHALL contain `"command": "colors"` +- **THEN** the report JSON SHALL contain `"version": 1` +- **AND** `"command"` SHALL be `"colors"` - **AND** `"config"` SHALL be the path to the PKL config file - **AND** `"startTime"` SHALL be an ISO8601 timestamp before `"endTime"` - **AND** `"duration"` SHALL be a positive number in seconds @@ -66,9 +67,9 @@ The report SHALL contain: `command` (string), `config` (string path to PKL confi --- -### Requirement: Stats object SHALL reuse ExportStats structure +### Requirement: Stats object SHALL contain asset counts -The `stats` object in the report SHALL include `colors`, `icons`, `images`, and `typography` integer counts matching the existing `ExportStats` structure from `BatchResult.swift`. +The `stats` object in the report SHALL include `colors`, `icons`, `images`, and `typography` integer counts. This uses a new `ReportStats: Encodable` struct with count fields only (analogous to `BatchReport.Stats`), since `ExportStats` contains non-Codable batch-only fields. #### Scenario: Colors export populates stats correctly @@ -90,7 +91,7 @@ The `stats` object in the report SHALL include `colors`, `icons`, `images`, and ### Requirement: All warnings SHALL be collected in the report -All warnings emitted via TerminalUI during export SHALL be collected and included in the report `warnings` array as strings. +When `--report` is specified, all warnings emitted via TerminalUI during export SHALL be collected by a `WarningCollector` and included in the report `warnings` array as strings. TerminalUI does not currently store warnings — a new collection mechanism is required. #### Scenario: Export with warnings includes them in report @@ -153,7 +154,7 @@ When the export itself fails with an error, the report SHALL still be written wi ### Requirement: Asset manifest SHALL track generated files -When manifest tracking is enabled, the report SHALL include a `manifest` object with a `files` array. Each file entry SHALL contain: `path` (string, relative to working directory), `action` (string enum), `checksum` (SHA256 hex string or null), and `assetType` (string). +When manifest tracking is enabled, the report SHALL include a `manifest` object with a `files` array. Each file entry SHALL contain: `path` (string, relative to working directory), `action` (string enum), `checksum` (FNV-1a 16-char hex string or null), and `assetType` (string). #### Scenario: Manifest lists all generated color files @@ -214,16 +215,16 @@ The system SHALL detect and report the following file actions: `created` (file d --- -### Requirement: SHA256 checksum SHALL be computed for manifest files +### Requirement: Content checksum SHALL be computed for manifest files -Each file in the manifest SHALL include a `checksum` field containing the SHA256 hex digest of the file content. This enables downstream tools to detect changes without reading file contents. +Each file in the manifest SHALL include a `checksum` field containing an FNV-1a 64-bit hex digest of the file content (using `FNV1aHasher.hashToHex()` already in the codebase). This enables downstream tools to detect changes without reading file contents. FNV-1a is non-cryptographic but sufficient for change detection — same algorithm used by the granular cache system. -#### Scenario: Written file has SHA256 checksum +#### Scenario: Written file has FNV-1a checksum - **GIVEN** an export writes a file with known content - **WHEN** the manifest entry is recorded -- **THEN** `checksum` SHALL be a 64-character lowercase hexadecimal string -- **AND** the value SHALL match the SHA256 hash of the written file content +- **THEN** `checksum` SHALL be a 16-character lowercase hexadecimal string +- **AND** the value SHALL match the FNV-1a hash of the written file content #### Scenario: Deleted file has null checksum @@ -235,4 +236,29 @@ Each file in the manifest SHALL include a `checksum` field containing the SHA256 - **GIVEN** an export that detects a file is unchanged - **WHEN** the manifest entry is recorded -- **THEN** `checksum` SHALL equal the SHA256 of the existing file content +- **THEN** `checksum` SHALL equal the FNV-1a hash of the existing file content + +--- + +### Requirement: Report SHALL include version field for forward compatibility + +The `version` field SHALL be an integer starting at `1`. It SHALL be incremented when breaking changes are made to the report schema structure. + +#### Scenario: Initial report version + +- **GIVEN** any export with `--report` +- **WHEN** the report is written +- **THEN** `"version"` SHALL be `1` + +--- + +### Requirement: Manifest SHALL handle zero-file exports gracefully + +When an export completes successfully but produces no output files, the manifest SHALL be present with an empty `files` array. + +#### Scenario: Export produces no files + +- **GIVEN** a valid config with no matching assets in Figma +- **WHEN** the export completes successfully with `--report` +- **THEN** `manifest.files` SHALL be an empty array `[]` +- **AND** `stats` SHALL reflect zero counts for the relevant asset type diff --git a/openspec/changes/export-report/tasks.md b/openspec/changes/export-report/tasks.md index f996ab65..8a58ce46 100644 --- a/openspec/changes/export-report/tasks.md +++ b/openspec/changes/export-report/tasks.md @@ -1,42 +1,44 @@ ## 1. ExportReport Struct & JSON Serialization -- [ ] 1.1 Create `ExportReport` struct in `Sources/ExFigCLI/Batch/ExportReport.swift` with fields: command, config, startTime, endTime, duration, success, error, stats (ExportStats), warnings -- [ ] 1.2 Add `Codable` conformance to `ExportReport` and serialization via `JSONCodec.encodePrettySorted()` -- [ ] 1.3 Add `Codable` conformance to `ExportStats` (subset: colors, icons, images, typography counts only — exclude batch-only fields) -- [ ] 1.4 Write unit tests for `ExportReport` JSON serialization (success case, failure case, empty warnings) +- [ ] 1.1 Create `ExportReport` struct in `Sources/ExFigCLI/Report/ExportReport.swift` with fields: version (Int, default 1), command, config, startTime, endTime, duration, success, error, stats (ReportStats), warnings +- [ ] 1.2 Create `ReportStats: Encodable` struct with count fields only (colors, icons, images, typography) — analogous to `BatchReport.Stats` in `Batch.swift:901`. Do NOT add Codable to `ExportStats` (it has non-Codable batch-only fields: `computedNodeHashes`, `granularCacheStats`, `fileVersions`) +- [ ] 1.3 Add `Encodable` conformance to `ExportReport` and serialization via `JSONCodec.encodePrettySorted()` +- [ ] 1.4 Write unit tests for `ExportReport` JSON serialization (success case, failure case, empty warnings, version field) ## 2. --report Flag on Export Commands -- [ ] 2.1 Add `@Option(name: .long) var report: String?` to `ExportColors.swift` -- [ ] 2.2 Add `@Option(name: .long) var report: String?` to `ExportIcons.swift` -- [ ] 2.3 Add `@Option(name: .long) var report: String?` to `ExportImages.swift` -- [ ] 2.4 Add `@Option(name: .long) var report: String?` to `ExportTypography.swift` -- [ ] 2.5 Extract shared `writeExportReport(report:path:)` helper (wraps write in do/catch with warning on failure) -- [ ] 2.6 Wire report generation into each export command's run() method: capture start time, build ExportReport after export, call writeExportReport +- [ ] 2.1 Add `@Option(name: .long, help: "Path to write JSON report") var report: String?` to `ExportColors.swift` +- [ ] 2.2 Add `@Option(name: .long, help: "Path to write JSON report") var report: String?` to `ExportIcons.swift` +- [ ] 2.3 Add `@Option(name: .long, help: "Path to write JSON report") var report: String?` to `ExportImages.swift` +- [ ] 2.4 Add `@Option(name: .long, help: "Path to write JSON report") var report: String?` to `ExportTypography.swift` +- [ ] 2.5 Extract shared `writeExportReport(report:path:)` helper (wraps write in do/catch with warning on failure — same pattern as `Batch.swift:710-716`) +- [ ] 2.6 Modify each export command's `run()` to capture results: currently `_ = try await performExport(...)` discards the result. Change to capture count from `performExport()` (or use `performExportWithResult()`) and wrap in do/catch to capture errors. Record `startTime = Date()` before export, `endTime = Date()` after, build `ExportReport`, call `writeExportReport` ## 3. Warning Collection -- [ ] 3.1 Expose collected warnings from `TerminalUI` as `[String]` accessor -- [ ] 3.2 Integrate warning collection into `ExportReport` construction in each export command -- [ ] 3.3 Write tests for warning collection (with warnings, empty warnings) +- [ ] 3.1 Create `WarningCollector` actor in `Sources/ExFigCLI/Report/WarningCollector.swift` — follow `SharedThemeAttributesCollector` pattern (`Sources/ExFigCLI/Batch/SharedThemeAttributes.swift`). Store warnings as `[String]`. Note: TerminalUI does NOT currently store warnings — it only prints them +- [ ] 3.2 Extend `TerminalUI.warning()` methods to forward formatted message to `WarningCollector` when one is active (pass via `@TaskLocal` or inject into TerminalUI). Only active when `--report` is specified +- [ ] 3.3 Integrate warning collection into `ExportReport` construction in each export command +- [ ] 3.4 Write tests for `WarningCollector` (add warnings, retrieve, empty state) ## 4. Asset Manifest (Phase 2) - [ ] 4.1 Create `AssetManifest` and `ManifestEntry` structs with fields: path, action, checksum, assetType - [ ] 4.2 Create `FileAction` enum: created, modified, unchanged, deleted -- [ ] 4.3 Add optional file tracking to `FileWriter`: record path + content hash before write, detect action -- [ ] 4.4 Implement SHA256 checksum computation from in-memory data during file write +- [ ] 4.3 Add optional file tracking to `FileWriter`: before writing, check if file exists and compute `FNV1aHasher.hashToHex()` of new content. Compare with existing file hash to determine action (created/modified/unchanged). Only active when `--report` is specified — zero overhead otherwise +- [ ] 4.4 Compute content checksum via `FNV1aHasher.hashToHex()` (already in `Sources/ExFigCLI/Cache/FNV1aHasher.swift`) — NOT SHA256 (no CryptoKit/swift-crypto dependency in project). Produces 16-char lowercase hex - [ ] 4.5 Add `manifest` field to `ExportReport` (optional, present when tracking enabled) - [ ] 4.6 Write unit tests for FileWriter tracking (created, modified, unchanged detection) - [ ] 4.7 Write unit tests for AssetManifest JSON serialization ## 5. Deleted File Detection -- [ ] 5.1 Implement `deleted` action detection by comparing current manifest against previous report file +- [ ] 5.1 Implement `deleted` action detection by comparing current manifest against previous report file at the same `--report` path - [ ] 5.2 Write tests for deleted file detection (file in previous report but not in current export) ## 6. Integration Testing -- [ ] 6.1 Write integration test: `exfig colors --report` produces valid JSON file +- [ ] 6.1 Write integration test: `exfig colors --report` produces valid JSON with version, command, stats, timestamps - [ ] 6.2 Write integration test: export failure still writes report with `success: false` - [ ] 6.3 Write integration test: report write failure does not fail the export +- [ ] 6.4 Write integration test: zero-file export produces report with empty manifest