chore: replace darglint with pydoclint#58
Conversation
|
Warning Review limit reached
More reviews will be available in 6 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (24)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ FIPS Compatibility Check
Status: ✅ PASSED What is FIPS?FIPS 140-2/140-3 is a US government standard for cryptographic modules. Common issues:
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Dependency ReviewThe following issues were found:
License Issuesuv.lock
OpenSSF Scorecard
Scanned Files
|
There was a problem hiding this comment.
Pull request overview
Migrates docstring validation from the archived darglint tool to pydoclint 0.8.4, replacing the dependency, pre-commit hook, and tool config, then propagates the new style (arg (Type): desc, Type: desc for returns, class-level Attributes sections with types, __init__ docs merged into class docstrings, expanded Raises sections) across the source tree.
Changes:
- Drop
darglintdep/config/hook and skip-list entry; addpydoclint>=0.8.4with a Google-style[tool.pydoclint]block and a pinned pre-commit hook. - Update docstrings throughout
src/to include parameter/return type annotations and move__init__docstrings up to class docstrings (with new "Args" sections). - Refresh
.secrets.baselineanduv.lockfor the dependency swap.
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Replace [tool.darglint] config and dev dep with [tool.pydoclint] and pydoclint>=0.8.4. |
| .pre-commit-config.yaml | Swap local darglint hook for upstream pydoclint hook pinned to 0.8.4; drop from ci.skip. |
| uv.lock | Remove darglint; add pydoclint and docstring-parser-fork dependency. |
| .secrets.baseline | Regenerated baseline with new entries for security/test files. |
| src/audio_processor/api/init.py | Add return types in docstrings (dict[str, str], JSONResponse, etc.). |
| src/audio_processor/api/routes.py | Annotate Args/Returns; add Exception entry to process_audio Raises (see comment). |
| src/audio_processor/api/security.py | Annotate header dependency types in Args/Returns. |
| src/audio_processor/cli.py | Type-annotate Args in CLI command docstrings. |
| src/audio_processor/core/cache.py | Type-annotate Args/Returns of cache helpers and decorators. |
| src/audio_processor/core/config.py | Collapse Settings attribute docs to a note; type return values of properties/validators. |
| src/audio_processor/core/exceptions.py | Move per-exception __init__ Args into class docstrings; add Returns types. |
| src/audio_processor/core/job_store.py | Type Args/Returns; merge RedisJobStore.__init__ docstring into class. |
| src/audio_processor/core/models.py | Add types to Attributes (incl. model_config) and helper return types. |
| src/audio_processor/core/sentry.py | Type Args/Returns for init/capture/breadcrumb helpers. |
| src/audio_processor/jobs/audio_tasks.py | Type Args/Returns; expand process_audio_job Raises with TranscriptionError. |
| src/audio_processor/jobs/worker.py | Type Args/Returns for ARQ lifecycle and enqueue_task. |
| src/audio_processor/services/audio_conditioner.py | Move __init__ doc into class; type Args/Returns. |
| src/audio_processor/services/audio_converter.py | Same pattern: class-level Args, typed Args/Returns. |
| src/audio_processor/services/deepgram_client.py | Class-level Args with Raises; typed Args/Returns on all methods. |
| src/audio_processor/services/dom_builder.py | Class-level Args; typed Args/Returns; # type: ignore[type-arg] for dict return. |
| src/audio_processor/services/quality_assessor.py | Class-level Args; typed Args/Returns on all methods. |
| src/audio_processor/services/transcript_formatter.py | Drop trivial __init__ doc; typed Args/Returns; expanded class docstring. |
| src/audio_processor/services/vad_processor.py | Class-level Args; typed Args/Returns; typed dataclass Attributes. |
| src/audio_processor/utils/logging.py | Type Args/Returns for logging helpers. |
| fuzz/fuzz_input_validation.py | Type data arg in fuzz harness docstring. |
| HTTPException: If validation fails or file is too large. | ||
| Exception: If an unexpected error occurs during processing. |
There was a problem hiding this comment.
Addressed in 83c08aa, but kept the entry rather than removing it. pydoclint runs with skip-checking-raises = false, and process_audio contains a bare raise inside except Exception: (the enqueue-failure path at routes.py:342-349). pydoclint does per-statement raise analysis with no whole-function escape tracking, so it counts Exception as raised and flags DOC503 if it is undocumented. Removing the line breaks the required pydoclint gate, and a # noqa: DOC503 would violate the project's no-suppression rule.
To resolve your accuracy concern, I reworded the entry to state explicitly that the exception is always caught by the outer handler and re-raised as HTTPException(500), never reaching the caller. So the docstring now matches both the linter's view of the body and the function's external contract.
PR ReviewAutomated multi-agent review (code-reviewer, comment-analyzer, silent-failure-hunter, security-auditor) plus Copilot, CodeRabbit, SonarCloud, and qlty. The diff is overwhelmingly an accurate, mechanical darglint to pydoclint docstring migration. Two mechanical blockers and two code-touch findings stand out. Critical (blocks merge, but mechanical):
Important:
Suggested:
Verified safe: the large 🤖 Generated with Claude Code |
Swaps darglint>=1.8.1 for pydoclint>=0.8.4 in pyproject.toml dev deps, replaces [tool.darglint] with [tool.pydoclint] (google style, type hints required), updates .pre-commit-config.yaml hook from local darglint to pinned pydoclint 0.8.4 (rev 88d83c94), removes darglint from ci: skip list, and regenerates uv.lock and .secrets.baseline accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes all 512 DOC109/110/105/203/301/101/103/601/603/605/404/501/503 violations reported by pydoclint 0.8.4 across the full source tree. Pattern: arg (Type): desc in Args sections, Type: desc in Returns, merge __init__ docstrings into class docstrings for DOC301, add type hints to Attributes sections for dataclasses and Pydantic models, document Annotated[...] FastAPI parameters with their full annotation strings, and align Raises sections with actual raise statements. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- dom_builder.export_to_json: replace `-> dict # type: ignore[type-arg]` with `-> dict[str, object]` and align the docstring Returns type, removing the blanket type suppression per the CLAUDE.md no-`type: ignore` rule. - routes.process_audio: clarify the `Exception` Raises entry to state it is always caught by the outer handler and re-raised as HTTPException(500), never reaching the caller. Kept (not removed) because the body contains a bare re-raise that pydoclint DOC503 requires documented; addresses the Copilot accuracy concern without breaking the required pydoclint gate. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
williaby
force-pushed
the
chore/replace-darglint-with-pydoclint
branch
from
9581d66 to
83c08aa
Compare
PR Fix SummaryResolved the blockers from the review in an isolated worktree ( Rebase (clears 2 of the issues at once):
Code fixes (commit
Verified locally: ruff format + lint, BasedPyright (0 errors), pydoclint (0 violations), TruffleHog, detect-secrets, Bandit, interrogate, em-dash, all green. Signed commit. Not changed (Suggested / Informational, left for author judgment): the 6 🤖 Generated with Claude Code |
|
Summary
[tool.darglint]config block; adds pydoclint>=0.8.4 and[tool.pydoclint]with google style, type hints required in both signature and docstring88d83c94); removes darglint fromci: skiplistarg (Type): desc, returns asType: desc,__init__docstrings merged into class docstrings, Pydantic/dataclass Attributes sections typed, Raises sections aligned with raise statementsPosture: keep raises, require docstring arg type hints, lenient on Returns/Yields sections.
Test plan
uv lock && uv sync --all-extrassucceeds; lockfile shows darglint removed and pydoclint addeduv run pydoclint src/exits clean (0 violations)pre-commit run pydoclint --all-filespassespre-commit run --all-filespasses (ruff, basedpyright, trufflehog, detect-secrets, interrogate all green)grep -rn darglint pyproject.toml .pre-commit-config.yamlreturns emptyGenerated with Claude Code