Skip to content

Adopt Skylos for generated dead-code detection - #43

Merged
leynos merged 2 commits into
mainfrom
vulture-adoption
Jul 24, 2026
Merged

Adopt Skylos for generated dead-code detection#43
leynos merged 2 commits into
mainfrom
vulture-adoption

Conversation

@leynos

@leynos leynos commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Summary

This branch records the proposed adoption of Skylos as the generated Python
template's dead-code detector. It closes a quality-gate gap in which plausible
but unused code, particularly residue from large language model-assisted
changes, can pass formatting, linting, typechecking and tests.

The decision keeps the default check local and deterministic, integrates it
with the existing lint interface, and requires narrow, reasoned exceptions. It
also rejects deadcode, Vulture and review-only detection, while keeping
Skylos' cloud, security, remediation and optional model-backed features outwith
the adopted scope.

Review walkthrough

  • Start with ADR-004
    for the problem statement and evaluated alternatives.
  • Continue through the decision outcome for the proposed make lint and
    guarded make skylos-allow interfaces, including the allowlist discipline.
  • Finish with the migration plan, risks and consequences to assess the
    authorized follow-up implementation and its validation boundaries.

Validation

  • markdownlint-cli2 docs/adr-004-adopt-skylos-for-dead-code-detection.md:
    passed with no errors.
  • make spelling: passed.
  • git diff --check: passed.
  • en-GB Oxford English review: passed.

Notes

This draft PR records a proposed architecture decision only. It does not yet
change the generated dependency set, Makefile targets, template guidance or
parent user documentation; those changes remain in the ADR's migration plan.

Record the rationale for adding Skylos to the generated Python lint gate.
Define a guarded, reason-required allowlist workflow and the documentation,
contract-test, and rollout work needed for adoption.

@sourcery-ai sourcery-ai 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.

Sorry @leynos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Changes

Skylos adoption

Layer / File(s) Summary
Decision context
docs/adr-004-adopt-skylos-for-dead-code-detection.md
Define the dead-code detection problem, decision drivers, and comparisons with alternative approaches.
Operating policy
docs/adr-004-adopt-skylos-for-dead-code-detection.md
Specify local Skylos linting, guarded skylos-allow usage, allowlist rules, goals, and non-goals.
Migration and consequences
docs/adr-004-adopt-skylos-for-dead-code-detection.md
Outline planned template, test, and documentation updates, plus known limitations and maintenance consequences.

Poem

Skylos scans where stray code hides,
A guarded gate now walks beside.
Allow with reason, precise and clear,
Keep cloud and auto-delete out of here.
Local checks make dead code known.

🚥 Pre-merge checks | ✅ 19 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Developer Documentation ⚠️ Warning FAIL: the ADR date is 2026-07-24 (tomorrow), and the branch adds no developer-guide update for the new Skylos workflow. Set the ADR date to the actual approval date and add the Skylos workflow notes to docs/developers-guide.md when the implementation lands.
✅ Passed checks (19 passed)
Check name Status Explanation
Title check ✅ Passed Match the title to the ADR’s Skylos dead-code detection adoption.
Description check ✅ Passed Keep the description aligned with the ADR by describing the Skylos adoption proposal and its scoped workflow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Testing (Overall) ✅ Passed Pass: the patch is documentation-only; no executable behaviour or tests were changed, so the testing guard does not apply.
User-Facing Documentation ✅ Passed Only an ADR changed; no user-facing functionality or behaviour was shipped, and the ADR defers users-guide updates to later migration work.
Module-Level Documentation ✅ Passed Only a docs ADR changed; no module files were added or edited, so the module docstring rule is not applicable to this PR.
Testing (Unit And Behavioural) ✅ Passed Treat this as PASS: the PR only adds an ADR and defers implementation/tests, so no behavioural boundary changes exist yet.
Testing (Property / Proof) ✅ Passed Treat this ADR as documentation-only: do not require property tests or proofs, because only the ADR changed and no executable invariant was added.
Testing (Compile-Time / Ui) ✅ Passed ADR-only markdown change; no Rust/TS compile-time path or UI output was added, so no trybuild/snapshot test is required.
Unit Architecture ✅ Passed Only a new ADR was added; no executable code, dependencies, or side-effects changed, so unit-architecture boundaries are untouched.
Domain Architecture ✅ Passed PASS: The PR only adds an ADR document; no domain, adapter, or infrastructure code changes are present, so the domain-boundary check is not applicable.
Observability ✅ Passed PASS: Limit the scope to a documentation-only ADR; no runtime behaviour, logging, metrics, tracing, or alerts changed.
Security And Privacy ✅ Passed Only docs/adr-004-adopt-skylos-for-dead-code-detection.md changed, and it explicitly keeps Skylos local, deterministic, and free of credentials, network, cloud, or LLM features.
Performance And Resource Use ✅ Passed PASS: The PR is ADR-only; no executable/template code changed, and the only performance note is a future migration-plan timing check.
Concurrency And State ✅ Passed Only an ADR document changed; it introduces no shared mutable state, async work, locks, or ordering-sensitive implementation.
Architectural Complexity And Maintainability ✅ Passed Treat this as documentation-only: the ADR proposes an explicit, bounded lint workflow and introduces no new code abstraction in the PR.
Rust Compiler Lint Integrity ✅ Passed Only a Markdown ADR changed; no Rust sources, lint suppressions, or clone sites were touched.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch vulture-adoption

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

@leynos
leynos marked this pull request as ready for review July 23, 2026 23:43

@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.

Actionable comments posted: 3

🤖 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 `@docs/adr-004-adopt-skylos-for-dead-code-detection.md`:
- Around line 7-9: Update the Date value in ADR-004 to 2026-07-23, unless the
actual approval date is different; preserve the existing date format.
- Around line 58-62: Update the deadcode rejection rationale in the ADR to
include an explicit as-of date and define the maintenance criterion used to
judge its activity as too weak. Retain the existing release information, but
connect it to that dated criterion rather than relying on release age alone.
- Around line 94-105: Update the `make skylos-allow` specification to require
Skylos’ exact `whitelist` interface: invoke `skylos whitelist --reason ...` for
one NAME/pattern and reason, and persist entries under `[tool.skylos.whitelist]`
in `pyproject.toml`. Keep the wrapper’s required-value validation and ensure
contract tests target this exact CLI and configuration surface without report
scraping or baseline generation.
🪄 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: 106a3754-43c1-4b8b-8a8d-ce3f0e2684fc

📥 Commits

Reviewing files that changed from the base of the PR and between 58da3b5 and 0a4dd6c.

📒 Files selected for processing (1)
  • docs/adr-004-adopt-skylos-for-dead-code-detection.md
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • leynos/whitaker (auto-detected)
  • leynos/cuprum (auto-detected)
  • leynos/mdtablefix (auto-detected)

Comment thread docs/adr-004-adopt-skylos-for-dead-code-detection.md
Comment thread docs/adr-004-adopt-skylos-for-dead-code-detection.md
Comment thread docs/adr-004-adopt-skylos-for-dead-code-detection.md
Require the generated wrapper to invoke Skylos' reasoned whitelist command
for one name and persist the result in the documented `pyproject.toml`
configuration surface. Make the corresponding contract-test expectations
explicit.
@leynos
leynos merged commit 9b1c470 into main Jul 24, 2026
6 checks passed
@leynos
leynos deleted the vulture-adoption branch July 24, 2026 00:56
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