Conversation
Reviewer's GuideAdds zero-config bootstrapping and standalone ruff.toml support to ruff-sync, refactors Ruff config parsing/merging to work for both pyproject and ruff.toml, tightens [tool.ruff-sync] config validation, and updates docs/tests (including new scaffold and config validation test suites). Sequence diagram for the updated pull command with zero-config init and ruff.toml supportsequenceDiagram
actor User
participant CLI as main
participant Parser as _get_cli_parser
participant Args as Arguments
participant FS as Filesystem
participant TOML as TOMLFile
participant HTTP as httpx_AsyncClient
participant CFG as get_ruff_config
participant MERGE as merge_ruff_toml
User->>CLI: invoke ruff_sync pull upstream_url --init
CLI->>Parser: build parser with pull subcommand and init flag
Parser-->>CLI: parsed argparse Namespace
CLI->>Args: Arguments.from_cli
Args-->>CLI: exec_args (includes init, source, upstream)
CLI->>CLI: _resolve_target_path(exec_args)
alt source is file
CLI-->>CLI: use args.source as target path
else ruff.toml exists
CLI-->>CLI: use source/ruff.toml
else .ruff.toml exists
CLI-->>CLI: use source/.ruff.toml
else upstream points to ruff.toml
CLI-->>CLI: use source/ruff.toml
else
CLI-->>CLI: use source/pyproject.toml
end
CLI->>FS: check if target path exists
alt file exists
FS-->>CLI: file found
CLI->>TOML: read existing document
TOML-->>CLI: source_doc
else file missing and init is true
FS-->>CLI: missing
CLI->>FS: mkdir parents, touch target file
CLI->>TOML: create empty tomlkit.document
TOML-->>CLI: source_doc
else file missing and init is false
CLI-->>User: print error and return 1
CLI-->>User: return
end
CLI->>HTTP: create AsyncClient
CLI->>HTTP: download upstream config
HTTP-->>CLI: upstream TOML text
CLI->>CFG: get_ruff_config(upstream_text, is_ruff_toml)
CFG-->>CLI: upstream_ruff_config (TOMLDocument or Table)
CLI->>MERGE: merge_ruff_toml(source_doc, upstream_ruff_config, is_ruff_toml)
MERGE-->>CLI: merged_doc
CLI->>TOML: write merged_doc to target path
TOML-->>CLI: write complete
CLI-->>User: print Updated relative_path
CLI-->>User: exit code 0
Class diagram for Arguments, Config, and Ruff configuration helpersclassDiagram
class Arguments {
+str command
+pathlib_Path source
+str upstream
+list~str~ exclude
+str branch
+str path
+bool semantic
+bool diff
+bool init
+classmethod from_cli(raw_args)
+@staticmethod fields()
}
class Config {
+Optional~str~ upstream
+Optional~list~ exclude
+Optional~str~ branch
+Optional~str~ path
+Optional~bool~ semantic
+Optional~bool~ diff
+Optional~bool~ init
}
class get_config {
+get_config(source)
}
class get_ruff_config {
+get_ruff_config(toml, is_ruff_toml, create_if_missing, exclude)
}
class merge_ruff_toml {
+merge_ruff_toml(source, upstream_ruff_doc, is_ruff_toml)
}
class is_ruff_toml_file {
+is_ruff_toml_file(path_or_url)
}
class _resolve_target_path {
+_resolve_target_path(args)
}
class check {
+check(args)
}
class pull {
+pull(args)
}
class _apply_exclusions {
+_apply_exclusions(tbl, exclude)
}
class _recursive_update {
+_recursive_update(source_table, upstream)
}
class main {
+main()
}
get_config --> Config : returns
main --> get_config : calls
main --> _resolve_target_path : uses via check, pull
main --> check : may call
main --> pull : may call
pull --> _resolve_target_path : resolve target path
pull --> is_ruff_toml_file : detect upstream and source type
pull --> get_ruff_config : read upstream ruff config
pull --> merge_ruff_toml : merge upstream into source
check --> _resolve_target_path : resolve target path
check --> is_ruff_toml_file : detect upstream and source type
check --> get_ruff_config : read upstream ruff config
check --> merge_ruff_toml : compute merged_doc
merge_ruff_toml --> get_ruff_config : obtain tool_ruff section when pyproject
merge_ruff_toml --> _recursive_update : merge structures
get_ruff_config --> _apply_exclusions : apply exclude paths
main --> Arguments : constructs
get_config --> Arguments : uses fields to validate keys
Flow diagram for resolving the target configuration pathflowchart TD
Start[[Start _resolve_target_path]]
SRC[args.source]
QFile{Is args.source a file?}
QRuff{Does source/ruff.toml exist?}
QDotRuff{Does source/.ruff.toml exist?}
QUpstream{Is args.upstream a ruff.toml?}
PyProj[source/pyproject.toml]
Ruff[source/ruff.toml]
DotRuff[source/.ruff.toml]
UseFile[Use args.source]
UseRuff[Use source/ruff.toml]
UseDotRuff[Use source/.ruff.toml]
UsePyProj[Use source/pyproject.toml]
End[[Return target path]]
Start --> SRC --> QFile
QFile -- Yes --> UseFile --> End
QFile -- No --> QRuff
QRuff -- Yes --> Ruff --> UseRuff --> End
QRuff -- No --> QDotRuff
QDotRuff -- Yes --> DotRuff --> UseDotRuff --> End
QDotRuff -- No --> QUpstream
QUpstream -- Yes --> Ruff --> UseRuff --> End
QUpstream -- No --> PyProj --> UsePyProj --> End
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #82 +/- ##
==========================================
- Coverage 92.13% 90.48% -1.65%
==========================================
Files 1 1
Lines 356 410 +54
==========================================
+ Hits 328 371 +43
- Misses 28 39 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This was
linked to
issues
Mar 9, 2026
Closed
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The new
is_ruff_toml_filehelper relies on a simple.endswith("ruff.toml")check, which will fail for common URL patterns with query strings or fragments (e.g.ruff.toml?raw=1); consider using URL parsing or a more robust suffix check to avoid misclassifying valid upstream configs. - The updated testing guidelines require each test file to end with a
__main__block, buttests/test_scaffold.pyis missing this, so it would be good to add the standardif __name__ == "__main__": pytest.main([...])footer there for consistency.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `is_ruff_toml_file` helper relies on a simple `.endswith("ruff.toml")` check, which will fail for common URL patterns with query strings or fragments (e.g. `ruff.toml?raw=1`); consider using URL parsing or a more robust suffix check to avoid misclassifying valid upstream configs.
- The updated testing guidelines require each test file to end with a `__main__` block, but `tests/test_scaffold.py` is missing this, so it would be good to add the standard `if __name__ == "__main__": pytest.main([...])` footer there for consistency.
## Individual Comments
### Comment 1
<location path="ruff_sync.py" line_range="429-431" />
<code_context>
return await download(url, client)
+def is_ruff_toml_file(path_or_url: str) -> bool:
+ """Return True if the path or URL indicates a ruff.toml file."""
+ return path_or_url.endswith("ruff.toml")
+
+
</code_context>
<issue_to_address>
**suggestion:** Relying on `endswith("ruff.toml")` might misclassify URLs with query strings or fragments.
This correctly handles plain paths and URLs that literally end with `ruff.toml` (including `.ruff.toml`), but will return `False` for cases like `...?file=ruff.toml` or `ruff.toml?ref=main`. If such URLs are expected, consider parsing with `urllib.parse.urlparse` and applying `endswith("ruff.toml")` to the path component, or otherwise checking only the part before any query string or fragment.
Suggested implementation:
```python
def is_ruff_toml_file(path_or_url: str) -> bool:
"""Return True if the path or URL indicates a ruff.toml file.
This handles:
- Plain paths (e.g. "ruff.toml", ".ruff.toml", "configs/ruff.toml")
- URLs with query strings or fragments (e.g. "ruff.toml?ref=main", "ruff.toml#L10")
by examining only the path component (or the part before any query/fragment).
"""
parsed = urlparse(path_or_url)
# If it's a URL with a scheme/netloc, use the parsed path component.
# Otherwise, fall back to stripping any query/fragment from the raw string.
if parsed.scheme or parsed.netloc:
path = parsed.path
else:
path = path_or_url.split("?", 1)[0].split("#", 1)[0]
return path.endswith("ruff.toml")
```
Add the missing import near the other imports at the top of `ruff_sync.py`:
- `from urllib.parse import urlparse`
This ensures `urlparse` is available for `is_ruff_toml_file`. No other changes should be required.
</issue_to_address>
### Comment 2
<location path="tests/test_scaffold.py" line_range="8-10" />
<code_context>
+"""
+
+
+@pytest.mark.asyncio
+async def test_pull_without_init_fails_on_missing_file(
+ mock_http: respx.MockRouter, fs: FakeFilesystem, capsys: pytest.CaptureFixture[str]
+):
+ target_dir = pathlib.Path(fs.create_dir("empty_dir").path) # type: ignore[arg-type]
+
+ upstream = URL("https://example.com/pyproject.toml")
+ result = await ruff_sync.pull(
+ ruff_sync.Arguments(
+ command="pull",
+ upstream=upstream,
+ source=target_dir,
+ exclude=(),
+ verbose=0,
+ init=False,
+ )
+ )
+
+ assert result == 1
+ captured = capsys.readouterr()
+ assert "Configuration file" in captured.out
+ assert "does not exist" in captured.out
+ assert "Pass the '--init' flag" in captured.out
+ assert not (target_dir / "pyproject.toml").exists()
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for `_resolve_target_path` when a config file already exists, and for `.ruff.toml` resolution
The current tests only cover the "no config present" case. Please also exercise `_resolve_target_path` when config files already exist, including its precedence of `ruff.toml` → `.ruff.toml` → `pyproject.toml`. Concretely, add tests that:
- Start with an existing `pyproject.toml` and verify `pull --init` uses it and does not create `ruff.toml`.
- Start with both `pyproject.toml` and `.ruff.toml` and assert `.ruff.toml` is selected.
- Start with only `.ruff.toml` and verify `pull` updates it.
This will cover the key branches in `_resolve_target_path` and guard against regressions in file selection behavior.
```suggestion
from httpx import URL, Response
import ruff_sync
@pytest.mark.asyncio
async def test_pull_init_uses_existing_pyproject_toml(
mock_http: respx.MockRouter, fs: "FakeFilesystem", capsys: pytest.CaptureFixture[str]
) -> None:
# Arrange: existing pyproject.toml, no ruff.toml or .ruff.toml
directory = fs.create_dir("project_with_pyproject")
target_dir = pathlib.Path(directory.path) # type: ignore[arg-type]
pyproject_path = target_dir / "pyproject.toml"
fs.create_file(str(pyproject_path), contents="[tool.ruff]\nline-length = 79\n")
upstream = URL("https://example.com/pyproject.toml")
mock_http.get(str(upstream), return_value=Response(200, text="[tool.ruff]\nline-length = 88\n"))
# Act
result = await ruff_sync.pull(
ruff_sync.Arguments(
command="pull",
upstream=upstream,
source=target_dir,
exclude=(),
verbose=0,
init=True,
)
)
# Assert: command succeeded, existing pyproject.toml was used as target
assert result == 0
captured = capsys.readouterr()
assert "pyproject.toml" in captured.out
assert pyproject_path.exists()
assert (target_dir / "ruff.toml").exists() is False
assert (target_dir / ".ruff.toml").exists() is False
contents = pyproject_path.read_text()
assert "line-length = 88" in contents
@pytest.mark.asyncio
async def test_pull_prefers_dot_ruff_toml_over_pyproject_toml(
mock_http: respx.MockRouter, fs: "FakeFilesystem", capsys: pytest.CaptureFixture[str]
) -> None:
# Arrange: both pyproject.toml and .ruff.toml exist
directory = fs.create_dir("project_with_pyproject_and_dot_ruff")
target_dir = pathlib.Path(directory.path) # type: ignore[arg-type]
pyproject_path = target_dir / "pyproject.toml"
dot_ruff_path = target_dir / ".ruff.toml"
fs.create_file(str(pyproject_path), contents="[tool.ruff]\nline-length = 79\n")
fs.create_file(str(dot_ruff_path), contents="[tool.ruff]\nline-length = 100\n")
upstream = URL("https://example.com/ruff.toml")
mock_http.get(str(upstream), return_value=Response(200, text="[tool.ruff]\nline-length = 88\n"))
# Act: no --init, should update existing config, preferring .ruff.toml
result = await ruff_sync.pull(
ruff_sync.Arguments(
command="pull",
upstream=upstream,
source=target_dir,
exclude=(),
verbose=0,
init=False,
)
)
# Assert: command succeeded and .ruff.toml was selected and updated
assert result == 0
captured = capsys.readouterr()
assert ".ruff.toml" in captured.out
assert pyproject_path.exists()
assert dot_ruff_path.exists()
pyproject_contents = pyproject_path.read_text()
dot_ruff_contents = dot_ruff_path.read_text()
# pyproject.toml should remain unchanged
assert "line-length = 79" in pyproject_contents
# .ruff.toml should be updated with the upstream contents
assert "line-length = 88" in dot_ruff_contents
@pytest.mark.asyncio
async def test_pull_updates_existing_dot_ruff_toml(
mock_http: respx.MockRouter, fs: "FakeFilesystem", capsys: pytest.CaptureFixture[str]
) -> None:
# Arrange: only .ruff.toml exists
directory = fs.create_dir("project_with_dot_ruff_only")
target_dir = pathlib.Path(directory.path) # type: ignore[arg-type]
dot_ruff_path = target_dir / ".ruff.toml"
fs.create_file(str(dot_ruff_path), contents="[tool.ruff]\nline-length = 79\n")
upstream = URL("https://example.com/ruff.toml")
mock_http.get(str(upstream), return_value=Response(200, text="[tool.ruff]\nline-length = 120\n"))
# Act
result = await ruff_sync.pull(
ruff_sync.Arguments(
command="pull",
upstream=upstream,
source=target_dir,
exclude=(),
verbose=0,
init=False,
)
)
# Assert: command succeeded, .ruff.toml was updated, and no new files created
assert result == 0
captured = capsys.readouterr()
assert ".ruff.toml" in captured.out
assert dot_ruff_path.exists()
assert (target_dir / "ruff.toml").exists() is False
assert (target_dir / "pyproject.toml").exists() is False
contents = dot_ruff_path.read_text()
assert "line-length = 120" in contents
```
</issue_to_address>
### Comment 3
<location path="tests/test_config_validation.py" line_range="26-35" />
<code_context>
+ LOGGER.propagate = original_propagate
+
+
+def test_get_config_warns_on_unknown_key(
+ tmp_path: pathlib.Path, caplog: pytest.LogCaptureFixture, clean_config_cache: None
+):
+ pyproject = tmp_path / "pyproject.toml"
+ pyproject.write_text(
+ """
+[tool.ruff-sync]
+upstream = "https://github.com/org/repo"
+unknown_key = "value"
+"""
+ )
+
+ # We need to ensure the logger is set up to capture the warning
+ # In ruff_sync.py, get_config is called before handlers are added in main()
+ # But in tests, caplog should catch it if the level is right.
+
+ with caplog.at_level(logging.WARNING):
+ config = get_config(tmp_path)
+
+ assert "Unknown ruff-sync configuration: unknown_key" in caplog.text
+ assert "upstream" in config
+ assert "unknown_key" not in config
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a positive test ensuring allowed keys are passed through without warnings
The current tests only cover disallowed keys and their warnings/filtering. Please also add a test where the config contains only allowed keys (e.g. `upstream`, `exclude`, `branch`) and assert that `get_config()` returns them unchanged and no warnings are logged (e.g. `caplog.text` does not contain "Unknown ruff-sync configuration"). This will help catch regressions where valid keys might be excluded in future changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
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.
🚀 Overview
This PR introduces the
--initflag, enabling a "Zero-Config" bootstrapping experience. It also adds first-class support for standaloneruff.tomland.ruff.tomlfiles, makingruff-syncmore versatile for projects that don't usepyproject.toml.✨ Key Changes
🛠️ Bootstrapping with
--initAdded a new
--initflag to thepullcommand. In a fresh project with no existing configuration, you can now run:ruff-sync pull https://github.com/my-org/standards --initThis will scaffold a new
pyproject.toml(orruff.toml) pre-filled with the upstream settings, eliminating the need for manual setup.📄 Standalone Config Support
is_ruff_toml_filehelper for intelligent file type detection.ruff-syncnow correctly identifies and handlesruff.tomland.ruff.tomlfiles as both sources and targets.💅 CLI & DX Improvements
✅ Updated pyproject.tomlinstead of absolute paths) for a cleaner UX.get_configto be more resilient and handle cases where files are missing during initialization.🧪 Testing
is_ruff_toml_fileto cover various URL formats (parameters, fragments, local paths).--initworkflow creates valid TOML files from scratch.