Merge pull request #156 from tpwalke2/feature/tpwalke2/144-context

Feature/tpwalke2/144 context

Author: tpwalke2

AGENTS.md

@@ -46,12 +46,19 @@ permission on PRs from forks by default in a public repo.
 
 ### Entry point and Minecraft hooks
 
-`BlueMapSignMarkersMod` (`DedicatedServerModInitializer`) wires three lifecycle hooks:
-- `SERVER_STARTING` → `SignProvider.loadSigns(...)` reads the world's persisted `signs.json`
+`BlueMapSignMarkersMod` (`DedicatedServerModInitializer`) wires four lifecycle hooks:
+- `SERVER_STARTING` → `SignProvider.loadSigns(...)` reads the world's persisted, region-sharded sign storage
+  (migrating a pre-sharding single `signs.json` on first boot after an upgrade)
 - `SERVER_STOPPING` → `SignProvider.saveSigns(...)` then `SignManager.stop()`
 - `BLOCK_ENTITY_LOAD` → for any loaded `SignBlockEntity`, calls `SignManager.addOrUpdate(...)`
+- `CHUNK_LOAD` → reconciles signs the mod's cache still knows about in a loading chunk against what's actually
+  there; if a tracked sign's block is gone (e.g. its region file was deleted/regenerated externally while
+  unloaded), calls `SignManager.remove(...)` for it. Addresses GitHub issue #110.
 
-The markers file path is per-world: `config/bluemapsignmarkers/<world-save-name>/signs.json`.
+Sign state is stored per-world, region-sharded: one JSON file per (dimension, 32x32-chunk region) under
+`{server_root}/bluemapsignmarkers/{level}/{dimension_namespace}/{dimension_path}/r.{regionX}.{regionZ}.json`. A
+pre-sharding single `config/bluemapsignmarkers/<world-save-name>/signs.json` is migrated in place on first boot
+after upgrading (backed up, not deleted).
 
 Two Mixins (`src/main/resources/bluemapsignmarkers.mixins.json`) catch the events the lifecycle hooks above can't:
 - `SignBlockEntityInject` injects into `SignBlockEntity.updateSignText` (a player edits a sign) → `SignManager.addOrUpdate`
@@ -62,13 +69,16 @@ Two Mixins (`src/main/resources/bluemapsignmarkers.mixins.json`) catch the event
 
 1. **`SignHelper`** builds a `SignEntry` (immutable snapshot: position/dimension key, player id, parsed front/back
    text) from a `SignBlockEntity`, running sign text through a `SignLinesParser` configured with the current
-   `MarkerGroup`s.
+   `MarkerGroup`s. `SignHelper.reloadParser()` rebuilds that parser from the current config; it's called on every
+   config reload (see below) so a sign parsed after `/bluemap reload` picks up an edited prefix/matchType.
 2. **`SignManager`** (singleton, holds a `ConcurrentMap<SignEntryKey, SignEntry>` cache of all known signs) is the
    decision point: given a new/updated `SignEntry`, it figures out whether this is an add, update, remove, or
    prefix-change (remove-then-add into a different marker group) relative to what's cached, then dispatches the
    corresponding `MarkerAction` (`AddMarkerAction`/`UpdateMarkerAction`/`RemoveMarkerAction`, built via
-   `ActionFactory`) to the BlueMap connector. It also implements `IResetHandler.reset()` to replay the whole sign
-   cache when BlueMap resets its state.
+   `ActionFactory`) to the BlueMap connector. It also implements `IResetHandler.reset()`, which BlueMap fires on
+   `/bluemap reload`: reloads config (`ConfigManager.reload()`, `SignHelper.reloadParser()`, rebuilding its
+   prefix→group lookup and `ActionFactory`) then replays the whole sign cache through the same add/update/remove
+   logic, so an edited marker-group's icon/offset/visibility/prefix takes effect without a server restart.
 3. **`BlueMapAPIConnector`** owns the `ReactiveQueue<MarkerAction>` and all actual BlueMap API calls. Because the
    BlueMap API is only available while BlueMap itself is enabled, actions are queued and only drained
    (`markerActionQueue.process()`) while `BlueMapAPI.getInstance().isPresent()`; `BlueMapAPI.onEnable`/`onDisable`
@@ -90,12 +100,17 @@ line to match the pattern (unlike `STARTS_WITH`, a regex prefix can't share its
 
 ### Sign persistence and versioning
 
-Sign state is persisted per-world as `signs.json`, wrapped in a `VersionedSignFile` envelope (`{version, data}`) so
-the format can evolve without breaking old saves. `SignProvider.loadSigns` tries, in order: the versioned-file loader
-(`VersionedFileSignEntryLoader`, handling V2→V3 migration via `Version3Converter`, and current V3 files directly),
-then falls back to `Version1SignEntryLoader` for pre-versioning files. When adding a new persisted field, bump
+Sign state is stored per-world, region-sharded (one file per dimension + 32x32-chunk region — see "Entry point"
+above), with each region file wrapped in a `VersionedSignFile` envelope (`{version, data}`) so the format can evolve
+without breaking old saves. `SignProvider.loadSigns` checks whether the storage root already has region files
+(`RegionShardedSignEntryLoader.hasSignData`); if so, it loads every region file the same version-aware way as
+before sharding — the versioned-file loader (`VersionedFileSignEntryLoader`, handling V2→V3 migration via
+`Version3Converter`, and current V3 files directly), falling back to `Version1SignEntryLoader` for pre-versioning
+files. If no region files exist yet, `LegacySignFileMigrator` reads a pre-sharding single `signs.json` (if present)
+through that same version chain, writes it out region-sharded, and backs up the legacy file (renamed, not deleted)
+only once every expected region file is confirmed on disk. When adding a new persisted field, bump
 `SignFileVersions` and add a loader/converter rather than changing an existing version's shape in place — old
-`signs.json` files on live servers must keep loading.
+region files (or a not-yet-migrated legacy `signs.json`) on live servers must keep loading.
 
 ### Adding a new marker/BlueMap action
 
@@ -107,15 +122,17 @@ compile.
 ### Testable vs. game-coupled code
 
 When adding logic, prefer keeping it in plain Java classes with no Minecraft/Fabric/BlueMap API types in their
-signature (like `SignLinesParser`, `SignEntryHelper`, `MarkerGroup`/`MarkerGroupMatchType`, `ConfigManager`/
-`ConfigProvider`, `ReactiveQueue`, the persistence loaders/converters, `ActionFactory`/`MarkerSetIdentifierCollection`)
+signature (like `SignLinesParser`/`ParsingContext`, `SignEntry`/`SignEntryHelper`, `SignChunkKey`/`SignChunkIndex`,
+`MarkerGroup`/`MarkerGroupMatchType`, `ConfigManager`/`ConfigProvider`, `ReactiveQueue`, `HtmlUtils`, `FileUtils`,
+the persistence loaders/converters (including `Version1SignEntryLoader`), `ActionFactory`/`MarkerSetIdentifierCollection`)
 — these can be unit tested directly (see `src/test/java/.../core/signs/SignLinesParserTest.java` for the pattern).
 Code that must reference game types (`SignHelper`, the mixins, `BlueMapSignMarkersMod`, `BlueMapAPIConnector`)
 should stay thin glue around the testable core, since it can only be verified manually via `runServer`.
 
-There is a documented, currently-unfixed gap here: `BlueMapAPIConnector` passes sign text into BlueMap's POI marker
-`detail` field unescaped, and BlueMap renders `detail` as raw HTML (unlike `label`, which BlueMap escapes itself) —
-see `plans/html-detail-escaping-plan.md` for the planned fix.
+`BlueMapAPIConnector` escapes sign text (`HtmlUtils.toHtmlDetail`, in `common`) before it reaches BlueMap's POI
+marker `detail` field — BlueMap renders `detail` as raw HTML (unlike `label`, which BlueMap escapes itself), and
+sign text is player-controlled, so this closes a live XSS vector. See `plans/html-detail-escaping-plan.md` for the
+design. Persisted sign data stays raw/unescaped; escaping happens only at this BlueMap API call site.
 
 ## Planning documents
 

agent-context/README.md

@@ -0,0 +1,36 @@
+# Agent Context — BlueMap Sign Markers
+
+BlueMap Sign Markers is a server-side Fabric mod (Java 25) for Minecraft. It watches in-game signs and, when a
+sign's text matches a configured prefix (e.g. `[poi]`), creates/updates/removes a corresponding marker on a
+[BlueMap](https://bluemap.bluecolored.de/) map. Signs are tracked persistently, sharded into one JSON file per
+(dimension, 32x32-chunk region) under `{server_root}/bluemapsignmarkers/{level}/`, so markers survive restarts.
+Multiple "marker groups" (prefix, match rule, icon, visibility) can be configured at once via
+`config/bluemapsignmarkers/BMSM-Core.json`.
+
+Human-facing docs already exist:
+- `README.md` (repo root) — end-user install/config instructions and the marker-group config format
+- `AGENTS.md` / `CLAUDE.md` (repo root) — AI agent guidance (build commands, high-level architecture, conventions)
+
+This `agent-context/` directory goes deeper than `AGENTS.md` on implementation specifics — exact decision logic,
+method-level behavior, data-shape history — so an agent doesn't have to re-derive it from source every time.
+
+## Documents
+
+| Document | What it covers |
+|----------|----------------|
+| `context/architecture.md` | Tech stack, directory map, package taxonomy, build/CI/publish tooling |
+| `context/core-pipeline.md` | Sign text → marker action: parsing state machine, `SignManager` decision logic, chunk-load sign reconciliation (`SignChunkKey`/`SignChunkIndex`), `ReactiveQueue`/`BlueMapAPIConnector` mechanics, marker ID/set identity scheme |
+| `context/config-and-persistence.md` | Marker-group config loading/migration (V1→V2), region-sharded sign persistence, per-region-file versioning (V1→V2→V3), legacy-file migration and backup-on-migrate behavior |
+| `context/testing.md` | Test infra, testable-vs-game-coupled split, CI test-result summarization, current coverage and known gaps |
+
+## Document Scopes
+
+| Document | Codebase paths watched |
+|----------|------------------------|
+| `context/architecture.md` | `build.gradle`, `gradle.properties`, `settings.gradle`, top-level dirs, `.github/workflows/` |
+| `context/core-pipeline.md` | `src/main/java/.../core/signs/` (excl. `persistence/`), `core/bluemap/`, `core/reactive/`, `core/markers/`, `mixin/`, `BlueMapSignMarkersMod.java` |
+| `context/config-and-persistence.md` | `src/main/java/.../config/`, `core/signs/persistence/`, `README.md` (config format section) |
+| `context/testing.md` | `src/test/java/`, `.github/workflows/build.yml`, `.github/workflows/publish.yml` |
+
+---
+*Last updated: 2026-07-21 | Verified against: 26.2-0.17.0 (72d4280)*

agent-context/context/architecture.md

@@ -0,0 +1,75 @@
+# Architecture
+
+## Tech stack
+
+| Layer | Technology |
+|-------|-----------|
+| Language | Java 25 (`sourceCompatibility`/`targetCompatibility` = `VERSION_25` in `build.gradle`) |
+| Mod platform | Fabric, `DedicatedServerModInitializer` (server-only, no client logic) |
+| Build | Gradle wrapper + Fabric Loom `1.17-SNAPSHOT` |
+| Target Minecraft | version pinned in `gradle.properties` (`minecraft_version`), currently `26.2` |
+| Fabric loader/API | `loader_version` / `fabric_api_version` in `gradle.properties` |
+| External integration | BlueMap API `blue_map_api_version` (`compileOnly` — provided by BlueMap itself at runtime), targeting BlueMap `blue_map_version` |
+| JSON | Gson, `Strictness.LENIENT` |
+| Logging | SLF4J, all loggers named via `Constants.MOD_ID` |
+| Testing | JUnit 5 (Jupiter), via `junit-bom:5.11.4` |
+| Publish target | Modrinth, via `com.modrinth.minotaur` Gradle plugin |
+| CI | GitHub Actions |
+
+All version numbers live in `gradle.properties` — never hardcode a version in `build.gradle` or source.
+`mod_version` follows `<minecraft_version>-<mod_semver>` (e.g. `26.2-0.17.0`).
+
+## Top-level layout
+
+```
+src/main/java/com/tpwalke2/bluemapsignmarkers/   mod source
+src/main/resources/                              fabric.mod.json, mixins config, icon asset
+src/test/java/com/tpwalke2/bluemapsignmarkers/   unit tests (plain-Java logic only)
+plans/                                           design/implementation plans, written before larger changes
+.github/workflows/                               build.yml (CI), publish.yml (manual Modrinth publish)
+build.gradle, gradle.properties                  build config and all version numbers
+README.md                                        end-user install + config-format docs
+AGENTS.md / CLAUDE.md                            AI agent guidance (source of truth CLAUDE.md defers to AGENTS.md)
+```
+
+## Package taxonomy
+
+| Package | Contents |
+|---------|----------|
+| `com.tpwalke2.bluemapsignmarkers` (root) | `BlueMapSignMarkersMod` (entrypoint, lifecycle hooks; implements `ServerPathProvider`), `Constants` (`MOD_ID`), `ServerPathProvider` (interface: `getMarkerStorageRoot(MinecraftServer): Path`, resolves `{server_root}/bluemapsignmarkers/{level}`) |
+| `common` | `FileUtils` — `createBackup`/`moveToBackup(originalPath, suffix, description)` (copy- vs. rename-based backup, used by config/sign migration code before overwriting or retiring an old-format file); `HtmlUtils` — `escape`/`toHtmlDetail`, escapes player-supplied sign text before it crosses into BlueMap's HTML-rendered marker `detail` field |
+| `config` | `ConfigManager` (lazy `volatile` singleton over the loaded config, `reload()` on first null `get()`), `ConfigProvider` (load/save/migrate `BMSM-Core.json`) |
+| `config.models` | `BMSMConfigV1` (legacy, single `poiPrefix` field), `BMSMConfigV2` (current runtime shape — `MarkerGroup[]`, ctor also seeds Gson defaults) |
+| `config.persistence` | `LoadingBMSMConfigV2` / `LoadingMarkerGroupV2` — a *nullable-boxed-field* mirror of `BMSMConfigV2`/`MarkerGroup` used only for Gson deserialization, so partially-specified user JSON can be detected and defaulted field-by-field in `ConfigProvider.convertToLoadedMarkerGroup` rather than Gson silently zeroing missing primitives |
+| `core` | `WorldMap` — just the `UNKNOWN` dimension-key sentinel used when a sign's `Level` is null |
+| `core.bluemap` | `BlueMapAPIConnector` (the only class that touches the BlueMap API), `IResetHandler` (callback interface `SignManager` implements to replay its cache on BlueMap reset) |
+| `core.bluemap.actions` | `MarkerAction` (abstract base, holds a `MarkerIdentifier`), `AddMarkerAction`/`UpdateMarkerAction`/`RemoveMarkerAction`, `ActionFactory` (builds actions, resolves `MarkerSetIdentifier`s) |
+| `core.markers` | `MarkerGroup` (record: the config unit), `MarkerGroupMatchType` (`STARTS_WITH`/`REGEX`), `MarkerGroupType` (currently only `POI`), `MarkerIdentifier`/`MarkerSetIdentifier`/`MarkerSetIdentifierCollection` (marker identity + dedup) |
+| `core.reactive` | `ReactiveQueue<T>` (generic buffer-while-unavailable queue) + its three functional-interface callbacks (`ShouldRunCallback`, `MessageProcessorCallback<T>`, `MessageProcessorErrorCallback`) — see `core-pipeline.md` |
+| `core.signs` | `SignEntry`/`SignEntryKey` (immutable sign snapshot + position/dimension key), `SignHelper` (game-coupled: builds `SignEntry` from a `SignBlockEntity`), `SignEntryHelper` (plain-Java: derives label/detail/prefix from a `SignEntry`), `SignLinesParser`/`ParsingContext`/`SignLinesParseResult` (plain-Java parsing state machine), `SignManager` (singleton decision point — cache + dispatch), `SignChunkKey`/`SignChunkIndex` (plain-Java chunk-position lookup backing chunk-load sign reconciliation, see `core-pipeline.md` §4) |
+| `core.signs.persistence` | `SignProvider` (load/save, dispatches to the region-sharded loader/writer or migration), `SignFileVersions` (`V1`/`V2`/`V3` — per-region-file schema version, unchanged by sharding), `VersionedSignFile` (`{version, data}` envelope), `SignRegionKey` (record: dimension + region coords; `forPosition` does the `floorDiv` region math, `relativeFilePath` builds the `{namespace}/{path}/r.{x}.{z}.json` path), `SignRegionPartitioner` (groups `SignEntry`s by `SignRegionKey`), `RegionShardedSignEntryWriter` (writes each region's file; region files that dropped to zero signs are quarantined with a `.stale` suffix, not deleted, unless any region write failed — in which case quarantine is skipped entirely, to avoid mistaking a load failure for a genuine "signs removed" case), `LegacySignFileMigrator` (one-shot: reads the pre-sharding single file through the existing V1/V2/V3 chain, writes it out sharded, backs up the legacy file only once every expected region file is confirmed written to disk) |
+| `core.signs.persistence.loaders` | `VersionedFileSignEntryLoader` (tries versioned envelope; V2 → converts + backs up), `Version1SignEntryLoader` (pre-versioning fallback; also normalizes legacy `nether`/`end`/`overworld` map-id strings to real dimension identifiers — the identifiers are spelled out as literal strings rather than read from `net.minecraft.world.level.Level` statics, so the class has no Minecraft-type dependency and is fully unit-testable), `Version3Converter` (V2 model → current `SignEntry`), `RegionShardedSignEntryLoader` (walks a storage root's directory tree, loads each region file via `VersionedFileSignEntryLoader`, flattens to one list; `hasSignData(Path)` is the migration-trigger check) |
+| `core.signs.persistence.models` | `SignEntryV2`, `SignLinesParseResultV2`, `MarkerTypeV2` — frozen shapes representing the on-disk V2 format, kept solely so `Version3Converter` can still read old files; do not evolve these, they must stay byte-compatible with historical `signs.json` |
+| `mixin` | `SignBlockEntityInject` (injects `SignBlockEntity.updateSignText` TAIL), `AbstractBlockInject` (injects `BlockBehaviour.affectNeighborsAfterRemoval` HEAD) |
+
+## Import / module resolution
+
+Standard Java package imports. Fabric Loom remaps Minecraft/Fabric API artifacts at build time; `build.gradle`'s
+`loom { mods { "bluemapsignmarkers" { sourceSet sourceSets.main } } }` registers the main source set as the mod's
+dev source for `runServer`/`runClient`.
+
+## Build / CI / publish tooling
+
+- `./gradlew build` — compile, run tests, produce jar in `build/libs/`; fails on any test failure
+- `./gradlew test` — JUnit 5 suite only (`src/test/java`)
+- `./gradlew runServer` / `runClient` — dev Minecraft instance with the mod + BlueMap loaded
+- `.github/workflows/build.yml` — runs on push/PR to `main` and `releases/**`: `./gradlew test` → summarize JUnit
+  XML results to `$GITHUB_STEP_SUMMARY` (plain shell `sed`, no third-party reporter action — see `testing.md` for
+  why) → `./gradlew build` → upload `build/libs/` as an artifact
+- `.github/workflows/publish.yml` — manual `workflow_dispatch` only: runs tests first (failure blocks publish), then
+  `./gradlew modrinth -PbuildNumber=<run_number>` (alpha, from a branch dispatch) or the same with `-PisRelease`
+  (release, from a `v*` tag dispatch)
+
+---
+*Last updated: 2026-07-21 | Verified against: 26.2-0.17.0 (72d4280)*
+