From 750a2cd36b295c9a34fc8f95cb15a9306749a73c Mon Sep 17 00:00:00 2001 From: hshum Date: Fri, 17 Jul 2026 03:33:22 -0700 Subject: [PATCH] fix(redesign): honor title-only requests behind any determiner detectRequestedResponseShape matched "return only the title" but not "return only its title": the determiner group listed articles and no possessives, so the possessive phrasing fell through to the five-section memo and silently overrode an explicit user output constraint on a paid run. The determiner set now lives in one TITLE_DETERMINER constant shared by every title pattern. It was previously duplicated across two regexes, which is how the two could disagree in the first place. Found by re-verifying the 2026-07-16 production audit against main instead of trusting it. Four of its five P1s were already fixed by #550/#561/#564 within hours of the run; only this one survived. The suite stayed green because every title case it asserted used the one phrasing that worked, so the new tests use the audit's verbatim production prompts and cover the negative cases the wider pattern could now over-match. Both test files are added to the CI runtime-smoke allowlist, which is an explicit file list rather than a full-suite run. Without that, the regression guard would never execute in CI and would repeat the same blind spot one level up. Also pins DEFAULT_TIERS to the runtime's modelForTier with a parity test. The two were hand-maintained mirrors with no test binding them; drift would make the paid preflight advertise a model the runtime never runs. The composer header comment still claimed "provider names appear only in the trace", the opposite of the disclosure contract shipped in #550. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 2 + CHANGELOG/pages/redesign-chat.md | 43 +++++++++++++++++++ .../redesign/chatRuns.responseShape.test.ts | 40 +++++++++++++++++ convex/domains/redesign/chatRuns.ts | 26 ++++++++--- .../components/UniversalComposer.test.tsx | 15 +++++++ .../redesign/components/UniversalComposer.tsx | 7 ++- 6 files changed, 126 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66305ebf8..5a17d3d6d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -100,6 +100,8 @@ jobs: src/features/dogfood/components/TasteBenchPanel.test.tsx src/features/evaluation/data/tasteBenchScenario.test.ts scripts/__tests__/releaseWorkflowContracts.test.ts + convex/domains/redesign/chatRuns.responseShape.test.ts + src/features/redesign/components/UniversalComposer.test.tsx scratchnode-launch-gates: name: ScratchNode launch gates diff --git a/CHANGELOG/pages/redesign-chat.md b/CHANGELOG/pages/redesign-chat.md index aa38aced6..3e00de388 100644 --- a/CHANGELOG/pages/redesign-chat.md +++ b/CHANGELOG/pages/redesign-chat.md @@ -3,6 +3,49 @@ Append-only lane for the public redesign chat, reproducible answer receipts, and their transition into an authenticated live conversation. Newest entries first. +## 2026-07-17 - Honor title-only requests behind any determiner + +`detectRequestedResponseShape` recognized `return only the title` but not `return only +its title`: the determiner group listed articles (`a`/`the`) and no possessives, so the +possessive phrasing fell through to the five-section memo and silently overrode an +explicit user output constraint on a paid run. The determiner set now lives in one +`TITLE_DETERMINER` constant shared by every title pattern, so the patterns cannot drift +apart again. User-visible effect: a prompt ending `return only its title` now returns one +plain title line instead of a Short answer / Why it matters / Evidence / Risks / Next +action memo. + +This was found by re-verifying the 2026-07-16 production audit against `main` rather than +trusting it. Four of its five P1s were already fixed by #550/#561/#564 within hours of the +run; only this one survived. The suite stayed green throughout because every title case it +asserted used the one phrasing that worked - so the regression tests here use the audit's +**verbatim** production prompts, and both new test files were added to the CI runtime-smoke +allowlist, which is an explicit file list rather than a full-suite run. + +Also corrected the `UniversalComposer` header comment, which still claimed "no provider +names in UI / provider names appear only in the trace" - the opposite of the disclosure +contract shipped in #550 - and pinned `DEFAULT_TIERS` to the runtime's `modelForTier` with +a parity test, since the two were hand-maintained mirrors with no test binding them. + +**PR / canonical main commit**: `PENDING #NNN MAIN SHA / FINAL QA`. + +**Evidence state**: +- Source: `pending` +- Checks: `npx tsc --noEmit --pretty false` -> 0 errors. `npx vitest run + convex/domains/redesign/chatRuns.responseShape.test.ts + src/features/redesign/components/UniversalComposer.test.tsx` -> 16 passed. Fix reverted + in isolation to prove the guard: audit prompt fails `expected { kind: 'memo' } to deeply + equal { kind: 'title_only' }` (2 failed / 11 passed), restored -> 16 passed. Redesign + sweep `npx vitest run convex/domains/redesign src/features/redesign shared/redesign` -> + 141 passed, 3 failed in `ScratchnodeEventsSurface.test.tsx`, confirmed pre-existing on + clean `main` with all edits stashed and unrelated to this change. +- Visual proof: `not recorded` - no rendered surface changed; the response body shape is + produced by the runtime, and the composer edit is a comment. +- Preview: `not recorded` +- Production live: `not recorded` + +**Author**: Homen Shum + Claude Opus 4.8. +**Touches**: this change is chat-runtime only; no other lane. + ## 2026-07-16 - Contract NodeBench to one decision workspace NodeBench now has one primary product surface: the runtime-backed conversation at diff --git a/convex/domains/redesign/chatRuns.responseShape.test.ts b/convex/domains/redesign/chatRuns.responseShape.test.ts index 34cdc7569..982f4b881 100644 --- a/convex/domains/redesign/chatRuns.responseShape.test.ts +++ b/convex/domains/redesign/chatRuns.responseShape.test.ts @@ -32,6 +32,46 @@ describe("redesign chat runtime response policy", () => { expect(detectRequestedResponseShape("Research Acme's market position.")).toEqual({ kind: "memo" }); }); + // Both prompts below are verbatim from the 2026-07-16 authenticated production + // runs. The possessive one silently rendered the five-section memo in prod while + // this suite stayed green, because every title case asserted here used "the". + it("honors the shape of the verbatim production prompts that regressed", () => { + expect( + detectRequestedResponseShape( + "Production recovery QA run 2026-07-16: From the attached Daily Brief, identify one unresolved claim and return only its title. Do not write, share, approve, or modify any data.", + ), + ).toEqual({ kind: "title_only" }); + + expect( + detectRequestedResponseShape( + "Production QA run 2026-07-16: Using the attached Daily Brief, return exactly two bullets: (1) the strongest supported claim with its best source, and (2) one concrete review gap. Do not write, share, approve, or modify any data.", + ), + ).toEqual({ kind: "bullets", count: 2 }); + }); + + it("detects a title request behind any determiner, not just an article", () => { + for (const prompt of [ + "Return only its title.", + "Return only their title.", + "Just give me his title.", + "Return only this title.", + "Return only title.", + "Provide its title only", + "Title-only please.", + ]) { + expect(detectRequestedResponseShape(prompt)).toEqual({ kind: "title_only" }); + } + }); + + it("does not mistake incidental prose about titles for a shape request", () => { + expect( + detectRequestedResponseShape("Summarize the report and explain why its title is misleading."), + ).toEqual({ kind: "memo" }); + expect( + detectRequestedResponseShape("Compare each vendor's job titles across the market."), + ).toEqual({ kind: "memo" }); + }); + it("returns one plain title line while omitting memo-only fields", () => { const shaped = applyDeterministicResponsePolicy( "Provide a title only", diff --git a/convex/domains/redesign/chatRuns.ts b/convex/domains/redesign/chatRuns.ts index 3976b7c1f..eff3107ff 100644 --- a/convex/domains/redesign/chatRuns.ts +++ b/convex/domains/redesign/chatRuns.ts @@ -842,13 +842,29 @@ function parseRequestedCount(value: string): number | null { return Number.isInteger(count) && count >= 1 && count <= 12 ? count : null; } +// Determiners that may precede "title". Articles alone drop possessive phrasings +// such as "return only its title", which then fall back to the five-section memo +// and silently override the user's explicit shape request. Kept in one place so +// the patterns below cannot drift apart. +const TITLE_DETERMINER = String.raw`(?:a |an |the |its |their |his |her |our |your |this |that )?`; + +const TITLE_ONLY_PATTERNS: RegExp[] = [ + /\btitle[- ]only\b/, + new RegExp( + String.raw`\b(?:only|just) (?:give|return|output|provide|write|respond with)?\s*(?:me )?` + + TITLE_DETERMINER + + String.raw`title\b`, + ), + new RegExp( + String.raw`\b(?:give|return|output|provide|write|respond with) (?:me )?` + + TITLE_DETERMINER + + String.raw`title only\b`, + ), +]; + export function detectRequestedResponseShape(prompt: string): RequestedResponseShape { const normalized = prompt.replace(/\s+/g, " ").trim().toLowerCase(); - if ( - /\btitle[- ]only\b/.test(normalized) - || /\b(?:only|just) (?:give|return|output|provide|write|respond with)?\s*(?:me )?(?:a |the )?title\b/.test(normalized) - || /\b(?:give|return|output|provide|write|respond with) (?:me )?(?:a |the )?title only\b/.test(normalized) - ) { + if (TITLE_ONLY_PATTERNS.some((pattern) => pattern.test(normalized))) { return { kind: "title_only" }; } diff --git a/src/features/redesign/components/UniversalComposer.test.tsx b/src/features/redesign/components/UniversalComposer.test.tsx index b48a0bd61..ee5eb3fff 100644 --- a/src/features/redesign/components/UniversalComposer.test.tsx +++ b/src/features/redesign/components/UniversalComposer.test.tsx @@ -1,6 +1,7 @@ import { act, fireEvent, render, screen } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { modelForTier } from "../../../../convex/domains/redesign/chatRuns"; import { CANCEL_ARM_DELAY_MS, DEFAULT_TIERS, @@ -28,6 +29,20 @@ describe("UniversalComposer runtime controls", () => { vi.clearAllMocks(); }); + // The preflight names a provider and model before a paid submit, but DEFAULT_TIERS + // is a hand-maintained mirror of the runtime's modelForTier. Drift between them + // makes the disclosure advertise a model the runtime never runs — and never prices. + it("discloses the exact model the chat runtime resolves for every offered tier", () => { + expect(DEFAULT_TIERS.length).toBeGreaterThan(0); + + for (const tier of DEFAULT_TIERS) { + expect({ id: tier.id, model: tier.model }).toEqual({ + id: tier.id, + model: modelForTier(tier.id), + }); + } + }); + it("does not let the second click of submit immediately cancel the new run", () => { const onSubmit = vi.fn(); const onStop = vi.fn(); diff --git a/src/features/redesign/components/UniversalComposer.tsx b/src/features/redesign/components/UniversalComposer.tsx index ad7281568..1d822be93 100644 --- a/src/features/redesign/components/UniversalComposer.tsx +++ b/src/features/redesign/components/UniversalComposer.tsx @@ -7,8 +7,11 @@ * - Bottom row: `+` tools menu (left) · mic (left) · keyboard hint · circular send (right). * - Tools / paid-tier hints / estimates live behind the dropdown popover, not as a 4-pill row. * - * Spec: "no provider names in UI" — the dropdown shows tiers (Auto / Quick / Deep / Compare), - * not model names. Provider names appear only in the trace. + * Disclosure contract: a paid run must reveal its resolved provider and model + * BEFORE submit, not only afterwards in the trace. `DEFAULT_TIERS` therefore + * mirrors `modelForTier` in convex/domains/redesign/chatRuns.ts, and a parity + * test pins the two together — if they drift, the preflight advertises a model + * the runtime does not charge for. * * Submit: Enter sends. Shift+Enter inserts newline. ⌘↵ also sends. */