fix(atlas): removing the last region keeps a country the user marked visited itself

Marking a region wrote its country with source 'manual', the same as ticking
the country off by hand, and removing the last region then took the country
away whatever its origin, Dawarich confirmations included. A country that
only comes in through a region is now stored as 'region', marking it visited
afterwards turns it into the user's own mark, and the cascade in unmarkRegion
only runs for a country without such a mark. Readers still see 'manual' for
those rows, so the country detail, the MCP resource and the plugin RPC answer
as before. Rows written before this change stay 'manual' and keep their
country.

REST, MCP and plugin RPC share the service. The atlas popup on desktop and
phone now asks the server whether the country is still marked before it
drops it from the map instead of guessing.

Refs #2597

client/src/pages/AtlasPage.wiring.test.tsx

@@ -44,6 +44,8 @@ beforeEach(() => {
     http.post('/api/addons/atlas/country/:code/mark', () => HttpResponse.json({ success: true })),
     http.post('/api/addons/atlas/region/:code/mark', () => HttpResponse.json({ success: true })),
     http.delete('/api/addons/atlas/region/:code/mark', () => HttpResponse.json({ success: true })),
+    // After a region goes the atlas asks whether the country is still marked (#2597).
+    http.get('/api/addons/atlas/country/:code', () => HttpResponse.json({ manually_marked: false })),
     http.post('/api/addons/atlas/bucket-list', () =>
       HttpResponse.json({ item: buildBucketItem({ id: 99, name: 'Germany', country_code: 'DE' }) }),
     ),

client/src/pages/atlas/useAtlasCountryActions.test.ts

@@ -1,4 +1,4 @@
-// FE-PAGE-ATLASACTIONS-001 to FE-PAGE-ATLASACTIONS-014: the country / region popup logic
+// FE-PAGE-ATLASACTIONS-001 to FE-PAGE-ATLASACTIONS-015: the country / region popup logic
 // behind both the desktop dialog and the phone sheet.
 import { act, renderHook } from '@testing-library/react';
 import { http, HttpResponse } from 'msw';
@@ -73,6 +73,9 @@ beforeEach(() => {
       deleted.push(new URL(request.url).pathname);
       return HttpResponse.json({ success: true });
     }),
+    http.get('/api/addons/atlas/country/:code', () =>
+      HttpResponse.json({ places: [], trips: [], manually_marked: false, marked_source: null, status: 'idea' })
+    ),
     http.post('/api/addons/atlas/bucket-list', async ({ request }) => {
       const body = (await request.json()) as Record<string, unknown>;
       posted.push({ url: '/api/addons/atlas/bucket-list', body });
@@ -202,6 +205,32 @@ describe('useAtlasCountryActions', () => {
     expect(result.current.confirmAction).toBeNull();
   });
 
+  it('FE-PAGE-ATLASACTIONS-015: a country the server still holds as marked stays when its last region goes (#2597)', async () => {
+    const init = () => ({
+      confirmAction: { type: 'unmark-region', code: 'ES', name: 'Galicia', regionCode: 'ES-GA' } as AtlasConfirmAction,
+      data: buildAtlasData({
+        countries: [{ code: 'ES', tripCount: 0, placeCount: 0, firstVisit: null, lastVisit: null }],
+      }),
+      visitedRegions: { ES: [{ code: 'ES-GA', name: 'Galicia', placeCount: 0 }] },
+    });
+    server.use(
+      http.get('/api/addons/atlas/country/:code', () =>
+        HttpResponse.json({ places: [], trips: [], manually_marked: true, marked_source: 'manual', status: 'visited' })
+      )
+    );
+    const marked = setup(init());
+    await act(() => marked.result.current.actions.unmarkRegion());
+    expect(marked.result.current.visitedRegions).toEqual({});
+    expect(marked.result.current.data?.countries.map((c) => c.code)).toEqual(['ES']);
+
+    // No answer: the country stays until the next load rather than vanishing on a guess.
+    server.use(http.get('/api/addons/atlas/country/:code', () => HttpResponse.json({}, { status: 500 })));
+    const unknown = setup(init());
+    await act(() => unknown.result.current.actions.unmarkRegion());
+    expect(unknown.result.current.data?.countries.map((c) => c.code)).toEqual(['ES']);
+    expect(addToast).not.toHaveBeenCalled();
+  });
+
   it('FE-PAGE-ATLASACTIONS-010: adding to the bucket list posts the entry, prepends it and runs beforeClose', async () => {
     const beforeClose = vi.fn();
     const { result } = setup({

client/src/pages/atlas/useAtlasCountryActions.ts

@@ -67,11 +67,12 @@ export function withRegionUnmarked(prev: VisitedRegionMap, countryCode: string,
 }
 
 /**
- * The atlas once a region is unmarked: if no visible region of the country remains
- * (however a region was derived: the server hides it either way and cascades to the
- * country the same way), the country goes too, but only when it has no place or trip
- * of its own. A country with real places is never hidden server side (#1490), so
- * removing it here would only flash and reappear on the next load.
+ * The atlas once a region is unmarked and the server no longer holds the country as
+ * marked: if no visible region of the country remains (however a region was derived:
+ * the server hides it either way and cascades to the country the same way), the
+ * country goes too, but only when it has no place or trip of its own. A country with
+ * real places is never hidden server side (#1490), so removing it here would only
+ * flash and reappear on the next load.
  */
 export function withCountryDroppedAfterRegionUnmark(
   prev: AtlasData | null,
@@ -93,6 +94,21 @@ export function withCountryDroppedAfterRegionUnmark(
   };
 }
 
+/**
+ * Whether the server still holds the country as marked after one of its regions was
+ * unmarked. A country the user marked itself survives its last region (#2597), so the
+ * atlas asks instead of guessing; when the answer does not come, the country stays
+ * until the next load settles it.
+ */
+async function isCountryStillMarked(code: string): Promise<boolean> {
+  try {
+    const r = await apiClient.get(`/addons/atlas/country/${code}`);
+    return r.data?.manually_marked !== false;
+  } catch {
+    return true;
+  }
+}
+
 /**
  * What the country / region action popup does, behind both the desktop dialog and the
  * phone sheet (which render their own markup and their own bucket list date picker):
@@ -155,7 +171,9 @@ export function useAtlasCountryActions({
     try {
       await apiClient.delete(`/addons/atlas/region/${regionCode}/mark`);
       setVisitedRegions((prev) => withRegionUnmarked(prev, countryCode, regionCode));
-      setData((prev) => withCountryDroppedAfterRegionUnmark(prev, countryCode, regionCode, visitedRegions));
+      if (!(await isCountryStillMarked(countryCode))) {
+        setData((prev) => withCountryDroppedAfterRegionUnmark(prev, countryCode, regionCode, visitedRegions));
+      }
     } catch (err) {
       toast.error(getApiErrorMessage(err, t('common.error')));
     }

client/tests/unit/mobile/atlas/MAtlasCountryPopup.test.tsx

@@ -30,6 +30,8 @@ beforeEach(() => {
     http.post('/api/addons/atlas/country/:code/mark', () => HttpResponse.json({ success: true })),
     http.post('/api/addons/atlas/region/:code/mark', () => HttpResponse.json({ success: true })),
     http.delete('/api/addons/atlas/region/:code/mark', () => HttpResponse.json({ success: true })),
+    // After a region goes the atlas asks whether the country is still marked (#2597).
+    http.get('/api/addons/atlas/country/:code', () => HttpResponse.json({ manually_marked: false })),
     http.post('/api/addons/atlas/bucket-list', () =>
       HttpResponse.json({ item: { id: 42, name: 'Germany', country_code: 'DE', lat: null, lng: null, notes: null, target_date: '2027-05' } }),
     ),

server/src/db/repositories/VisitedCountries.repository.ts

@@ -20,6 +20,15 @@ export interface VisitedCountryRow {
 type VisitedCountriesReadKyselyDB = Pick<DB, 'visited_countries'>;
 type VisitedCountriesWriteKyselyDB = Pick<DB, 'visited_countries'>;
 
+/**
+ * The source as callers see it. 'region' marks a country that only came onto
+ * the map through one of its regions and goes again with the last of them
+ * (#2597); to everyone else it reads as a mark made by hand, as it always did.
+ */
+function publicSource(source: string): string {
+  return source === 'region' ? 'manual' : source;
+}
+
 /**
  * `visited_countries` — the user's own explicit "I've been here" marks
  * (Plan 3f Task 1, atlas). `markVisited` (AT12) needs the `changes > 0`
@@ -43,12 +52,13 @@ export class VisitedCountriesRepository extends TrekRepository<VisitedCountries>
 
   /** AT10 (`listVisitedCountries`) — `SELECT country_code, created_at, source FROM visited_countries WHERE user_id = ? ORDER BY created_at DESC`. */
   async listForUser(userId: number): Promise<VisitedCountryRow[]> {
-    return await this.readDb()
+    const rows = await this.readDb()
       .selectFrom('visited_countries')
       .select(['country_code', 'created_at', 'source'])
       .where('user_id', '=', userId)
       .orderBy('created_at', 'desc')
       .execute();
+    return rows.map((row) => ({ ...row, source: publicSource(row.source) }));
   }
 
   /**
@@ -69,12 +79,29 @@ export class VisitedCountriesRepository extends TrekRepository<VisitedCountries>
 
   /** AT8/AT9 (`countryPlaces`'s no-trip early return, `countryPlaces`'s mark lookup — identical text) — `SELECT source FROM visited_countries WHERE user_id = ? AND country_code = ?`. */
   async findSource(userId: number, countryCode: string): Promise<{ source: string } | undefined> {
-    return await this.writeDb()
+    const row = await this.writeDb()
       .selectFrom('visited_countries')
       .select('source')
       .where('user_id', '=', userId)
       .where('country_code', '=', countryCode)
       .executeTakeFirst();
+    return row && { source: publicSource(row.source) };
+  }
+
+  /**
+   * `unmarkRegion`'s cascade check: whether the user marked the country visited
+   * themselves (by hand or from Dawarich), rather than it only coming in
+   * through one of its regions (#2597).
+   */
+  async hasOwnMark(userId: number, countryCode: string): Promise<boolean> {
+    const row = await this.writeDb()
+      .selectFrom('visited_countries')
+      .select('country_code')
+      .where('user_id', '=', userId)
+      .where('country_code', '=', countryCode)
+      .where('source', '!=', 'region')
+      .executeTakeFirst();
+    return !!row;
   }
 
   /**
@@ -103,20 +130,34 @@ export class VisitedCountriesRepository extends TrekRepository<VisitedCountries>
 
   /**
    * AT19 (`markRegion`, inside its transaction) — `INSERT OR IGNORE INTO
-   * visited_countries (user_id, country_code) VALUES (?, ?)`, a DISTINCT
-   * 2-column variant of {@link markVisited}'s 3-column insert. The legacy
-   * statement's column list omits `source`, letting the schema's own
-   * `DEFAULT 'manual'` fill it — reproduced here by passing that literal
-   * explicitly (`VisitedCountries.entity.ts`'s `source: p.text()
-   * .default('manual')`, the exact same value the omitted column would
-   * have taken). No `changes` signal here; the legacy statement never
-   * checked it either.
+   * visited_countries (user_id, country_code, source) VALUES (?, ?, 'region')`.
+   * A country that only came onto the map through one of its regions is
+   * written as 'region', not 'manual', so removing its last region can tell
+   * it apart from a country the user ticked off itself (#2597). The conflict
+   * is ignored, so a country already marked keeps its own source. Rows written
+   * before this change carry 'manual' and are left as they are. No `changes`
+   * signal here; the legacy statement never checked it either.
    */
   async markFromRegion(userId: number, countryCode: string): Promise<void> {
     await this.writeDb()
       .insertInto('visited_countries')
-      .values({ user_id: userId, country_code: countryCode, source: 'manual' })
+      .values({ user_id: userId, country_code: countryCode, source: 'region' })
       .onConflict((oc) => oc.doNothing())
       .execute();
   }
+
+  /**
+   * `markCountry`, inside its transaction, right after {@link markVisited}: a
+   * country that only came in through a region becomes the user's own mark
+   * once they mark it visited themselves. Any other row is left alone.
+   */
+  async promoteFromRegion(userId: number, countryCode: string, source: 'manual' | 'dawarich'): Promise<void> {
+    await this.writeDb()
+      .updateTable('visited_countries')
+      .set({ source })
+      .where('user_id', '=', userId)
+      .where('country_code', '=', countryCode)
+      .where('source', '=', 'region')
+      .execute();
+  }
 }

server/src/nest/atlas/atlas.mcp.ts

@@ -283,7 +283,8 @@ export class AtlasMcp {
 
   @Tool({
     name: 'unmark_region_visited',
-    description: 'Remove a region from the visited list.',
+    description:
+      'Remove a region from the visited list. Removing the last region of a country also removes the country, unless it was marked visited itself.',
     inputSchema: {
       regionCode: z.string(),
     },

server/src/nest/atlas/atlas.service.ts

@@ -685,14 +685,13 @@ export class AtlasService {
    * every mark was until an integration could make one, and `INSERT OR IGNORE`
    * means a country already ticked by hand keeps that provenance — confirming
    * it again from a recording does not relabel somebody's own work as imported.
-   */
-  /**
    * Returns whether this actually added the country, so a caller reporting
    * "3 countries added" is not counting the ones that were already there.
    */
   async markCountry(userId: number, code: string, source: 'manual' | 'dawarich' = 'manual'): Promise<boolean> {
     return await this.uow.transactional(async () => {
       const inserted = await this.visitedCountries.markVisited(userId, code, source); // AT12
+      await this.visitedCountries.promoteFromRegion(userId, code, source); // #2597
       // Marking it visited again lifts a previous removal.
       await this.hiddenCountries.unhide(userId, code); // AT13
       return inserted;
@@ -763,10 +762,11 @@ export class AtlasService {
       if (countryCode) {
         await this.hiddenRegions.hide(userId, code, countryCode); // AT27
 
-        // If that was the country's last visible region, hide the country too — otherwise it
-        // keeps showing "visited" on the world map with nothing left to drill into.
+        // If that was the country's last visible region, hide the country too, or it keeps showing
+        // "visited" with nothing left to drill into. One the user marked itself stays (#2597).
+        const ownMark = await this.visitedCountries.hasOwnMark(userId, countryCode);
         const hidden = await this.getHiddenRegions(userId);
-        if (!(await this.hasVisibleRegionForCountry(userId, countryCode, hidden))) {
+        if (!ownMark && !(await this.hasVisibleRegionForCountry(userId, countryCode, hidden))) {
           await this.unmarkCountry(userId, countryCode);
         }
       }

server/tests/integration/atlas.test.ts

@@ -424,6 +424,43 @@ describe('Mark/unmark region', () => {
     expect(codes).toContain('DE');
   });
 
+  it('ATLAS-011: unmark last region keeps a country the user marked visited itself (#2597)', async () => {
+    const { user } = createUser(testDb);
+    const cookie = authCookie(user.id);
+
+    await request(app).post('/api/addons/atlas/country/NO/mark').set('Cookie', cookie);
+    await request(app)
+      .post('/api/addons/atlas/region/NO-03/mark')
+      .set('Cookie', cookie)
+      .send({ name: 'Oslo', country_code: 'NO' });
+    await request(app).delete('/api/addons/atlas/region/NO-03/mark').set('Cookie', cookie);
+
+    const stats = await request(app).get('/api/addons/atlas/stats').set('Cookie', cookie);
+    expect((stats.body.countries as { code: string }[]).map((c) => c.code)).toContain('NO');
+    const country = await request(app).get('/api/addons/atlas/country/NO').set('Cookie', cookie);
+    expect(country.body).toMatchObject({ manually_marked: true, marked_source: 'manual' });
+    const regions = await request(app).get('/api/addons/atlas/regions').set('Cookie', cookie);
+    expect(regions.body.regions['NO']).toBeUndefined();
+  });
+
+  it('ATLAS-011: a country first reached through a region stays once it is marked itself (#2597)', async () => {
+    const { user } = createUser(testDb);
+    const cookie = authCookie(user.id);
+
+    await request(app)
+      .post('/api/addons/atlas/region/NO-03/mark')
+      .set('Cookie', cookie)
+      .send({ name: 'Oslo', country_code: 'NO' });
+    const before = await request(app).get('/api/addons/atlas/country/NO').set('Cookie', cookie);
+    expect(before.body).toMatchObject({ manually_marked: true, marked_source: 'manual' });
+
+    await request(app).post('/api/addons/atlas/country/NO/mark').set('Cookie', cookie);
+    await request(app).delete('/api/addons/atlas/region/NO-03/mark').set('Cookie', cookie);
+
+    const stats = await request(app).get('/api/addons/atlas/stats').set('Cookie', cookie);
+    expect((stats.body.countries as { code: string }[]).map((c) => c.code)).toContain('NO');
+  });
+
   it('ATLAS-011 — regions are isolated between users', async () => {
     const { user: user1 } = createUser(testDb);
     const { user: user2 } = createUser(testDb);

server/tests/unit/db/repositories/VisitedCountries.repository.test.ts

@@ -10,7 +10,7 @@ import { VisitedCountries } from '../../../../src/db/entities/VisitedCountries.e
 import type { VisitedCountriesRepository } from '../../../../src/db/repositories/VisitedCountries.repository';
 import { createSnapshotTestDb } from '../../../helpers/db-mock';
 import { createUser } from '../../../helpers/factories';
-import { insertRows } from '../../../helpers/factories/rows';
+import { findRow, insertRows } from '../../../helpers/factories/rows';
 import { resetTestDb } from '../../../helpers/test-db';
 import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
 
@@ -82,3 +82,46 @@ describe('VisitedCountriesRepository.listCodesForUser (AT5/AT7/AT43)', () => {
     expect(await repo.listCodesForUser(user.id)).toEqual([]);
   });
 });
+
+describe('VisitedCountriesRepository: countries that came in through a region (#2597)', () => {
+  it('VISCOUNTRYREPO-005: markFromRegion writes region, and the readers still answer manual', async () => {
+    const { user } = createUser(testDb);
+    await repo.markFromRegion(user.id, 'NO');
+
+    expect((await findRow(t, VisitedCountries, { user: user.id, country_code: 'NO' }))?.source).toBe('region');
+    expect(await repo.findSource(user.id, 'NO')).toEqual({ source: 'manual' });
+    expect((await repo.listForUser(user.id)).map((r) => r.source)).toEqual(['manual']);
+    expect(await repo.hasOwnMark(user.id, 'NO')).toBe(false);
+  });
+
+  it('VISCOUNTRYREPO-006: markFromRegion leaves a country the user marked itself as it was', async () => {
+    const { user } = createUser(testDb);
+    await repo.markVisited(user.id, 'NO', 'dawarich');
+    await repo.markFromRegion(user.id, 'NO');
+
+    expect(await repo.findSource(user.id, 'NO')).toEqual({ source: 'dawarich' });
+    expect(await repo.hasOwnMark(user.id, 'NO')).toBe(true);
+  });
+
+  it('VISCOUNTRYREPO-007: promoteFromRegion turns a region row into the given mark and touches nothing else', async () => {
+    const { user } = createUser(testDb);
+    await repo.markFromRegion(user.id, 'NO');
+    await repo.markVisited(user.id, 'SE', 'manual');
+
+    await repo.promoteFromRegion(user.id, 'NO', 'dawarich');
+    await repo.promoteFromRegion(user.id, 'SE', 'dawarich');
+
+    expect(await repo.findSource(user.id, 'NO')).toEqual({ source: 'dawarich' });
+    expect(await repo.findSource(user.id, 'SE')).toEqual({ source: 'manual' });
+    expect(await repo.hasOwnMark(user.id, 'NO')).toBe(true);
+  });
+
+  it('VISCOUNTRYREPO-008: hasOwnMark is false without a row and scoped to the user', async () => {
+    const { user } = createUser(testDb);
+    const { user: other } = createUser(testDb, { username: 'other' });
+    await repo.markVisited(other.id, 'NO', 'manual');
+
+    expect(await repo.hasOwnMark(user.id, 'NO')).toBe(false);
+    expect(await repo.findSource(user.id, 'NO')).toBeUndefined();
+  });
+});

server/tests/unit/mcp/tools-atlas-expanded.test.ts

@@ -11,6 +11,7 @@ import { HiddenRegions } from '../../../src/db/entities/HiddenRegions.entity';
 import { PlaceRegions } from '../../../src/db/entities/PlaceRegions.entity';
 import { Places } from '../../../src/db/entities/Places.entity';
 import { ReservationEndpoints } from '../../../src/db/entities/ReservationEndpoints.entity';
+import { VisitedCountries } from '../../../src/db/entities/VisitedCountries.entity';
 import { VisitedRegions } from '../../../src/db/entities/VisitedRegions.entity';
 import {
   createUser,
@@ -424,6 +425,21 @@ describe('Tool: unmark_region_visited', () => {
     });
   });
 
+  it('keeps a country the user marked itself when its last region goes, like REST (#2597)', async () => {
+    const { user } = createUser(testDb);
+    await markCountryVisited(orm, user.id, 'NO');
+    await markRegion(user.id, 'NO-03', 'Oslo', 'NO');
+    await insertRow(orm, VisitedCountries, { user: user.id, country_code: 'SE', source: 'region' });
+    await markRegion(user.id, 'SE-AB', 'Stockholm', 'SE');
+    await withHarness(user.id, async (h) => {
+      await h.client.callTool({ name: 'unmark_region_visited', arguments: { regionCode: 'NO-03' } });
+      await h.client.callTool({ name: 'unmark_region_visited', arguments: { regionCode: 'SE-AB' } });
+    });
+    expect(await findRow(orm, VisitedCountries, { user: user.id, country_code: 'NO' })).toBeTruthy();
+    // A country that only came in through its region still goes with it.
+    expect(await findRow(orm, VisitedCountries, { user: user.id, country_code: 'SE' })).toBeNull();
+  });
+
   it('succeeds even when region was not marked (no-op)', async () => {
     const { user } = createUser(testDb);
     await withHarness(user.id, async (h) => {

wiki/MCP-Addon-Tools.md

@@ -58,7 +58,7 @@ Requires `atlas:read` or `atlas:write` scope.
 | `list_visited_regions` | List all manually visited sub-country regions for the current user. |
 | `locate_atlas_region` | Resolve a coordinate to the country and region codes the Atlas map can highlight, which is where the pair `mark_region_visited` expects comes from. A point outside every bundled polygon answers with `null` fields rather than an error. For a postal address use `reverse_geocode` instead. |
 | `mark_region_visited` | Mark a sub-country region as visited (e.g. `"US-CA"`). |
-| `unmark_region_visited` | Remove a region from the visited list. |
+| `unmark_region_visited` | Remove a region from the visited list. Removing the last region of a country also removes the country, unless it was marked visited itself. |
 | `get_country_atlas_places` | Get places saved in the user's atlas for a specific country. |
 | `create_bucket_list_item` | Add a destination to your personal bucket list with optional coordinates, country code, target date and `region_code` (a state or province, hatched on the Atlas map). The same destination for the same target date is rejected as a duplicate. |
 | `update_bucket_list_item` | Update a bucket list item (name, notes, coordinates, target date). |