Spaces:
Sleeping
Sleeping
HatmanStack commited on
Commit ·
fd53f23
1
Parent(s): ec807bc
ci(deps): consolidate to pyproject.toml, remove requirements files
Browse files- .github/workflows/ci.yml +5 -5
- docs/plans/2026-03-25-audit-streamlit-nba/Phase-4.md +206 -0
- requirements-dev.txt +0 -6
- requirements.txt +0 -5
.github/workflows/ci.yml
CHANGED
|
@@ -17,15 +17,15 @@ jobs:
|
|
| 17 |
uses: actions/setup-python@v6
|
| 18 |
with:
|
| 19 |
python-version: "3.11"
|
| 20 |
-
|
|
|
|
|
|
|
| 21 |
|
| 22 |
- name: Install dependencies
|
| 23 |
-
run:
|
| 24 |
-
python -m pip install --upgrade pip
|
| 25 |
-
pip install -r requirements-dev.txt
|
| 26 |
|
| 27 |
- name: Run tests
|
| 28 |
-
run: pytest --cov=src --cov-report=term-missing
|
| 29 |
|
| 30 |
- name: Run ruff
|
| 31 |
run: ruff check src/ tests/
|
|
|
|
| 17 |
uses: actions/setup-python@v6
|
| 18 |
with:
|
| 19 |
python-version: "3.11"
|
| 20 |
+
|
| 21 |
+
- name: Install uv
|
| 22 |
+
run: pip install uv
|
| 23 |
|
| 24 |
- name: Install dependencies
|
| 25 |
+
run: uv pip install -e ".[dev]" --system
|
|
|
|
|
|
|
| 26 |
|
| 27 |
- name: Run tests
|
| 28 |
+
run: pytest --cov=src --cov-report=term-missing --cov-fail-under=70
|
| 29 |
|
| 30 |
- name: Run ruff
|
| 31 |
run: ruff check src/ tests/
|
docs/plans/2026-03-25-audit-streamlit-nba/Phase-4.md
ADDED
|
@@ -0,0 +1,206 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
# Phase 4: [FORTIFIER] Guardrails
|
| 2 |
+
|
| 3 |
+
## Phase Goal
|
| 4 |
+
|
| 5 |
+
Add CI hardening, pre-commit hooks, dependency consolidation, and type rigor improvements. These are additive guardrails that prevent regression.
|
| 6 |
+
|
| 7 |
+
**Success criteria:** Pre-commit hooks run ruff and mypy. Dependencies consolidated to `pyproject.toml` only. CI uses `uv`. Type annotations tightened (no unnecessary `Any`). Coverage enforcement in CI.
|
| 8 |
+
|
| 9 |
+
**Estimated tokens:** ~20k
|
| 10 |
+
|
| 11 |
+
## Prerequisites
|
| 12 |
+
|
| 13 |
+
- Phase 3 complete (tests passing at 70% coverage)
|
| 14 |
+
- All lint and type checks passing
|
| 15 |
+
|
| 16 |
+
## Tasks
|
| 17 |
+
|
| 18 |
+
### Task 1: Consolidate dependencies to pyproject.toml
|
| 19 |
+
|
| 20 |
+
**Goal:** Remove `requirements.txt` and `requirements-dev.txt` as duplicate dependency sources. Update CI to install from `pyproject.toml`. Addresses health audit finding #14 (MEDIUM) and ADR-5.
|
| 21 |
+
|
| 22 |
+
**Files to Modify:**
|
| 23 |
+
- `requirements.txt` - Delete this file
|
| 24 |
+
- `requirements-dev.txt` - Delete this file (after verifying its contents match `[project.optional-dependencies] dev`)
|
| 25 |
+
- `.github/workflows/ci.yml` - Update install step to use `pyproject.toml`
|
| 26 |
+
|
| 27 |
+
**Prerequisites:** None
|
| 28 |
+
|
| 29 |
+
**Implementation Steps:**
|
| 30 |
+
- Read `requirements-dev.txt` to verify it matches or is a subset of `pyproject.toml` `[project.optional-dependencies] dev`.
|
| 31 |
+
- Delete `requirements.txt`.
|
| 32 |
+
- Delete `requirements-dev.txt` (if it matches pyproject.toml dev deps; if it has extra deps, add them to pyproject.toml first).
|
| 33 |
+
- Update `.github/workflows/ci.yml`:
|
| 34 |
+
- Replace `pip install -r requirements-dev.txt` with `pip install -e ".[dev]"`.
|
| 35 |
+
- Optionally switch to `uv` in CI for faster installs:
|
| 36 |
+
```yaml
|
| 37 |
+
- name: Install uv
|
| 38 |
+
run: pip install uv
|
| 39 |
+
- name: Install dependencies
|
| 40 |
+
run: uv pip install -e ".[dev]" --system
|
| 41 |
+
```
|
| 42 |
+
- Update CI to add `--cov-fail-under=70` to the pytest command if not already there.
|
| 43 |
+
|
| 44 |
+
**Verification Checklist:**
|
| 45 |
+
- [x] `requirements.txt` deleted
|
| 46 |
+
- [x] `requirements-dev.txt` deleted
|
| 47 |
+
- [x] CI workflow installs from `pyproject.toml`
|
| 48 |
+
- [x] CI workflow still runs tests, ruff, and mypy successfully
|
| 49 |
+
|
| 50 |
+
**Testing Instructions:** Push to a branch and verify CI passes, or run the install and test commands locally.
|
| 51 |
+
|
| 52 |
+
**Commit Message Template:**
|
| 53 |
+
```text
|
| 54 |
+
ci(deps): consolidate to pyproject.toml, remove requirements files
|
| 55 |
+
```
|
| 56 |
+
|
| 57 |
+
---
|
| 58 |
+
|
| 59 |
+
### Task 2: Add pre-commit hooks
|
| 60 |
+
|
| 61 |
+
**Goal:** Add `.pre-commit-config.yaml` with ruff and mypy hooks to catch issues before commit. Addresses eval git-hygiene remediation target.
|
| 62 |
+
|
| 63 |
+
**Files to Modify:**
|
| 64 |
+
- `.pre-commit-config.yaml` - Create new file
|
| 65 |
+
|
| 66 |
+
**Prerequisites:** Task 1
|
| 67 |
+
|
| 68 |
+
**Implementation Steps:**
|
| 69 |
+
- Create `.pre-commit-config.yaml` at the project root with:
|
| 70 |
+
```yaml
|
| 71 |
+
repos:
|
| 72 |
+
- repo: https://github.com/astral-sh/ruff-pre-commit
|
| 73 |
+
rev: v0.8.0 # Use latest stable version
|
| 74 |
+
hooks:
|
| 75 |
+
- id: ruff
|
| 76 |
+
args: [--fix]
|
| 77 |
+
- id: ruff-format
|
| 78 |
+
- repo: https://github.com/pre-commit/mirrors-mypy
|
| 79 |
+
rev: v1.13.0 # Use latest stable version
|
| 80 |
+
hooks:
|
| 81 |
+
- id: mypy
|
| 82 |
+
additional_dependencies: [pandas-stubs]
|
| 83 |
+
args: [--config-file=pyproject.toml]
|
| 84 |
+
pass_filenames: false
|
| 85 |
+
entry: mypy src/
|
| 86 |
+
```
|
| 87 |
+
- Add `pre-commit` to the dev dependencies in `pyproject.toml`:
|
| 88 |
+
```
|
| 89 |
+
"pre-commit>=3.0.0",
|
| 90 |
+
```
|
| 91 |
+
- Verify the hooks work: `uvx pre-commit run --all-files`.
|
| 92 |
+
- Note: Use the latest stable versions of ruff and mypy that are compatible. Check PyPI for current versions.
|
| 93 |
+
|
| 94 |
+
**Verification Checklist:**
|
| 95 |
+
- [ ] `.pre-commit-config.yaml` exists at project root
|
| 96 |
+
- [ ] `pre-commit` is in dev dependencies
|
| 97 |
+
- [ ] `uvx pre-commit run --all-files` passes
|
| 98 |
+
|
| 99 |
+
**Testing Instructions:** Run `uvx pre-commit run --all-files` and verify all hooks pass.
|
| 100 |
+
|
| 101 |
+
**Commit Message Template:**
|
| 102 |
+
```text
|
| 103 |
+
ci(hooks): add pre-commit config with ruff and mypy hooks
|
| 104 |
+
```
|
| 105 |
+
|
| 106 |
+
---
|
| 107 |
+
|
| 108 |
+
### Task 3: Tighten type annotations
|
| 109 |
+
|
| 110 |
+
**Goal:** Replace imprecise type annotations (`Any`, `tuple[Any, ...]`, `list[list]`) with specific types. Addresses eval type-rigor score of 7/10.
|
| 111 |
+
|
| 112 |
+
**Files to Modify:**
|
| 113 |
+
- `src/database/queries.py` - Fix return types
|
| 114 |
+
- `scripts/compile_model.py` - Fix type hints
|
| 115 |
+
|
| 116 |
+
**Prerequisites:** Phase 1 (dead code removed, so fewer files to fix)
|
| 117 |
+
|
| 118 |
+
**Implementation Steps:**
|
| 119 |
+
- In `src/database/queries.py`:
|
| 120 |
+
- Find `tuple[Any, ...]` return types (around line 36) and replace with specific types. If the function returns player data columns, use a `TypedDict` or a specific tuple type like `tuple[str, str, float, ...]` matching the actual columns returned.
|
| 121 |
+
- Find `list[tuple[str]]` return types (around line 14) and simplify to `list[str]` if the function returns a flat list of strings.
|
| 122 |
+
- Remove `Any` import if no longer needed.
|
| 123 |
+
- In `scripts/compile_model.py`:
|
| 124 |
+
- Change `list[list]` to `list[list[float]]` or more specific parameterized types (around line 85).
|
| 125 |
+
- Review `src/models/player.py` for any remaining `Any` imports that are no longer needed after `PlayerStats` removal.
|
| 126 |
+
|
| 127 |
+
**Verification Checklist:**
|
| 128 |
+
- [ ] No `tuple[Any, ...]` in `queries.py`
|
| 129 |
+
- [ ] No bare `list[list]` in `compile_model.py`
|
| 130 |
+
- [ ] `mypy src/` passes
|
| 131 |
+
- [ ] `pytest` passes
|
| 132 |
+
|
| 133 |
+
**Testing Instructions:** Run `mypy src/` and verify no new errors. Run existing tests.
|
| 134 |
+
|
| 135 |
+
**Commit Message Template:**
|
| 136 |
+
```text
|
| 137 |
+
refactor(types): tighten type annotations, remove unnecessary Any usage
|
| 138 |
+
```
|
| 139 |
+
|
| 140 |
+
---
|
| 141 |
+
|
| 142 |
+
### Task 4: Add coverage enforcement to CI
|
| 143 |
+
|
| 144 |
+
**Goal:** Ensure CI fails if coverage drops below the threshold. Addresses eval reproducibility concerns.
|
| 145 |
+
|
| 146 |
+
**Files to Modify:**
|
| 147 |
+
- `.github/workflows/ci.yml` - Add `--cov-fail-under=70` to pytest command
|
| 148 |
+
|
| 149 |
+
**Prerequisites:** Phase 3 Task 5 (coverage threshold set)
|
| 150 |
+
|
| 151 |
+
**Implementation Steps:**
|
| 152 |
+
- In `.github/workflows/ci.yml`, update the pytest command:
|
| 153 |
+
```yaml
|
| 154 |
+
- name: Run tests
|
| 155 |
+
run: pytest --cov=src --cov-report=term-missing --cov-fail-under=70
|
| 156 |
+
```
|
| 157 |
+
- This ensures CI fails if coverage drops, not just locally.
|
| 158 |
+
|
| 159 |
+
**Verification Checklist:**
|
| 160 |
+
- [ ] CI pytest command includes `--cov-fail-under=70`
|
| 161 |
+
|
| 162 |
+
**Testing Instructions:** Verify the CI workflow file has the correct command.
|
| 163 |
+
|
| 164 |
+
**Commit Message Template:**
|
| 165 |
+
```text
|
| 166 |
+
ci(coverage): enforce 70% coverage threshold in CI
|
| 167 |
+
```
|
| 168 |
+
|
| 169 |
+
---
|
| 170 |
+
|
| 171 |
+
### Task 5: Clean up ruff ignores
|
| 172 |
+
|
| 173 |
+
**Goal:** Remove ruff ignore rules that are no longer relevant after Phase 1 cleanup. Addresses tech debt in linter config.
|
| 174 |
+
|
| 175 |
+
**Files to Modify:**
|
| 176 |
+
- `pyproject.toml` - Update ruff ignore list
|
| 177 |
+
|
| 178 |
+
**Prerequisites:** Phase 1 (SQL injection code removed), Phase 2 (error handling fixed)
|
| 179 |
+
|
| 180 |
+
**Implementation Steps:**
|
| 181 |
+
- Review the ruff `ignore` list in `pyproject.toml`:
|
| 182 |
+
- `S608` (SQL injection false positive): Remove this. SQL injection code was deleted in Phase 1.
|
| 183 |
+
- `S110` (try-except-pass): Review if still needed after the `finally: pass` removal. If no `except: pass` patterns remain, remove it.
|
| 184 |
+
- Keep the other ignores that are still relevant (`S101`, `PLR0913`, `SIM105`, `PLR2004`, `S311`, `E501`).
|
| 185 |
+
- Run `ruff check src/ tests/` to verify no new violations surface.
|
| 186 |
+
|
| 187 |
+
**Verification Checklist:**
|
| 188 |
+
- [ ] `S608` removed from ruff ignores
|
| 189 |
+
- [ ] `S110` removed if no longer needed
|
| 190 |
+
- [ ] `ruff check src/ tests/` passes with the updated config
|
| 191 |
+
|
| 192 |
+
**Testing Instructions:** `ruff check src/ tests/`
|
| 193 |
+
|
| 194 |
+
**Commit Message Template:**
|
| 195 |
+
```text
|
| 196 |
+
chore(config): remove obsolete ruff ignore rules
|
| 197 |
+
```
|
| 198 |
+
|
| 199 |
+
## Phase Verification
|
| 200 |
+
|
| 201 |
+
1. Run `pytest --cov=src --cov-report=term-missing --cov-fail-under=70` and confirm pass.
|
| 202 |
+
2. Run `ruff check src/ tests/` with no errors.
|
| 203 |
+
3. Run `mypy src/` with no errors.
|
| 204 |
+
4. Run `uvx pre-commit run --all-files` and confirm all hooks pass.
|
| 205 |
+
5. Verify `requirements.txt` and `requirements-dev.txt` no longer exist.
|
| 206 |
+
6. Verify `.pre-commit-config.yaml` exists.
|
requirements-dev.txt
DELETED
|
@@ -1,6 +0,0 @@
|
|
| 1 |
-
-r requirements.txt
|
| 2 |
-
pytest>=7.4.0
|
| 3 |
-
pytest-cov>=4.1.0
|
| 4 |
-
mypy>=1.7.0
|
| 5 |
-
ruff>=0.1.6
|
| 6 |
-
pandas-stubs>=2.0.0
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
requirements.txt
DELETED
|
@@ -1,5 +0,0 @@
|
|
| 1 |
-
streamlit>=1.28.0
|
| 2 |
-
tensorflow>=2.15.0
|
| 3 |
-
numpy>=1.24.0
|
| 4 |
-
pandas>=2.0.0
|
| 5 |
-
pydantic>=2.5.0
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|