[48849b22] Identify and fix the failing make quality step on roboco-api master (#260) (#261) (#262)

This commit is contained in:
Renzo F
2026-06-25 17:17:46 +02:00
committed by GitHub
parent 9702955f0c
commit 99cf56dff3
3 changed files with 49 additions and 2 deletions
+4
View File
@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
## [Unreleased] ## [Unreleased]
### Fixed
- **Mypy [unreachable] error in test_pr_gate_records_verdict resolved.** A test assigned `t.notes_structured = None` in the function body, causing mypy to narrow the attribute type to `None`. Since the test's helper function took the object as `Any`, mypy did not reset its narrowing after the call, treating `assert t.notes_structured is not None` as statically always-False and marking the next line as `[unreachable]`, failing the quality gate. Fixed by introducing `_TaskWithNoNotes` — a helper class that declares `notes_structured: dict[str, Any] | None = None` in `__init__` — so mypy uses the declared union type rather than a narrowed literal. All tests pass with no suppressions. This pattern is documented in the testing standards for future reference.
## [0.11.1] - 2026-06-25 ## [0.11.1] - 2026-06-25
### Fixed ### Fixed
+32
View File
@@ -73,6 +73,38 @@ pnpm lint
pnpm typecheck pnpm typecheck
``` ```
## Type Narrowing and mypy
When you assign `None` to an attribute inside a test function, mypy narrows that attribute's type to `None`, and does **not** invalidate this narrowing after a function call — even when the called function takes the object as `Any` or modifies it.
**Problem:** This causes mypy to treat subsequent assertions as unreachable, failing the quality gate.
```python
# ❌ BAD: mypy narrows notes to None and treats the assertion as unreachable
def test_example() -> None:
t = _Task()
t.notes = None # mypy narrows type to None
process(t) # Even though process may write to t.notes
assert t.notes is not None # [unreachable] — mypy sees this as always False
```
**Solution:** Use a helper class whose `__init__` declares the attribute with its full union type, so mypy uses the declared type (not a narrowed literal) when accessed in your test:
```python
# ✅ GOOD: Annotation-typed class preserves union type
class _TaskWithNoNotes:
"""Variant where notes starts as None (no prior state)."""
def __init__(self) -> None:
self.id = uuid4()
self.notes: dict[str, Any] | None = None # Declared as union, not narrowed
def test_example() -> None:
t = _TaskWithNoNotes() # Use the helper instead
process(t)
assert t.notes is not None # ✅ Reachable — mypy sees the union type
```
## Quality Gates ## Quality Gates
All tests MUST pass before: All tests MUST pass before:
@@ -45,6 +45,15 @@ class _Task:
self.pr_reviewer_notes = "stale" self.pr_reviewer_notes = "stale"
class _TaskWithNoNotes:
"""Variant where notes_structured starts as None (no prior verdict history)."""
def __init__(self) -> None:
self.id = uuid4()
self.notes_structured: dict[str, Any] | None = None
self.pr_reviewer_notes: str = ""
def test_pr_fail_overwrites_stale_passed_verdict() -> None: def test_pr_fail_overwrites_stale_passed_verdict() -> None:
c = _make_choreographer() c = _make_choreographer()
t = _Task() t = _Task()
@@ -60,8 +69,10 @@ def test_pr_fail_overwrites_stale_passed_verdict() -> None:
def test_pr_pass_records_passed_verdict() -> None: def test_pr_pass_records_passed_verdict() -> None:
c = _make_choreographer() c = _make_choreographer()
t = _Task() # Use the None-initial variant so mypy sees the broader declared type
t.notes_structured = None # (dict[str, Any] | None) rather than a narrowed None from an in-body
# assignment, which would make the post-call assertions look unreachable.
t = _TaskWithNoNotes()
c._record_gate_verdict( c._record_gate_verdict(
t, "pr_pass", "Assembled root scope is clean; every criterion is covered." t, "pr_pass", "Assembled root scope is clean; every criterion is covered."
) )