Просмотр исходного кода

Give the AI's own halt a name in the archive (issue #2946) (#2954)

Kouki Ojima 18 часов назад
Родитель
Сommit
a6ce26205a
2 измененных файлов с 188 добавлено и 6 удалено
  1. 36 5
      backend/app/main.py
  2. 152 1
      backend/tests/unit/test_failure_reason_derivation.py

+ 36 - 5
backend/app/main.py

@@ -513,7 +513,7 @@ _printer_offline_notify_tasks: dict[int, asyncio.Task] = {}
 _PRINTER_OFFLINE_NOTIFY_DEBOUNCE_SECONDS = 60.0
 
 
-# HMS short-code → human-readable failure reason. Used by _dispatch_archive_update
+# HMS short-code → failure_reason key. Used by _dispatch_archive_update
 # when status="failed" to label the print's failure_reason in archives.
 #
 # Earlier code matched on `module` alone (e.g. "any module 0x0C HMS → Layer shift"),
@@ -561,6 +561,35 @@ _HMS_FAILURE_REASONS: dict[str, str] = {
     "0701_8007": "cloggedNozzle",
     "0701_8013": "cloggedNozzle",
     "0702_8003": "cloggedNozzle",
+    # AI print monitoring — spaghetti / the model coming off the plate.
+    # `spaghettiDetached` is a key the archive editor already offers for this
+    # failure mode, not a new one, so a derived reason opens the dropdown on
+    # that option rather than blank — and survives the next save, which clears
+    # any value the editor does not recognise.
+    #
+    # A module-0x0C row is safe here despite the warning above: that warning is
+    # about matching on the module alone, and 0C00_8042 is a full short code
+    # with a documented meaning ("The AI print monitor has detected a spaghetti
+    # defect", hms_errors.py). The H2D cancel echo is 0C00_001B, so the two
+    # cannot collide.
+    #
+    # 0300_8003's own text ends "before continuing your print", but on an X2D
+    # it arrives with the print already paused, offering only
+    # RESUME_PRINTING_DEFECTS / STOP_PRINTING — a halt waiting on the user.
+    #
+    # Two neighbours are left out on purpose, so this does not get re-derived:
+    #   * 0C00_C004 "Possible spaghetti failure was detected." — "possible"
+    #     reads as a warning about a print that is still running, not a halt.
+    #   * 0300_800A is AI monitoring too, but it reports a filament pile-up in
+    #     the waste chute. That is not the print failing.
+    #
+    # That line is drawn from the text, not from `severity`, because severity
+    # cannot draw it: 0300_8003 reaches us through `print_error`, a bare
+    # module/error word with no level in it, and bambu_mqtt.py gives every
+    # print_error entry a flat severity=3. Matching on the short code alone is
+    # the right shape for derive_failure_reason, not an omission.
+    "0300_8003": "spaghettiDetached",
+    "0C00_8042": "spaghettiDetached",
 }
 
 
@@ -575,11 +604,13 @@ def _hms_short_code(attr: int, code: int | str) -> str:
 
 
 def derive_failure_reason(status: str, hms_errors: list[dict] | None) -> str | None:
-    """Derive a human-readable failure_reason for an archived print.
+    """Derive the failure_reason key for an archived print.
 
-    Returns "User cancelled" for cancelled/aborted prints; for failed prints,
-    returns the first matching reason from _HMS_FAILURE_REASONS, or None when
-    no HMS code matches (don't guess — null is honest).
+    Returns "userCancelled" for cancelled/aborted prints; for failed prints,
+    returns the first matching key from _HMS_FAILURE_REASONS, or None when
+    no HMS code matches (don't guess — null is honest). The keys are the
+    archive editor's vocabulary (_FAILURE_REASON_KEYS in print_log.py) and are
+    translated at render time.
     """
     if status in ("aborted", "cancelled"):
         return "userCancelled"

+ 152 - 1
backend/tests/unit/test_failure_reason_derivation.py

@@ -7,9 +7,27 @@ matched by the old broad heuristic (`module == 0x0C → Layer shift`).
 
 from __future__ import annotations
 
+import re
+from pathlib import Path
+
 import pytest
 
-from backend.app.main import derive_failure_reason
+from backend.app.main import _HMS_FAILURE_REASONS, derive_failure_reason
+
+REPO_ROOT = Path(__file__).resolve().parents[3]
+_EN_TS = REPO_ROOT / "frontend" / "src" / "i18n" / "locales" / "en.ts"
+_EDIT_ARCHIVE_MODAL = REPO_ROOT / "frontend" / "src" / "components" / "EditArchiveModal.tsx"
+
+# Dockerfile.test copies backend/, pyproject.toml and the requirements files and
+# nothing else, so frontend/ does not exist inside the test image and the two
+# tests below that read it have nothing to check. A source checkout always has
+# it and keeps those guards live on every test_backend.sh run.
+# frontend/package.json is present in every checkout and never in the image,
+# which is what the launcher config tests use to tell the two apart.
+_needs_the_frontend_tree = pytest.mark.skipif(
+    not (REPO_ROOT / "frontend" / "package.json").is_file(),
+    reason="frontend/ isn't shipped in the Docker test image; the guards run in native runs",
+)
 
 # ---------------------------------------------------------------------------
 # Status-based reasons (no HMS lookup needed)
@@ -99,6 +117,72 @@ def test_int_code_field_accepted() -> None:
     assert derive_failure_reason("failed", hms) == "layerShift"
 
 
+# ---------------------------------------------------------------------------
+# AI print monitoring (issue #2946)
+# ---------------------------------------------------------------------------
+
+
+def test_ai_spaghetti_detection_is_classified() -> None:
+    """0300_8003 is what the onboard AI raises when it halts a print for spaghetti.
+
+    Taken from the archive that reported this: the printer sent
+    ``attr=50364419, code='0x8003'``, which is 0x0300_8003, and the archive was
+    written with failure_reason=None because the map had no row for it. The text
+    for the code was already in the tree twice — hms_errors.py and
+    HMSErrorModal.tsx — so this was a missing key, not a missing meaning.
+
+    The dict is the one bambu_mqtt.py builds for it: attr holding the whole
+    word is the print_error branch, which is also where severity=3 comes from.
+    That 3 is a constant for every print_error entry, not a level the printer
+    sent, and nothing here depends on it.
+    """
+    hms = [{"code": "0x8003", "attr": 50364419, "module": 0x03, "severity": 3}]
+    assert derive_failure_reason("failed", hms) == "spaghettiDetached"
+
+
+def test_the_ai_monitors_other_code_is_classified_too() -> None:
+    """0C00_8042 is the same event reported from the motion-controller module.
+
+    hms_errors.py documents it as "The AI print monitor has detected a spaghetti
+    defect", so it is a full short code with a published meaning rather than the
+    module-0x0C guessing the map header rules out.
+    """
+    hms = [{"code": "0x8042", "attr": 0x0C00_0000, "module": 0x0C}]
+    assert derive_failure_reason("failed", hms) == "spaghettiDetached"
+
+
+@pytest.mark.parametrize(
+    ("short_code", "attr", "code"),
+    [
+        # "Possible spaghetti failure was detected." — a warning about a print
+        # that is still running, not a print that stopped.
+        ("0C00_C004", 0x0C00_0000, "0xC004"),
+        # AI monitoring, but a filament pile-up in the waste chute.
+        ("0300_800A", 0x0300_0000, "0x800A"),
+    ],
+)
+def test_the_ai_monitors_warnings_are_left_unclassified(short_code: str, attr: int, code: str) -> None:
+    """Being AI monitoring is not the criterion — halting the print is.
+
+    Both of these are in hms_errors.py and both would be easy to sweep in with
+    the two that are mapped. Neither means the print failed, and a wrong reason
+    on an archive is worse than none, so they stay out and this says so.
+    """
+    assert short_code not in _HMS_FAILURE_REASONS
+    hms = [{"code": code, "attr": attr, "module": attr >> 24}]
+    assert derive_failure_reason("failed", hms) is None
+
+
+def test_ai_detection_and_its_runout_neighbour_are_distinct() -> None:
+    """0300_8003 and 0300_8004 are one hex digit apart and arrive by the same
+    path. The runout side was already mapped; this keeps them from drifting into
+    each other."""
+    ai = [{"code": "0x8003", "attr": 0x0300_0000, "module": 0x03}]
+    runout = [{"code": "0x8004", "attr": 0x0300_0000, "module": 0x03}]
+    assert derive_failure_reason("failed", ai) == "spaghettiDetached"
+    assert derive_failure_reason("failed", runout) == "filamentRunout"
+
+
 # ---------------------------------------------------------------------------
 # One vocabulary in storage (issue #2974)
 # ---------------------------------------------------------------------------
@@ -151,3 +235,70 @@ def test_the_stale_paths_write_a_key_the_editor_will_not_discard() -> None:
     assert text.count('failure_reason = "noStatusUpdate"') == 2
     assert "Stale - print likely cancelled" not in text
     assert "Stale - reconciled after reconnect" not in text
+
+
+# ---------------------------------------------------------------------------
+# The vocabulary spans two languages, and only a comment says so
+# ---------------------------------------------------------------------------
+
+
+def _keys_the_dropdown_offers() -> set[str]:
+    """The `FAILURE_REASON_KEYS` array exported from EditArchiveModal.tsx."""
+    source = _EDIT_ARCHIVE_MODAL.read_text(encoding="utf-8")
+    block = re.search(r"export const FAILURE_REASON_KEYS = \[(.*?)\] as const;", source, re.S)
+    assert block is not None, f"no FAILURE_REASON_KEYS array in {_EDIT_ARCHIVE_MODAL}"
+    return set(re.findall(r"'([^']+)'", block.group(1)))
+
+
+def _keys_the_frontend_can_translate() -> set[str]:
+    """Every key in the `editArchive.failureReasons` block of en.ts."""
+    source = _EN_TS.read_text(encoding="utf-8")
+    # Up to the brace that closes the block on its own line, so a `}` inside a
+    # label (an ICU placeholder, say) does not cut the block short.
+    block = re.search(r"failureReasons:\s*\{(.*?)^\s*\}", source, re.S | re.M)
+    assert block is not None, f"no failureReasons block in {_EN_TS}"
+    # Either quote: a label with an apostrophe is written double-quoted in TS.
+    return set(re.findall(r"^\s*(\w+):\s*['\"]", block.group(1), re.M))
+
+
+@_needs_the_frontend_tree
+def test_the_backend_vocabulary_matches_the_one_the_frontend_offers() -> None:
+    """The only thing holding the two lists together is a comment asking nicely.
+
+    ``_FAILURE_REASON_KEYS`` in api/routes/print_log.py gates every write, and
+    its own comment says "Keep these two lists in sync if the EditArchiveModal
+    options ever change". Nothing enforces it, and the drift is silent in both
+    directions: a key the frontend offers but the backend rejects turns a save
+    into a 400 the modal has no surface for, and a key the backend accepts but
+    the dropdown omits is a value the editor discards the next time anyone
+    opens that archive.
+
+    Asserting one Python literal against another cannot see either — the other
+    end lives in TypeScript, so the check has to read it.
+    """
+    from backend.app.api.routes.print_log import _FAILURE_REASON_KEYS
+
+    offered = _keys_the_dropdown_offers()
+    assert offered, "the FAILURE_REASON_KEYS array parsed as empty; the regex has gone stale"
+
+    # "" is the backend's "clear the classification" value; the dropdown spells
+    # that as its own placeholder option rather than a key, so it is not drift.
+    backend_keys = set(_FAILURE_REASON_KEYS) - {""}
+
+    assert backend_keys == offered, (
+        f"backend-only: {sorted(backend_keys - offered)}, frontend-only: {sorted(offered - backend_keys)}"
+    )
+
+
+@_needs_the_frontend_tree
+def test_every_offered_key_has_english_text() -> None:
+    """A key with no en.ts entry renders as the raw key in the dropdown.
+
+    The parity script covers the other 13 locales against en.ts, so en.ts is the
+    one end of this that nothing else checks.
+    """
+    translatable = _keys_the_frontend_can_translate()
+    assert translatable, "the failureReasons block parsed as empty; the regex has gone stale"
+
+    untranslated = sorted(_keys_the_dropdown_offers() - translatable)
+    assert not untranslated, f"offered by the dropdown with no editArchive.failureReasons entry: {untranslated}"