FEnigma/TODO.md

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

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

• 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).

• 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).

• 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).

• 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.

• "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.

• 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.

• 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.

• 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.

• 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/.

• 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/.

• 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 " deadline (no "Requested"/ dashes), and either a direct "on our position at " 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).

• 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.

• 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)

• "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.
• "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).
• 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.
  
  Follow-ups from user feedback after the above landed:
  - `_accept_proposal` now actually USES `detected_id` (it was only
    being logged before, never applied) -- an accepted proposal's
    entity id prefers the detected number over auto-assignment,
    falling back on a collision. 4 new regression tests
    (`tests/test_app_accept_proposal.py`).
  - Auto-assignment itself (`Board.add_target`/`add_ally` with no
    `id_`/no usable detection) changed from one shared letter
    sequence per group (targets, or allies) to its own sequence per
    TYPE within each group -- Tank#A/Infantry#A rather than
    Tank#A/Infantry#B. This directly reverses an earlier deliberate
    fix in this same file (see the "Allies and enemies seem to share
    indices" entry above, which moved FROM per-type TO
    shared-per-group) -- that fix is still correct for what it fixed
    (targets-vs-allies must stay separate namespaces), just not for
    per-type-vs-shared, which the user has now clarified the other
    way. Tests in `test_models.py` updated to match (renamed
    `test_auto_id_is_shared_across_types...` ->
    `test_auto_id_is_per_type...`, since it now asserts the opposite).
  
    First pass at this ALSO switched auto-assignment from letters to
    plain numbers (1/2/3...), reasoning that it should match what
    `detected_id` looks like when read successfully. Wrong -- caught
    by the user immediately: auto-assignment (no real id known, a
    manual add or an accept with no confident read) and a genuinely
    detected id need to stay visually distinct, or a made-up
    auto-assigned number could collide with, or be mistaken for, a
    real one. Reverted back to `_next_free_id` (letters, rolling
    over to "AA"/"AB"/... past 26 rather than raising
    `StopIteration`) as the auto-assignment fallback, scoped per
    type same as above; plain numbers are reserved for an id
    `_accept_proposal` is actually confident was read off the
    marker itself (`Proposal.detected_id`), passed straight through
    as `id_` and never touching auto-assignment at all.
  - `_accept_proposal` now actually USES `detected_id` (it was only
    being logged before, never applied) -- an accepted proposal's
    entity id prefers the detected number over auto-assignment,
    falling back on a collision (scoped per type, same bug fixed
    in two places: this collision pre-check, and the "Change ID"
    popover's own check, which still enforced the OLD shared-per-
    group rule after the auto-assignment change above and rejected
    valid renames across types). 4 new regression tests
    (`tests/test_app_accept_proposal.py`).
  - `detected_id` is now shown, not just logged: the proposal
    popover's heading (", id #8") and the pending-proposal's own
    on-map label (`? #8 G8 5:4`) both show it while there's still a
    screenshot up to check it against by eye.
  

Windows build (packaging/windows) -- real progress, not yet a clean pass

Booted the actual dockur/windows build VM and drove it live (VNC) to find out what's really failing, rather than guessing from the README's own "UNTESTED end to end" note. Three real, separate bugs found and fixed, each confirmed live against the real VM, not just read off a diff:

• The build watcher was never actually installed at all, despite install_progress.log claiming every provisioning step succeeded. C:\OEM (dockur's /oem staging dir) doesn't reliably persist past Windows Setup finishing -- exactly what the (already-uncommitted, now committed) install.bat/watch_build.bat fix diagnosed, just never verified against a real run before now. A 2-day-old BUILD_REQUEST had been sitting unclaimed the whole time. Manually re-applied the fix's logic live once (copied the corrected files to C:\FenigmaBuild, registered the Startup-folder entry) and confirmed on a full container restart that the watcher now auto-starts on login and picks up a pending request with zero manual intervention -- the actual fix, not just my live patch, is what's doing that.

• pip install pytesseract fails outright: MSYS2's mingw64 Python enforces PEP 668 ("externally-managed-environment"), which install.bat never accounted for. Needs --break-system-packages.

• import fenigma.app fails with ModuleNotFoundError: No module named 'cv2' even after pacman -S mingw-w64-x86_64-opencv succeeds -- that package is the C++ library only. The actual Python bindings are a SEPARATE package, mingw-w64-x86_64- python-opencv, that install.bat's dependency list simply never included. (pip install opencv-python-headless as a fallback doesn't work either and shouldn't be relied on: MSYS2's mingw64 Python uses a different ABI than PyPI's Windows wheels -- cp314-mingw_x86_64_msvcrt_gnu vs win_amd64 -- so pip can never use a prebuilt wheel there, only build from source, which then needs a full separate native toolchain -ninja/cmake/gcc- this VM doesn't have either.)

  All three are one-line fixes once known. install.bat's pacman package list and pip install line need these applied for a from-scratch VM to provision correctly (currently they're only proven fixed live on this session's VM, not yet folded back into the committed install.bat -- do that before relying on a fresh ./build_windows.sh run from scratch).

  With all three fixed, import fenigma.app succeeds and a real build attempt got all the way through source copy, sanity check, dist-tree assembly, and WiX harvest+compile (candle.exe) -- further than this pipeline has ever gotten. Two more issues surfaced right at the finish line:

  • product.wxs's Version needs strict WiX x.x.x.x numeric form -- a 0.1.0-test version string (my own test invocation, not build_windows.sh's real default) fails candle.exe with CNDL0108/CNDL0010. Not a real bug, just don't pass a version with a suffix.
  • light.exe (final MSI linking) did not finish within 15 minutes on this VM (4 CPU / 8GB RAM) before the RAM-conscious auto- shutdown killed it -- process was genuinely active (343MB working set, not hung on a dialog), just slow, likely from the ~1GB+ bulk-copied mingw64 runtime (see README's own "dist tree is fat, not lean" note) combined with this VM's disk/CPU being shared with the host. Needs either a longer timeout on a retry, or the dist-tree-trimming work the README already flags as a known follow-up (tracing the real DLL/typelib closure instead of bulk-copying all of mingw64) to make light.exe have less to compress in the first place.

OCR: new fire-support-request grammar gaps (from real user-pasted messages)

• A "taking fire" report's reporting unit ("Infantry#11 taking fire!...") was being added as a hostile Target, not a friendly Ally -- see the id-scheme entry above for the "no Friendly/Hostile prefix word exists in this grammar" root cause and the fix (_TAKING_FIRE_RE, force_ally, and splitting the shell/deadline into a synthetic StrikeRequest even for the no-offset "on our position" case, which previously kept them on the entity itself -- fine when it was wrongly a Target, silently lost once correctly an Ally, since Ally tuples carry no shell/deadline fields at all). 4 existing tests updated, all still passing plus the rest of the suite (51 total).

• A multi-shell sequential request ("Requesting TEAR Shell first, then HE Shell, at bearing...") only captures the FIRST shell (TEAR) into the structured shell field -- "then HE Shell" isn't parsed into anything. Less urgent than it first looked though: the full original message text (both shells, in order) is already preserved as-is and shown to the player via the coord dialog's description view (Location.desc_raw, set from the same raw every merged target/ally carries) -- nothing is silently LOST, it's just not machine-parsed into a queryable second-shell field. Real scope question before building that: does the board/firing-panel data model even have a place to put a second shell for one strike request today, or does this need a new field/shape entirely -- worth confirming it's actually wanted (vs. "read the raw text yourself, it's right there") before spending the design effort.

• "Answer by 10:30:00" turned out to be one bug, not two. The phrasing itself was never the problem -- _TAKING_FIRE_TIME_RE already matches any before|by <time>, "Answer BY 10:30:00" included. The REAL bug: a same-message "Important: ..." follow-up line was misread as a brand new named entity header (the last-resort bare-<name>: fallback rule matched "Important:" itself with nothing excluding common prose lead-ins), creating a bogus Target#Important that stole "Answer by 10:30:00" into ITS own requested_time instead of the real report's. Fixed with a blocklist (_BARE_NAME_HEADER_BLOCKLIST: important/note/warning/ attention/caution/alert/reminder/priority) on that fallback rule -- once the phantom split stopped happening, the deadline resolved onto the right entry with no separate fix needed. New regression test, confirmed against the user's real pasted message (with an assumed Infantry#N taking fire! header line prepended, since their paste seems to have been cropped before it).

• "<Type>#<id> Destroyed" kill-feed parsing already exists and already marks the matching Target dead (parse_destroyed, _merge_targets's own destroyed-handling block in app.py) -- confirmed working end-to-end against a real 9-entry kill-feed paste, including multi-word types ("Enemy Mechanized Infantry#2 Destroyed" correctly resolved to INFANTRY_MECHANIZED). This was already-existing, working functionality, not something needing to be built. One real gap found in the same test: "Enemy Field Gun#1 Destroyed" silently dropped. Two bugs stacked, both fixed: - [x] "Field Gun" is just the game's own alt name for plain Artillery (confirmed by the user directly) -- not a missing unit type needing a new enum member/icon after all. Added to _TYPE_WORD_ALIASES next to AmmoCache/CoastalBattery. - [x] Even with that alias, it still didn't resolve: _ALLY_PREFIX_RE only ever stripped a leading "Friendly"/"Hostile" word, never "Enemy" -- so squash_multiword_ids's "EnemyFieldGun#1" token got alias-looked-up and fuzzy-matched as a WHOLE ("EnemyFieldGun" vs "Artillery", nowhere close), not just its "FieldGun" part. "Enemy Mechanized Infantry#2" only ever worked by fuzzy-match ACCIDENT (a long, distinctive type string still clears the ratio threshold with "Enemy" stuck to the front; a short, unrelated one like Artillery doesn't). _ALLY_PREFIX_RE now strips "Enemy" too, with a lookahead requiring something after it -- a BARE "Enemy#N" is TargetType.ENEMY itself (its own value IS "Enemy"), stripping unconditionally would've left an empty type_word and broken every ad-hoc "Enemy#N Destroyed" report instead. 3 new regression tests, all passing (54 total).