fix(retain): drop the source-language rule when an output language is set - #3777
fix(retain): drop the source-language rule when an output language is set#3777feniix wants to merge 6 commits into
Conversation
Strix Security ReviewWarning This pull request has 13 commits after the last Strix review ( No security issues found. Updated for Reviewed by Strix |
|
Thanks for this — I verified the bug and the fix independently, and the fix itself is correct. Running the real But could you please scope this PR down to only what's needed to fix #3776? Right now it's 14 commits spanning two unrelated workstreams. Only these are the language fix:
The rest are #3756 retain-memory follow-ups — content-hash derivation, the sub-batch splitter, the screening cache, Two things I noticed that support splitting:
One heads-up on CI: this is a fork PR, and |
9af4356 to
968a7a0
Compare
|
Scoped down: the five commits you listed, rebased on current The #3756 follow-ups are #3787. Splitting them surfaced that
Also expanded the PR description, which never covered the reflect half. On CI: 55 passed locally here, 54 on #3787, |
|
Added the It found that the reflect fix didn't work. With the contradicting rule gone and nothing left in the prompt disagreeing with the directive, a Chinese question with the output language set to English still came back in Chinese:
Retain is unaffected. I ran it separately rather than assuming: its directive is followed from the system prompt and the extraction test passes unchanged. I don't have an explanation for the difference that I've actually tested, so I've left one out. Every deterministic test passed throughout, including the two I'd added — the prompt was correct and only its position was wrong. Four prompt-assembly tests encoded the old layout and are updated to pin the directive to the user prompt. Verified on Retain follow-ups and |
… set (vectorize-io#3776) `HINDSIGHT_API_LLM_OUTPUT_LANGUAGE` was a silent no-op for fact extraction. The extraction prompt hardcodes a "detect the input language and never translate" rule near the top, and `output_language_directive()` appends the opposite instruction at the end, so the prompt contradicted itself. The model resolves that in favour of the first rule — it comes ~6KB earlier and is phrased far more forcefully ("STRICTLY FORBIDDEN from translating" vs "Respond exclusively in X") — and facts kept being stored in the source language while the setting appeared to be in effect. Consolidation already solves this: `build_consolidation_system_prompt()` omits its default rule when a language is configured, and `test_consolidation_directive_replaces_source_language_rule` guards it. Retain never got the same treatment, and its tests only asserted the directive was present, never that the contradicting rule was absent — which is how this shipped. Hoist the rule into `_DEFAULT_LANGUAGE_RULE` and emit it through a `{language_section}` placeholder, deferred through the two-stage format the mode templates already use for `{retain_mission_section}`. All four extraction modes are affected and all four are fixed. The rule is kept when no output language is set: it exists for vectorize-io#181 (CONCISE mode drifting to Japanese/Chinese on English/Italian input), so this makes the two mutually exclusive rather than removing it. The unset path is byte-identical to before this change in every mode. Per the cacheable-prefix invariant in `test_retain_cacheable_prefix_invariant_to_per_bank_freetext`, output language is a low-cardinality structural toggle that may change the prefix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Retain and consolidation each spelled out the same invariant — an explicit output language drops the preserve-the-source-language rule instead of arguing with it — as an inline `"" if language else ...` plus a paragraph of comment explaining why. Two copies of a rule that must never diverge. `default_language_section()` in prompt_utils now owns the selection and the reasoning, next to `output_language_directive()`, the other half it excludes. Each pipeline keeps its own wording of the rule; only the choice is shared. Also flattens `_build_extraction_prompt_and_schema`: every template takes the same two placeholders and only custom mode substitutes a third, so the modes now differ in which constant they name rather than each repeating the `.format()` call. The language tests assert against the rule constants instead of quoting fragments of their text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`mode_extras: dict[str, str]` only ever held one key, in one branch, and the project standard is that a dict with a known key set should be a scalar or a typed model. `str.format` ignores a keyword no template references, so passing `custom_instructions` unconditionally is the same behaviour without asking the reader which other keys can appear. Prompt output is unchanged: all 27 variants (4 modes x 3 language settings x custom-instructions on/off, plus consolidation) hash identically to 067ed1e. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s set Same defect vectorize-io#3776 fixed for retain, still live in the third pipeline. `build_final_system_prompt` appended `_FINAL_LANGUAGE_RULE` unconditionally and then `output_language_directive(...)`, so a prompt built with HINDSIGHT_API_LLM_OUTPUT_LANGUAGE set told the model both to answer in the question's language and to translate everything into the configured one. The rule carries an escape hatch — "if a directive above specifies a response language, follow the directive" — but the directive is appended at the very END of the prompt, so "above" never matched it. The model followed the rule, which comes first and is phrased more forcefully, and the setting was silently no-opped for reflect answers. Reflect now routes through `default_language_section()` like the other two, so the mutual exclusion has one owner. The reasoning/agent prompt is untouched: it never receives `llm_output_language` (only the final synthesis call does), so its language rule has nothing to contradict. Verified the new test fails against the previous code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tract
`test_final_prompt_output_language_override_is_appended_last` asserted the
override merely came *after* the default LANGUAGE rule, on the theory that
appending it last made it win. That is the theory the previous commit
disproved: the rule is phrased more forcefully and comes first, so the model
followed it and the configured language was a no-op.
With the rule now dropped when a language is set, `prompt.index("## LANGUAGE")`
raised ValueError. Renamed and rewritten to assert what actually has to hold —
the directive is present and the default rule is gone. Also renamed
`test_final_prompt_always_includes_language_rule`, which only ever covered the
no-output-language case, so "always" no longer overstates it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…follow it Dropping the contradicting source-language rule was necessary but not sufficient, and only a real-LLM test could show it. With the rule gone and nothing in the prompt disagreeing with it, a Chinese question with the output language set to English still came back in Chinese 12 times out of 12 on gemini-2.5-flash-lite, and 5/5 on gemini-2.5-flash. The directive was appended to the end of `build_final_system_prompt`, which is last in the *system* message — but the question and the retrieved data arrive after it, in the user message. "Last in the system prompt" is not last, and the model answered in the language of the text nearest the end. `build_final_prompt` and `build_reduce_prompt` now close the user message with it instead: 12/12 and 5/5 English, same prompts otherwise. Both, not just the first: `_forced_final_synthesis` picks between them on whether the retrieved data fits one chunk, and a setting that only worked below that threshold would be the same silent no-op wearing a different hat. This also takes a per-bank toggle out of the cacheable system prefix, which can only help the prefix cache. Retain needed no such move — verified independently, its directive is followed from the system prompt. The difference is the task: extraction rewrites the input, where the instruction is the whole job; answering a question inherits a very strong prior to mirror the asker's language. Tests: `test_retain_reflect_output_language.py` drives the real synthesis call and judges the answer, and is what caught this — the deterministic tests passed throughout, because the prompt was correct and only its layout was wrong. The prompt-assembly tests now pin the directive to the user prompt, including that it comes last. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0573d1b to
34897e3
Compare
2 participants
Fixes #3776.
Problem
HINDSIGHT_API_LLM_OUTPUT_LANGUAGEis silently a no-op for retain. The extraction prompt hardcodes a "detect the input language and never translate" rule near the top, andoutput_language_directive()appends the opposite instruction at the end — the prompt contradicts itself.The model resolves that in favour of the first rule: it lands ~6KB earlier (position 221 vs 6595 in the concise prompt) and is phrased far more forcefully — "STRICTLY FORBIDDEN from translating" against "Respond exclusively in X". Facts keep getting stored in the source language while
/configreports the setting as active.Consolidation already solves exactly this.
build_consolidation_system_prompt()omits its default rule when a language is configured,consolidation/prompts.py:22-27writes down the invariant ("the two must never both be present or they contradict each other"), andtest_consolidation_directive_replaces_source_language_ruleguards it. Retain never got the same treatment.Fix
Hoist the rule into
_DEFAULT_LANGUAGE_RULEand emit it through a{language_section}placeholder, deferred through the same two-stage.format()the mode templates already use for{retain_mission_section}.All four extraction modes carried the rule (the base template feeds concise/custom/verbatim; verbose had its own copy) and all four are fixed:
The rule is deliberately kept when no output language is set. It exists for #181 (CONCISE mode drifting to Japanese/Chinese on English/Italian input), so removing it outright would regress that. This makes the two mutually exclusive rather than deleting either.
The unset path is byte-identical to before this change in all four modes — verified by diffing the built prompts against
main.On the cacheable prefix:
test_retain_cacheable_prefix_invariant_to_per_bank_freetextalready documents output language as a low-cardinality structural toggle that may change the prefix and is correctly keyed by the cache fingerprint, so gating on it is in line with the existing invariant. High-cardinality free-text (mission, custom instructions) still cannot leak in — that test passes unchanged.Tests
Two new parametrized tests over all four modes, mirroring the consolidation pair:
test_retain_directive_replaces_source_language_rule— the contradicting rule is gone when a language is set. Fails on all 4 modes without this fix.test_retain_unset_requires_source_language— the rule is still there when unset. Passes either way by design; it's the guard against "fixing" this by simply deleting the rule and regressing [Bug] CONCISE mode outputs Japanese/Chinese for English/Italian input due to prompt priming #181.The existing retain tests only asserted the directive was present, never that the contradicting rule was absent — which is how this shipped.
The two failures and six errors in
tests/test_retain.pyare pre-existing in this environment (they requireHINDSIGHT_API_LLM_API_KEY); they fail identically on unmodifiedmain.Verification against a live deployment
Reproduced on
0.9.1with provideropenai-codex/gpt-5.6-lunaandHINDSIGHT_API_LLM_OUTPUT_LANGUAGE=English.Spanish input:
Before — stored fact still Spanish:
After:
Russian and Chinese inputs likewise produced English only after the change. The
llm_requestsaudit table confirmed both contradicting instructions were present in the same prompt beforehand.Reflect had the same contradiction
The issue assumed reflect was fine. It was not —
build_reflect_system_prompt()appended its source-language rule unconditionally and relied on in-prompt precedence, so a configured output language hit exactly the same silent no-op. It now goes through the shared helper too, anddefault_language_section()'s docstring records that all three pipelines call it: nothing may append a rule of its own without reintroducing the contradiction.Scope
Narrowed at review request — this PR is now only the language fix: 5 commits over 6 files, all prompt assembly. The #3756 retain-memory follow-ups that were originally stacked here (content-hash derivation, the sub-batch splitter, the screening cache,
_SubBatch.index,_flush(), the benchmark imports) have moved to #3787 and land on their own merits.The reflect fix needed a second half — found by adding the LLM test
Adding the
hs_llm_corecounterpart totest_consolidation_output_language.py(which thedeterministic tests had no way to substitute for) showed that dropping the contradicting rule
did not actually change reflect's behaviour. With the rule gone and nothing in the prompt
disagreeing with the directive, a Chinese question with the output language set to English
still came back in Chinese:
build_final_system_promptappended the directive at the end of the system message — but thequestion and the retrieved data arrive after it, in the user message. "Last in the system prompt"
is not last.
build_final_promptandbuild_reduce_promptnow close the user message with it.Both, because
_forced_final_synthesispicks between them on whether the retrieved data fits onechunk, and a setting that only worked below that threshold is the same silent no-op in a new
disguise. It also takes a per-bank toggle out of the cacheable system prefix.
Retain needed no such move — verified independently, its directive is followed from the system
prompt. The difference is the task: extraction rewrites its input, where the instruction is the
whole job; answering a question carries a strong prior to mirror the asker's language.
Tests
tests/test_retain_reflect_output_language.py— the behavioural half, mirroringtest_consolidation_output_language.py, which had no retain or reflect counterpart. Chinesesource material so "did it translate?" is a script test rather than a vocabulary guess; the judge
confirms the phrasing. Both tests drive the real pipeline:
extract_facts_from_textfor retain,and for reflect the same system/user prompts and tool-less call
_forced_final_synthesismakes.Verified against
gemini-2.5-flash-lite, the model the Core LLM job uses — 2 passed, fourconsecutive runs. The deterministic prompt-assembly tests passed throughout the broken reflect
behaviour, which is precisely why this file exists.