Skip to content

[FEATURE] Make the Ruff lint and format checks actually gate CI, and clear the backlog behind them #272

Description

@Nitjsefnie

Is your feature request related to a problem?

Yes. The two Ruff steps in CI cannot fail the build, so nothing enforces them:

# .github/workflows/ci.yml:45-51
      - name: Run Ruff linter
        run: uv run ruff check . || true
        continue-on-error: true

      - name: Run Ruff formatter check
        run: uv run ruff format --check . || true
        continue-on-error: true

Both the shell-level || true and the step-level continue-on-error: true are present; either one alone is enough to swallow the exit code.

This looks deliberate rather than a slip, and I want to say so plainly: the MyPy step immediately below carries an explicit comment about the same construct (ci.yml:55-56"A missing target is a config error, not a finding: surface it instead of letting || true report success"), and the same shape is used for integration tests and coverage. So this is a question about intent and the debt that has accumulated behind it, not a report that something is broken.

Measured on main at 2ad932b, using the Ruff version uv.lock pins (0.15.22):

  • uv run ruff check .Found 228 errors, across 107 files (104 .py plus 3 notebooks under examples/)
  • uv run ruff format --check .122 files would be reformatted, 153 files already formatted

Most frequent rule codes: TC003 28, UP006 13, E402 11, UP042 11, UP017 10, W293 9, B904 7. ([tool.ruff.lint] selects TCH; Ruff 0.15.x reports those findings under the renamed TC codes — same rules.)

The practical cost falls on contributors. CONTRIBUTING.md:84-96 tells them to run exactly these two commands, and both are red at HEAD, so someone new cannot separate their own violations from the pre-existing ones.

Describe the Solution You'd Like

Burn the backlog down, then remove || true and continue-on-error: true from the two Ruff steps so they actually gate.

Describe Alternatives You've Considered

  • Leave as-is and document it. Cheapest — the steps stay informational. But the count keeps growing and CONTRIBUTING.md keeps pointing contributors at a command that is red before they touch anything.
  • Gate formatting only, first. ruff format is mechanical and reviewable at a glance (122 files, no judgement calls), so it could be enforced well before ruff check is worked down.
  • Ratchet instead of clearing. Baseline the current findings and fail only on new ones. Avoids a bulk diff entirely, at the cost of carrying the existing 228 indefinitely.

Use Case

The end state, for the linter step:

      - name: Run Ruff linter
        run: uv run ruff check .

      - name: Run Ruff formatter check
        run: uv run ruff format --check .

so that a PR introducing a new lint violation goes red instead of merging green.

Additional Context

Surfaced while working on #270, where an F401 in a new test file shipped green because of this — that one is fixed, but it prompted checking whether the gate could have caught it, and it could not.

Related: #250 (the MyPy step targeting a non-existent src/) was the same family — a gate that silently was not gating. The test -d openagent_eval guard added there is what makes the intent behind || true legible now.

If you would like the cleanup, I would suggest several reviewable PRs split by rule family rather than one 107-file diff, and the formatter pass kept separate from the linter pass since the two gates are independent.

Would you be willing to contribute this feature?

  • Yes, I'd like to implement this feature
  • I can help with testing
  • No, I'm just suggesting

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