Cache config file layer discovery on startup (#319)#367
Conversation
The startup path discovered and loaded configuration file layers twice: once in `resolve_merged_diag_json` (the diagnostic-mode pre-pass) and again in `merge_with_config` (the full merge), opening, reading, and deserialising every config file twice per invocation. Introduce `ConfigFileLayers`, loaded once per invocation, and `_with_layers` variants of both consumers that share it. `main` now loads the layers once and passes them to the pre-pass and the merge. The original two functions remain as thin wrappers that load layers themselves, preserving the existing API for tests and embedders. The build script's `verify_public_api_symbols` gains entries for the new public symbols, following the established convention that keeps the shared modules warning-free when compiled into the build script. A regression test proves the I/O is shared: it loads the layers, deletes the config file, and verifies both the diag pre-pass and the merge still honour the file's contents.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideIntroduces a cached configuration file layer abstraction that is loaded once on startup and shared between the diagnostic pre-pass and full config merge, eliminating duplicate config-file I/O while preserving the existing public API surface. Sequence diagram for cached config file layer reuse on startupsequenceDiagram
participant Run as run_with_args
participant Layers as ConfigFileLayers
participant Diag as resolve_merged_diag_json_with_layers
participant MergeEntry as merge_cli_or_exit
participant Merge as merge_with_config_layers
participant Composer as MergeComposer
Run->>Layers: ConfigFileLayers::load(parsed_cli)
activate Layers
Layers-->>Run: ConfigFileLayers
deactivate Layers
Run->>Diag: resolve_merged_diag_json_with_layers(parsed_cli, matches, &config_layers)
activate Diag
Diag->>Layers: ConfigFileLayers::as_result()
Diag-->>Run: bool (DiagMode input)
deactivate Diag
Run->>MergeEntry: merge_cli_or_exit(parsed_cli, matches, mode, &config_layers)
activate MergeEntry
MergeEntry->>Merge: merge_with_config_layers(parsed_cli, matches, &config_layers)
activate Merge
Merge->>Composer: MergeComposer::with_capacity(4)
Merge->>Merge: push_defaults_layer(composer, errors)
Merge->>Merge: push_file_layers(&config_layers, composer, errors)
Merge->>Composer: push_environment_layer(composer, errors)
Merge->>Composer: push_cli_layer(cli, matches, composer, errors)
Merge-->>MergeEntry: OrthoResult<Cli>
deactivate Merge
MergeEntry-->>Run: Result<Cli, ExitCode>
deactivate MergeEntry
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary
Closes #319
Eliminates the duplicate config-file I/O on startup:
resolve_merged_diag_jsonandmerge_with_configeach independently discovered and loaded all file layers, so every config file was opened, read, and deserialised twice per run.Stacked on #365 (merge-pipeline debug logging) because both touch
merge.rs/discovery.rs/main.rs; this PR retargets tomainautomatically once #365 merges.Changes
src/cli/discovery.rs: newConfigFileLayers(discovery outcome loaded once;MergeLayerisClone, so the merge clones cached layers instead of re-reading disk).push_file_layersnow consumes the cache;collect_diag_file_layersis gone.src/cli/diag.rs/src/cli/merge.rs: newresolve_merged_diag_json_with_layersandmerge_with_config_layers; the original functions remain as thin self-loading wrappers, keeping the existing API for tests.src/main.rs: loads the layers once and shares them between the pre-pass and the merge.build.rs:verify_public_api_symbolsentries for the new symbols (project convention for the build-script copy of the CLI modules).Testing
config_layers_are_loaded_once_and_sharedtest: loads the layers, deletes the config file, then verifies both the diag pre-pass and the merge still honour the file's contents — proving neither re-reads the disk.Validation
make check-fmt/make lint/make test— pass (37 suites)🤖 Generated with Claude Code
Summary by Sourcery
Cache configuration file layer discovery so diagnostic pre-pass and full merge share loaded config layers instead of re-reading from disk on startup.
Enhancements:
Tests: