From d2002cc9138d23cc5c3982d2cb5a57dce794b518 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 5 Nov 2026 15:18:56 +0100 Subject: [PATCH] fix: keep the audit follow-ups inside the domain boundaries The field-to-column map of shared place details moves to @trek/shared, where places and place-contrib both read it without depending on each other, and the assistant reads a recorded call's arguments with a helper of its own instead of reaching into the AI providers. The catch-up's country from the address is taken back out: it made place-contrib depend on atlas, which already depends on it, and it waits for a shared country lookup both can use. --- .../PlaceContribOutbox.repository.ts | 2 -- .../src/nest/assistant/assistant-history.ts | 18 ++++++++++++--- .../place-contrib-backfill.service.ts | 10 ++++----- .../place-contrib-sender.service.ts | 2 +- .../place-contrib/place-contrib.helpers.ts | 15 ------------- .../src/nest/places/place-origins.helpers.ts | 2 +- .../place-contrib-backfill.service.test.ts | 22 +++---------------- shared/src/place/place-contrib.schema.ts | 15 +++++++++++++ 8 files changed, 40 insertions(+), 46 deletions(-) diff --git a/server/src/db/repositories/PlaceContribOutbox.repository.ts b/server/src/db/repositories/PlaceContribOutbox.repository.ts index 0cb25b0d3..26d83186a 100644 --- a/server/src/db/repositories/PlaceContribOutbox.repository.ts +++ b/server/src/db/repositories/PlaceContribOutbox.repository.ts @@ -27,7 +27,6 @@ export interface PlaceTypedRow { name: string; lat: number | null; lng: number | null; - address: string | null; osm_id: string | null; website: string | null; phone: string | null; @@ -306,7 +305,6 @@ export class PlaceContribOutboxRepository extends TrekRepository { + let parsed = value; + if (typeof value === 'string') { + try { + parsed = JSON.parse(value); + } catch { + return {}; + } + } + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? (parsed as Record) : {}; +} + /** * A thread's stored messages, read two ways: as the conversation the * provider is sent, and as the messages the client shows. @@ -58,11 +70,11 @@ function recordedCalls(state: AiProviderState | null): AiToolCall[] { state?.adapter === 'anthropic-messages' ? state.blocks .filter((block) => block.type === 'tool_use') - .map((block) => ({ id: block.id, name: block.name, args: asRecord(block.input) })) + .map((block) => ({ id: block.id, name: block.name, args: recordOf(block.input) })) : state?.adapter === 'openai-responses' ? state.items .filter((item) => item.type === 'function_call') - .map((item) => ({ id: item.call_id, name: item.name, args: parseToolArgs(asText(item.arguments)) })) + .map((item) => ({ id: item.call_id, name: item.name, args: recordOf(item.arguments) })) : []; return calls.flatMap(({ id, name, args }) => typeof id === 'string' ? [{ id, name: typeof name === 'string' ? name : '', args }] : [], diff --git a/server/src/nest/place-contrib/place-contrib-backfill.service.ts b/server/src/nest/place-contrib/place-contrib-backfill.service.ts index 1ff511c20..c23f17bc1 100644 --- a/server/src/nest/place-contrib/place-contrib-backfill.service.ts +++ b/server/src/nest/place-contrib/place-contrib-backfill.service.ts @@ -1,18 +1,18 @@ import { PlaceContribOutbox } from '../../db/entities/PlaceContribOutbox.entity'; import type { PlacesContributeBackfill } from '../../db/json-columns'; import type { PlaceContribOutboxRepository, PlaceTypedRow } from '../../db/repositories/PlaceContribOutbox.repository'; -import { resolveCountryCodeSync } from '../atlas/atlas-geo'; +import { getCountryFromCoords } from '../../utils/country-borders'; import { logInfo } from '../audit/audit-log.logger'; import { inOrder } from '../common/in-order'; import { valueHash } from '../common/value-hash'; import { UnitOfWork } from '../database/unit-of-work'; import { PlaceContribQueue } from './place-contrib-queue'; import { PlaceContribSettings } from './place-contrib-settings'; -import { PLACE_CONTRIB_COLUMN, contribKeyOf } from './place-contrib.helpers'; +import { contribKeyOf } from './place-contrib.helpers'; import type { ContribWrite } from './place-contrib.types'; import { InjectRepository } from '@mikro-orm/nestjs'; import { Injectable } from '@nestjs/common'; -import type { PlaceContribField } from '@trek/shared'; +import { PLACE_CONTRIB_COLUMN, type PlaceContribField } from '@trek/shared'; /** Places read per step of the catch-up. */ const BACKFILL_PAGE = 100; @@ -104,8 +104,8 @@ function typedWrite(row: PlaceTypedRow, fields: readonly PlaceContribField[]): C name: place.name, lat: place.lat, lng: place.lng, - // Resolved the way a save resolves it, so a place goes out with the same country either way. - country: values.length > 0 ? resolveCountryCodeSync(place) : null, + country: + values.length > 0 && place.lat !== null && place.lng !== null ? getCountryFromCoords(place.lat, place.lng) : null, values, dropped: [], }; diff --git a/server/src/nest/place-contrib/place-contrib-sender.service.ts b/server/src/nest/place-contrib/place-contrib-sender.service.ts index d75bde230..63825e6d5 100644 --- a/server/src/nest/place-contrib/place-contrib-sender.service.ts +++ b/server/src/nest/place-contrib/place-contrib-sender.service.ts @@ -23,7 +23,6 @@ import { PLACES_PER_RUN, } from './place-contrib.constants'; import { - PLACE_CONTRIB_COLUMN, UPLOADED_IMAGE_PREFIX, baseHashOf, dayOf, @@ -37,6 +36,7 @@ import { import { InjectRepository } from '@mikro-orm/nestjs'; import { Injectable } from '@nestjs/common'; import { + PLACE_CONTRIB_COLUMN, PLACE_CONTRIB_TERMS_VERSION, type PlaceContribField, type PlaceContribImageAnswer, diff --git a/server/src/nest/place-contrib/place-contrib.helpers.ts b/server/src/nest/place-contrib/place-contrib.helpers.ts index f3e8a374e..b619c6ea6 100644 --- a/server/src/nest/place-contrib/place-contrib.helpers.ts +++ b/server/src/nest/place-contrib/place-contrib.helpers.ts @@ -17,21 +17,6 @@ const OSM_ID = /^(node|way|relation):(\d{1,15})$/; /** Where the place pictures users upload live (the places domain's `placeImageUrl`). */ export const UPLOADED_IMAGE_PREFIX = '/uploads/places/'; -/** - * The places column each field is stored in. The origin hash, the catch-up - * and the sender all read a field through this one map: a value is shared - * only while the column's hash matches the origin, so two copies drifting - * apart would skip a field without a word. - */ -export const PLACE_CONTRIB_COLUMN = { - website: 'website', - phone: 'phone', - email: 'email', - opening_hours: 'opening_hours', - description: 'description', - image: 'image_url', -} as const satisfies Record; - /** * The key the service files a place under: `gers:` for a place from the * TREK index, `osm:/` for an OpenStreetMap element. A place without diff --git a/server/src/nest/places/place-origins.helpers.ts b/server/src/nest/places/place-origins.helpers.ts index 03a12f738..eaf0a15ec 100644 --- a/server/src/nest/places/place-origins.helpers.ts +++ b/server/src/nest/places/place-origins.helpers.ts @@ -1,8 +1,8 @@ import { PLACE_DETAILS_CACHE_PAYLOAD } from '../../db/json-columns'; import type { CachedProviderPayload, PlaceShareableRow } from '../../db/repositories/PlaceFieldOrigin.repository'; import { decodeJson } from '../../utils/json-column'; -import { PLACE_CONTRIB_COLUMN } from '../place-contrib/place-contrib.helpers'; import { + PLACE_CONTRIB_COLUMN, PLACE_CONTRIB_FIELDS, normalizePlaceWebsite, type PlaceContribField, diff --git a/server/tests/unit/nest/place-contrib/place-contrib-backfill.service.test.ts b/server/tests/unit/nest/place-contrib/place-contrib-backfill.service.test.ts index 275535f33..a05d3f976 100644 --- a/server/tests/unit/nest/place-contrib/place-contrib-backfill.service.test.ts +++ b/server/tests/unit/nest/place-contrib/place-contrib-backfill.service.test.ts @@ -115,15 +115,7 @@ async function agree(fields: PlaceContribFieldSwitches = ALL): Promise { /** A place with a website, whose origin is recorded as `origin` for the value given (the current one by default). */ async function placeWith( - opts: { - osmId?: string | null; - origin?: string; - typed?: string; - website?: string; - lat?: number | null; - lng?: number | null; - address?: string; - } = {}, + opts: { osmId?: string | null; origin?: string; typed?: string; website?: string } = {}, ): Promise { const website = opts.website ?? 'https://cafe-luna.de'; const { user } = await makeUser(orm()); @@ -132,9 +124,8 @@ async function placeWith( name: 'Café Luna', osm_id: opts.osmId === undefined ? GERS : opts.osmId, website, - lat: opts.lat === undefined ? 54.08 : opts.lat, - lng: opts.lng === undefined ? 12.13 : opts.lng, - address: opts.address ?? null, + lat: 54.08, + lng: 12.13, }); await insertRow(orm(), PlaceFieldOrigin, { place: id, @@ -372,11 +363,4 @@ describe('PlaceContribBackfill', () => { afterId: 0, }); }); - - it('PCB-018: the country comes from the address when the coordinates do not give one, as on a save', async () => { - await placeWith({ lat: null, lng: null, address: 'Lange Straße 1, 18055 Rostock, Germany' }); - await agree(); - expect(await backfill().step(NOW)).toBe(1); - expect((await findRows(orm(), PlaceContribOutbox, {})).map((row) => row.country)).toEqual(['DE']); - }); }); diff --git a/shared/src/place/place-contrib.schema.ts b/shared/src/place/place-contrib.schema.ts index bac65b498..d1a885410 100644 --- a/shared/src/place/place-contrib.schema.ts +++ b/shared/src/place/place-contrib.schema.ts @@ -19,6 +19,21 @@ export const PLACE_CONTRIB_FIELDS = ['website', 'phone', 'email', 'opening_hours export const placeContribFieldSchema = z.enum(PLACE_CONTRIB_FIELDS); export type PlaceContribField = z.infer; +/** + * The places column each field is stored in. The origin hash, the catch-up + * and the sender all read a field through this one map: a value is shared + * only while the column's hash matches the origin, so two copies drifting + * apart would skip a field without a word. + */ +export const PLACE_CONTRIB_COLUMN = { + website: 'website', + phone: 'phone', + email: 'email', + opening_hours: 'opening_hours', + description: 'description', + image: 'image_url', +} as const satisfies Record; + /** * Where the current value of a field came from. Only `user` is ever shared, and * only while the stored hash still matches the value. The server decides