fix(engine): add_location defaults discovered=True so runtime-named places stay on the Atlas (#261/#371 follow-up)

[100yenadmin]

TL;DR

Reviewing an external suggestion, I verified a real regression the additive discovered field (PR #371 / #261) introduced on the runtime path: add_location never set discovered, so a place the DM names into the world mid-play serialized discovered=False and the Atlas hid it until the party visited it. Pre-#371 those places were visible. This restores that with discovered: bool = True as the default, keeps deliberate fog-of-war expressible via discovered=False, and adds a 4-test regression suite.

The bug (verified, file:line anchored)

• servers/engine/models.py:828 — Location.discovered: bool = False (added by fix(openworlds): atlas-seed — #261 #371 so fog-of-war seeds must opt in).
• servers/engine/server.py (add_location) — the new-Location constructor was Location(name=…, description=…, hex=coords, region=region, travel_times=tt) with no discovered= → every runtime-added place serialized discovered=False.
• viewer/server.py:2241-2244 (_atlas_visible_location_ids) — a place shows when visited OR discovered is True OR discovered is None; discovered is False and not visited → hidden. Old snapshots (field absent → None) stay visible, so the regression bites only the runtime add_location path, where the model default makes it an explicit False.

Net: a far-off / future-destination place the DM adds (without make_current) is invisible on the Atlas until visited — a silent change from pre-#371 behavior.

Why discovered=True is the right default (not fog-of-war)

Signal	Implication
seed_world sets day-1 regions discovered=True	"known" == True
Pre-#371, add_location'd places were visible	restoring, not inventing
The model already has a separate hidden flag (predicate line 2240) for deliberate suppression	discovered was never the runtime "hide it" lever
The False default exists so fog-of-war seeds opt in	the runtime path was simply missed when the field landed

A place the DM names into the world mid-play is, by that act, known — the player should see it. Deliberate fog-of-war stays available via discovered=False.

The fix (additive, engine-only)

• add_location gains a discovered: bool = True param, threaded into the new-Location constructor only.
• Update path (location_id reuse) is untouched → an existing place's discovered is preserved, never clobbered by the default (so editing a hidden place's description doesn't reveal it).
• make_current=True still arrives the party (visited=True) → visible regardless; unchanged.
• Docstring documents the default + the discovered=False rumour path + the hidden-vs-discovered distinction.

Tests — servers/engine/tests/test_add_location_discovered.py (4)

1. default add_location → discovered=True and visited=False (proves visibility comes from the flag, not a visit — the regression fix)
2. discovered=False → fog-of-war opt-in preserved (discovered/visited both False)
3. update path (location_id reuse, default discovered=True) → existing discovered=False preserved, description updated
4. make_current=True → visited=True + becomes current_location_id (visible regardless)

Verification: ast.parse clean on both files. Engine pytest left to CI (this canonical checkout is not on the local test-execution allowlist; same approach as my prior engine PRs).

Process notes

• Authored from an isolated git worktree off origin/main — the shared canonical checkout is used by parallel sessions, so no branch-op touched it.
• Does NOT touch PR fix(openworlds): atlas-seed — #261 #371, the viewer predicate, or any seed. Sibling to the still-open fix(openworlds): restore Atlas rumoured/known tier split + 5 missing BG POIs + Sword Coast regional pin (#261 follow-up) #380 (atlas rumoured/known tier work).

Collision audit

• servers/engine/server.py add_location region is not in the main agent's current hot path (their recent work is viewer/ + the DM skill — skill(dm): force the engine combat loop when combat is warranted #404, fix(viewer): render chronicle narration exactly once, in order, via a stable key (#405) #407). Additive param + one constructor kwarg; the update path is untouched.

DO NOT MERGE yet

Per owner — the parallel agent is still at work. Ready for review.

Provenance

External-AI suggestion ("Decide discovered= for runtime add_location"), independently verified against the tree before implementing. Follow-up to #371 / #261.

Summary by CodeRabbit

New Features

• Added configurable visibility settings for locations. New locations appear on the Atlas by default, with an optional fog-of-war mode to keep them hidden until discovered. Existing location visibility is preserved when locations are updated.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 87ecf98e-a511-45a2-9eb4-315cc1fc6ffe

📥 Commits

Reviewing files that changed from the base of the PR and between 7221f7b and eafd9b5.

📒 Files selected for processing (2)

• servers/engine/server.py
• servers/engine/tests/test_add_location_discovered.py

---

📝 Walkthrough

Walkthrough

The PR extends the add_location MCP tool with a new discovered parameter (default True) to control whether runtime-generated locations are immediately visible on the Atlas. The implementation forwards this parameter to the Location constructor, documents preservation behavior for update paths, and includes comprehensive regression tests covering default visibility, fog-of-war opt-in, state preservation, and current location semantics.

Changes

Location visibility discovery parameter

Layer / File(s)	Summary
Function signature, documentation, and implementation servers/engine/server.py	add_location gains discovered: bool = True parameter. Docstring documents discovery defaults and state preservation on updates. Location constructor call forwards the discovered value explicitly.
Regression test suite for discovered behavior servers/engine/tests/test_add_location_discovered.py	Four test scenarios validate default visibility (discovered=True), fog-of-war opt-in (discovered=False), update-path state preservation, and current-location behavior independent of discovered. Includes module documentation and helper utilities.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

A location appears on the Atlas bright,
Discovered by default, shining with light!
Or hide it in fog, if you prefer the night—
The parameter chosen, the visibility right. 🗺️✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description comprehensively covers the bug (with file/line anchors), justification for the fix, implementation details, test coverage, and verification notes. However, the required CLA checklist and licensing section from the template are completely missing.	Add the required Licensing / CLA section with the three checkboxes as specified in the description template before merging.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main fix: adding a discovered=True default to add_location to restore pre-#371 behavior for runtime-named places on the Atlas, with follow-up references.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}