feat(config): three-scope config resolution (User → Project → Local) - #73
Merged
Conversation
Add SCOPE_LOCKED_PATHS, DEDUP_LIST_FIELDS, ENV_FIELD_MAP constants and _set_nested, _lookup_provenance helper functions to support the scoped config pipeline. These pure functions handle nested dict operations and provenance tracking across config scopes.
Implements _load_scoped(project_root) as the core scoped config resolution pipeline: Ingest → Guard → Merge → Env → Validate. Reads user, project (.pythinker/config.toml), and local (.pythinker/config.local.toml) scopes; enforces scope locks before merge; overlays env vars last; attributes validation errors to their source scope; populates source_scopes on the returned Config and auto-gitignores config.local.toml when present. Also wraps the Task-7 source_scopes description to fix a pre-existing ruff E501 violation (blocked the ruff gate). Adds 8 integration tests covering user-only, project-overrides-user, local-overrides-project, hook concatenation, scope-lock violation, validation error attribution, env override, and source_scopes metadata. Note: 3 provided tests used "solarized" as a theme value, which is not in Literal["dark","light"] and fails Pydantic validation. Tests were corrected to "light"/"dark" while preserving their override-precedence intent. The merge-layer three-scope ordering is independently covered by test_merge_scalar_three_scopes. Pyright debt: 60 errors (45 pre-existing bare-dict annotations in Tasks 3-6 helpers + 15 same-class cascades from _load_scoped). No real type mismatches. Deferred to Task 9 typing sweep.
When called with no explicit file path, load_config now discovers User → Project → Local scopes relative to the nearest .git root, merges them with type-based rules, overlays PYTHINKER_* env vars, and validates once through Pydantic with provenance-enriched errors. Explicit --config path continues to bypass scope resolution. Also seeds a default user config.toml when no config exists after JSON migration (e.g. corrupt JSON backed up) so existing behaviour is preserved. Two backward-compatibility tests added.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a three-scope config resolution pipeline (user → project → local) with guarded merges, provenance tracking, PYTHINKER_* env overlays, validation errors annotated with provenance, and auto-gitignore of local config. Adds ChangesConfiguration scoping system
Sequence Diagram(s)sequenceDiagram
participant UserConfig as User Config (~/.pythinker/config.toml)
participant ProjectConfig as Project Config (.pythinker/config.toml)
participant LocalConfig as Local Config (.pythinker/config.local.toml)
participant Merger as _type_based_merge
participant EnvOverlay as _apply_env_vars (PYTHINKER_*)
participant Guard as _check_scope_locks
participant Validator as Pydantic Validator
participant Result as Resolved Config
UserConfig->>Merger: load user scope dict + provenance
ProjectConfig->>Merger: overlay project scope dict + provenance
LocalConfig->>Merger: overlay local scope dict + provenance
Merger->>EnvOverlay: merged dict + provenance
EnvOverlay->>Guard: apply env overrides, update provenance
Guard->>Validator: enforce scope locks
Validator->>Result: validate & attach source_scopes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/pythinker_code/config.py`:
- Around line 156-167: When _type_based_merge encounters a dict override (value
is dict) but base[key] exists as a non-dict, avoid recursing into the non-dict
by first detecting this type conflict and replacing base[key] with a new dict
and normalizing provenance[key] to a dict before calling _type_based_merge;
i.e., in the branch handling isinstance(value, dict) (inside _type_based_merge)
add a check like "if key in base and not isinstance(base[key], dict): base[key]
= {}" and ensure "provenance[key] = {}" (or convert it to a dict) so the
recursive call operates on dicts only and prevents the TypeError so validation
can raise a ConfigError later.
In `@tests/core/test_config.py`:
- Around line 589-597: Add an integration test that verifies _load_scoped
invokes ensure_gitignored when a local config file exists: create a tmp
project_root, write a local config file (e.g., project_root/"config.toml") and
the existing .pythinker/config.toml, then monkeypatch the module-level
ensure_gitignored (the function named ensure_gitignored) with a spy/stub that
records invocation, call _load_scoped(project_root=project_root), and assert the
spy was called with the project_root (or path to the local config) and that
source_scopes include "local"; reference the _load_scoped and ensure_gitignored
symbols when adding the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 61d37245-81aa-4567-aaca-70220e313296
📒 Files selected for processing (4)
src/pythinker_code/config.pysrc/pythinker_code/utils/gitignore.pytests/core/test_config.pytests/utils/test_gitignore.py
Add missing type arguments to bare dict/tuple generics across all five pipeline helpers (_set_nested, _lookup_provenance, _check_scope_locks, _type_based_merge, _apply_env_vars) and the nested _read_toml function. Import Any and cast from typing; use cast() at isinstance-narrowing call sites where dict[Unknown, Unknown] would otherwise propagate. Remove the now-redundant isinstance(prov, dict) guard in _lookup_provenance (prov is already narrowed to dict[str, Any] after the early-return str branch). Break the long _type_based_merge signature across lines to satisfy E501. Also wire Pythoughts-labs branding into constant.py (ORGANIZATION/CONTACT), pyproject.toml (authors/description), --version output, and pythinker info. Update __all__ in constant.py to multi-line form for line-length compliance. Add CHANGELOG entry for both the scoped config feature and identity update.
Add a 'Product Identity' block at the top of the default agent system prompt that instructs the agent to identify itself as Pythinker by Pythoughts-labs when asked, and to not name the underlying model. The underlying model (e.g. MiniMax, Claude) would otherwise fall back to its own built-in identity, causing responses like "I am MiniMax-M3" instead of the correct product identity.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/pythinker_code/agents/default/system.md`:
- Line 1: Add a top-level H1 to the start of the Markdown file so the first line
is a proper heading (fixing MD041); insert a descriptive H1 (for example "#
Pythinker") immediately before the existing opening sentence "You are
**Pythinker** — a think-first software engineering agent..." in
src/pythinker_code/agents/default/system.md so the document begins with an H1 on
line 1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b44c33c9-5e9c-44e1-a653-37e3583db1c1
📒 Files selected for processing (1)
src/pythinker_code/agents/default/system.md
- Guard dict/scalar type conflict in _type_based_merge: when base holds a scalar or list for a key that an overlay wants to replace with a dict, let the overlay win outright instead of recursing into a non-dict (which crashes with TypeError before validation can surface a ConfigError) - Add integration test verifying _load_scoped auto-gitignores config.local.toml when the file is present (test_load_scoped_gitignores_local_config) - Add MD041-compliant H1 heading to agents/default/system.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@tests/core/test_default_agent.py`:
- Around line 21-29: The current test in tests/core/test_default_agent.py is
brittle because it asserts the full prompt snapshot; update the test (the test
function that currently checks the identity prompt) to remove the large snapshot
assertion and instead assert focused invariants: confirm the prompt contains the
header "## Product Identity", the tokens "Pythinker" and "Pythoughts-labs", and
the explicit rule phrase forbidding naming/describing the underlying model
(e.g., assert "do not name" or "do not name or describe the underlying model" in
the prompt). Replace the snapshot equality with a few targeted in/substring
assertions and keep any existing helper that loads the prompt string. Ensure the
test uses plain membership checks (e.g., "in") rather than exact multi-line
matches so unrelated prompt edits won’t break it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 69984bf2-f511-4b75-9db0-70fc48e205bd
📒 Files selected for processing (1)
tests/core/test_default_agent.py
- config.py: in _type_based_merge, when a dict overlay hits a scalar/list at the same key, normalize base[key] and provenance[key] to empty dicts before recursing instead of short-circuiting with `continue`. The old approach left provenance[key] as a string, which would crash a subsequent dict-merge on the same key with TypeError. - test_default_agent.py: replace the full-prompt inline_snapshot in test_default_agent with four targeted substring assertions covering the Product Identity invariants (section header, product name, developer, and the no-model-name rule). The builtin_types snapshot is unchanged.
5 tasks
This was referenced Jun 11, 2026
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.
Summary
load_config()with a five-step scoped pipeline: Ingest → Guard → Merge → Env → Validate~/.pythinker/config.toml(user) →.pythinker/config.toml(project) →.pythinker/config.local.toml(local); local wins on scalar fields, lists concatenate across scopesproviders,services,feedback.api_key) are blocked in project/local scopes before merging;PYTHINKER_*env vars overlay all file scopesconfig.local.tomlis auto-added to.gitignorewhen first loaded;Config.source_scopesfield tracks which files contributed--configpath bypasses scope resolution entirely (backward-compatible)New files
src/pythinker_code/utils/gitignore.py—ensure_gitignoredutilitytests/utils/test_gitignore.py— 5 testsModified files
src/pythinker_code/config.py— all new constants, helpers, pipeline functions,source_scopesfield,load_configwiringtests/core/test_config.py— unit + integration tests for every new functionTest Plan
pytest tests/core/test_config.py— 66 tests covering all pipeline functions and backward-compat pathspytest tests/utils/test_gitignore.py— 5 tests forensure_gitignoredpytest tests/ --ignore=tests/e2e— full suite (4281 passed, 0 failures)make check-pythinker-code— ruff format + check cleanSummary by CodeRabbit
New Features
Documentation
Tests