#7 Implement line markers

[tpwalke2]

What

Adds LINE marker groups: signs sharing a group's prefix and label become ordered points of one BlueMap line marker, alongside the existing POI groups.

Why

Issue #7 — users want to draw trails/roads/borders from signs, not just point markers.

Changes

• New MarkerGroupType.LINE config option, with lineWidth/lineColor fields (POI-only fields on a LINE group, or vice versa, log a warning instead of erroring).
• LineGroupResolver groups signs by (group, label) and orders points by SignEntry.createdAtMillis (new field, backfilled via V3→V4 sign-file migration).
• New marker actions SetLineMarkerAction, RemoveLineMarkerAction, GroupTransitionMarkerAction (replaces ChangeGroupMarkerAction) built through ActionFactory; BlueMapAPIConnector renders/updates/removes line markers.
• SignManager's add/update/remove/move-between-groups/type-flip decisions are unified into a Representation diff, resolved by a new extracted, unit-tested SignTransitionResolver (used both for live sign changes and for /bluemap reload re-diffing every cached sign against the new config).
• Fixes: POI label edits on an unchanged group now go through remove+add instead of a direct update; a reload regression where redispatch stopped working after a config swap.
• README.md/AGENTS.md/agent-context/ docs updated for LINE groups and the new transition-resolver architecture.
• Version bump: 26.2-0.18.0 → 26.2-0.19.0.

Testing

• ./gradlew test — new/updated coverage: SignTransitionResolverTest, LineGroupResolverTest, ColorUtilsTest, Version4ConverterTest, plus expanded ActionFactoryTest/ConfigProviderTest.
• Manual: ./gradlew runServer, place several signs with a LINE-type prefix and the same label, confirm the line marker appears at 2+ signs, updates on point add/remove, and disappears below 2; verify /bluemap reload picks up group edits without orphaning markers.

[Copilot]

Pull request overview

This PR adds support for LINE marker groups to BlueMap Sign Markers, allowing multiple signs that share a prefix and label to render as an ordered polyline (in placement order) on BlueMap. It extends the existing POI-marker pipeline with new marker-group configuration, marker actions, persistence versioning, and a representation-diff transition resolver to unify add/update/remove/move/type-flip behavior.

Changes:

• Introduces MarkerGroupType.LINE with lineWidth/lineColor config fields, plus validation/defaulting and updated user-facing docs.
• Adds line-marker rendering end-to-end: membership resolution + ordering (createdAtMillis), new marker identifiers/actions, and BlueMap connector support.
• Updates sign persistence to V4 by adding createdAtMillis with V3→V4 migration logic and expanded unit test coverage.

Reviewed changes

Copilot reviewed 84 out of 84 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignLinesParserTest.java	Updates helper MarkerGroup construction for new lineWidth/lineColor fields.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignEntryTest.java	Extends equality/hashCode/withKey coverage for new createdAtMillis.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignEntryHelperTest.java	Updates SignEntry/MarkerGroup creation for new persisted/config fields.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/SignRegionPartitionerTest.java	Updates fixtures to include createdAtMillis.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/RegionShardedSignEntryWriterTest.java	Updates expected sign-file version to V4 and fixtures to include createdAtMillis.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/RegionShardedSignEntryLoaderTest.java	Updates fixtures to include createdAtMillis.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/VersionedFileSignEntryLoaderTest.java	Adds/updates coverage for V3→V4 migration and V4 passthrough behavior + backups.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/Version4ConverterTest.java	New tests for V3→V4 conversion behavior (createdAtMillis backfill).
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/Version3ConverterTest.java	Updates config fixtures for MarkerGroup signature changes.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/Version1SignEntryLoaderTest.java	Updates fixtures for MarkerGroup signature changes.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/LegacySignFileMigratorTest.java	Updates migration tests to expect V4 legacy behavior and createdAtMillis fixtures.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/ParsingContextTest.java	Updates config fixtures for MarkerGroup signature changes.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/LineGroupResolverTest.java	New tests for line membership filtering, ordering, and deterministic tie-breaking.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/reactive/ReactiveQueueTest.java	Updates documentation reference to GroupTransitionMarkerAction/applySingleAction.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/markers/MarkerSetIdentifierCollectionTest.java	Updates config fixtures for MarkerGroup signature changes.
src/test/java/com/tpwalke2/bluemapsignmarkers/core/bluemap/actions/ActionFactoryTest.java	Expands factory tests for new line actions and new group-transition bundling behavior.
src/test/java/com/tpwalke2/bluemapsignmarkers/config/ConfigProviderTest.java	Adds/extends tests for LINE defaults/validation and POI-with-line-fields warning-only behavior.
src/test/java/com/tpwalke2/bluemapsignmarkers/common/ColorUtilsTest.java	New tests for hex parsing/validation used by lineColor.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignTransitionResolver.java	New extracted transition-table resolver for representation diffs and action dispatch decisions.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignManager.java	Replaces ad-hoc POI logic with representation diff + transition-table dispatch; updates reload diffing.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignHelper.java	Adds createdAtMillis initialization when creating new SignEntry from live sign state.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignEntry.java	Adds persisted createdAtMillis and updates equals/hashCode/withKey/toString.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/SignFileVersions.java	Adds V4 sign-file version constant.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/RegionShardedSignEntryWriter.java	Updates writer to emit V4 sign files.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/models/SignEntryV3.java	New frozen legacy model for pre-createdAtMillis entries to support V3→V4 conversion.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/VersionedFileSignEntryLoader.java	Adds V3→V4 migration path and threads file mtime into createdAtMillis backfill.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/Version4Converter.java	New converter implementing V3→V4 backfill logic.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/Version3Converter.java	Adjusts V2→V3 conversion to produce frozen SignEntryV3 model.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/Version1SignEntryLoader.java	Threads file mtime into V1→V4 migration via V3 + V4 converters.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/LineGroupResolver.java	New pure helper to compute line membership and deterministic ordering.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/markers/MarkerIdentifier.java	Implements generalized DispatchedMarkerIdentifier interface.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/markers/MarkerGroupType.java	Adds new LINE group type.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/markers/MarkerGroup.java	Adds lineWidth/lineColor fields and updates defaults and withType.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/markers/LinePoint.java	New value type representing one point in a rendered line.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/markers/LineMarkerIdentifier.java	New content-keyed identifier for line markers (line: + label).
src/main/java/com/tpwalke2/bluemapsignmarkers/core/markers/DispatchedMarkerIdentifier.java	New identifier interface shared by POI and line marker identifiers.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/bluemap/BlueMapAPIConnector.java	Adds Set/Remove line marker handling, generalized identifiers, and reload detection fix.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/bluemap/actions/SetLineMarkerAction.java	New action carrying line details, points, width/color, and log-only “first appearance” flag.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/bluemap/actions/RemoveLineMarkerAction.java	New action for removing a line marker by content-keyed id.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/bluemap/actions/MarkerAction.java	Generalizes marker identifier type and removes POI-only coordinate helpers.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/bluemap/actions/GroupTransitionMarkerAction.java	New action bundling multiple effects into one dispatched unit to avoid ordering hazards.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/bluemap/actions/ChangeGroupMarkerAction.java	Removes old fixed-shape change-group action in favor of generalized transition effects.
src/main/java/com/tpwalke2/bluemapsignmarkers/core/bluemap/actions/ActionFactory.java	Adds factory methods for line actions and updates group-change to return a bundled transition.
src/main/java/com/tpwalke2/bluemapsignmarkers/config/persistence/LoadingMarkerGroupV2.java	Adds nullable lineWidth/lineColor to config loading model.
src/main/java/com/tpwalke2/bluemapsignmarkers/config/persistence/LoadingBMSMConfigV2.java	Propagates new loading-model fields through defaults.
src/main/java/com/tpwalke2/bluemapsignmarkers/config/ConfigProvider.java	Adds LINE field resolution/validation and warnings for POI/LINE field mismatches.
src/main/java/com/tpwalke2/bluemapsignmarkers/common/ColorUtils.java	New utility for parsing/validating hex colors used by line markers.
README.md	Documents LINE marker groups, lineWidth/lineColor, and adds a LINE example.
gradle.properties	Bumps mod_version to 26.2-0.19.0.
AGENTS.md	Updates architecture documentation for LINE groups, transition resolver, and V4 persistence.
agent-context/reviews/adversarial-review-feature-tpwalke2-7-line-markers-2026-08-14.md	Adds an internal adversarial review note for the feature branch.
agent-context/README.md	Updates documentation index to reflect LINE feature additions and new version chain.
agent-context/plans/unit-testing-plan.md	Adjusts internal plan doc paths/wording for current repo structure.
agent-context/plans/unit-test-coverage-gaps.md	Updates internal references and coverage notes for current structure.
agent-context/plans/stale-prefix-orphaned-signs-fix.md	Adds plan doc for addressing stale-prefix orphaned signs.
agent-context/plans/signmanager-transition-resolver-test-coverage-plan.md	Adds plan doc for extracting/testing the transition table (now implemented).
agent-context/plans/sign-storage-refactor-options.md	Updates internal references/paths to match current repo layout.
agent-context/plans/reload-marker-redispatch-fix.md	Adds plan doc describing reload redispatch behavior and proposed fix strategy.
agent-context/plans/region-sharded-sign-persistence-plan.md	Updates internal references/paths to match current repo layout.
agent-context/plans/marker-group-reload-followups-todo.md	Updates internal references/paths; tracks follow-up items.
agent-context/plans/marker-group-config-reload-plan.md	Updates internal references/paths to match current repo layout.
agent-context/plans/line-marker-noop-dispatch-fix.md	Adds plan doc addressing prior noop-redispatch and O(N²) reload concerns.
agent-context/plans/html-detail-escaping-plan.md	Updates internal references/paths to match current repo layout.
agent-context/plans/fabric-api-module-split-plan.md	Updates internal references/paths to match current repo layout.
agent-context/plans/codebase-review-2026-07-11.md	Updates internal references/paths to match current repo layout.
agent-context/plans/chunk-load-sign-reconciliation-plan.md	Updates internal references/paths to match current repo layout.
agent-context/context/testing.md	Updates test inventory/coverage docs for new LINE + V4 + new unit tests.
agent-context/context/config-and-persistence.md	Updates config/persistence docs for LINE fields and V4 version chain/migrations.
agent-context/context/architecture.md	Updates architecture docs for LINE feature surface area and module layout.
.scratch/line-markers/issues/09-agents-md-update.md	Adds scratch issue doc tracking AGENTS.md updates for LINE feature.
.scratch/line-markers/issues/08-version-bump-verification.md	Adds scratch issue doc tracking version bump + manual verification checklist.
.scratch/line-markers/issues/07-config-reload-fix-id-scheme-change.md	Adds scratch issue doc tracking reload behavior when id scheme changes.
.scratch/line-markers/issues/06-signmanager-transition-table.md	Adds scratch issue doc tracking transition-table implementation work.
.scratch/line-markers/issues/05-bluemapapiconnector-line-rendering.md	Adds scratch issue doc tracking BlueMap line marker rendering work.
.scratch/line-markers/issues/04-marker-action-model-line-support.md	Adds scratch issue doc tracking action/identifier model changes.
.scratch/line-markers/issues/03-linegroupresolver.md	Adds scratch issue doc tracking LineGroupResolver implementation work.
.scratch/line-markers/issues/02-persistence-v3-v4-createdatmillis.md	Adds scratch issue doc tracking persistence V3→V4 createdAtMillis migration.
.scratch/line-markers/issues/01-config-marker-group-type-line.md	Adds scratch issue doc tracking config/type additions for LINE groups.
.scratch/codebase-review-followups/issues/12-configprovidertest-line-field-coverage.md	Adds scratch follow-up doc tracking new config test coverage for LINE fields.
.scratch/codebase-review-followups/issues/11-actionfactorytest-line-action-coverage.md	Adds scratch follow-up doc tracking new ActionFactory test coverage for line actions.
.scratch/codebase-review-followups/issues/10-reload-clear-and-replay-orphans-markers-on-id-scheme-change.md	Updates/records follow-up doc about reload behavior when marker id scheme changes.

Suppressed comments (1)

src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/VersionedFileSignEntryLoader.java:69

• Returning null when a .v3.bak backup cannot be created causes RegionShardedSignEntryLoader to drop the entire region file’s entries for this session. That can lead to markers disappearing and (on save) the original file being quarantined as “stale”. Consider continuing to load/convert the entries even if backup creation fails, and keep this as an error-level warning about potential overwrite risk on next save.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 84 out of 84 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/VersionedFileSignEntryLoader.java:100

• This helper’s comment still refers to “v2-to-v3 conversion”, but the loader now converts V2�V3 and then V3�V4 (adding createdAtMillis). Keeping this comment accurate matters because it explains why per-entry failures are isolated rather than aborting the whole load.
  src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignTransitionResolver.java:115
• The comment on lineJoinAction assumes a same-group/label recompute can only happen when a line marker already existed, but isReload=true also forces recomputes to recreate markers after a BlueMap reload. That makes the comment inaccurate and can mislead future changes around the isFirstAppearance flag (which is log-only).
  src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignTransitionResolver.java:147
• joinLineDetail joins member details using the platform line separator, but HtmlUtils.toHtmlDetail() only replaces \n with <br>. On Windows (\r\n), this will leave stray \r characters in the rendered HTML detail. Normalizing to \n here avoids OS-dependent output for line marker detail.

[Copilot]

Pull request overview

Copilot reviewed 84 out of 84 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/main/java/com/tpwalke2/bluemapsignmarkers/core/bluemap/actions/GroupTransitionMarkerAction.java:16

• GroupTransitionMarkerAction passes null to the MarkerAction base constructor. While the current BlueMapAPIConnector path special-cases this action, a null identifier makes the type fragile (e.g., any future logging/dispatch path that calls getMarkerIdentifier().parentSet() will NPE). It’s safer to use the first effect’s marker identifier and implicitly require a non-empty effects list.
  .scratch/line-markers/issues/08-version-bump-verification.md:44
• The PR description says manual runServer verification was performed, but this tracking doc still marks the full manual verification checklist as “not yet performed”. Either the checklist needs to be updated to reflect completed testing, or the PR description’s testing section should be adjusted so reviewers can rely on it.

**Status:** in progress

- [x] `README.md` reviewed end to end for the `LINE` feature — `type`/`lineWidth`/`lineColor` documented, example
      config includes a `LINE` group, prose reads coherently
- [x] `mod_version` bumped in `gradle.properties` (`26.2-0.18.0` → `26.2-0.19.0`)
- [x] `./gradlew test` and `./gradlew build` pass; `LineGroupResolverTest`, `ColorUtilsTest`,
      `Version4ConverterTest` all present and passing
- [ ] All 7 manual verification scenarios above pass — requires interactive `runServer` testing, not yet performed
- [ ] Known limitations section of the spec (no reordering, prefix-rename needs live re-edit, no closed/filled
      shapes) still accurate — no surprises found during manual testing that should be added there (pending the
      manual pass above)

[Copilot]

Pull request overview

Copilot reviewed 84 out of 84 changed files in this pull request and generated no new comments.

Suppressed comments (7)

src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignManager.java:147

• removeByKey() always copies the entire signCache via getAllSigns(), even though the full sign list is only needed for LINE transitions (to recompute remaining members). For POI removals this is unnecessary O(N) work.
  agent-context/plans/region-sharded-sign-persistence-plan.md:27
• This references ase-review-2026-07-11.md, but that file doesn’t exist in the repo (the codebase review doc is codebase-review-2026-07-11.md). This looks like a typo that will break the cross-reference.
  agent-context/plans/unit-test-coverage-gaps.md:54
• This references ase-review-2026-07-11.md, which doesn’t exist in the repo (the codebase review doc is codebase-review-2026-07-11.md). Likely a typo that breaks the reference.
  agent-context/plans/marker-group-config-reload-plan.md:166
• This references ase-review-2026-07-11.md, but that file doesn’t exist in the repo (the codebase review doc is codebase-review-2026-07-11.md). This appears to be a typo.
  src/main/java/com/tpwalke2/bluemapsignmarkers/core/bluemap/BlueMapAPIConnector.java:248
• LINE markers are created without applying the configured minDistance/maxDistance from the MarkerGroup, so those fields have no effect for LINE groups (unlike POI markers, which set them on the built marker).
  src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignManager.java:132
• addOrUpdateSign() always copies the entire signCache via getAllSigns(), even when both the old/new representations are POI (or NONE) and the transition logic will never consult the full sign list. This makes every sign load/edit/removal pay an O(N) allocation cost unnecessarily.

This issue also appears on line 147 of the same file.
agent-context/context/config-and-persistence.md:138

• This persistence doc says VersionedFileSignEntryLoader’s V2/V3 branches abort migration (return null) when the .bak copy fails, but the current implementation logs and continues returning converted entries in-memory (see VersionedFileSignEntryLoader.loadSignEntries). The doc should match the real behavior so operators know what to expect on backup failure.

     post-migration, duplicating markers as "new" signs. `Version1SignEntryLoader`, the `V2` branch, and the `V3`
     branch of `VersionedFileSignEntryLoader` each back up the file before migrating (`.v1.bak`/`.v2.bak`/`.v3.bak`
     respectively) and **abort the migration if that backup fails** (ticket 02) rather than overwriting the
     original with no recoverable copy — `Version1SignEntryLoader` throws `IllegalStateException` (uncaught here,
     isolated instead by `LegacySignFileMigrator`'s own try/catch around the whole chain), while
     `VersionedFileSignEntryLoader`'s `V2`/`V3` branches log an error and return `null` (falling through to the V1
     loader, same as any other failure to load at that version).

[Copilot]

Pull request overview

Copilot reviewed 84 out of 84 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/main/java/com/tpwalke2/bluemapsignmarkers/config/ConfigProvider.java:229

• resolveLineColor validates/log-warns for malformed lineColor even when the group is POI (where lineColor is documented as ignored). This can produce noisy or misleading warnings for POI groups; it would be clearer to only validate lineColor when the group is LINE and otherwise just default the stored value.
  src/main/java/com/tpwalke2/bluemapsignmarkers/config/ConfigProvider.java:219
• resolveLineWidth validates/log-warns for non-positive lineWidth even when the group is POI (where lineWidth is documented as ignored). This can lead to confusing/duplicate warnings (one for type mismatch, one for non-positive width) and implies the value matters for POI groups. Consider only validating (and warning about invalid values) when type == LINE, and otherwise silently defaulting the stored value.

This issue also appears on line 225 of the same file.

[Copilot]

Pull request overview

Copilot reviewed 84 out of 84 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/VersionedFileSignEntryLoader.java:77

• The catch-all warning when versioned sign-file parsing fails drops the exception, which makes diagnosing bad/malformed files much harder. Include the exception and the file path in the log message.
  src/main/java/com/tpwalke2/bluemapsignmarkers/core/bluemap/BlueMapAPIConnector.java:201
• Line marker labels are derived from sign text (player-controlled) but are logged without sanitization, which can allow log injection/noise (e.g., embedded newlines/control characters). Sanitize the label the same way POI detail text is sanitized.

[Copilot]

Pull request overview

Copilot reviewed 84 out of 84 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignTransitionResolver.java:142

• Same issue as lineJoinAction: lineLeaveAction sets the line marker’s detail to the label, so line hover text won’t reflect member sign details after a member leaves (or during reload recomputes). Compute the line detail deterministically from the remaining members’ details.
  src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/SignTransitionResolver.java:127
• SetLineMarkerAction is built with the line marker’s detail set to rep.label(), which ignores the sign detail text entirely and makes the detail field effectively meaningless for line markers. The spec/docs indicate the line detail should be derived from member sign details (in point order) to give useful hover text.

This issue also appears on line 133 of the same file.