From b41c8b988f28b0a3d313e4558f47317bb2678d09 Mon Sep 17 00:00:00 2001 From: "Hardening v2.7" Date: Sat, 29 Aug 2026 00:09:31 +0530 Subject: [PATCH] fix(modes): honour an explicit reference-files switch in interview-prep modes Follow-up to #520. T8 gave technical-interview a reference pool and put `reference_files` into its permitted switches, so the "Primary knowledge source" control offers it and an attached file became REACHABLE -- second-person reachability went 0/6 to 6/6. But `buildUserSourceContract` pinned `defaultOwner: 'profile'` for interview-prep templates REGARDLESS of what the user ticked, so the saved contract still resolved `profile_only`, `documentGroundedFromContract` still returned false, and `forceDocumentGrounding` stayed off. Measured, identical user selection, two modes: technical-interview + reference_files -> profile_only docGrounded=false general + reference_files -> reference_files_primary docGrounded=true So everything gated on that switch stayed off in the one mode whose users are most likely to upload project documents: topK 6 and a 1800-token budget instead of 12/3600, no per-file floor, no answerability scoring, no section-target or positional restore, no identity block, no query normalization. A user could ask for their reference files and be handed a materially weaker retrieval than the same files in General -- which is the mode inversion the beta report described, surviving the fix that was supposed to close it. #520's commit message claimed this clause of the decision ("restores documentCentricMode routing") was delivered. It was not, and I did not verify it. This is that verification, and the fix. UPLOAD IS STILL NOT CONSENT. This reads the user's EXPLICIT switch, never the presence of a file. A mode without the tick keeps `profile` and behaves exactly as before -- asserted for both interview-prep templates, with no switches and with profile+JD only. THE MEMORY-POLICY CONSEQUENCE IS REAL and is pinned in the suite rather than left to surface as a bug report. Reference-files ownership forbids Hindsight (invariant #3), so a mode the user grounds this way loses: allowHindsight true -> FALSE cross-meeting recall is off allowPriorAssistantFacts true -> FALSE the assistant's own prior output stops counting as evidence, which is a fabrication vector closing allowPriorAssistantReferents stays TRUE, so follow-ups still resolve That is a consequence of the user's own explicit choice, and the interview-prep seed comment says document-grounded modes must not use Hindsight. Flagged for the owner because it is a live behaviour change, not because it is unintended. Behind NATIVELY_RETRIEVAL_INTERVIEW_PREP_HONORS_REFERENCE_SWITCH, default ON via a literal, proven to reproduce the pre-fix behaviour when off. Non-interview templates are asserted byte-identical with the flag in either position. test:intelligence 2037/0; services 3402/0. Reviewed but not executed on macOS and Windows (pure TypeScript, no platform branch). The suite runs under the services glob, which is Windows-ADVISORY (`continue-on-error`), so this is `Covered by automated macOS branch tests` and `Requires physical Windows verification`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014D8QkwcTrpGVPmkWHrp4Nz --- .../contracts/retrieval-flags.ts | 6 + electron/services/ModesManager.ts | 32 +++- ...viewPrepReferenceSwitch2026_08_29.test.mjs | 143 ++++++++++++++++++ 3 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 electron/services/__tests__/InterviewPrepReferenceSwitch2026_08_29.test.mjs diff --git a/electron/context-intelligence/contracts/retrieval-flags.ts b/electron/context-intelligence/contracts/retrieval-flags.ts index a51e7896d..d7a6be2cb 100644 --- a/electron/context-intelligence/contracts/retrieval-flags.ts +++ b/electron/context-intelligence/contracts/retrieval-flags.ts @@ -81,6 +81,12 @@ export const RETRIEVAL_FLAGS = { default: true, why: 'RC5: a global score sort across ports discarded the status partition, per-type round-robin and per-document interleave, and compared incomparable score scales', }, + /** Interview-prep modes honour an EXPLICIT reference-files switch (2026-08-29). */ + interviewPrepHonorsReferenceSwitch: { + env: 'NATIVELY_RETRIEVAL_INTERVIEW_PREP_HONORS_REFERENCE_SWITCH', + default: true, + why: 'RC4 remainder: T8 made reference files REACHABLE in technical-interview, but buildUserSourceContract pinned defaultOwner=profile, so forceDocumentGrounding stayed off even when the user ticked the switch', + }, /** T7 — referent resolution compares the turn's scope before reusing a topic. */ referentScopeCheck: { env: 'NATIVELY_RETRIEVAL_REFERENT_SCOPE_CHECK', diff --git a/electron/services/ModesManager.ts b/electron/services/ModesManager.ts index 40ab9de9a..fcc32adc2 100644 --- a/electron/services/ModesManager.ts +++ b/electron/services/ModesManager.ts @@ -1,5 +1,6 @@ import * as crypto from 'crypto'; import { DatabaseManager } from '../db/DatabaseManager'; +import { isRetrievalFixEnabled } from '../context-intelligence/contracts/retrieval-flags'; import type { EmbeddingPipeline } from '../rag/EmbeddingPipeline'; import { ModeContextRetriever, type ModeRetrievalOptions, type RetrieveOptions } from './ModeContextRetriever'; import type { ModeRetrievedContext as HybridContext } from './modes/ModeHybridRetriever'; @@ -822,7 +823,36 @@ export class ModesManager { const isInterviewPrep = input.templateType === 'looking-for-work' || input.templateType === 'technical-interview'; const switches = input.switches.filter((s) => s !== 'transcript'); - const defaultOwner: ModeSourceOwner = isInterviewPrep ? 'profile' : 'reference_files'; + // Interview-prep modes are profile-first BY DEFAULT — but not when the + // user has explicitly said otherwise (2026-08-29). + // + // THE GAP THIS CLOSES. T8 gave technical-interview a reference pool and + // put `reference_files` in its permitted switches, so the "Primary + // knowledge source" control offers it and the file is now REACHABLE. + // But this function pinned `defaultOwner: 'profile'` for interview-prep + // regardless of what the user ticked, so the contract still resolved + // `profile_only`, `documentGroundedFromContract` still returned false, + // and `forceDocumentGrounding` stayed OFF. Measured: ticking "Reference + // files" in Technical Interview produced sourceAuthority=profile_only + // and docGrounded=false, while the identical selection in General + // produced reference_files_primary / true. + // + // Everything gated on that switch therefore stayed off in the one mode + // whose users are most likely to upload project documents: topK 6 and a + // 1800-token budget instead of 12/3600, no per-file floor, no + // answerability scoring, no section-target or positional restore, no + // identity block, no query normalization. The user could ask for their + // reference files and be given a materially weaker retrieval than the + // same files in General. + // + // The upload-is-not-consent rule is untouched: this reads the user's + // EXPLICIT switch, not the presence of a file. A mode with no + // `reference_files` tick keeps `profile` and behaves exactly as before. + const userChoseReferenceFiles = switches.includes('reference_files') + && isRetrievalFixEnabled('interviewPrepHonorsReferenceSwitch'); + const defaultOwner: ModeSourceOwner = (isInterviewPrep && !userChoseReferenceFiles) + ? 'profile' + : 'reference_files'; return buildUserSelectedSourceContract({ defaultOwner, allowedExplicitSwitches: switches as any, diff --git a/electron/services/__tests__/InterviewPrepReferenceSwitch2026_08_29.test.mjs b/electron/services/__tests__/InterviewPrepReferenceSwitch2026_08_29.test.mjs new file mode 100644 index 000000000..9ce4a7a50 --- /dev/null +++ b/electron/services/__tests__/InterviewPrepReferenceSwitch2026_08_29.test.mjs @@ -0,0 +1,143 @@ +// Ticking "Reference files" in an interview-prep mode now actually grounds in +// them. +// +// THE GAP. T8 (2026-08-28) gave technical-interview a reference pool and put +// `reference_files` into its permitted switches, so the "Primary knowledge +// source" control offers it and an attached file became REACHABLE — measured +// second-person reachability went 0/6 to 6/6. But `buildUserSourceContract` +// pinned `defaultOwner: 'profile'` for interview-prep templates regardless of +// what the user ticked, so the saved contract still resolved `profile_only`, +// `documentGroundedFromContract` still returned false, and +// `forceDocumentGrounding` stayed OFF. +// +// Measured before this fix — the identical user selection, two modes: +// +// technical-interview + reference_files -> profile_only docGrounded=false +// general + reference_files -> reference_files_primary docGrounded=true +// +// So everything gated on that switch stayed off in the one mode whose users are +// most likely to upload project documents: topK 6 and a 1800-token budget +// instead of 12/3600, no per-file floor, no answerability scoring, no +// section-target or positional restore, no identity block, no query +// normalization. The user could ask for their reference files and be handed a +// materially weaker retrieval than the same files in General. +// +// UPLOAD IS STILL NOT CONSENT. This reads the user's EXPLICIT switch, never the +// presence of a file — the rule T8 was built around is unchanged and is +// asserted below. + +import { test, describe } from 'node:test'; +import assert from 'node:assert/strict'; +import path from 'node:path'; +import { createRequire } from 'node:module'; +import { fileURLToPath } from 'node:url'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const repoRoot = path.resolve(__dirname, '../../..'); +const cjsRequire = createRequire(import.meta.url); + +const { ModesManager } = cjsRequire(path.resolve(repoRoot, 'dist-electron/electron/services/ModesManager.js')); +const msc = cjsRequire(path.resolve(repoRoot, 'dist-electron/electron/services/modeSourceContract.js')); + +const ENV = 'NATIVELY_RETRIEVAL_INTERVIEW_PREP_HONORS_REFERENCE_SWITCH'; +const withFlag = (value, fn) => { + const original = process.env[ENV]; + process.env[ENV] = value; + try { return fn(); } finally { + if (original === undefined) delete process.env[ENV]; + else process.env[ENV] = original; + } +}; + +/** The real builder, without standing up a database. */ +const build = (templateType, switches) => + ModesManager.prototype.buildUserSourceContract.call( + Object.create(ModesManager.prototype), + { modeId: 'm', templateType, switches }); + +const INTERVIEW_PREP = ['technical-interview', 'looking-for-work']; + +describe('an explicit reference-files switch grounds interview-prep modes', () => { + for (const template of INTERVIEW_PREP) { + test(`${template}: ticking reference files enables document grounding`, () => { + const c = build(template, ['profile', 'job_description', 'reference_files']); + assert.equal(c.sourceAuthority, 'reference_files_primary'); + assert.equal(msc.documentGroundedFromContract(c, true), true, + 'forceDocumentGrounding must now be reachable for this mode'); + }); + + test(`${template}: reference files ALONE also grounds`, () => { + const c = build(template, ['reference_files']); + assert.equal(msc.documentGroundedFromContract(c, true), true); + }); + } + + test('it matches what the same selection already did in General', () => { + // The asymmetry was the bug: identical user intent, opposite outcome. + const ti = build('technical-interview', ['reference_files']); + const general = build('general', ['reference_files']); + assert.equal(ti.sourceAuthority, general.sourceAuthority); + }); +}); + +describe('NON-REGRESSION: upload is not consent, and profile-first is still the default', () => { + for (const template of INTERVIEW_PREP) { + test(`${template}: WITHOUT the switch it stays profile-first`, () => { + const c = build(template, ['profile', 'job_description']); + assert.equal(c.defaultOwner, 'profile'); + assert.equal(c.sourceAuthority, 'profile_only'); + assert.equal(msc.documentGroundedFromContract(c, true), false, + 'a mode the user has not switched must not document-ground'); + }); + + test(`${template}: no switches at all stays profile-first`, () => { + assert.equal(build(template, []).sourceAuthority, 'profile_only'); + }); + } + + test('non-interview templates are completely unaffected', () => { + for (const t of ['general', 'sales', 'recruiting', 'team-meet', 'lecture']) { + const on = build(t, ['reference_files']); + const off = withFlag('0', () => build(t, ['reference_files'])); + assert.deepEqual(on, off, `${t} changed`); + } + }); + + test('the kill switch restores the pre-fix behaviour exactly', () => { + withFlag('0', () => { + for (const template of INTERVIEW_PREP) { + const c = build(template, ['profile', 'job_description', 'reference_files']); + assert.equal(c.sourceAuthority, 'profile_only'); + assert.equal(msc.documentGroundedFromContract(c, true), false); + } + }); + }); +}); + +describe('the memory-policy consequence is real, and is pinned here on purpose', () => { + // Switching an interview-prep mode to reference-files ownership ALSO changes + // its memory policy, because invariant #3 forbids Hindsight for any + // document-grounded owner. That is a deliberate consequence of the user's own + // choice, not a side effect to discover later in a bug report: + // + // allowHindsight true -> FALSE (cross-meeting recall is off) + // allowPriorAssistantFacts true -> FALSE (the assistant's own prior output + // stops counting as evidence — a + // fabrication vector closing, and + // arguably a gain) + // allowPriorAssistantReferents stays TRUE, so follow-ups still resolve + // ("what did you monitor after that?") + test('grounding an interview-prep mode turns Hindsight off', () => { + const grounded = build('technical-interview', ['reference_files']); + assert.equal(grounded.memoryPolicy.allowHindsight, false); + assert.equal(grounded.memoryPolicy.allowPriorAssistantFacts, false); + assert.equal(grounded.memoryPolicy.allowPriorAssistantReferents, true, + 'follow-up resolution must survive — losing it would break "and what about X?"'); + }); + + test('an ungrounded interview-prep mode keeps both', () => { + const profileFirst = build('technical-interview', ['profile']); + assert.equal(profileFirst.memoryPolicy.allowHindsight, true); + assert.equal(profileFirst.memoryPolicy.allowPriorAssistantFacts, true); + }); +});