Skip to content

fix(graph): skip malformed hallway records - #2271

Draft
fallenmi wants to merge 1 commit into
MemPalace:developfrom
fallenmi:codex/fix-malformed-hallway-records
Draft

fix(graph): skip malformed hallway records#2271
fallenmi wants to merge 1 commit into
MemPalace:developfrom
fallenmi:codex/fix-malformed-hallway-records

Conversation

@fallenmi

Copy link
Copy Markdown

What does this PR do?

  • validates decoded hallway records centrally in _load_hallways
  • keeps records with non-empty string IDs, wings, and entity pairs
  • validates optional counts and room lists when present while preserving unrelated legacy metadata
  • supports both the current envelope and legacy bare-list storage formats
  • skips unusable rows with one aggregated warning containing only the path and count
  • diagnoses unsupported root/envelope payload types without auto-wrapping or logging their contents
  • treats invalid UTF-8 and JSON decoder limit errors like the existing corrupt-file fallback
  • rejects records that cannot be safely saved as UTF-8

The root cause was that _load_hallways validated neither the envelope payload nor individual records. Consumers then performed dict access, entity sorting, count sorting, room iteration, and UTF-8 persistence on arbitrary decoded JSON values, so one malformed row could block every valid hallway behind it and surface through MCP as an internal tool error.

Read-only calls do not rewrite the sidecar. A later successful delete or recomputation naturally persists only the valid set through the existing atomic envelope write path. An unsuccessful delete preserves the original file bytes, and unsupported roots are not destructively repaired.

Closes #2270

How to test

  • .venv/bin/python -m pytest -q tests/test_hallways.py tests/test_cli_hallways.py — 40 passed
  • .venv/bin/python -m pytest -q tests/test_mcp_server.py -k hallway — 7 passed
  • .venv/bin/python -m pytest -q --ignore=tests/test_init.py --deselect=tests/test_mcp_server.py::TestStaleLibraryGate::test_startup_baseline_survives_module_reload — 4303 passed, 31 skipped
  • installed-package subprocess matrix from outside the checkout — 9 passed
  • .venv/bin/pre-commit run --all-files

The split full-suite invocation keeps cwd/import-path tests in their intended installed-package context; together the two pytest runs cover all 4312 executed tests.

Checklist

  • Tests pass
  • No hardcoded paths
  • Linter passes

@igorls igorls added bug Something isn't working area/kg Knowledge graph needs-rebase PR has merge conflicts with develop and needs rebase labels Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/kg Knowledge graph bug Something isn't working needs-rebase PR has merge conflicts with develop and needs rebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: malformed hallways.json records crash hallway operations

2 participants