aws-samples / aws-samples/sample-agent-cost-bench

black / ruff / mypy all fail on main; propose a staged cleanup + CI enforcement

Open
#7 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
74
Forks
19
Avg merge
30m
Merged PRs (30d)
2

Description

## Summary

The repo configures `black`, `ruff` and `mypy` in `pyproject.toml` (line-length 100,
`select = ["E","F","I","N","W"]`, `python_version = "3.10"`), and the README documents

```
black . && ruff check . && mypy agent_cost_bench
```

as the format/lint/type-check step — but all three currently fail on `main`:

| Tool | State on `main` |
|---|---|
| `black --check` | 36 files would be reformatted (13 clean) |
| `ruff check` | 69 errors |
| `mypy agent_cost_bench` | 38 errors in 8 files |

Because nothing enforces this, the drift grows quietly and every contributor sees a wall of
pre-existing failures when they run the documented command — which makes it hard to tell
whether *their* change introduced anything.

## Breakdown

`ruff` (69):

| Rule | Count |
|---|---|
| `E501` line too long | 53 |
| `F401` unused import | 7 |
| `I001` unsorted imports | 5 |
| `N806` non-lowercase variable in function | 2 |
| `F811` redefinition of unused name | 1 |
| `E402` import not at top of file | 1 |

A representative `mypy` sample:

```
agent_cost_bench/runner.py:403: error: Incompatible types in assignment
(expression has type "SpecDrivenExecutor", variable has type "VibeExecutor")
agent_cost_bench/cli.py:655: error: Need type annotation for "verify_files"
```

The `runner.py:403` one is arguably a real latent issue rather than pure noise —
`VibeExecutor` and `SpecDrivenExecutor` are assigned to the same variable without a common
annotated base type.

## Suggested path

Splitting this up so no single PR is unreviewable:

1. **Safe autofixes** — done in #6 (F401 / I001 / F811 / E402; 9 files, no behaviour change).
2. **Formatting pass** — run `black` across the repo in one dedicated commit. This fixes 45
of the 53 `E501`s but touches ~1,540 lines in 36 files, so it wants to land on its own,
ideally when little else is in flight. Worth adding to `.git-blame-ignore-revs` so it
doesn't pollute `git blame`.
3. **Residual `E501`** — the ~8 lines `black` cannot split (long string literals / URLs in
comments), wrapped by hand.
4. **`mypy`** — separately, and probably incrementally. `strict = false` today, so this is a
judgement call about how far to take it.
5. **CI** — once green, run the three tools in a GitHub Action on PRs so it stays that way.
Without this step the drift simply returns.

Happy to send PRs for any of 2–4 if that's useful — I held off on the formatting pass
specifically because CONTRIBUTING asks contributors not to reformat the codebase alongside a
functional change, and a repo-wide reformat felt like the maintainers' call rather than
something to arrive unannounced in someone else's PR.

## Context

Noticed while preparing #4 and #5 (two Codex cost-accounting fixes). Both branches keep the
lint numbers from getting worse — #4 actually reduces ruff 69 → 67 — but neither can make the
repo green without dragging in a reformat.

Contributor guide

Open the contributing guide

Research direction

Start with pyproject.toml and the documented command in README, then run black --check, ruff check, and mypy agent_cost_bench to reproduce the reported failures. Review the staged cleanup plan, including the formatting changes, residual E501 cases, and mypy decisions. Done means the selected cleanup stages are complete and a GitHub Action enforces the three checks on pull requests.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, python
Domain
ci-cd, developer-experience, tooling
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.