feat(cli): make rate-limit, max-retries, concurrent-downloads and batch settings configurable via PKL#82
Merged
Conversation
…ch settings configurable via PKL Adds 6 PKL fields so CI workflows pin network/orchestration knobs in exfig.pkl instead of repeating CLI flags everywhere. Pure-additive — configs that don't set the new fields keep v3.3.0 behavior. PKL schema: - figma.rateLimit / maxRetries / concurrentDownloads - new Batch.pkl module with batch.parallel / failFast / resume - PklProject 3.3.0 -> 3.4.0 Precedence per knob: CLI flag > PKL config > built-in default. Boolean flags (--fail-fast, --resume) use OR semantics (CLI || config). exfig batch reads `batch:` and `figma.*` rate-limiting fields ONLY from the first config in argv; per-target `batch:` blocks are debug-logged and ignored. fetch is config-free. colors/typography ignore figma.concurrentDownloads with a debug log under -v. Help text on all 7 flags unified to "(overrides config, default: N)". Tests: 12 new precedence cases in FaultToleranceOptionsTests, 7 cases in new BatchSettingsResolverTests. Full suite: 1954 passed / 0 failed.
Plug review-discovered gaps in the CLI > PKL > defaults precedence, silent failures, and missing tests: - Pass figma.concurrentDownloads to standalone icons/images exporters (was silently ignored outside batch). - Promote loadFirstConfig PKL errors from debug to warning so users see why their batch settings dropped to defaults. - Sanitize PKL configValue (clamp out-of-range with warning) and tighten CLI bounds to match PKL isBetween constraints. - Cache the first-config PKL evaluation via PKLModuleCache so BatchConfigRunner reuses it instead of re-evaluating. - Replace try? in logIgnoredPerTargetSettings with do/catch and warn on ignored figma.* fields in non-first batch configs. - Centralize fault-tolerance defaults in FaultToleranceDefaults + FaultToleranceValidator; deduplicate resolveClient via RateLimitClientFactory protocol. - Make ResolvedBatchSettings init fileprivate so its "Resolved" promise is enforced. - Sync ExFigCommand.version (3.3.0 -> 3.4.0) and exfig.usage.kdl with PklProject so generated configs reference an existing package. - Add tests for multi-config first-wins, Float64 timeout truncation, verbose ignored-block scan, partial PKL config, sanitizer clamps, defaults parity, and CLI upper-bound rejection.
Critical: - Wire HeavyFaultToleranceOptions through `download all` (was bypassing rate-limiter, retries, and CLI flags entirely). - Drop per-config `figma.timeout` fallback in BatchConfigRunner — use resolved batch-level timeout only, matching the documented "first-config-wins" rule and ignoredPerTargetFigmaRateLimiting warning. - Make `validateUsing(preloadedModule:)` throwing and call `resolveConfigPath()` so cached configs go through the same validation surface as `validate()`. Important: - Sync `failFast` x `maxRetries` in batch RetryPolicy (failFast forces 0 retries, matching HeavyFaultToleranceOptions.createRetryPolicy). - Promote PKL parse errors during verbose pre-check from debug to `batchSettingsPreloadFailed` warning. - Replace brittle "not found" substring match in `isFileNotFound` with structural FileManager + NSError domain checks. A real PKL error like "module member 'foo' not found" is no longer reclassified as missing-file. - Cap `concurrentDownloads * parallel` at 1000 with a warning to prevent EMFILE / cryptic CDN throttling at extreme combinations. - Add precedence integration tests for ExportColors/Icons/Images/Typography via subcommand wiring. - Add PKLModuleCache write/hit tests covering verbose pre-check path and URL standardization. - Add cliTimeout precedence tests in BatchSettingsResolverExtendedTests. Suggestions: - Dedupe `invalidConfigValue` warnings per (key, value) pair via FaultToleranceValidator.warnOnce so repeated sanitize() calls don't flood the log. - Add ExFigWarningFormatter tests for the four new fault-tolerance warnings + new `excessiveDownloadSlots` case. - Add upper-bound sanitizer tests for parallel / timeout / concurrentDownloads. - Verbose Batch logs now print resolved batch settings (parallel, failFast, resume, concurrentDownloads, timeout) for traceability. Tests: 1989 passed, lint clean.
- fault-tolerance.md: rephrase batch `--timeout` row to make "CLI > first config's figma.timeout > default" precedence explicit; per-target timeouts in subsequent configs are always ignored. - Usage.md: add `download all` to the list of commands supporting heavy fault-tolerance flags; add footnote clarifying that `batch.failFast` and `batch.resume` PKL keys apply only to `exfig batch` (standalone icons/images accept the CLI flags but don't read PKL fields). - Regenerate llms-full.txt.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request centralizes fault tolerance and batch processing logic, establishing a clear precedence hierarchy (CLI > PKL > Defaults) and enhancing performance through a new PKL module cache. The changes include a new settings resolver, centralized validation, and updated documentation. Feedback was provided to simplify a factory method by removing redundant local variable assignments in favor of direct property access.
…falls fault-tolerance.md: - Document BatchSettingsResolver as single merge point for CLI/PKL/defaults. - Note BatchConfigRunner must NOT re-read per-config figma.timeout. - Document FaultToleranceValidator.warnOnce dedup pattern with reset hook. - Document PKLModuleCache as the way to avoid re-evaluating configs across resolver / pre-check / runner. - Document slot product cap and failFast x maxRetries coupling in batch. gotchas.md: - pkl-swift requires .0 suffix for Number?-typed PKL fields in test fixtures (DecodingError.typeMismatch otherwise). - PKL constraints validate at amends-time, not at sanitize-time — tests for out-of-range values must call FaultToleranceValidator.sanitized* directly. - Prefer splitting test classes over `swiftlint:disable file_length` (e.g. PKLModuleCacheTests was extracted from BatchSettingsResolverExtendedTests).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
figma.rateLimit,figma.maxRetries,figma.concurrentDownloads,figma.timeout. CLI flags override PKL; PKL overrides built-in defaults.batch.parallel,batch.failFast,batch.resumePKL keys, read only from the first config inexfig batchargv. Per-targetbatch:blocks and per-targetfigma:*rate-limiting fields in subsequent configs are ignored. The resolver warns about them under--verbose.BatchSettingsResolvermerges CLI flags, the first config's PKL values, and built-in defaults. Out-of-range PKL values fall back to defaults with a deduplicatedinvalidConfigValuewarning per(key, value)pair.HeavyFaultToleranceOptionsthroughexfig download allso retry, rate limiting, timeout, and concurrent downloads work the same way as indownload colors / icons / images.PKLModuleCacheso the first config (and any verbose pre-checked ones) are not re-evaluated byBatchConfigRunner.figma.timeoutfallback inBatchConfigRunner. Resolver-level timeout is the single source of truth in batch mode.failFasttomaxRetries: 0in the batchRetryPolicy, matchingHeavyFaultToleranceOptions.createRetryPolicy()behavior outside batch mode.concurrentDownloads * parallelat 1000 with anexcessiveDownloadSlotswarning. Stops extreme combinations (e.g. 200 × 50 = 10000) from blowing past OS file descriptor limits.isFileNotFoundwith a structuralFileManager+NSErrorcheck. Real PKL errors likemodule member 'foo' not foundare no longer reclassified as missing-file.batchSettingsPreloadFailedwarning.Usage.md,BatchProcessing.md,Configuration.md,PKLGuide.md,.claude/rules/fault-tolerance.md,exfig.usage.kdl,llms-full.txt.Additional notes
BatchSettingsResolverTests,BatchSettingsResolverExtendedTests,PKLModuleCacheTests,SubcommandFaultTolerancePrecedenceTests, plus extensions toFaultToleranceOptionsTestsandExFigWarningFormatterTests. 1989 tests pass; lint clean.FaultToleranceDefaultsis the single source of truth. Parity with PKL schema defaults is asserted at runtime byBatchSettingsResolverExtendedTests.testPKLDefaultsMatchSwiftDefaults.PKLEvaluator.swiftwent from 41 to 43 (Batch.Module+Batch.BatchConfig), guarded byPKLEvaluatorTests.batch.failFastandbatch.resumePKL keys apply only toexfig batch. Standaloneiconsandimagesaccept the corresponding CLI flags but do not read these PKL fields.FaultToleranceValidator.warnOncededup strategy. It uses a process-level cache with a reset hook for tests. A ui-scoped alternative was considered, but the chosen approach matches theLock<T>patterns already used elsewhere in the codebase.