Skip to content

lint: ruff cleanup (82 pre-existing violations) + wire ruff into CI as required check #33

Description

@scottconverse

Why this issue exists

The v1.3.0 release directive expected ruff: 0 violations as part of the 3d Verification Log. The actual tree state at v1.3.0 cut (2026-04-25) was:

  • docker compose exec -T api ruff check . → exit 1
  • 82 errors (72 auto-fixable with --fix, 1 hidden fix available with --unsafe-fixes)
  • Pre-existing condition — none of the 82 violations were introduced by the v1.3.0 source bumps (which were pure version-string edits across 20 files)
  • CI does not run ruff anywhere in .github/workflows/ci.yml — the only jobs are backend pytest, T2C bootstrap-failure smoke, and frontend vitest+build. So the 82 errors accumulated silently because the merge bar never enforced ruff.

Path B ruling (Scott, 2026-04-25 conversation): ship v1.3.0 source-only without bundling the ruff cleanup, file a follow-up issue, document the divergence honestly in the v1.3.0 Verification Log. Reasoning: release scope is mechanics, not lint cleanup; CI doesn't gate on ruff anyway; bundling 82 lint fixes into a release-mechanics PR would be exactly the scope-creep Scott has repeatedly warned against.

Two-part scope (BOTH parts required to close this issue)

Part 1 — Fix the 82 violations

  1. Run docker compose exec -T api ruff check . --fix → expect ~72 auto-fixed
  2. Re-run docker compose exec -T api ruff check . → expect ~10 remaining violations (the unsafe-fixable subset)
  3. Manually address each remaining violation (don't auto-apply --unsafe-fixes — review each)
  4. Verify clean: docker compose exec -T api ruff check . → exit 0, "All checks passed!"
  5. Ensure the fixes don't regress any other tests: docker compose run --rm api python -m pytest tests -q → still all pass

Part 2 — Wire ruff into CI as a required check (FORCING FUNCTION)

Without this part, the next release will surface the same divergence. Lint cleanup with no enforcement is a temporary state, not a fix.

  1. Add a ruff job (or a step inside the existing backend job) to .github/workflows/ci.yml. Suggested placement: a separate fast job that runs in parallel with the backend pytest job, so it doesn't block the long-pole pytest run.
  2. The job should run docker compose run --rm --no-deps api ruff check . (no live DB needed for lint; matches the --no-deps pattern already used in CI).
  3. Job MUST exit non-zero on any violation. Standard ruff exit code semantics handle this naturally.
  4. Make it a required check in the master branch protection settings (Settings → Branches → master). Without "required" status, the job runs but doesn't block merges.
  5. Add a brief note in CONTRIBUTING.md instructing contributors to run ruff check . locally before pushing.

Recommended timing

  • Not blocking any release. v1.3.0 ships with 82 ruff violations documented in the 3d Verification Log.
  • Should land before: the next records-ai PR that touches Python code, so the new code doesn't accumulate additional violations on top of the existing 82.
  • Could land before: Phase 2 work begins (clean lint baseline for the next extraction).

Severity

Medium. Lint violations are not runtime bugs, but they:

  • Indicate accumulated technical debt
  • Hide real problems behind noise (when ruff has 82 errors, nobody reads the output, so a real new violation goes unnoticed)
  • Block compliance with the release directive's standard 3d gate

The forcing-function part (Part 2) is what elevates this above "low" — without CI enforcement, the violations will recur even after a one-time cleanup.

Refs

  • v1.3.0 PR: #32
  • Path B ruling (Scott, 2026-04-25 conversation): ship source-only, file follow-up
  • v1.3.0 Verification Log entry (will appear in PR release: v1.3.0 (Phase 1 CivicCore extraction) #32 / release notes): "ruff: 82 violations, pre-existing, not gated by CI, deferred to this issue"
  • Sister deferral: #31 (binary doc regen — same Path B pattern)
  • feedback_own_preexisting_bugs.md (Scott's "never dismiss pre-existing" rule — this issue exists so the obligation isn't dismissed)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions