mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
[chore] mypy tests/: clear all 15 pre-existing type errors so make quality can go green
The branch tip had 15 mypy tests/ errors in files this bundle did not author,
which blocked CI's make quality mypy step (mypy roboco/ tests/) regardless of
the bundle's own commits. Pre-existing is still existing — fix every one:
- test_schemas_v1_flow.py (8): the StrList coercion tests intentionally pass
SDK-nested list-of-strings input ([[['...']]], {'item':{'$text':'...'}}, int,
dict). Annotate those literals as list[Any] locals so mypy accepts the
coerce-able shape; the StrList BeforeValidator still flattens to list[str] at
runtime. No type:ignore.
- test_pr_gate_records_verdict.py (3): notes_structured is dict|None; narrow
with 'assert t.notes_structured is not None' before indexing (the existing
pattern at line 90).
- test_pr_review_hand_format_guard.py (1 site, 2 errors): the _verb_runner()
spy assertion — use the cc: Any = c alias idiom so assert_not_awaited
resolves; drops the now-unused type:ignore[union-attr].
- test_pr_gate_notifies_pm.py (1): drop the unused type:ignore[method-assign]
on the a2a.send reassignment.
- test_content_models.py (1): narrow coerced with isinstance(coerced,
PrReviewContent) before reading .issues (the base _Content lacks the field).
Gates: rm -rf .mypy_cache && mypy roboco/ tests/ = Success (855 files);
ruff check + format clean; 5 affected suites = 40 passed.
This commit is contained in:
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from typing import Any
|
||||||
from uuid import uuid4
|
from uuid import uuid4
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
@@ -67,6 +68,14 @@ def test_i_will_plan_request_flattens_sdk_nested_technical_considerations() -> N
|
|||||||
valid string``. The ``StrList`` BeforeValidator must flatten it to a flat
|
valid string``. The ``StrList`` BeforeValidator must flatten it to a flat
|
||||||
``list[str]`` so the verb body receives clean strings.
|
``list[str]`` so the verb body receives clean strings.
|
||||||
"""
|
"""
|
||||||
|
# The SDK nests list-of-strings tool input as nested arrays / dict-wrapped
|
||||||
|
# text (``[[["…"]]]``, ``{"item": {"$text": "…"}}``). Annotated ``list[Any]``
|
||||||
|
# so mypy accepts the coerce-able shape; the ``StrList`` BeforeValidator
|
||||||
|
# flattens it to ``list[str]`` at runtime (no ``type: ignore`` owed).
|
||||||
|
technical_considerations: list[Any] = [
|
||||||
|
[[["Empty state distinct from loaded state, coverage target 80%"]]],
|
||||||
|
[{"item": {"$text": "Use asyncpg prepared statements"}}],
|
||||||
|
]
|
||||||
req = IWillPlanRequest(
|
req = IWillPlanRequest(
|
||||||
task_id=uuid4(),
|
task_id=uuid4(),
|
||||||
plan="Plan narrative describing the approach in full sentences.",
|
plan="Plan narrative describing the approach in full sentences.",
|
||||||
@@ -75,10 +84,7 @@ def test_i_will_plan_request_flattens_sdk_nested_technical_considerations() -> N
|
|||||||
"enforced on the plan's Approach field so the Plan tab is fully "
|
"enforced on the plan's Approach field so the Plan tab is fully "
|
||||||
"populated for audit and tracing instead of rendering an empty view."
|
"populated for audit and tracing instead of rendering an empty view."
|
||||||
),
|
),
|
||||||
technical_considerations=[
|
technical_considerations=technical_considerations,
|
||||||
[[["Empty state distinct from loaded state, coverage target 80%"]]],
|
|
||||||
[{"item": {"$text": "Use asyncpg prepared statements"}}],
|
|
||||||
],
|
|
||||||
)
|
)
|
||||||
assert req.technical_considerations == [
|
assert req.technical_considerations == [
|
||||||
"Empty state distinct from loaded state, coverage target 80%",
|
"Empty state distinct from loaded state, coverage target 80%",
|
||||||
@@ -92,9 +98,12 @@ def test_i_will_work_on_request_flattens_dict_wrapped_technical_considerations()
|
|||||||
"""Same coercion on the developer planning verb — a dict-wrapped string
|
"""Same coercion on the developer planning verb — a dict-wrapped string
|
||||||
(``{"item": {"$text": "…"}}``, the SDK's element-text marker) must reduce
|
(``{"item": {"$text": "…"}}``, the SDK's element-text marker) must reduce
|
||||||
to the bare string, not ``str(dict)``."""
|
to the bare string, not ``str(dict)``."""
|
||||||
|
technical_considerations: list[Any] = [
|
||||||
|
{"item": {"$text": "Cache the lookup result"}}
|
||||||
|
]
|
||||||
req = IWillWorkOnRequest(
|
req = IWillWorkOnRequest(
|
||||||
task_id=uuid4(),
|
task_id=uuid4(),
|
||||||
technical_considerations=[{"item": {"$text": "Cache the lookup result"}}],
|
technical_considerations=technical_considerations,
|
||||||
)
|
)
|
||||||
assert req.technical_considerations == ["Cache the lookup result"]
|
assert req.technical_considerations == ["Cache the lookup result"]
|
||||||
|
|
||||||
@@ -104,6 +113,10 @@ def test_delegate_request_flattens_sdk_nested_acceptance_criteria() -> None:
|
|||||||
the SDK can nest (this is the ``delegate``-verb analogue of the MegaTask
|
the SDK can nest (this is the ``delegate``-verb analogue of the MegaTask
|
||||||
Bug 3 crash). The ``StrList`` field must flatten the nested input so the
|
Bug 3 crash). The ``StrList`` field must flatten the nested input so the
|
||||||
VARCHAR[] insert downstream never sees a dict/list element."""
|
VARCHAR[] insert downstream never sees a dict/list element."""
|
||||||
|
acceptance_criteria: list[Any] = [
|
||||||
|
[[["returns 200 for valid input"]]],
|
||||||
|
[{"item": {"$text": "rejects malformed input with 400"}}],
|
||||||
|
]
|
||||||
req = DelegateRequest(
|
req = DelegateRequest(
|
||||||
parent_task_id=uuid4(),
|
parent_task_id=uuid4(),
|
||||||
title="t",
|
title="t",
|
||||||
@@ -113,10 +126,7 @@ def test_delegate_request_flattens_sdk_nested_acceptance_criteria() -> None:
|
|||||||
task_type="code",
|
task_type="code",
|
||||||
nature="technical",
|
nature="technical",
|
||||||
estimated_complexity="medium",
|
estimated_complexity="medium",
|
||||||
acceptance_criteria=[
|
acceptance_criteria=acceptance_criteria,
|
||||||
[[["returns 200 for valid input"]]],
|
|
||||||
[{"item": {"$text": "rejects malformed input with 400"}}],
|
|
||||||
],
|
|
||||||
)
|
)
|
||||||
assert req.acceptance_criteria == [
|
assert req.acceptance_criteria == [
|
||||||
"returns 200 for valid input",
|
"returns 200 for valid input",
|
||||||
@@ -129,8 +139,9 @@ def test_strlist_drops_non_string_junk_instead_of_crashing() -> None:
|
|||||||
is dropped — the field never raises on garbage the SDK might emit; only
|
is dropped — the field never raises on garbage the SDK might emit; only
|
||||||
real strings survive. An all-junk payload yields an empty list (the
|
real strings survive. An all-junk payload yields an empty list (the
|
||||||
delegate min_length=1 gate then rejects it cleanly, not a 500)."""
|
delegate min_length=1 gate then rejects it cleanly, not a 500)."""
|
||||||
|
technical_considerations: list[Any] = [42, {"foo": 123}, [[" "]], "real note"]
|
||||||
req = IWillWorkOnRequest(
|
req = IWillWorkOnRequest(
|
||||||
task_id=uuid4(),
|
task_id=uuid4(),
|
||||||
technical_considerations=[42, {"foo": 123}, [[" "]], "real note"],
|
technical_considerations=technical_considerations,
|
||||||
)
|
)
|
||||||
assert req.technical_considerations == ["real note"]
|
assert req.technical_considerations == ["real note"]
|
||||||
|
|||||||
@@ -246,6 +246,7 @@ def test_pr_review_issues_default_empty_and_single_scalar_coerced() -> None:
|
|||||||
"issues": "lone issue string",
|
"issues": "lone issue string",
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
assert isinstance(coerced, PrReviewContent)
|
||||||
assert coerced.issues == ["lone issue string"]
|
assert coerced.issues == ["lone issue string"]
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -233,7 +233,7 @@ async def test_pr_fail_a2a_failure_is_swallowed() -> None:
|
|||||||
|
|
||||||
c = _make_choreographer()
|
c = _make_choreographer()
|
||||||
_stub_gate_path(c, reviewer_id=reviewer_id, t_before=t_before, t_after=t_after)
|
_stub_gate_path(c, reviewer_id=reviewer_id, t_before=t_before, t_after=t_after)
|
||||||
c.a2a.send = AsyncMock(side_effect=RuntimeError("db hiccup")) # type: ignore[method-assign]
|
c.a2a.send = AsyncMock(side_effect=RuntimeError("db hiccup"))
|
||||||
|
|
||||||
env = await c.pr_fail(reviewer_id, task_id, ["a concrete actionable issue"])
|
env = await c.pr_fail(reviewer_id, task_id, ["a concrete actionable issue"])
|
||||||
# Verdict still landed — the owning PM is in needs_revision.
|
# Verdict still landed — the owning PM is in needs_revision.
|
||||||
|
|||||||
@@ -103,6 +103,7 @@ def test_pr_fail_stores_issues_structurally_not_summary_only() -> None:
|
|||||||
"Issues:\n- seam mismatch\n- docs lag the diff",
|
"Issues:\n- seam mismatch\n- docs lag the diff",
|
||||||
issues=("seam mismatch", "docs lag the diff"),
|
issues=("seam mismatch", "docs lag the diff"),
|
||||||
)
|
)
|
||||||
|
assert t.notes_structured is not None
|
||||||
slot = t.notes_structured["pr_review"]
|
slot = t.notes_structured["pr_review"]
|
||||||
assert slot["verdict"] == "failed"
|
assert slot["verdict"] == "failed"
|
||||||
assert slot["issues"] == ["seam mismatch", "docs lag the diff"]
|
assert slot["issues"] == ["seam mismatch", "docs lag the diff"]
|
||||||
@@ -128,6 +129,7 @@ def test_pr_fail_summary_does_not_duplicate_issues() -> None:
|
|||||||
"Issues:\n- seam mismatch\n- docs lag the diff",
|
"Issues:\n- seam mismatch\n- docs lag the diff",
|
||||||
issues=("seam mismatch", "docs lag the diff"),
|
issues=("seam mismatch", "docs lag the diff"),
|
||||||
)
|
)
|
||||||
|
assert t.notes_structured is not None
|
||||||
slot = t.notes_structured["pr_review"]
|
slot = t.notes_structured["pr_review"]
|
||||||
assert slot["verdict"] == "failed"
|
assert slot["verdict"] == "failed"
|
||||||
# Issues live in the structured issues slot...
|
# Issues live in the structured issues slot...
|
||||||
@@ -151,6 +153,7 @@ def test_pr_pass_leaves_issues_slot_empty() -> None:
|
|||||||
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."
|
||||||
)
|
)
|
||||||
|
assert t.notes_structured is not None
|
||||||
slot = t.notes_structured["pr_review"]
|
slot = t.notes_structured["pr_review"]
|
||||||
assert slot["verdict"] == "passed"
|
assert slot["verdict"] == "passed"
|
||||||
assert slot.get("issues", []) == []
|
assert slot.get("issues", []) == []
|
||||||
|
|||||||
@@ -96,7 +96,11 @@ async def test_hand_formatted_verdict_body_with_no_findings_is_rejected() -> Non
|
|||||||
assert "findings" in body["remediate"].lower()
|
assert "findings" in body["remediate"].lower()
|
||||||
# Nothing posted / transitioned — the guard fired before any side effect.
|
# Nothing posted / transitioned — the guard fired before any side effect.
|
||||||
c.git.post_pr_review.assert_not_awaited()
|
c.git.post_pr_review.assert_not_awaited()
|
||||||
c._verb_runner().run_intent.assert_not_awaited() # type: ignore[union-attr]
|
# ``c._verb_runner()`` is a ``MagicMock`` at runtime (stubbed above) but the
|
||||||
|
# declared return is a coroutine — index the spy through an ``Any`` alias so
|
||||||
|
# ``assert_not_awaited`` resolves without a ``type: ignore``.
|
||||||
|
cc: Any = c
|
||||||
|
cc._verb_runner().run_intent.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|||||||
Reference in New Issue
Block a user