Fix ally/target bugs, OCR fire-support parsing, add debug capture

- Board.clear() now also drops allies; the "clear board?" guard checks allies too. New Board.clear_units() + Clear button right-click menu ("clear enemies, units & flights", keeps Nest/spotters/RPs). - An Ally with the ad-hoc TargetType.ENEMY showed "Enemy" on the map popover/toast instead of "Ally" (icons.target_type_label already had the fix for the picker, now reused everywhere else via app.py's _display_name). - Firing panel drag-reorder no longer triggers a full app refresh (solver + dedupe + map redraw) on every drop, just a local rebuild. - "Always show geo" didn't draw for Allies (missing from the overlay candidate list); blast radius only respected selection, not the show_geo_desc pin. - ocr.py: added a second fire-support-request grammar ("Infantry#N taking fire ... Requesting X Shell on our position at <coord> before <time>", plus a bearing/distance-from-position variant), distinct from the existing Marine Garrison one. - New debug_capture.py: saves screenshots (+ metadata) the app handled badly, for later tuning of map_vision/ocr against real failures: map-read errors, user grid corrections (paired with the auto-detected grid), screenshots that read as text but may have been a map, and marker-detection ground truth (every proposal's accept/reject verdict plus units added with no matching proposal) captured whenever a screenshot stops being the active one. - README: Known issues section (map screenshot reading, grid + unit detection, is unreliable and fails often). - 14 new tests (tests/test_models.py, tests/test_debug_capture.py, + additions to tests/test_ocr.py), 38/38 passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-11 17:35:37 +02:00
parent 136492b197
commit 1ddb532325
13 changed files with 823 additions and 44 deletions
@@ -0,0 +1,100 @@
+"""debug_capture just needs to reliably write what it's given and never
+raise into caller code -- these are format/plumbing checks, not vision
+tests."""
+import json
+
+import numpy as np
+import pytest
+from PIL import Image
+
+from fenigma import debug_capture
+
+
+@pytest.fixture(autouse=True)
+def _isolated_debug_dir(tmp_path, monkeypatch):
+    monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path))
+    return tmp_path
+
+
+def _tiny_png_bytes() -> bytes:
+    import io
+    buf = io.BytesIO()
+    Image.new("RGB", (4, 4), (10, 20, 30)).save(buf, format="PNG")
+    return buf.getvalue()
+
+
+def test_save_map_read_failure_writes_png_and_reason(_isolated_debug_dir):
+    path = debug_capture.save_map_read_failure(_tiny_png_bytes(), "too few grid line families")
+    assert path is not None
+    assert path.exists()
+    meta = json.loads(path.with_suffix(".json").read_text())
+    assert meta["reason"] == "too few grid line families"
+
+
+def test_save_maybe_map_accepts_numpy_bgr_array(_isolated_debug_dir):
+    bgr = np.zeros((4, 4, 3), dtype=np.uint8)
+    bgr[..., 0] = 200  # blue channel, would come out red if BGR/RGB got swapped
+    path = debug_capture.save_maybe_map(bgr)
+    assert path is not None
+    saved = Image.open(path)
+    assert saved.getpixel((0, 0)) == (0, 0, 200)  # still blue, not swapped to red
+
+
+def test_save_grid_correction_writes_both_solutions(_isolated_debug_dir):
+    class FakeSolution:
+        def __init__(self, du):
+            self.H = np.eye(3)
+            self.si, self.sj, self.du, self.dv = 1, 1, du, 0
+
+    path = debug_capture.save_grid_correction(_tiny_png_bytes(), FakeSolution(0.0), FakeSolution(0.3))
+    assert path is not None
+    meta = json.loads(path.with_suffix(".json").read_text())
+    assert meta["original"]["du"] == 0.0
+    assert meta["corrected"]["du"] == 0.3
+
+
+def test_unsupported_image_type_returns_none_without_raising(_isolated_debug_dir):
+    assert debug_capture.save_maybe_map(object()) is None
+
+
+def _proposal(**overrides):
+    from fenigma.map_import import Proposal
+    defaults = dict(side="hostile", label="K8", sub_x=3, sub_y=4, unit="Armor_Tank.png",
+                    centre=(0, 0), box=(0, 0, 0, 0))
+    defaults.update(overrides)
+    return Proposal(**defaults)
+
+
+def test_marker_ground_truth_records_verdict_per_proposal(_isolated_debug_dir):
+    accepted = _proposal(accepted=True, confirmed_type="TANK")
+    rejected = _proposal(label="K9", rejected=True)
+    undecided = _proposal(label="L1")
+
+    path = debug_capture.save_marker_ground_truth(_tiny_png_bytes(), [accepted, rejected, undecided])
+    assert path is not None
+    meta = json.loads(path.with_suffix(".json").read_text())
+    by_label = {p["label"]: p for p in meta["proposals"]}
+    assert by_label["K8"]["verdict"] == "accepted"
+    assert by_label["K8"]["confirmed_type"] == "TANK"
+    assert by_label["K9"]["verdict"] == "rejected"
+    assert by_label["L1"]["verdict"] == "undecided"
+
+
+def test_marker_ground_truth_records_units_with_no_matching_proposal(_isolated_debug_dir):
+    from fenigma.models import Board, Coord, TargetType
+
+    board = Board()
+    manual_target = board.add_target(TargetType.TANK, Coord("K", 8, 3, 4))
+    manual_ally = board.add_ally(TargetType.INFANTRY, Coord("K", 9, 0, 0))
+
+    path = debug_capture.save_marker_ground_truth(
+        _tiny_png_bytes(), [], added_targets=[manual_target], added_allies=[manual_ally])
+    assert path is not None
+    meta = json.loads(path.with_suffix(".json").read_text())
+    kinds = {(u["kind"], u["type"], u["coord"]) for u in meta["added_units"]}
+    assert ("target", "TANK", "K8 3:4") in kinds
+    assert ("ally", "INFANTRY", "K9 0:0") in kinds
+
+
+def test_marker_ground_truth_skips_when_nothing_to_say(_isolated_debug_dir):
+    assert debug_capture.save_marker_ground_truth(_tiny_png_bytes(), []) is None
@@ -0,0 +1,79 @@
+"""Regression coverage for Board's bulk-mutation helpers (clear/clear_units)
+and the id namespaces Target/Ally are supposed to keep separate."""
+from fenigma.models import Board, Coord, TargetType
+
+
+def _coord(x=0, y=0):
+    return Coord(X="A", Y=1, x=x, y=y)
+
+
+def test_clear_drops_allies_too():
+    """Board.clear() used to leave self.allies untouched -- the "clear
+    board" action then reported success but a previously-placed ally
+    stayed on the map."""
+    board = Board()
+    board.nest.coord = _coord()
+    board.add_spotter(_coord())
+    board.add_reference_point(_coord())
+    board.add_target(TargetType.TANK, _coord())
+    board.add_ally(TargetType.TANK, _coord())
+    board.add_scout_flight((1.0, 1.0), 45.0)
+
+    board.clear()
+
+    assert board.nest.coord is None
+    assert board.spotters == []
+    assert board.reference_points == []
+    assert board.targets == []
+    assert board.allies == []
+    assert board.scout_flights == []
+
+
+def test_clear_units_keeps_recon_infrastructure():
+    """The Clear button's right-click "Clear enemies, units & flights"
+    option: drops targets/allies/scout flights but keeps the Nest,
+    spotters, and reference points."""
+    board = Board()
+    board.nest.coord = _coord()
+    sp = board.add_spotter(_coord())
+    rp = board.add_reference_point(_coord())
+    board.add_target(TargetType.TANK, _coord())
+    board.add_ally(TargetType.TANK, _coord())
+    board.add_scout_flight((1.0, 1.0), 45.0)
+
+    board.clear_units()
+
+    assert board.nest.coord is not None
+    assert board.spotters == [sp]
+    assert board.reference_points == [rp]
+    assert board.targets == []
+    assert board.allies == []
+    assert board.scout_flights == []
+
+
+def test_ally_and_target_ids_are_independent_namespaces():
+    """An ally Tank#1 and a hostile Target Tank#1 are unrelated -- adding
+    one must never be influenced by the other's ids, and auto-assignment
+    on each side starts from 'A' independently."""
+    board = Board()
+    t1 = board.add_target(TargetType.TANK, _coord(), id_="1")
+    a1 = board.add_ally(TargetType.TANK, _coord(), id_="1")
+    assert t1.id == a1.id == "1"
+    assert t1 is not a1
+
+    t_auto = board.add_target(TargetType.TANK, _coord())
+    a_auto = board.add_ally(TargetType.TANK, _coord())
+    assert t_auto.id == "A"  # first free letter among *targets* only
+    assert a_auto.id == "A"  # first free letter among *allies* only, unaffected by the target above
+
+
+def test_find_by_name_prefers_target_over_same_named_ally():
+    """find_by_name() (used to resolve Clue references) checks targets
+    before allies -- documented, deliberate priority, not a namespace
+    collision: an ally and a same-typed/same-id target are still two
+    distinct objects, this only matters when something's Clue names one
+    ambiguously by the shared display name."""
+    board = Board()
+    target = board.add_target(TargetType.TANK, _coord(x=1), id_="1")
+    board.add_ally(TargetType.TANK, _coord(x=2), id_="1")
+    assert board.find_by_name("Tank#1") is target
@@ -10,6 +10,7 @@ being noticed (or not) days later.
 """
 from fenigma import ocr
 from fenigma.models import Coord, TargetType
+from fenigma.shells import Shell


 def test_standard_target_and_rp_blocks():
@@ -236,3 +237,54 @@ Tank#3  Spotted. 095, 3.00km from Spotter#1
    assert (TargetType.TANK, "1") not in info.targets
    assert (TargetType.TANK, "2") in info.targets
    assert (TargetType.TANK, "3") in info.targets
+
+
+def test_infantry_taking_fire_direct_position_request():
+    """A different fire-support-request grammar from Marine Garrison's:
+    shell word order reversed ('Requesting X Shell' not 'X Shells
+    requested'), deadline is a bare 'before <time>' with no 'Requested'/
+    dashes. The '<b>id1</b>' attacker mention is just prose here, not
+    parsed into anything -- only the request itself (shell, position,
+    deadline) matters."""
+    text = ("Infantry#1 taking fire from <b>id1</b>!\n"
+            "Requesting <u><b>SMK Shell</b></u> on our position at <b>J6 2:7</b> "
+            "before <u>10:38:57</u>!")
+    info = ocr.parse_text(text)
+    assert (TargetType.INFANTRY, "1") in info.targets
+    raw, clues, coord, shell, requested_time = info.targets[(TargetType.INFANTRY, "1")]
+    assert coord == Coord("J", 6, 2, 7)
+    assert shell is Shell.SMK
+    assert requested_time == "10:38:57"
+
+
+def test_infantry_taking_fire_no_attacker_mention():
+    text = ("Infantry#3 taking fire!\n"
+            "Requesting <u><b>SMK Shell</b></u> on our position at <b>J6 2:5</b> "
+            "before <u>10:37:52</u>!")
+    info = ocr.parse_text(text)
+    assert (TargetType.INFANTRY, "3") in info.targets
+    raw, clues, coord, shell, requested_time = info.targets[(TargetType.INFANTRY, "3")]
+    assert coord == Coord("J", 6, 2, 5)
+    assert shell is Shell.SMK
+    assert requested_time == "10:37:52"
+
+
+def test_infantry_taking_fire_bearing_distance_from_position():
+    """The other request shape: the shell isn't wanted right on top of the
+    reporting unit, but at a bearing/distance offset from its own
+    (inline-given) position -- 'our position' isn't a named board entity
+    to hang a Clue off of, so this resolves straight to an absolute
+    coord."""
+    text = ("Infantry#3 taking fire!\n"
+            "Requesting <u><b>HE Shell</b></u> at bearing <b>239°</b>, distance "
+            "<b>10.76km</b> from our position, <b>J6 2:5</b>, by <u>10:38:18</u> "
+            "or we will be overrun!")
+    info = ocr.parse_text(text)
+    assert (TargetType.INFANTRY, "3") in info.targets
+    raw, clues, coord, shell, requested_time = info.targets[(TargetType.INFANTRY, "3")]
+    assert shell is Shell.HE
+    assert requested_time == "10:38:18"
+    from fenigma import solver
+    expected = solver.point_to_coord(
+        solver.point_from_bearing_distance(Coord("J", 6, 2, 5).as_fraction(), 239.0, 10.76))
+    assert coord == expected