From 400f81fa6caa32da26f50af9cfe257caa08dd360 Mon Sep 17 00:00:00 2001 From: Adrian Bonpin Date: Sun, 4 Oct 2026 15:50:45 +0800 Subject: [PATCH] fix: stop reporting a timezone's name as the user's city get_location printed "City: Manila", derived from the last segment of the IANA zone name. That is the timezone database's representative place, not the user's: Asia/Manila is one zone covering the entire Philippines, Asia/Kolkata covers all of India, Europe/Oslo all of Norway. It was wrong for anyone outside the zone's namesake, and confidently so, because a model asked about the weather uses it without hesitating. The README claimed the value was region-level, so the output also contradicted its own docs. - The zone's label is still reported, as `Timezone area`, and named for what it is. Both the field and the tool note say outright that it is not the user's city, and the tool description repeats it. - The locale region subtag is now used, which is a declaration rather than an inference: a locale of en-PH is the Philippines because the user set it. - A Location preference was added. It is the only exact place this extension has, it is reported verbatim and first, and when it is set the note tells the model to prefer it over anything inferred. - The continent grouping from the zone name was dropped: "Asia" next to a region of "Philippines" reads as two answers to the same question. - ToolContext takes the place as an injected value, keeping tools.ts free of Vicinae imports as the module wiring test requires. Verified: 316 tests, tsc, eslint, prettier, and vici build. --- package.json | 7 +++ src/__tests__/location.test.ts | 64 ++++++++++++++++++++---- src/__tests__/preferences.test.ts | 12 +++++ src/__tests__/tools.test.ts | 33 ++++++++++++- src/lib/agent.ts | 1 + src/lib/hooks/useChat.ts | 6 ++- src/lib/location.ts | 82 ++++++++++++++++++++++++++----- src/lib/preferences.ts | 10 ++++ src/lib/tools.ts | 45 +++++++++++++---- 9 files changed, 225 insertions(+), 35 deletions(-) diff --git a/package.json b/package.json index cfd9b83..eb67b68 100644 --- a/package.json +++ b/package.json @@ -65,6 +65,13 @@ "type": "textfield", "required": false }, + { + "name": "location", + "title": "Location", + "description": "The place Lam should treat as yours, for example Cebu City, Philippines. Optional. Without it Lam falls back to your device timezone and locale, which are accurate to a region and not to a city, so anything that depends on exactly where you are (weather, opening hours) is worth filling in.", + "type": "textfield", + "required": false + }, { "name": "webSearchEnabled", "title": "Web Tools", diff --git a/src/__tests__/location.test.ts b/src/__tests__/location.test.ts index b61e40f..3f6be9e 100644 --- a/src/__tests__/location.test.ts +++ b/src/__tests__/location.test.ts @@ -1,21 +1,53 @@ import { describe, expect, it } from "vitest"; -import { describeTimezone, getLocationContext, utcOffsetLabel, weekdayName } from "../lib/location"; +import { + describeTimezoneArea, + getLocationContext, + localeRegion, + utcOffsetLabel, + weekdayName, +} from "../lib/location"; -describe("describeTimezone", () => { - it("splits a simple zone into region and city", () => { - expect(describeTimezone("Asia/Manila")).toEqual({ region: "Asia", city: "Manila" }); +describe("describeTimezoneArea", () => { + it("keeps the zone's own label", () => { + expect(describeTimezoneArea("Asia/Manila")).toEqual({ area: "Manila" }); }); it("handles zones with more than two segments", () => { - expect(describeTimezone("America/Argentina/Salta")).toEqual({ region: "America", city: "Salta" }); + expect(describeTimezoneArea("America/Argentina/Salta")).toEqual({ area: "Salta" }); }); - it("underscores become spaces", () => { - expect(describeTimezone("America/New_York")).toEqual({ region: "America", city: "New York" }); + it("turns underscores into spaces", () => { + expect(describeTimezoneArea("America/New_York")).toEqual({ area: "New York" }); + }); + + it("drops the continent grouping, which answers a question nobody asked", () => { + // "Asia" beside a locale region of "Philippines" reads as two answers to + // the same question. + expect(describeTimezoneArea("Asia/Manila")).not.toHaveProperty("region"); }); it("returns nothing for a bare zone like UTC", () => { - expect(describeTimezone("UTC")).toEqual({}); + expect(describeTimezoneArea("UTC")).toEqual({}); + }); +}); + +describe("localeRegion", () => { + it("reads the region the user configured, rather than inferring one", () => { + expect(localeRegion("en-PH")).toEqual({ code: "PH", name: "Philippines" }); + }); + + it("handles other regions", () => { + expect(localeRegion("de-DE")?.name).toBe("Germany"); + expect(localeRegion("en-US")?.code).toBe("US"); + }); + + it("has nothing to say for a locale with no region", () => { + expect(localeRegion("en")).toBeUndefined(); + }); + + it("does not throw on a malformed locale", () => { + expect(localeRegion("not a locale")).toBeUndefined(); + expect(localeRegion("")).toBeUndefined(); }); }); @@ -54,10 +86,11 @@ describe("getLocationContext", () => { const context = getLocationContext(new Date("2026-01-02T03:04:05Z"), "Asia/Manila"); expect(context.timezone).toBe("Asia/Manila"); - expect(context.city).toBe("Manila"); - expect(context.region).toBe("Asia"); expect(context.utcOffset).toContain("+08:00"); expect(context.locale.length).toBeGreaterThan(0); + // The zone's label is reported as such, never as the user's city. + expect(context.timezoneArea).toBe("Manila"); + expect(context).not.toHaveProperty("city"); }); it("still returns a usable context for an unknown zone", () => { @@ -65,4 +98,15 @@ describe("getLocationContext", () => { expect(context.timezone).toBe("Not/AReal_Zone"); expect(context.utcOffset).toBe("UTC"); }); + + it("carries the configured place through verbatim", () => { + const context = getLocationContext(new Date(), "Asia/Manila", " Cebu City, Philippines "); + expect(context.place).toBe("Cebu City, Philippines"); + }); + + it("treats an empty or whitespace place as unset", () => { + expect(getLocationContext(new Date(), "Asia/Manila", "").place).toBeUndefined(); + expect(getLocationContext(new Date(), "Asia/Manila", " ").place).toBeUndefined(); + expect(getLocationContext(new Date(), "Asia/Manila").place).toBeUndefined(); + }); }); diff --git a/src/__tests__/preferences.test.ts b/src/__tests__/preferences.test.ts index 1a894f8..25f703b 100644 --- a/src/__tests__/preferences.test.ts +++ b/src/__tests__/preferences.test.ts @@ -53,6 +53,18 @@ describe("resolvePreferences", () => { expect(resolved.showReasoning).toBe(true); expect(resolved.think).toBeUndefined(); expect(resolved.resumeTimeoutMs).toBe(HOUR); + // An empty string rather than undefined, so callers never branch on it. + expect(resolved.place).toBe(""); + }); + + it("keeps the configured location, trimmed", () => { + expect(resolvePreferences({ location: " Cebu City, Philippines " }).place).toBe( + "Cebu City, Philippines", + ); + }); + + it("treats a whitespace-only location as unset", () => { + expect(resolvePreferences({ location: " " }).place).toBe(""); }); it("honours the display toggles being switched off", () => { diff --git a/src/__tests__/tools.test.ts b/src/__tests__/tools.test.ts index 7265b4c..ffd806f 100644 --- a/src/__tests__/tools.test.ts +++ b/src/__tests__/tools.test.ts @@ -225,13 +225,42 @@ describe("local tools", () => { expect(result.content).toContain("Current date and time:"); }); - it("describes location as a region rather than an address", async () => { + it("describes location as a region rather than an address when nothing is configured", async () => { const result = await executeTool("get_location", {}, context); expect(result.isError).toBe(false); expect(result.content).toContain("Timezone:"); expect(result.content).toContain("Locale:"); - expect(result.content).toContain("not a precise address"); + expect(result.content).toContain("no Location is set in preferences"); + expect(result.content).toContain("Ask which city they are in"); + }); + + it("never claims a city derived from the timezone name", async () => { + // `Asia/Manila` is one zone covering the whole Philippines, so reporting + // "City: Manila" was wrong for anyone in Cebu and confidently so. + const result = await executeTool("get_location", {}, context); + + expect(result.content).not.toMatch(/^City:/m); + expect(result.content).toContain("The timezone area is the timezone's own name, not the user's city."); + }); + + it("puts the configured place first and says to prefer it", async () => { + const result = await executeTool("get_location", {}, { + ...context, + place: "Cebu City, Philippines", + }); + + expect(result.content).toContain("Place: Cebu City, Philippines"); + expect(result.content).toContain("prefer it"); + expect(result.content).not.toContain("no Location is set"); + expect(result.content.indexOf("Place:")).toBeLessThan(result.content.indexOf("Timezone:")); + }); + + it("still reports the timezone alongside a configured place, for local time questions", async () => { + const result = await executeTool("get_location", {}, { ...context, place: "Cebu City" }); + + expect(result.content).toContain("Timezone:"); + expect(result.content).toContain("UTC offset:"); }); }); diff --git a/src/lib/agent.ts b/src/lib/agent.ts index 34d7005..c50c9e1 100644 --- a/src/lib/agent.ts +++ b/src/lib/agent.ts @@ -114,6 +114,7 @@ export async function runAgent(options: AgentOptions): Promise { const result = await executeTool(name, args, { host: options.toolContext.host, apiKey: options.toolContext.apiKey, + place: options.toolContext.place, now: options.toolContext.now, signal: options.signal, }); diff --git a/src/lib/hooks/useChat.ts b/src/lib/hooks/useChat.ts index 71b6aa0..dec8604 100644 --- a/src/lib/hooks/useChat.ts +++ b/src/lib/hooks/useChat.ts @@ -174,7 +174,11 @@ export function useChat(options: UseChatOptions): UseChat { think: preferences.think, messages, tools: toolsFor({ webEnabled: preferences.webEnabled }), - toolContext: { host: preferences.host, apiKey: preferences.apiKey }, + toolContext: { + host: preferences.host, + apiKey: preferences.apiKey, + place: preferences.place, + }, signal: controller.signal, events: { onContentDelta: (delta) => { diff --git a/src/lib/location.ts b/src/lib/location.ts index 9e1dcdf..754defe 100644 --- a/src/lib/location.ts +++ b/src/lib/location.ts @@ -1,30 +1,76 @@ /** * Time and place, derived locally. * - * `get_location` is intentionally timezone-derived rather than a GPS or IP - * lookup. It answers the questions people actually ask a launcher ("what time - * is it there", "is it late in London") without shipping the user's IP address - * to a third-party geolocation service on every tool call. It is also pure, so - * it is cheap to test. + * `get_location` is timezone-derived rather than a GPS or IP lookup, so the + * user's IP is never shipped to a third-party geolocation service on every tool + * call. It is also pure, so it is cheap to test. + * + * The correction that matters here is that a timezone name is **not** a city. + * The IANA database names a zone after one representative place, and a single + * zone can cover a whole country: "Asia/Manila" is the Philippines, + * "Asia/Kolkata" is India, "Europe/Oslo" is Norway, "Australia/Brisbane" is + * Queensland. Reporting "Manila" as the user's city is simply wrong for anyone + * in Cebu, and being confidently wrong is worse than being vague, because a + * model asked about the weather will use it without hesitating. So the zone's + * label is still reported, but named for what it is instead of being dressed up + * as the user's city. + * + * The only exact answer available without asking a third party is the Location + * preference, which the user fills in themselves. It is reported verbatim and + * preferred over anything inferred. */ export interface LocationContext { timezone: string; utcOffset: string; locale: string; + /** Country code from the locale, e.g. "PH". The user's own setting rather than an inference. */ + regionCode?: string; + /** Readable name for that region, e.g. "Philippines". */ region?: string; - city?: string; + /** + * The timezone's own area label, e.g. "Manila". + * + * Deliberately not called a city. See the note at the top of this file. + */ + timezoneArea?: string; + /** The user's configured Location preference, verbatim. The only exact value here. */ + place?: string; } -/** Splits an IANA zone like "Asia/Manila" or "America/Argentina/Salta" into readable parts. */ -export function describeTimezone(timeZone: string): { region?: string; city?: string } { +/** + * Splits an IANA zone like "Asia/Manila" or "America/Argentina/Salta". + * + * Only the last segment is kept, and the continent grouping is dropped: "Asia" + * next to a locale region of "Philippines" reads as two answers to the same + * question, and the zone's grouping answers a question nobody asked. + */ +export function describeTimezoneArea(timeZone: string): { area?: string } { const parts = timeZone.split("/").filter(Boolean); if (parts.length < 2) return {}; - const region = parts[0]; - const city = parts[parts.length - 1].replace(/_/g, " "); + return { area: parts[parts.length - 1].replace(/_/g, " ") || undefined }; +} - return { region, city: city || undefined }; +/** + * The region from the locale subtag. + * + * This is a declaration rather than an inference: a locale of "en-PH" says the + * user set their region to the Philippines. That makes it a better answer than + * anything read out of a zone name, and it costs nothing. + */ +export function localeRegion(locale: string): { code: string; name: string } | undefined { + try { + const code = new Intl.Locale(locale).region; + if (!code) return undefined; + + const name = new Intl.DisplayNames(["en"], { type: "region" }).of(code); + return { code, name: name ?? code }; + } catch { + // A malformed locale, or an ICU build without region display names. Not + // worth failing the whole answer over. + return undefined; + } } /** "GMT+08:00" on a modern V8; falls back to a short name if `longOffset` is unavailable. */ @@ -56,7 +102,11 @@ function resolveTimeZone(timeZone?: string): string { } } -export function getLocationContext(date: Date = new Date(), timeZone?: string): LocationContext { +export function getLocationContext( + date: Date = new Date(), + timeZone?: string, + place?: string, +): LocationContext { const zone = resolveTimeZone(timeZone); let locale = "en-US"; @@ -66,11 +116,17 @@ export function getLocationContext(date: Date = new Date(), timeZone?: string): // Keep the fallback locale. } + const region = localeRegion(locale); + const configured = place?.trim(); + return { timezone: zone, utcOffset: utcOffsetLabel(date, zone), locale, - ...describeTimezone(zone), + regionCode: region?.code, + region: region?.name, + timezoneArea: describeTimezoneArea(zone).area, + place: configured && configured.length > 0 ? configured : undefined, }; } diff --git a/src/lib/preferences.ts b/src/lib/preferences.ts index 95dce44..8e7740d 100644 --- a/src/lib/preferences.ts +++ b/src/lib/preferences.ts @@ -15,6 +15,7 @@ export interface LamPreferences { model?: string; ollamaHost?: string; systemPrompt?: string; + location?: string; webSearchEnabled?: boolean; thinking?: string; showToolCalls?: boolean; @@ -28,6 +29,14 @@ export interface ResolvedPreferences { model: string; host: string; systemPrompt: string; + /** + * The place the user says they are, verbatim, or an empty string. + * + * The only exact location this extension has. Everything else is inferred from + * the device timezone and locale, which is accurate to a region rather than a + * city, and a timezone name is not a city at all. + */ + place: string; webEnabled: boolean; /** Undefined when reasoning is off, which is what the API client expects. */ think?: string; @@ -78,6 +87,7 @@ export function resolvePreferences(raw: LamPreferences): ResolvedPreferences { model: (raw.model ?? "").trim() || DEFAULT_MODEL, host: (raw.ollamaHost ?? "").trim() || DEFAULT_HOST, systemPrompt: (raw.systemPrompt ?? "").trim(), + place: (raw.location ?? "").trim(), // Defaults to on: the preference is a checkbox the user has to actively untick. webEnabled: raw.webSearchEnabled ?? true, think: thinking && thinking !== "off" ? thinking : undefined, diff --git a/src/lib/tools.ts b/src/lib/tools.ts index 5a00bdd..9917cae 100644 --- a/src/lib/tools.ts +++ b/src/lib/tools.ts @@ -19,6 +19,13 @@ export interface ToolContext { signal?: AbortSignal; /** Injectable clock so tests are deterministic. */ now?: Date; + /** + * The user's Location preference, if they set one. + * + * Injected rather than read here, because this module is deliberately free of + * Vicinae imports and preferences arrive through that API. + */ + place?: string; } export interface ToolResult { @@ -45,7 +52,7 @@ export const TOOL_DEFINITIONS: ToolDefinition[] = [ function: { name: "get_location", description: - "Get the user's approximate location context: timezone, UTC offset, locale, region, and city. Use it for local questions such as weather, business hours, or travel timing. This is derived from the device timezone, not GPS, so treat it as a region rather than an address.", + "Get where the user is, for local questions such as weather, business hours, or travel timing. Returns the place they configured in preferences, plus the timezone, UTC offset, locale and region. Without a configured place this is region-level, not an address, and the timezone area it reports is the timezone's own name rather than the user's city, so never treat it as one.", parameters: { type: "object", properties: {}, required: [] }, }, }, @@ -140,7 +147,7 @@ export async function executeTool( return { content: describeNow(context.now), isError: false }; case "get_location": - return { content: describeLocation(context.now), isError: false }; + return { content: describeLocation(context.now, context.place), isError: false }; case "web_search": return await runWebSearch(args, context); @@ -172,18 +179,38 @@ function describeNow(now?: Date): string { ].join("\n"); } -function describeLocation(now?: Date): string { +function describeLocation(now?: Date, place?: string): string { const date = now ?? new Date(); - const location = getLocationContext(date); + const location = getLocationContext(date, undefined, place); - const lines = [ + const lines: string[] = []; + + // The configured place comes first because it is the only exact thing here, + // and the model should reach for it before anything else. + if (location.place) lines.push(`Place: ${location.place}`); + + lines.push( `Timezone: ${location.timezone}`, `UTC offset: ${location.utcOffset}`, `Locale: ${location.locale}`, - ]; - if (location.city) lines.push(`City: ${location.city}`); - if (location.region) lines.push(`Region: ${location.region}`); - lines.push("Note: derived from the device timezone. It indicates region, not a precise address."); + ); + + if (location.region) { + lines.push(`Region: ${location.region}${location.regionCode ? ` (${location.regionCode})` : ""}`); + } + if (location.timezoneArea) { + lines.push(`Timezone area: ${location.timezoneArea}`); + } + + lines.push( + location.place + ? "Note: Place is the user's own setting, so prefer it for anything that depends on exactly where they are." + : "Note: no Location is set in preferences. Everything above is derived from the device timezone and locale, so it is accurate to a region and not to a city. Ask which city they are in when an exact place matters.", + ); + + if (location.timezoneArea) { + lines.push("The timezone area is the timezone's own name, not the user's city."); + } return lines.join("\n"); }