From e7c678dbe546af6bf087b1ad9dd67b56547f7c40 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 9 Jul 2026 18:51:56 +0000 Subject: [PATCH] fix(prompting): allowed magic keyword punctuation Magic keyword matching now treats sentence punctuation and quotes as prose boundaries while still rejecting casing changes, inflections, and path/file-extension occurrences. Added regression coverage for detection and highlighting across ultrathink, orchestrate, and workflowz. Fixes #4965 --- packages/coding-agent/CHANGELOG.md | 4 +++ .../src/modes/magic-keyword-boundary.ts | 22 ++++++++++++++++ .../coding-agent/src/modes/orchestrate.ts | 11 ++++---- packages/coding-agent/src/modes/ultrathink.ts | 9 ++++--- packages/coding-agent/src/modes/workflow.ts | 11 ++++---- .../src/prompts/system/workflow-notice.md | 10 +++---- .../test/modes/magic-keywords.test.ts | 26 +++++++++++++++++-- .../test/modes/orchestrate.test.ts | 20 +++++++++++--- .../coding-agent/test/modes/workflow.test.ts | 22 ++++++++++++---- 9 files changed, 105 insertions(+), 30 deletions(-) create mode 100644 packages/coding-agent/src/modes/magic-keyword-boundary.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d1ad895e4..901cf0671 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `ultrathink`, `orchestrate`, and `workflowz` magic keywords not triggering when adjacent to sentence punctuation or quotes while still ignoring inflections and path/file-extension occurrences. ([#4965](https://github.com/can1357/oh-my-pi/issues/4965)) + ## [16.3.13] - 2026-07-09 ### Fixed diff --git a/packages/coding-agent/src/modes/magic-keyword-boundary.ts b/packages/coding-agent/src/modes/magic-keyword-boundary.ts new file mode 100644 index 000000000..4149bbf21 --- /dev/null +++ b/packages/coding-agent/src/modes/magic-keyword-boundary.ts @@ -0,0 +1,22 @@ +/** Characters that bind a magic keyword into an identifier or path segment. */ +const LEFT_BOUNDARY = String.raw`(? 150 + t * 130, }); diff --git a/packages/coding-agent/src/modes/ultrathink.ts b/packages/coding-agent/src/modes/ultrathink.ts index 8d673cc5d..1f9d104dd 100644 --- a/packages/coding-agent/src/modes/ultrathink.ts +++ b/packages/coding-agent/src/modes/ultrathink.ts @@ -1,5 +1,6 @@ import ultrathinkNotice from "../prompts/system/ultrathink-notice.md" with { type: "text" }; import { createGradientHighlighter, type KeywordHighlighter } from "./gradient-highlight"; +import { magicKeywordRegex } from "./magic-keyword-boundary"; import { keywordInProse } from "./markdown-prose"; /** @@ -13,15 +14,15 @@ import { keywordInProse } from "./markdown-prose"; * trigger either behavior. */ -// Detection: lowercase keyword flanked by whitespace or a string edge. Non-global so `.test` stays stateless. -const ULTRATHINK_WORD = /(? t * 330, }); diff --git a/packages/coding-agent/src/modes/workflow.ts b/packages/coding-agent/src/modes/workflow.ts index e8d4832cc..44ed5b662 100644 --- a/packages/coding-agent/src/modes/workflow.ts +++ b/packages/coding-agent/src/modes/workflow.ts @@ -1,6 +1,7 @@ import { prompt } from "@oh-my-pi/pi-utils"; import workflowNoticeTemplate from "../prompts/system/workflow-notice.md" with { type: "text" }; import { createGradientHighlighter, type KeywordHighlighter } from "./gradient-highlight"; +import { magicKeywordRegex } from "./magic-keyword-boundary"; import { keywordInProse } from "./markdown-prose"; /** @@ -10,13 +11,13 @@ import { keywordInProse } from "./markdown-prose"; * amber→green gradient ({@link highlightWorkflow}); submitting a message that * mentions it appends a hidden workflow notice that steers the model to author * a deterministic multi-subagent workflow through the active task schema. - * Matching is whitespace-delimited and case-sensitive (lowercase only) — + * Matching is prose-delimited and case-sensitive (lowercase only) — * "workflowz" triggers, but "workflowzed", "Workflowz", and "workflowz.ts" * never do. */ -// Detection: lowercase keyword flanked by whitespace or a string edge. Non-global so `.test` stays stateless. -const WORKFLOW_WORD = /(? 30 + t * 120, }); diff --git a/packages/coding-agent/src/prompts/system/workflow-notice.md b/packages/coding-agent/src/prompts/system/workflow-notice.md index 74eddee21..3e62ca5b0 100644 --- a/packages/coding-agent/src/prompts/system/workflow-notice.md +++ b/packages/coding-agent/src/prompts/system/workflow-notice.md @@ -51,20 +51,20 @@ Decompose first, then {{#if taskBatch}}batch the independent leaves{{else}}issue {{#if taskBatch}} task( - context: "# Goal\nReview the auth diff...\n# Constraints\nRead-only...\n# Contract\nReturn findings as severity/file/line/fix...", + context: "# Goal\nReview the auth diff…\n# Constraints\nRead-only…\n# Contract\nReturn findings as severity/file/line/fix…", tasks: [ - { id: "AuthOwner", role: "Auth Storage Reviewer", assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nTrace credential selection...\n# Acceptance\nReturn confirmed findings only..." }, - { id: "PromptOwner", role: "Prompt Contract Reviewer", assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance...\n# Acceptance\nReturn mismatches and exact prompt lines..." }, + { id: "AuthOwner", role: "Auth Storage Reviewer", assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nTrace credential selection…\n# Acceptance\nReturn confirmed findings only…" }, + { id: "PromptOwner", role: "Prompt Contract Reviewer", assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance…\n# Acceptance\nReturn mismatches and exact prompt lines…" }, ] ) {{else}} task( role: "Auth Storage Reviewer", - assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nReview the auth diff. Shared contract: read-only; return findings as severity/file/line/fix.\n# Acceptance\nReturn confirmed findings only..." + assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nReview the auth diff. Shared contract: read-only; return findings as severity/file/line/fix.\n# Acceptance\nReturn confirmed findings only…" ) task( role: "Prompt Contract Reviewer", - assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance. Shared contract: read-only; return mismatches and exact prompt lines.\n# Acceptance\nReturn confirmed findings only..." + assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance. Shared contract: read-only; return mismatches and exact prompt lines.\n# Acceptance\nReturn confirmed findings only…" ) {{/if}} diff --git a/packages/coding-agent/test/modes/magic-keywords.test.ts b/packages/coding-agent/test/modes/magic-keywords.test.ts index 68b4fd311..993211901 100644 --- a/packages/coding-agent/test/modes/magic-keywords.test.ts +++ b/packages/coding-agent/test/modes/magic-keywords.test.ts @@ -22,6 +22,16 @@ describe("highlightMagicKeywords", () => { } }); + it("paints punctuation-adjacent prose keywords without changing visible text", () => { + const input = 'first "ultrathink," then orchestrate. Finally workflowz!'; + const decorated = highlightMagicKeywords(input); + expect(decorated).not.toBe(input); + expect(Bun.stripANSI(decorated)).toBe(input); + for (const keyword of ["ultrathink", "orchestrate", "workflowz"]) { + expect(decorated).not.toContain(keyword); + } + }); + it("never paints keywords inside code spans, fenced blocks, or XML sections", () => { const input = "`ultrathink`\n```\norchestrate\n```\nworkflowz"; expect(highlightMagicKeywords(input)).toBe(input); @@ -71,10 +81,22 @@ describe("hasMagicKeyword", () => { expect(hasMagicKeyword("just workflowz the steps")).toBe(true); }); - it("rejects keywords embedded in longer words or paths", () => { + it("detects standalone keywords beside prose punctuation and quotes", () => { + for (const text of ["please ultrathink.", 'say "orchestrate" now', "then workflowz, please"]) { + expect(hasMagicKeyword(text)).toBe(true); + } + }); + + it("rejects casing, inflections, old workflow names, and paths", () => { + expect(hasMagicKeyword("Ultrathink")).toBe(false); + expect(hasMagicKeyword("ORCHESTRATE")).toBe(false); + expect(hasMagicKeyword("workflow")).toBe(false); + expect(hasMagicKeyword("workflows")).toBe(false); expect(hasMagicKeyword("ultrathinking is fun")).toBe(false); - expect(hasMagicKeyword("orchestrate.ts is a file")).toBe(false); expect(hasMagicKeyword("workflowzed already")).toBe(false); + expect(hasMagicKeyword("src/modes/ultrathink.ts")).toBe(false); + expect(hasMagicKeyword("orchestrate.ts is a file")).toBe(false); + expect(hasMagicKeyword("packages/coding-agent/test/modes/workflowz.test.ts")).toBe(false); }); it("rejects keywords inside code spans, fences, and xml sections", () => { diff --git a/packages/coding-agent/test/modes/orchestrate.test.ts b/packages/coding-agent/test/modes/orchestrate.test.ts index 01f5e0836..c1cac442a 100644 --- a/packages/coding-agent/test/modes/orchestrate.test.ts +++ b/packages/coding-agent/test/modes/orchestrate.test.ts @@ -22,7 +22,13 @@ describe("orchestrate keyword detection", () => { expect(containsOrchestrate("do it now\norchestrate")).toBe(true); }); - it("ignores casing, inflections, punctuation-adjacent, and path-embedded forms", () => { + it("matches the lowercase word beside prose punctuation and quotes", () => { + for (const text of ["do it. orchestrate.", "please orchestrate, then report", 'say "orchestrate" now']) { + expect(containsOrchestrate(text)).toBe(true); + } + }); + + it("ignores casing, inflections, and path-embedded forms", () => { expect(containsOrchestrate("Orchestrate")).toBe(false); expect(containsOrchestrate("ORCHESTRATE")).toBe(false); expect(containsOrchestrate("orchestrated the build")).toBe(false); @@ -30,9 +36,8 @@ describe("orchestrate keyword detection", () => { expect(containsOrchestrate("a clean orchestration")).toBe(false); expect(containsOrchestrate("it orchestrates well")).toBe(false); expect(containsOrchestrate("reorchestrate everything")).toBe(false); - // The reported bug: a path/extension is not whitespace, so the word never triggers. + // A path/extension must not trigger even though sentence punctuation does. expect(containsOrchestrate("packages/coding-agent/src/modes/orchestrate.ts")).toBe(false); - expect(containsOrchestrate("do it. orchestrate.")).toBe(false); expect(containsOrchestrate("nothing to see here")).toBe(false); }); @@ -53,9 +58,16 @@ describe("orchestrate keyword highlighting", () => { expect(Bun.stripANSI(decorated)).toBe("please orchestrate this"); }); + it("decorates punctuation-adjacent prose while preserving visible text", () => { + const input = 'please "orchestrate," then continue'; + const decorated = highlightOrchestrate(input); + expect(decorated).not.toBe(input); + expect(Bun.stripANSI(decorated)).toBe(input); + }); + it("leaves text without the standalone keyword untouched", () => { expect(highlightOrchestrate("nothing here")).toBe("nothing here"); - // Probe hits the substring but the whitespace boundary fails — no decoration. + // Probe hits the substring but token/path boundaries fail — no decoration. expect(highlightOrchestrate("orchestrated builds")).toBe("orchestrated builds"); expect(highlightOrchestrate("Orchestrate this")).toBe("Orchestrate this"); // The reported bug: a filename must not be painted. diff --git a/packages/coding-agent/test/modes/workflow.test.ts b/packages/coding-agent/test/modes/workflow.test.ts index 61a5ae838..c0ed30e16 100644 --- a/packages/coding-agent/test/modes/workflow.test.ts +++ b/packages/coding-agent/test/modes/workflow.test.ts @@ -13,23 +13,28 @@ beforeAll(() => { }); describe("workflow keyword detection", () => { - it("matches the lowercase trigger word delimited by whitespace", () => { + it("matches the lowercase trigger word delimited by whitespace or a string edge", () => { expect(containsWorkflow("workflowz")).toBe(true); expect(containsWorkflow("please workflowz this rollout")).toBe(true); expect(containsWorkflow("design the workflowz")).toBe(true); expect(containsWorkflow("run these workflowz")).toBe(true); }); - it("ignores old triggers, casing, inflections, punctuation-adjacent, and path-embedded forms", () => { + it("matches the lowercase trigger word beside prose punctuation and quotes", () => { + for (const text of ["do it. workflowz.", "please workflowz, then report", 'say "workflowz" now']) { + expect(containsWorkflow(text)).toBe(true); + } + }); + + it("ignores old triggers, casing, inflections, and path-embedded forms", () => { expect(containsWorkflow("workflow")).toBe(false); expect(containsWorkflow("workflows")).toBe(false); expect(containsWorkflow("Workflowz")).toBe(false); expect(containsWorkflow("WORKFLOWZ")).toBe(false); expect(containsWorkflow("workflowzed the build")).toBe(false); expect(containsWorkflow("reworkflowz everything")).toBe(false); - // A path/extension is not whitespace, so the word never triggers. + // A path/extension must not trigger even though sentence punctuation does. expect(containsWorkflow("packages/coding-agent/test/modes/workflowz.test.ts")).toBe(false); - expect(containsWorkflow("do it. workflowz.")).toBe(false); expect(containsWorkflow("nothing to see here")).toBe(false); }); }); @@ -43,8 +48,15 @@ describe("workflow keyword highlighting", () => { expect(Bun.stripANSI(decorated)).toBe(input); }); + it("decorates punctuation-adjacent prose while preserving visible text", () => { + const input = 'please "workflowz," then continue'; + const decorated = highlightWorkflow(input); + expect(decorated).not.toBe(input); + expect(Bun.stripANSI(decorated)).toBe(input); + }); + it("leaves text without the standalone keyword untouched", () => { - // Probe hits the substring but the whitespace boundary fails — no decoration. + // Probe hits the substring but token/path boundaries fail — no decoration. expect(highlightWorkflow("workflowzed builds")).toBe("workflowzed builds"); expect(highlightWorkflow("Workflowz this")).toBe("Workflowz this"); const filePath = "packages/coding-agent/test/modes/workflowz.test.ts";