FEnigma/TODO.md

# Bug Backlog (from user report, 2026-08-11)

Status legend: [x] fixed+tested, [~] partially addressed, [ ] open/needs input

- [x] Allies and enemies seem to share indices.
      First pass on this was wrong: I only checked that targets and allies
      are separate id namespaces (they are, always were) and stopped there.
      The actual bug was one level down: `Board.add_target`/`add_ally`'s
      auto-id assignment (`used = {t.id for t in self.targets if t.type ==
      type_}`) was scoped **per type**, not per group — a Tank and an
      Infantry auto-added back to back both got id "A", each type getting
      its own independent A/B/C... sequence instead of sharing one across
      the whole group. Fixed: the id namespace split is targets-vs-allies
      ONLY, type never subdivides it further. New regression test
      (`test_auto_id_is_shared_across_types_within_targets_and_within_allies`).
- [x] Regression FROM the fix above, caught via a real traceback: sharing
      one A/B/C... sequence across a whole group (instead of per-type)
      made it much easier to actually run out of the 26 letters --
      `next(c for c in string.ascii_uppercase if c not in used)` raises
      `StopIteration` the instant all 26 are taken, silently killing
      whatever button click triggered `add_target`/`add_ally` (this is
      what "Accept as"/"Accept all" doing nothing turned out to be, see
      below). Fixed with `_next_free_id()`: rolls over to two-letter ids
      ("AA", "AB", ...) instead of raising, can't run out. New test
      (`test_auto_id_survives_past_26_entities_in_one_group`).
- [x] Ally type 'ally' is called Enemy on map title.
      `icons._target_type_label` (now public `icons.target_type_label`)
      already special-cased this for the type picker, but the map's
      right-click popover heading, "Change type (...)" button, and toast all
      printed `obj.type.value` directly instead, so an Ally with the
      ad-hoc TargetType.ENEMY still showed "Enemy" everywhere except the
      picker itself. Fixed in `app.py` (`_display_name`, and the three
      spots using it).
- [x] Reordering firing commands lags UI hard.
      `FiringPanel._reorder()` was calling `self.on_change()` — app.py's
      full-app refresh (re-solve every target's clue graph, dedupe, redraw
      the map, THEN rebuild the panel) — on every single drag-drop, even
      though reordering touches no location/clue/coord state at all. Now
      calls a local `self.refresh()` instead.
- [x] "Always show geo" doesn't reliably work / blast radius should stay
      shown too.
      `GridCanvas._draw_geo_overlays()`'s candidate list was
      `reference_points + targets` only — Allies have a `show_geo_desc` pin
      in the UI and can carry OCR'd clues too, but were never drawn.
      Added. `_draw_blast_radius()` only ever looked at `self.selected`,
      ignoring `show_geo_desc` entirely, so pinning it and then selecting/
      deselecting something else made it vanish; now iterates every
      selected-or-pinned target.
- [x] Clearing the board doesn't clear allies.
      `Board.clear()` cleared everything except `self.allies`. Fixed, plus
      the "clear board?" confirm-dialog's early-return guard (which skipped
      the whole action if only allies were on the board) now checks allies
      too.
- [x] Allow right-click on Clear button: clear all enemies/units/flights,
      keep spotters/RPs/nest.
      New `Board.clear_units()` + a right-click popover on the header's
      Clear button wired to it.
- [x] On map-reading error: save a screenshot locally to adapt the algo.
      New `debug_capture.py` — `save_map_read_failure()` writes the PNG +
      the solver's rejection reason under
      `$XDG_DATA_HOME/fenigma/debug_captures/failures/`, wired into
      `app.py`'s `_start_map_import`.
- [x] When the user corrects the grid, store screenshot + ground truth too.
      `debug_capture.save_grid_correction()`, wired into `_accept_grid`:
      fires only when the accepted `GridSolution` isn't the one auto-solve
      produced (the user actually dragged a handle in GridFixDialog), saves
      both solutions under `.../debug_captures/corrections/`.
- [x] Many map screenshots seem to get read as text; if nothing relevant is
      found, also store the image to check whether it was actually a map.
      `debug_capture.save_maybe_map()`, wired into `_ocr_png`: fires when a
      screenshot (not a plain-text paste) fell through to the OCR/text path
      and `_merge_all` found nothing at all. Saved under
      `.../debug_captures/maybe_map/`.
- [x] Unable to parse 3 given chat messages (Infantry "taking fire" fire-
      support requests).
      A different grammar from the existing Marine Garrison fire-support
      request: reversed shell word order ("Requesting X Shell" vs "X Shells
      requested"), a bare "before/by <time>" deadline (no "Requested"/
      dashes), and either a direct "on our position at <coord>" or a
      bearing/distance offset from that same inline position (not a named
      board entity, so resolved directly via
      `solver.point_from_bearing_distance` rather than through a Clue).
      New extractors in `ocr.py`, wired into `parse_intel_blocks`'s
      `flush()`. 3 new regression tests, all passing (`tests/test_ocr.py`).
- [x] Follow-up bug in the above: the bearing/distance-offset variant
      names TWO different places (the reporting unit's own position, and
      a separate fire point offset from it), but only produced one Target
      entity, sitting at the offset point but still labeled with the
      unit's own type/id (e.g. "Infantry#11" at a spot no infantry is
      actually at). Math itself was right; the single-entity shape wasn't.
      Now produces two entries: the original (Infantry#N etc.) keeps its
      own reported position with no shell/deadline, and a new synthetic
      `Strike#<TypeWord><id>` entry (e.g. `Strike#Infantry11`) carries the
      shell/deadline at the computed offset coord. 2 more regression tests.

- [x] When the user deletes/replaces the map screenshot, capture whatever
      units they confirmed as ground truth for it.
      `ScreenshotImport.baseline_targets`/`baseline_allies` (a snapshot of
      `board.targets`/`board.allies` taken when the grid is confirmed,
      `Target`/`Ally` are identity-hashable so these are plain sets of the
      live objects) let `app.py` tell "added while this screenshot was up"
      apart from "was already on the board". `Proposal` also now records
      `confirmed_type` (what the user actually accepted it as, which can
      differ from the detector's own guess via "Accept as..."). All of it
      -- every proposal's accept/reject/undecided verdict, plus every
      target/ally added with no matching proposal at all (a manual add or
      an OCR-text merge run alongside the screenshot) -- is saved via
      `debug_capture.save_marker_ground_truth()` under
      `.../debug_captures/marker_ground_truth/`. Wired into all three
      places a screenshot stops being "the active one": explicit drop, a
      new screenshot pasted straight over it, and window close.

## Resolved via a real traceback (not guessed)

- [x] "Accept as" / "Accept all" on proposed targets doing nothing.
      A real traceback from the running app nailed it: `StopIteration`
      from `Board.add_ally`'s id auto-assignment once 26 allies existed
      already (see the id-namespace regression entry above) — every
      accept attempt after that silently died before the ally/target
      ever got added, popover already closed by the time it happened.
      Fixed there; not a separate bug.
- [x] "Accept as…" (the type-picker submenu on a proposal, and "Change
      type" on an already-placed entity) opening to a visibly empty/
      unchanged popover. This one left no traceback at all -- confirmed
      live with temporary debug prints that the button's `clicked` signal
      fires, the icon grid builds successfully (all N types), and
      `Popover.set_child()` on the already-open outer popover reports the
      right `visible=True`/width/height afterward... but the compositor
      never actually repaints that reused surface, so nothing new ever
      appeared on screen. Fixed by not resizing the existing open
      popover at all: popping it down and opening a genuinely new one
      (fresh native surface) at the same anchor point instead. Same fix
      applied to both call sites (`_open_proposal_menu`'s `show_type`,
      `_open_entity_menu`'s `show_type`, the latter refactored to share
      the same `_reopen_with()` helper).
- [x] New: mark a Target as underground, at a hardening tier (1-3),
      rendered as the game's own Armor-tier additive badge stacked on
      the icon. `Target.underground_tier: int | None`, a "Mark
      underground" entry in the entity-edit popover (tier picker reusing
      the same fresh-popover fix above), and `GridCanvas` draws the
      badge above the marker's icon, overlapping down into it by
      `_ADDITIVE_OVERLAP_PX` -- both the diamond icon's top corner and
      the badge's bottom are tapered to a near-point, not a flat edge,
      so bbox-exact touching still read as a gap; a real pixel overlap
      is what actually looks contiguous (confirmed against the game's
      own stacked-badge screenshots). Badge is scaled/positioned off the
      art's real opaque content (PIL `getbbox()`), not its PNG canvas --
      the additive files carry a lot of off-center transparent padding
      that made the badge look tiny and floating if sized off the raw
      canvas.

## Needs more scope / your input before I keep going

- [~] Enemy type detection needs to be more robust; read the entity id
      label so dedup is reliable; detect death from the log.

      Started on the id-reading piece: `map_vision.read_marker_id` reads
      each marker's own small "#<N>" label (distinct from the big
      per-cell grid label `read_cell_label` reads) via the SAME template-
      correlation approach as `read_cell_label`, not OCR -- this text
      sits over the same aerial-photo backdrop that this module's own
      docstring says defeated every detection-based approach tried for
      grid labels, so pytesseract (already tried elsewhere in this repo,
      `ocr.py`, for a different image domain: flat scanned paper, not
      photo-textured) was skipped in favor of the approach already proven
      here. Wired end-to-end: `find_markers` -> `Proposal.detected_id` ->
      `debug_capture.save_marker_ground_truth`'s JSON. Reads against
      `ScreenshotImport.full_image` (sharper than the WORK_W image
      detection itself runs against) when available. Crop region and
      `MIN_MARKER_ID_SCORE` are a single-screenshot calibration (see
      `read_marker_id`'s own docstring) -- UNVALIDATED against a real
      ground-truth batch (none of the 6 existing captures have a
      confirmed id to check against, they all predate this). New unit
      tests (`tests/test_map_vision_marker_id.py`) only cover the
      synthetic-render round-trip, not real-screenshot accuracy.

      Measured type-detection reliability against the 6 existing
      `marker_ground_truth` captures (72 accepted proposals total,
      2026-08-13): **0/72 (0%) had ANY confident `detected_unit` guess**
      -- `classify_marker` returned `None` on every single one, every
      side, every capture. Not "guesses wrong" -- never confident enough
      to answer at all. Spot-checked directly against one real marker
      crop (a hostile Infantry, confirmed by the user): best match was
      "Underground Fort" at score 0.376 (Infantry wasn't even in the top
      8), against a `min_score=0.55` floor `classify_marker` requires --
      not a close miss, a real correlation failure. The clean rendered
      icon templates `icon_bank()` matches against apparently don't
      correlate well with how markers actually look in a real screenshot
      (compression/blur/aerial-photo texture underneath), unlike text
      glyphs (`read_cell_label`'s measured 0.73-0.87 vs 0.40-0.56) where
      the same template-correlation idea works well. Added `unit_score`/
      `unit_margin` to `Proposal`/ground-truth JSON (previously only
      pass/fail `unit` was logged) so every future capture shows exactly
      how far off a guess was, not just None -- there was no way to tell
      "barely missed the bar" from "wildly wrong" before this.

      Death-detection-from-log is still fully unstarted -- no log-parsing
      code exists in this repo at all yet, real scope work (find/access
      the game's log, agree a "<Type>#<id> Destroyed" grammar, wire it
      into a dedup key) rather than a quick pass.