Adding a region scoping page for region rollouts

[ITblacksheep]

This adds in a new javascript page to allow searching for what region scopes to put on a repeater.

[Jeymz]

Overall PR Review — Changes Requested

Thank you for the work on PR #187. The new Region Def tool is a valuable addition to CoreScope and provides a useful workflow for determining repeater region configurations from addresses, coordinates, and map interactions.

The overall implementation is well structured. In particular, the separation of geographic matching logic from the UI, the manifest-driven configuration, integration with existing navigation and theming, and the addition of unit and E2E tests are all positive improvements.

After reviewing the changes against the existing repository, I recommend requesting changes before merge, with a clear distinction between items that should be resolved in this PR and improvements that can be tracked separately.

🔴 Required Before PR Approval / Merge

1. Verify canonical repeater region identifiers

• The current manifest uses shapeISO for state identifiers, which produces values such as us-oh, while the documentation and test examples reference ohio.
• Confirm the expected identifier format supported by the repeater firmware/configuration.
• Ensure the generated commands, production manifest, documentation, and tests consistently use the correct identifiers.

2. Prevent incomplete region configurations from being presented as ready to apply

• The tool currently generates commands using successfully loaded layers even when other configured layers fail.
• This could result in operators applying incomplete region definitions without realizing a lookup was degraded.
• Add a prominent incomplete-result warning or prevent command generation when required layers fail.
• Clarify how intentionally unmatched regions should be handled versus actual loading failures.

3. Validate the shipped geographic datasets

• Current E2E tests use synthetic GeoJSON fixtures and therefore do not verify the geographic files included in this PR.
• Add a production-data integration or smoke test that validates the actual manifest and representative locations.
• At minimum, verify expected region assignments for CVG, DAY, IND, and FWA, along with representative macro, mesh, and state matches.
• Confirm that missing meshregion properties in certain states are intentional and accurately reflect the planned rollout.

4. Confirm CI validation and test results

• Ensure the new Region Def tests pass within the repository's standard CI workflow.
• Verify that E2E failures correctly propagate to the overall CI result.
• Confirm that the shipped manifest and referenced files load successfully.

🟡 Recommended Follow-Up Issues / Future Backlog

These items should not block this PR once the correctness and operational safeguards above are addressed, assuming no significant performance or security issue is discovered during validation.

1. Optimize GeoJSON loading and rendering

• Cache shared GeoJSON requests rather than downloading the same datasets separately for multiple layers.
• Consider polygon simplification and rendering optimizations for large geographic files.
• Profile mobile/browser performance and create a separate optimization issue if necessary.

2. Strengthen manifest and geometry validation

• Add schema validation for layer definitions and canonical identifiers.
• Expand coverage for malformed polygons, invalid coordinates, duplicate region identifiers, and groups spanning multiple geographic files.
• Consider formal support for required versus optional layers.

3. Improve frontend maintainability

• Consolidate the duplicated theme-aware map tile switching logic with the existing map.js implementation.
• Consider lazy-loading the Region Def page assets.
• Tighten route matching and manifest-relative path validation.

4. Improve operator experience and accessibility

• Enhance mobile layouts and handling of long results or warning messages.
• Consider copy-to-clipboard functionality for generated commands.
• Improve visual distinctions between overlapping geographic regions.

5. Expand documentation and data governance

• Document the source, ownership, update process, and accuracy expectations for geographic boundaries.
• Align documentation examples with the actual production manifest.
• Clarify privacy behavior relating to geocoding and map tile requests.
• Document the procedures for updating region definitions and troubleshooting missing coverage.

Final Assessment

Recommendation: Request Changes — Approve after the required items are addressed.

The overall architecture and feature direction are solid, and I do not believe the optional refactoring or performance improvements should unnecessarily delay delivery.

However, because this tool produces configuration commands intended for operational repeater deployments, correctness of region identifiers, completeness of generated configurations, and validation against real geographic data should be treated as merge requirements.

Once those issues are resolved and CI checks pass, I would be comfortable approving the PR, with the remaining improvements captured as separate, actionable backlog issues.

Thank you again for the implementation and test coverage. This is a strong foundation for the region rollout workflow.

[Jeymz]

Overall PR Review — Follow-Up Review — Changes Requested

Thank you for the substantial work addressing the feedback from the original review of PR #187. The improvements demonstrate a thoughtful response to the concerns raised, particularly around configuration correctness, incomplete results, geographic data validation, and test coverage.

After reviewing the latest implementation, associated tests, production geographic datasets, and CI results, I am pleased to report that the majority of our original requested changes have been satisfactorily addressed.

The implementation is now significantly more robust, and the original architectural approach remains sound. However, this follow-up review identified a few additional operational correctness concerns when comparing the generated repeater commands against the upstream MeshCore firmware implementation.

My recommendation remains requesting changes before merge, although the remaining work is considerably narrower than in the original review.

✅ Previously Requested Changes — Addressed

1. Canonical repeater region identifiers — Substantially Addressed

• The production manifest now explicitly defines canonical region codes, including us-midwest, oki, us-oh, and cvg.
• Manifest validation prevents unrecognized region codes from silently appearing in generated configurations.
• Documentation and production-data tests are substantially more consistent with the actual implementation.
• A final firmware-compatibility validation is identified below.

2. Prevent incomplete region configurations — Addressed

• Required-layer failures now prevent incomplete commands from being generated.
• Optional-layer failures produce prominent warnings identifying missing information.
• Full-coverage declarations allow the application to distinguish unexpected geographic gaps from intentional exclusions.
• Error handling now covers missing files, malformed responses, timeouts, and unusable geographic data.

3. Validate shipped geographic datasets — Addressed

• Production-data integration tests now exercise the actual deployed GeoJSON files and manifest.
• Representative locations for CVG, DAY, IND, and FWA are covered.
• Additional regression tests verify macro, mesh, state, and metropolitan assignments, including state boundaries and intentionally excluded areas.
• Geographic integrity checks validate canonical codes, polygon structure, and overlapping regions.

4. Confirm CI validation and test results — Addressed

• The latest CI pipeline completed successfully.
• Go build/unit tests and Playwright E2E tests passed.
• The Region Def E2E suite reported 36 passing tests with no failures.
• E2E execution uses pipefail to preserve failure reporting.
• Production geographic validation and known-point assertions passed.

These changes satisfactorily resolve the principal correctness and testing concerns from the original review.

🔴 Required Before PR Approval / Merge

1. Correct repeater command execution and persistence order

The current generated sequence executes region save before region allowf *.

According to the upstream MeshCore firmware implementation, region save explicitly invokes saveRegions(), whereas region allowf modifies the region's flood-permission flags without independently persisting the change.

Reference: MeshCore CommonCLI.cpp — lines 1044–1056

As a result, a flood-permission change applied after region save may not survive a repeater restart.

Please generate the commands in the following order:

region def us-midwest oki us-oh cvg
region allowf *
region save

This ensures that all intended region configuration changes are applied before the final persistence operation.

2. Align canonical region identifier validation with firmware constraints

• The current CODE_RE validation accepts underscores and identifiers up to 64 characters.
• MeshCore firmware imposes more restrictive region-name requirements than the application's current validator.
• Please align the permitted characters and maximum identifier length with the supported firmware implementation.
• Also validate the total generated region def command length against the firmware's command-input limits.
• Add regression tests verifying that identifiers and commands exceeding those constraints are rejected rather than presented as valid repeater configurations.

3. Clarify existing-region behavior and wildcard flood permissions

• MeshCore's region def command does not automatically clear all previously configured regions.
• Operators applying a generated configuration may therefore retain obsolete region entries unless the existing configuration is inspected and cleaned up.
• Please document this behavior prominently in the operator instructions, including how to verify the resulting region configuration.
• Additionally, confirm that automatically including region allowf * is intentional for the intended repeater deployment policy, since it enables wildcard/unscoped flooding.
• These behaviors should be explicitly communicated so operators understand the effect of applying the generated commands.

🟡 Recommended Follow-Up Issues / Future Backlog

The following improvements should not block this PR once the remaining operational correctness requirements are addressed.

1. Strengthen geographic data provenance and maintenance

• Document the authoritative sources, licensing, ownership, and verification dates for the geographic boundary files.
• Establish an update procedure for boundary revisions and associated known-point fixtures.
• Continue adding regression points as regional coverage expands.

2. Improve asynchronous lookup handling

• Prevent an outstanding geocoder request from replacing a newer location selected through map interaction or coordinate entry.
• Consider request cancellation or sequence identifiers to discard stale responses.
• Evaluate an application-controlled geocoding service if usage increases beyond the practical limits of direct browser requests.

3. Continue frontend and performance improvements

• Consider extracting shared theme-aware map tile switching into a reusable helper.
• Evaluate lazy-loading the Region Def page assets.
• Continue monitoring mobile performance as additional geographic datasets are introduced.
• Preserve the current improvements to responsive layouts and visual layer distinctions.

4. Further strengthen validation and CI safeguards

• Consider making the production manifest mandatory in CI rather than allowing geographic validation to succeed when the manifest is absent.
• Expand malformed geometry validation to cover additional nested-coordinate edge cases.
• Consider atomic output handling for geographic simplification so invalid intermediate data cannot be mistaken for successfully processed output.

5. Refine operator documentation and usability

• Clearly distinguish synthetic example manifests from the actual production configuration.
• Consider copy-to-clipboard functionality for the generated command sequence.
• Add a concise operator checklist covering existing regions, generated commands, flood permissions, persistence, and post-configuration verification.

Final Assessment

Recommendation: Request Changes — Approve after the remaining operational correctness items are addressed.

The author has made substantial and meaningful improvements since the original review. The four original merge requirements have either been addressed or substantially resolved through explicit configuration validation, safer command generation, production-data regression tests, and successful CI execution.

The remaining concerns are primarily focused on ensuring that the commands produced by the tool behave correctly when applied to actual MeshCore repeaters.

Because this feature produces configuration commands intended for operational deployments, the correct persistence sequence, firmware-compatible region identifiers, and clear handling of existing regions and flood permissions should be treated as merge requirements.

I do not believe the remaining performance optimizations, additional data-governance improvements, or optional frontend refinements should unnecessarily delay delivery. Those can be captured as separate, actionable follow-up issues.

Once the three remaining required items are addressed, the relevant tests are updated, and CI remains green, I would be comfortable approving PR #187, assuming the supported firmware confirms the intended region identifiers and command behavior.

Thank you again for responding thoroughly to the original review feedback. The implementation is now considerably stronger, and these final adjustments should help ensure the feature is not only technically correct within CoreScope but also reliable and safe for operators configuring real repeaters.

[Jeymz]

Overall PR Review — Third Review — Changes Requested

Thank you for the additional updates to PR #187. This revision resolves the remaining firmware-facing concerns from our previous review and makes the Region Def implementation substantially more deployment-aware.

I reviewed the latest changes at commit 85ca2a2, including the updated application logic, documentation, tests, and GitHub Actions results.

✅ Previously Requested Changes — Addressed

1. Correct repeater command persistence order — Addressed

• Commands now execute region allowf * before region save.
• Documentation explains why the save operation must occur last.
• Unit and E2E tests verify the corrected sequence.

2. Align generated commands with firmware limits — Addressed

• Canonical names are limited to 30 characters.
• Generated region def commands are checked against the firmware CLI line-length limit.
• Invalid or oversized commands are withheld rather than presented as valid configurations.
• Regression coverage has been added.

3. Document existing-region behavior and wildcard flooding — Addressed

• Documentation explains that region def does not clear existing regions.
• Operators are instructed to inspect and remove obsolete entries.
• The effect of region allowf * is explained, including how to retain a policy that denies unscoped flooding.

4. CI and application-level regression coverage — Addressed

• The latest CI/CD pipeline completed successfully.
• Region Def unit tests passed.
• The Region Def Playwright suite reported 36 passed and 0 failed.
• E2E failures continue to propagate to the workflow result.

These improvements resolve the three operational correctness items identified in the previous review.

🔴 Required Before PR Approval / Merge

1. Establish and validate the production region-data deployment path

The current revision changes the delivery model: layers.json, the production geographic boundary files, and production known-point expectations are no longer committed.

That may be the right choice if geographic datasets are meant to be environment-specific. However, it introduces a deployment-readiness gap:

• A clean application image does not contain the required region-layers/layers.json.
• The Region Def navigation link is still displayed.
• Without additional provisioning, the page cannot load its region definitions.
• The latest CI run skips both deployed-data unit checks and the production-data E2E smoke test.
• The Dockerfile does not itself define a mount or provisioning mechanism for external region data.

Requested resolution:

• Document and verify how real region data reaches the deployed application, such as a Docker bind mount, deployment-specific copy step, or another explicit mechanism.
• Ensure the tool clearly communicates an unconfigured state, or hide/disable it until data is available.
• Establish a required validation check for the geographic manifest and known-point expectations in the deployment workflow that supplies those files.
• Confirm actual rollout regions and expected command assignments against that deployed data.

This does not necessarily require committing production geography to the public repository. It does require a demonstrated, verifiable deployment path.

2. Restore unrelated analytics regression tests

The PR removes three tests from tests/unit/test-frontend-helpers.js for:

• Decimal-string channel hash formatting.
• HTML escaping of non-numeric hashes.
• Numeric ordering of channel hashes.

These removals are not part of the Region Def feature, and no corresponding analytics implementation change is included.

Please restore the tests or provide a specific explanation showing why their assertions are obsolete. Unrelated regression protection should not be reduced as part of this feature.

3. Reconcile deployment documentation with the actual repository and CI behavior

• .gitignore excludes region data that the documentation still instructs operators to commit.
• The documentation describes a dedicated strict production-data CI step that is not present in .github/workflows/deploy.yml.
• References to currently mapped OKI coverage need to distinguish an actual deployment from the synthetic examples.

Please align these instructions with the intended operator-managed data model so future deployments do not mistake a skipped validation for a successful production-data check.

🟡 Recommended Follow-Up Issues / Future Backlog

These should not block approval once the remaining deployment and regression-coverage issues are addressed.

1. Improve geocoder request sequencing

• Discard stale geocoding responses when the user selects a newer coordinate or map location.

2. Continue frontend maintainability work

• Consolidate shared theme-aware map tile behavior.
• Consider lazy-loading Region Def assets.

3. Expand data governance and verification

• Record boundary data provenance, licensing, and update history.
• Add additional known-point regression locations as rollout coverage grows.

4. Harden geographic preprocessing

• Consider atomic output publication following successful simplification and validation.

Final Assessment

Recommendation: Request Changes — Approve once deployment readiness and unrelated test preservation are addressed.

The Region Def implementation itself is now considerably stronger, and the firmware command-order, validation, and operator-safety concerns raised previously have been resolved.

The outstanding concern is no longer the command generator. It is whether the feature can be deployed with verified geographic data and whether the application communicates its configuration state accurately.

Additionally, removing unrelated analytics regression tests should not be part of delivering this tool.

Once the deployment contract is demonstrated, the documentation and checks reflect that contract, and the analytics tests are restored or justified, I would be comfortable approving this PR with the remaining optimizations tracked separately.

Thank you again for the continued improvements and for addressing the earlier feedback.