Skip to content

feat: dch-config init - #2

Merged
bobrykov merged 2 commits into
masterfrom
feat/dch-config
Jul 10, 2026
Merged

feat: dch-config init#2
bobrykov merged 2 commits into
masterfrom
feat/dch-config

Conversation

@bobrykov

@bobrykov bobrykov commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ab8b95c6-2fb0-4d9d-9ba5-d869b6c3a5c4

📥 Commits

Reviewing files that changed from the base of the PR and between c42ed30 and 3fff061.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • Cargo.toml
  • crates/dch-config/Cargo.toml
  • crates/dch-config/src/lib.rs
  • rust-toolchain.toml

📝 Walkthrough

Walkthrough

The PR adds workspace dependencies and replaces the dch-config stub with a public TOML configuration model, file loading and precedence rules, loopctl conversion, config-directory resolution, toolchain configuration, and tests for defaults, parsing, errors, mapping, and provider URLs.

Changes

Configuration support

Layer / File(s) Summary
Configuration model and dependency wiring
Cargo.toml, crates/dch-config/Cargo.toml, crates/dch-config/src/lib.rs, rust-toolchain.toml
Adds workspace-managed dependencies, stable Rust tooling components, public configuration structs and enums, provider URL mapping, serde defaults, configuration errors, and model tests.
Configuration loading and loopctl conversion
crates/dch-config/src/lib.rs
Resolves ~/.dch, loads config.local.toml before config.toml, falls back to defaults when files are absent, maps values into loopctl::config::LoopConfig, and tests parsing, precedence, malformed TOML, and conversion.

Sequence Diagram(s)

sequenceDiagram
  participant DchConfig
  participant ConfigDirectory
  participant TomlParser
  participant LoopConfig
  DchConfig->>ConfigDirectory: resolve ~/.dch
  DchConfig->>ConfigDirectory: select config.local.toml or config.toml
  ConfigDirectory->>TomlParser: parse selected TOML
  TomlParser-->>DchConfig: return configuration values
  DchConfig->>LoopConfig: map configuration fields
Loading
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately reflects the main change: initializing the dch-config crate.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 50.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/dch-config

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
crates/dch-config/src/lib.rs (1)

221-233: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Confirm intended config.local.toml precedence: full replacement, not field merge.

When config.local.toml exists, it is parsed alone and config.toml is ignored entirely, so a local file that sets only api.model discards every other value defined in config.toml (falling back to Default, not to the main file). This is consistent with the doc comment and test_local_overrides_main, but .local overlays commonly imply per-field overrides. Please verify this whole-file semantic is intended.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/dch-config/src/lib.rs` around lines 221 - 233, The config loader’s
local-file behavior needs confirmation and explicit documentation or adjustment:
inspect load_from_dir and its related doc comment and test_local_overrides_main,
then either preserve the current full-replacement semantics and update
naming/documentation to make that contract clear, or implement field-level
merging where config.local.toml overrides values from config.toml while
retaining unspecified fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/dch-config/src/lib.rs`:
- Around line 221-233: The config loader’s local-file behavior needs
confirmation and explicit documentation or adjustment: inspect load_from_dir and
its related doc comment and test_local_overrides_main, then either preserve the
current full-replacement semantics and update naming/documentation to make that
contract clear, or implement field-level merging where config.local.toml
overrides values from config.toml while retaining unspecified fields.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a52b8adc-8646-44f0-bd78-606c06fb22fc

📥 Commits

Reviewing files that changed from the base of the PR and between c42ed30 and 8e5d38b.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • Cargo.toml
  • crates/dch-config/Cargo.toml
  • crates/dch-config/src/lib.rs

@bobrykov
bobrykov merged commit d42841b into master Jul 10, 2026
9 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Jul 10, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Aug 8, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Aug 16, 2026
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