feat(polish): cross-mode universal directives field#376
Open
katanumahotori wants to merge 3 commits into
Open
Conversation
Adds a single multi-line text field — `polish_universal_directives` on
UserPreferences — whose contents are injected into every polish (and
translate) system prompt, regardless of which polish mode the user has
selected. Empty / whitespace-only values are no-ops, so the change is
behavior-preserving by default.
## Why
Style modes (Light / Structured / Formal) describe *voice* — how
verbose, how rewriting-friendly, how formal. They do not (and should
not) encode user-specific *typography* conventions, which the same user
wants applied across every mode. Today a Japanese user who insists on
full-width punctuation has no way to express that without forking each
of the four mode prompts. Same for English users who want the Oxford
comma, Chinese users who want full-width paragraph indents, etc.
The dictionary covers vocabulary biasing, but not output formatting
rules. There is no other prompt knob that survives a mode switch. This
field fills that gap.
## What
- **`types.rs`** — adds `polish_universal_directives: String`
(default empty) on `UserPreferences`; mirrored on `UserPreferencesWire`
with `#[serde(default)]` so older preferences.json deserializes
cleanly. No migration needed.
- **`polish.rs`** — `compose_system_prompt` takes a third arg
`universal_directives: &str`. When non-blank, it is appended after the
mode prompt and before the hotword block, with a Chinese-language
section header consistent with the existing hotword block ("通用规则
（不论 polish mode 如何，始终遵守，与上述模式指示并存）"). The order matters: mode
prompt establishes voice → universal rules layer typography on top →
hotwords are last (most specific, easiest to attend to).
- **`polish.rs`** — `polish()` and `translate_to()` both accept the new
arg. Translation gets it too because the user wants their typography
conventions in translated output as well, not just polished input.
- **`coordinator.rs`** — `polish_or_passthrough`, `polish_text`,
`translate_or_passthrough`, `translate_text`, and the `repolish`
command all thread the directive through. Read once from
`prefs.polish_universal_directives` at session start; cloned String
passed by ref through the chain.
- **`commands.rs`** — `validate_llm_provider` (the "test the LLM
endpoint" call) passes an empty string; the validator does not need
user typography rules to verify connectivity.
- **Frontend (`Style.tsx`)** — multi-line textarea added at the top of
the Style page, above the existing 4-mode grid. Persists on `blur`
to avoid 1 IPC per keystroke. Shares the same save-error /
rollback path as the master toggle.
- **Types & i18n** — `UserPreferences` TS type, `defaultUserPrefs`
fixtures (ipc.ts, stylePrefs.test.ts), and 5 locales (ja / en / ko /
zh-CN / zh-TW) with label, description, and a placeholder example
appropriate to that language's typography concerns.
## Tests
Four new tests in `polish::tests`:
- empty / blank directives produce no `通用规则` block (legacy parity)
- non-empty directives append the section, content reproduced verbatim
- order contract: directives appear before hotwords in the composed
prompt (`directives_pos < hotwords_pos`)
- leading / trailing whitespace is trimmed without dropping interior
newlines
The pre-existing
`compose_system_prompt_prefers_correct_spelling_for_hotwords` test is
updated to pass `""` for the new arg, exercising the no-op path.
## Out of scope
- Raw mode bypasses the LLM entirely, so directives have no effect there
(documented in the field doc-comment and in the i18n description).
- QA chat (`answer_chat_streaming`) and other non-polish/non-translate
LLM paths are intentionally untouched — those are conversational, not
output-formatting, surfaces.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
PR Reviewer Guide 🔍(Review updated until commit 05a2683)Here are some key observations to aid the review process:
|
added 2 commits
May 9, 2026 12:46
The pre-existing `chat_completion_omits_authorization_when_api_key_is_empty` integration test calls `provider.polish(...)` directly. Updated to pass `""` for the new `universal_directives` parameter; this was missed in the initial commit because the test file was below the search radius. CI caught it cleanly on macOS / Windows / Linux.
…t-in prompt collision
The Chinese-language built-in mode prompts already contain a `# 通用规则`
(common rules) section. Naming the new universal-directives section
header `通用规则（...)` collided on the substring, which made
`!prompt.contains("通用规则")` in the new "empty directives = legacy
parity" test fail on every CI runner — the built-in section is always
present.
Renamed the new section header to `用户全局指令（...)` so it is unique to
this feature and unambiguously distinguishable from any built-in
section. Updated the three new tests to assert against the new keyword,
and tightened the order test's hotword anchor to `热词（` (the start of
the hotword block) rather than the substring `热词` (which appears in
discussion text inside the built-in prompt). Translate path renamed
in lockstep so polish and translate share the same marker.
User-facing strings (i18n, doc-comments) did not reference the Chinese
header text and need no changes.
Caught by upstream CI on macOS / Linux / (Windows pending). The local
test runner crashes with STATUS_ENTRYPOINT_NOT_FOUND on this
contributor's machine, so this regression had to be caught at PR push
time.
|
Persistent review updated to latest commit 8c19eb0 |
|
Persistent review updated to latest commit 05a2683 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Why
Style modes (Light / Structured / Formal) describe voice — how verbose, how rewriting-friendly, how formal. They do not (and should not) encode user-specific typography conventions, which the same user wants applied across every mode.
Today a Japanese user who insists on full-width punctuation (
、。instead of,.) and full-width spaces after！？has no way to express that without forking each of the four mode prompts. Same for English users who want the Oxford comma, Chinese users who want full-width paragraph indents, Korean users who want specific particle conventions, etc.The dictionary covers vocabulary biasing, but not output formatting rules. There is no other prompt knob that survives a mode switch. This PR adds one.
What
A single new
Stringpreference,polish_universal_directives(default empty), whose contents are injected into every polish system prompt right after the mode-specific instructions and before the hotword block. Empty / whitespace values are no-ops — behavior is byte-for-byte unchanged for users who never touch the field.Backend
types.rs— adds the field onUserPreferenceswith#[serde(default)]onUserPreferencesWire, so olderpreferences.jsonfiles deserialize cleanly. No migration needed.polish.rs—compose_system_prompttakes a third arg. When non-blank, it is appended with a Chinese section header consistent with the existing hotword block (通用规则（不论 polish mode 如何，始终遵守，与上述模式指示并存）). Order: mode prompt → universal rules → hotwords. Mode establishes voice; universal rules layer typography on top; hotwords are last (most specific, easiest for the model to attend to).polish.rs—polish()andtranslate_to()both accept the new arg. Translation deliberately receives it too: a user who wants full-width punctuation in their dictation also wants it in their translated output. Skipping translation would have produced confusing inconsistency.coordinator.rs—polish_or_passthrough,polish_text,translate_or_passthrough,translate_text, and therepolishcommand all thread the directive through. Read once fromprefs.polish_universal_directivesat session start.commands.rs—validate_llm_provider(the test-LLM-endpoint call) passes an empty string; connectivity validation does not need typography rules.Frontend
Style.tsx— multi-line textarea at the top of the Style page, above the existing 4-mode grid. Persists onblur(no IPC per keystroke). Shares the master-toggle save-error / rollback path.UserPreferencesinterface,defaultUserPrefsinipc.ts, the test fixture instylePrefs.test.ts.Tests
Four new tests in
polish::tests:通用规则block emitted (legacy parity)通用规则appears before热词in the composed promptThe pre-existing
compose_system_prompt_prefers_correct_spelling_for_hotwordstest is updated to pass\"\"for the new arg, exercising the no-op path.Backwards compatibility
#[serde(default)]means missing keys in olderpreferences.jsondeserialize without error.Out of scope
answer_chat_streaming) and other non-polish/non-translate LLM paths are intentionally untouched — those are conversational, not output-formatting, surfaces. Adding directives there would risk turning conversational answers into stilted typography exercises.PR Type
Enhancement, Tests
Description
Add
polish_universal_directivesfield toUserPreferencesInject directives into polish and translation system prompts
compose_system_promptappends them after mode instructions, before hotwordstranslate_toappends them so translated output follows the same rulesThread the new field through all coordinator polish/translate call chains
polish_text,translate_text,polish_or_passthrough,translate_or_passthroughAdd Style page UI with a textarea and i18n strings for five locales
Diagram Walkthrough
File Walkthrough
5 files
Add `polish_universal_directives` field to UserPreferences and wireInject universal directives into system prompt, add testsPass directives through all polish/translate call chainsAdd universal directives textarea UI with blur commitAddpolishUniversalDirectivesto TypeScript UserPreferences interface3 files
Update test call to include empty directivesUpdate mock settings with new field defaultAdd `polishUniversalDirectives: ''` to test mock preferences5 files
Add English UI labels for universal directivesAdd Japanese UI labels for universal directivesAdd Korean UI labels for universal directivesAdd Simplified Chinese UI labels for universal directivesAdd Traditional Chinese UI labels for universal directives