Add df12 pylint checkers and ambrleaks snapshot scanner - #6
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Important Review skippedNo new commits to review since the last review. ⚙️ 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:
WalkthroughThe package becomes a registered Pylint plugin with nine checkers and adds the ChangesLint plugin and snapshot scanning
Sequence Diagram(s)sequenceDiagram
participant User
participant AmbrleaksCLI
participant Config
participant Scanner
participant SnapshotFiles
User->>AmbrleaksCLI: Run ambrleaks with paths and options
AmbrleaksCLI->>Config: Load TOML rules and allowlists
AmbrleaksCLI->>SnapshotFiles: Discover and read .ambr files
SnapshotFiles-->>Scanner: Snapshot text
Scanner-->>AmbrleaksCLI: Findings and fingerprints
AmbrleaksCLI-->>User: Findings and exit status
Poem
🚥 Pre-merge checks | ✅ 19 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (19 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
leynos
marked this pull request as ready for review
July 23, 2026 11:17
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
df12_python_lints/ambrleaks/rules.py (1)
89-98: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winStill missing absolute POSIX paths outside the enumerated directory list.
Broaden the pattern past this fixed directory allowlist —
/srv/data/key-style paths under an unlisted top-level directory still slip through undetected.🤖 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 `@df12_python_lints/ambrleaks/rules.py` around lines 89 - 98, Update the pattern in the “snapshot-posix-path” Rule to detect absolute POSIX paths regardless of their top-level directory, including paths such as /srv/data/key when the directory is not explicitly listed. Preserve the existing path-boundary and termination constraints while removing the fixed top-level directory allowlist.
🤖 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 `@df12_python_lints/_chains.py`:
- Around line 25-98: Complete the public numpydoc sections: in
df12_python_lints/_chains.py lines 25-98, add Parameters and Returns
documentation to repeated_subject, narrowing_prefix, and elif_chain_tests; in
df12_python_lints/constant_chain.py lines 141-149,
df12_python_lints/match_dispatch.py lines 101-109, and
df12_python_lints/suppressions.py lines 121-129, add Attributes sections
documenting each checker’s name and msgs fields.
In `@df12_python_lints/ambrleaks/cli.py`:
- Around line 268-286: Update the exception tuple in main to include
UnicodeDecodeError alongside the existing scan errors, so invalid UTF-8 input
follows the existing ambrleaks error message and exit-code-2 path.
In `@df12_python_lints/constant_chain.py`:
- Around line 56-81: In _is_constant_like, replace the isinstance(expr,
nodes.Name) check within the nodes.Attribute branch with a nested structural
match on expr. Preserve the existing behavior: return true for an uppercase
attribute name or a Name expression whose name begins with an uppercase
character, and false otherwise.
In `@tests/test_crosshair.py`:
- Around line 49-55: Update the CrossHair invocation in the test to include
--report_all, then assert that stdout contains “Confirmed over all paths” for
each kernel under verification. Retain the existing return-code and “no
checkable functions” checks, ensuring a quiet vacuous pass cannot satisfy the
test.
---
Duplicate comments:
In `@df12_python_lints/ambrleaks/rules.py`:
- Around line 89-98: Update the pattern in the “snapshot-posix-path” Rule to
detect absolute POSIX paths regardless of their top-level directory, including
paths such as /srv/data/key when the directory is not explicitly listed.
Preserve the existing path-boundary and termination constraints while removing
the fixed top-level directory allowlist.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f82b440f-8f80-46cd-bcb8-2976a9255e54
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (37)
AGENTS.mdMakefileREADME.mddf12_python_lints/__init__.pydf12_python_lints/_chains.pydf12_python_lints/_expressions.pydf12_python_lints/ambrleaks/__init__.pydf12_python_lints/ambrleaks/cli.pydf12_python_lints/ambrleaks/rules.pydf12_python_lints/ambrleaks/scanner.pydf12_python_lints/assert_messages.pydf12_python_lints/constant_chain.pydf12_python_lints/match_dispatch.pydf12_python_lints/pure.pydf12_python_lints/reexports.pydf12_python_lints/snapshot_asserts.pydf12_python_lints/suppressions.pydf12_python_lints/wrappers.pydocs/contents.mddocs/developers-guide.mddocs/users-guide.mdpyproject.tomltests/test_ambrleaks.pytests/test_assert_messages.pytests/test_constant_chain.pytests/test_crosshair.pytests/test_e2e_shim.pytests/test_match_dispatch.pytests/test_plugin.pytests/test_properties.pytests/test_reexports.pytests/test_snapshot_asserts.pytests/test_stub.pytests/test_suppressions.pytests/test_wrappers.pytypos.local.tomltypos.toml
💤 Files with no reviewable changes (2)
- tests/test_stub.py
- df12_python_lints/pure.py
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/conftest.py`:
- Around line 1-16: Expand the module docstring in tests/conftest.py to explain
the shared fixtures’ purpose, utility, and usage as a test API. Update the
write_snapshot fixture docstring with NumPy-style Parameters and Returns
sections plus a meaningful usage example, while preserving its existing behavior
and type contract.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8a96078c-b4a7-4a30-9e0e-4c018c045b54
📒 Files selected for processing (6)
tests/conftest.pytests/test_ambrleaks.pytests/test_ambrleaks_cli.pytests/test_constant_chain.pytests/test_snapshot_asserts.pytests/test_wrappers.py
Owner
Author
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (2 errors)
|
This comment was marked as resolved.
This comment was marked as resolved.
Turn the package into a pylint plugin registering two checkers: - `prefer-structural-pattern-matching` (R9101) flags `isinstance` dispatch on a single subject, both as `if`/`elif` chains and as consecutive terminating guard `if` statements, and recommends a `match` statement with class patterns. - `assert-missing-message` (C9102) flags `assert` statements without a failure message, so a shrunk property-test counterexample names the violated invariant. Both checkers came out of recurring review feedback on downstream projects. Remove the generated `hello` scaffold and its stub test, and add `pylint` as a runtime dependency of the plugin.
Add `prefer-match-over-constant-chain` (R9103), reporting `if`/`elif` chains where every branch compares one subject with constants, enumeration members, or literals — by equality, by membership in a literal collection, or by an `or` combination of such comparisons. The message recommends a `match` statement over an enumeration of the accepted values. Extract the shared `if`/`elif` chain traversal helpers from the dispatch checker into `_chains.py` so both chain-oriented checkers use one implementation.
Describe the three checkers, their message identifiers, and how to load the plugin in the users' guide, and replace the template README blurb with a summary of the plugin.
`make fmt` rewraps AGENTS.md and `make spelling` refreshes the generated `typos.toml` from the shared en-GB-oxendict base.
Add four checkers rounding out the initial lint set: - `trivial-attribute-wrapper` (R9104) reports functions whose body only returns an attribute of a parameter or calls through such an attribute with the function's own parameters forwarded unchanged. Decorated functions are exempt because decorators such as `property` make the forwarding deliberate. - `reexport-by-assignment` (C9105) reports module-level names bound by assigning an imported name or an attribute of an imported module, recommending a real `from ... import ... as ...` binding instead. - `lint-suppression-without-explanation` (C9106) and `typecheck-suppression-without-explanation` (C9107) are token-based checks requiring every suppression pragma (`noqa`, `ruff: noqa`, `pylint: disable`; `type: ignore`, `pyright: ignore`, `ty: ignore`, `mypy:`) to record a reason, either inline or as a standalone comment on the line above. Extract the pure attribute-chain helper into `_expressions.py` for shared use by the wrapper and re-export checkers.
Extend the users' guide and README with the four new checkers, their message identifiers, and their exemptions.
Ship an `ambrleaks` console script (installable with `uv tool install df12-python-lints`) that scans syrupy `.ambr` snapshot files for values that should have been redacted with a matcher before recording: long hex strings (entropy-gated), UUIDs, email addresses, E.164 phone numbers (opt-in), URLs, and absolute POSIX and Windows paths. Findings are attributed to their `# name:` test block. Pylint only feeds Python modules to checkers, so snapshot files need a file-level tool; the rule model (strict regex, entropy floor, allowlist) follows gitleaks. Because syrupy rewrites `.ambr` files wholesale on `--snapshot-update`, inline suppression markers cannot survive; suppression instead lives in an `ambrleaks.toml` allowlist (values, test-name globs, path globs, per-rule enablement) and an optional JSON baseline whose fingerprints exclude line numbers so they survive snapshot regeneration.
Add `prefer-snapshot-assertion` (R9108) and `prefer-snapshot-substring` (R9109), reporting assertions in `test_`-named functions that would carry their contract more clearly as a syrupy snapshot: - equality against a large inline literal — a collection with eight or more constant/name leaves (counted on the AST so formatting is irrelevant) or a multiline/long string, including `textwrap.dedent` wrappers; and - three or more substring probes (`assert "..." in subject`) against one subject in a single test. Comparisons with names such as `expected` fixtures, small literals, and asserts outside test functions are never reported.
Replace the checker listing with the standard df12 README structure: tagline, value proposition, quick start with a runnable example and the reported message, feature list, documentation signposts, licence, and contributing sections.
Add `trivial-alias-wrapper` (R9110), reporting functions whose body only calls another function with the wrapper's own parameters forwarded unchanged, such as `def foo(qux): return bar(qux)`. The message is separate from `trivial-attribute-wrapper` (R9104) so the two can be enabled independently: bare-name forwarding is more contestable than attribute-chain forwarding. To keep false positives out, the target must resolve to a module-level function or an import — calling through a parameter is higher-order code, and wrapping a class constructor or a builtin is a factory with a deliberate name, so neither fires. Decorated functions remain exempt.
Move the dispatch-selection logic out of the match-dispatch checker into two pure functions in `_chains.py` — `repeated_subject` and `narrowing_prefix` — annotated with builtin-only types and PEP 316 contracts. The narrowing logic was the subtlest part of the guard-run detection, so it now carries machine-checkable postconditions. Add `make crosshair`, an opt-in gate (RUN_CROSSHAIR=1) that runs `crosshair check --analysis_kind=PEP316` over the kernels via a pytest wrapper, per the verification tiering: symbolic search runs on kernel changes, not on every push. The wrapper also fails when CrossHair reports "no checkable functions", so a vacuous pass cannot hide the contracts silently unresolving again (as happened when the annotations referenced a TYPE_CHECKING-only import). Truthiness on the tuple parameter became `len()` comparisons because symbolic execution cannot coerce `__bool__` on symbolic sequences.
Treat a unary operation on a constant (`-1`, `+1`, `~1`) as constant-like in `prefer-match-over-constant-chain`. Negative number literals parse as `UnaryOp` nodes rather than `Const`, so chains such as `value == 0` / `value == -1` were silently missed. Found by a Hypothesis property over generated constant chains; the shrunk counterexample is pinned as a named regression test.
Cover the input families the example suites only sample: constant chains of any length always fire and any variable branch poisons them; isinstance guard runs of any length report exactly once; the snapshot literal threshold depends only on AST leaf count, not nesting; generated suppression pragmas are classified uniformly with and without prose; the pure chain kernels honour their contracts; and Shannon entropy respects its bounds and repetition invariance. Identifiers and prose are constructed (prefixed alphabets) rather than filtered, avoiding the rejection-sampling trap.
Lint fixture modules through `leynos/pylint-pypy-shim` — the same runner as the project's lint gate, at the same pinned ref, read from the Makefile so the two cannot drift — with the plugin loaded from PYTHONPATH. One fixture holds a violation per checker and must produce all ten message symbols; a clean fixture must produce none. This proves the checkers work under PyPy-backed pylint, not only under the CPython test harness.
Address two review findings: - Pass-through detection now compares the call's arguments with the wrapper's ordered parameter list. Reordering (`target(b, a)`), repeating (`target(a, a)`), or omitting a parameter adapts the call — that is behaviour, so such wrappers are no longer reported by `trivial-attribute-wrapper`. Membership checking alone treated them as trivial. - `trivial-alias-wrapper` no longer accepts every import binding as a function. Imported names are resolved through astroid inference and only count when they infer to a function, so an imported class constructor (`from collections import OrderedDict`) is exempt like a locally defined one, matching the documented contract. Each false positive is pinned as a regression test, alongside checks that true positives (in-order proxies, imported functions such as `json.dumps`) still fire.
The users' guide now names the astroid AST library, which the en-GB-oxendict gate corrected to "asteroid". Add it to the accepted words in `typos.local.toml` and regenerate `typos.toml`.
Fingerprints deliberately exclude line numbers, so repeated values in one snapshot block share a fingerprint. The baseline previously deduplicated on write and suppressed by set membership on load, so a newly introduced duplicate of a baselined value passed silently. Write one baseline entry per occurrence (sorted, so ordering stays deterministic) and load the baseline as occurrence counts with `collections.Counter`: each recorded fingerprint suppresses exactly one occurrence, in the scan's existing deterministic order, and any surplus occurrence is reported and fails the run. Existing JSON baselines (a list of fingerprint strings) remain compatible — prior deduplicated baselines now simply accept one occurrence per entry.
Replace the raw astroid isinstance checks in `_chains.py` (`elif_chain_tests`, `is_elif_branch`) and `match_dispatch.py` (`_isinstance_subjects`, `_guard_subjects`) with match/case patterns in the house style the plugin itself recommends, preserving behaviour exactly. Remove the now-unused `_TERMINAL_STATEMENTS` tuple. The contract-carrying pure kernels are untouched, keeping the CrossHair model checks valid.
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/users-guide.md (1)
126-128: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
from module import name as aliashere, or drop the parenthetical.import module as aliasbinds the module object, not the exported name shown byjoin = os.path.join, so it changes the public API.🤖 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 `@docs/users-guide.md` around lines 126 - 128, Update the documentation guidance to recommend only `from module import name as alias` for exposing an imported name; remove the parenthetical suggesting `import module as alias`, since that binds the module object and changes the documented API.
🤖 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 `@df12_python_lints/ambrleaks/cli.py`:
- Around line 58-70: Update the public discover() docstring to follow NumPy
style by adding a Parameters section documenting paths and a Yields section
documenting the yielded pathlib.Path values, including that directory inputs
produce .ambr files and file inputs yield themselves.
- Around line 138-149: Replace the multi-line docstring on the private helper
_collect_findings with a concise single-line summary. Apply the same single-line
summary policy to the other referenced private helper as well, preserving only
its essential purpose and omitting extended implementation details.
- Line 73: Update the scan generator around the rglob traversal to avoid sorted
unbounded materialization: traverse directories in deterministic order while
yielding paths incrementally, so next() does not require consuming the full
tree. Refactor baseline filtering and writing to process findings incrementally
without sorting all fingerprints or converting findings to a list, while
preserving ordering and bounded state; add regression coverage for early yield
and incremental baseline-path handling.
In `@docs/developers-guide.md`:
- Around line 78-81: Update the sentence in the documentation around discover
and _collect_findings by adding a comma before “and” in “before the next file is
read, and the full finding set is never held in memory.”
- Around line 82-87: Revise the baseline-mode documentation to state explicitly
that --baseline materializes findings for apply_baseline and --write-baseline
sorts the complete fingerprint collection, so memory scales with input size.
Remove the claim that the snapshot tree is a fixed, bounded fixture set, and
either document this materialization as intentional or update the implementation
to stream both operations and add coverage.
---
Outside diff comments:
In `@docs/users-guide.md`:
- Around line 126-128: Update the documentation guidance to recommend only `from
module import name as alias` for exposing an imported name; remove the
parenthetical suggesting `import module as alias`, since that binds the module
object and changes the documented API.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2ec11472-2c99-4c23-a2d0-3f773970b04a
📒 Files selected for processing (4)
df12_python_lints/ambrleaks/cli.pydocs/developers-guide.mddocs/users-guide.mdtests/test_ambrleaks_cli.py
Resolve the Performance And Resource Use warning by removing every
whole-result materialisation from the scan path.
- discover(): yield straight from path.rglob("*.ambr") — drop the
sorted() that materialised each directory's listing. Paths now arrive
in filesystem order (documented); direct-file and recursive discovery
are unchanged.
- apply_baseline(): refactor from list->list into a lazy generator over
Iterable[Finding]. It copies the accepted mapping into a local Counter
(caller state untouched), consumes one occurrence per matching
fingerprint, and yields each survivor immediately.
- write_baseline(): new streaming filesystem boundary. It appends one
JSON-encoded fingerprint per finding as it arrives (`[`, comma-joined
entries, `]\n`), building the array in a same-directory temporary file
that is atomically replaced, so a write failure never leaves a partial
baseline and errors still reach main() as exit 2. Empty scans write
`[]`. read_baseline stays compatible with the JSON-array format.
- _run(): the --baseline branch streams _collect_findings through
apply_baseline into _print_findings, tallying scanned findings with a
small pass-through counter so scanned/suppressed logs need no list(),
len() over a materialised set, or second scan. The --write-baseline
branch delegates to write_baseline. No list(findings)/sorted() over the
full scan remains. Occurrence semantics, exit statuses, masking,
--show-values, and allowlist filtering are unchanged.
scan_file() stays a per-file list API — _collect_findings still finishes
one discovered file before the next, so scanner memory stays bounded per
file and no scanner change was needed.
Tests: prove discover() yields its first path after pulling exactly one
item from a spied walk (not after exhausting/sorting it); prove baseline
suppression yields the first survivor before reading the rest; prove
write_baseline consumes a single-use iterator into a valid JSON array
(one entry per occurrence) and writes an empty array for an empty scan.
Existing multiplicity, masking, UTF-8, JSON-error, and base_dir coverage
retained. Users' and developers' guides updated for streaming and
discovery-order baselines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- discover(): add NumPy Parameters and Yields sections documenting the paths argument and the yielded pathlib.Path values (directory inputs produce .ambr files; file inputs yield themselves). - _collect_findings(): condense the multi-line docstring to a single-line summary, in line with the other single-line private helpers. - users-guide: drop the "(or import module as alias)" alternative from the reexport-by-assignment guidance — that binds the module object, not the name, so it does not expose the re-exported name. Documentation-only change; no executable code touched. The related streaming/performance and test-coverage findings were already addressed in an earlier commit (discover no longer sorts; apply_baseline is a lazy generator; write_baseline streams incrementally; regression tests prove early yield and incremental baseline handling), and the two developers-guide punctuation/materialisation findings targeted prose that was already rewritten, so no change was needed there. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (1 error, 1 warning)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== ambrleaks CLI outline ==='
ast-grep outline df12_python_lints/ambrleaks/cli.py --items all
printf '%s\n' '=== streaming and baseline implementation ==='
rg -n -A190 -B20 \
'^def (discover|_collect_findings|apply_baseline|write_baseline|read_baseline|_run|_print_findings)\b|--write-baseline|--baseline' \
df12_python_lints/ambrleaks/cli.py
printf '%s\n' '=== full-result materialization / traversal locations ==='
rg -n -C3 --glob '*.py' \
'\bsorted\s*\(|\blist\s*\(\s*findings\b|\blist\s*\(|\.rglob\s*\(|json\.(dump|dumps)\s*\(' \
df12_python_lints/ambrleaks tests/test_ambrleaks*.py
printf '%s\n' '=== focused test outlines ==='
ast-grep outline tests/test_ambrleaks_cli.py --items all
ast-grep outline tests/test_ambrleaks_core.py --items all
ast-grep outline tests/test_ambrleaks.py --items all
printf '%s\n' '=== streaming and baseline tests ==='
rg -n -A80 -B10 --glob 'test_ambrleaks*.py' \
'(stream|lazy|discover|_collect_findings|baseline|write_baseline|apply_baseline)' \
tests
printf '%s\n' '=== quality gates ==='
rg -n -C3 '^(all|check-fmt|lint|typecheck|test):|^\.PHONY' MakefileLength of output: 70602
The baseline behaviour remains intact: duplicate fingerprints are stored once per occurrence, suppression consumes only the recorded occurrence quota, and surplus duplicates are reported. I validated this from the current source and test graph. I did not execute repository test commands because the available inspection sandbox is restricted from executing repository files. |
Replace the multi-paragraph docstring on the private helper _print_findings with a concise single-line summary; behaviour and type signature are unchanged, and no other code is touched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
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 `@df12_python_lints/ambrleaks/cli.py`:
- Around line 271-283: Shorten the docstring of the private function
_count_stream to a single-line summary describing that it yields findings while
incrementing total[0]. Remove the extended narrative paragraph and preserve the
function’s implementation unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9b0b3ac7-9eaf-4632-b09f-fbe40f08cdd5
📒 Files selected for processing (5)
df12_python_lints/ambrleaks/cli.pydocs/developers-guide.mddocs/users-guide.mdtests/test_ambrleaks_cli.pytests/test_ambrleaks_core.py
The streaming write_baseline added an error-cleanup branch (remove the temporary file and re-raise on a mid-write failure) that no test exercised, dropping coverage past the tolerance. Add a regression that feeds a source raising partway through the stream and asserts the write propagates the error while leaving neither a baseline nor a temporary file behind. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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/test_ambrleaks_core.py`:
- Around line 288-291: Update the _source function docstring to include a clear
usage-and-outcome example showing that it yields one Finding and then raises
_WriteError, while preserving the existing behavior and summary.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9326b205-0f94-4320-8105-c2eeee4b3966
📒 Files selected for processing (1)
tests/test_ambrleaks_core.py
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
- cli.py: shorten the private helper _count_stream's docstring to a single-line summary (yields findings while incrementing total[0]), dropping the narrative paragraph; implementation unchanged. - test_ambrleaks_core.py: add a usage-and-outcome Examples section to the write-failure test's _source generator, showing it yields one Finding then raises _WriteError; summary and behaviour unchanged. Documentation-only change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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/test_ambrleaks_core.py`:
- Around line 288-295: Shorten the private nested helper `_source` docstring to
a single-line summary describing that it yields one finding before failing;
remove the multi-line explanation and Examples section that duplicates the test
flow.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7042fd76-ecdc-4f32-acab-aa958e51a955
📒 Files selected for processing (2)
df12_python_lints/ambrleaks/cli.pytests/test_ambrleaks_core.py
|
@coderabbitai review |
✅ Action performedReview finished.
|
This was referenced Jul 27, 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
2 participants
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
This branch turns the generated package skeleton into the df12 pylint
plugin. It registers twelve messages encoding recurring review feedback —
prefer structural pattern matching over
isinstanceand constantdispatch, require failure messages on asserts, remove trivial wrappers
and assignment re-exports, demand explanations on suppression pragmas,
move snapshot-worthy assertions into syrupy, and adopt modern syntax
(PEP 695
typestatements; nofrom __future__ import annotationsona 3.14+ baseline) — and ships
ambrleaks,a standalone console script that scans syrupy
.ambrsnapshot files forunredacted hex strings, UUIDs, emails, phone numbers, URLs, and absolute
paths.
The checkers are verified at three tiers: example-based pytest suites,
Hypothesis property tests over whole input families, and end-to-end
tests through the pinned
leynos/pylint-pypy-shimrunner. The puredispatch kernels carry PEP 316 contracts model-checked by CrossHair
behind an opt-in
make crosshairgate. A Hypothesis property found areal detection gap during development (negative number literals parse as
UnaryOp, notConst); the fix is included with the shrunkcounterexample pinned as a regression test.
Messages
prefer-structural-pattern-matchingisinstancedispatch chains and terminating guard runsassert-missing-messageprefer-match-over-constant-chainif/elifchains comparing one subject with constantstrivial-attribute-wrapperreexport-by-assignmentlint-suppression-without-explanationnoqa/ruff/pylintpragmas with no recorded reasontypecheck-suppression-without-explanationtype: ignore/pyright/ty/mypypragmas with no reasonprefer-snapshot-assertionprefer-snapshot-substringtrivial-alias-wrapperprefer-type-statementtypestatementredundant-future-annotationsfrom __future__ import annotationson a 3.14+ baselineReview walkthrough
df12_python_lints/__init__.pyfor the plugin entry point and message inventory.
df12_python_lints/_chains.py(contract-carrying) and are consumed by
df12_python_lints/match_dispatch.pyand
df12_python_lints/constant_chain.py.df12_python_lints/wrappers.pyholds both wrapper messages with their resolution guards;
df12_python_lints/reexports.pyis the assignment re-export checker.
df12_python_lints/suppressions.pyis token-based; note the segment classification that distinguishes
pragma code lists from explanatory prose.
df12_python_lints/snapshot_asserts.pyimplements the syrupy-adoption heuristics (AST leaf counting, so
formatting cannot change whether a literal fires).
df12_python_lints/ambrleaks/subpackage is the snapshot scanner: rules with entropy gating in
rules.py,the
.ambrblock parser inscanner.py,and config/baseline suppression in
cli.py.Suppression lives entirely outside the snapshot files because syrupy
rewrites them wholesale on update.
tests/test_properties.py(Hypothesis),
tests/test_crosshair.py(opt-in symbolic checks), and
tests/test_e2e_shim.py(all ten symbols firing under the PyPy shim, with the shim ref read
from the Makefile to prevent drift).
docs/users-guide.mdand the rewritten
README.md;the verification tiering is described in
docs/developers-guide.md.Validation
make all(build, check-fmt, lint, typecheck, test, spelling): exit 0make test: 163 passed, 1 skipped (opt-in CrossHair gate)docstring coverage;
ty check: all checks passedmake crosshair: 1 passed — all four kernel postconditions confirmed over all pathspylint-pypy-shim; a clean module stays silentNotes
custom checkers.
snapshot-phoneambrleaks rule ships disabled by default becausenational number formats collide with identifiers; enable it per
project via
ambrleaks.toml.review feedback on downstream projects.
References