diff --git a/scripts/seed-forecasts.mjs b/scripts/seed-forecasts.mjs index 53b9ffad6f..881043d9fd 100644 --- a/scripts/seed-forecasts.mjs +++ b/scripts/seed-forecasts.mjs @@ -4604,14 +4604,16 @@ function summarizeImpactPathScore(path = null) { if (path.simulationAdjustmentDetail !== undefined) { const d = path.simulationAdjustmentDetail; summary.simDetail = { - bucketChannelMatch: Boolean(d.bucketChannelMatch), - actorOverlapCount: Number(d.actorOverlapCount), - candidateActorCount: Number(d.candidateActorCount), - actorSource: d.actorSource, - resolvedChannel: d.resolvedChannel || '', - channelSource: d.channelSource, - invalidatorHit: Boolean(d.invalidatorHit), - stabilizerHit: Boolean(d.stabilizerHit), + bucketChannelMatch: Boolean(d.bucketChannelMatch), + actorOverlapCount: Number(d.actorOverlapCount), + roleOverlapCount: Number(d.roleOverlapCount ?? d.actorOverlapCount), + keyActorsOverlapCount: Number(d.keyActorsOverlapCount ?? 0), + candidateActorCount: Number(d.candidateActorCount), + actorSource: d.actorSource, + resolvedChannel: d.resolvedChannel || '', + channelSource: d.channelSource, + invalidatorHit: Boolean(d.invalidatorHit), + stabilizerHit: Boolean(d.stabilizerHit), }; } } @@ -11427,7 +11429,7 @@ function negatesDisruption(stabilizer, candidatePacket) { */ function computeSimulationAdjustment(expandedPath, simTheaterResult, candidatePacket) { let adjustment = 0; - const details = { bucketChannelMatch: false, actorOverlapCount: 0, invalidatorHit: false, stabilizerHit: false, resolvedChannel: '', channelSource: 'none', candidateActorCount: 0, actorSource: 'none', simPathConfidence: 1.0 }; + const details = { bucketChannelMatch: false, actorOverlapCount: 0, roleOverlapCount: 0, keyActorsOverlapCount: 0, invalidatorHit: false, stabilizerHit: false, resolvedChannel: '', channelSource: 'none', candidateActorCount: 0, actorSource: 'none', simPathConfidence: 1.0 }; const { topPaths = [], invalidators = [], stabilizers = [] } = simTheaterResult || {}; const pathBucket = expandedPath?.direct?.targetBucket @@ -11484,13 +11486,27 @@ function computeSimulationAdjustment(expandedPath, simTheaterResult, candidatePa adjustment += +parseFloat((0.08 * simConf).toFixed(3)); details.bucketChannelMatch = true; details.simPathConfidence = simConf; - const simActors = new Set((Array.isArray(bucketChannelMatch.keyActors) ? bucketChannelMatch.keyActors : []).map(normalizeActorName)); - const overlap = candidateActors.filter((a) => simActors.has(a)); - details.actorOverlapCount = overlap.length; - // Overlap bonus fires only when both sides have named geo-political actors. - // Macro-financial theaters with role-based stateSummary.actors (e.g. "Commodity traders", - // "Central banks") will have actorOverlapCount=0 — this is expected, not a bug. - if (overlap.length >= 2) { + // Role overlap: candidate stateSummary.actors vs sim keyActorRoles (role-category vocabulary). + // Drives +0.04 bonus when actorSource=stateSummary. keyActorRoles absent → overlap=0 (graceful). + if (actorSrc === 'stateSummary') { + const simRoles = new Set((Array.isArray(bucketChannelMatch.keyActorRoles) ? bucketChannelMatch.keyActorRoles : []).map(normalizeActorName).filter(Boolean)); + details.roleOverlapCount = candidateActors.filter((a) => simRoles.has(a)).length; + } + + // Entity overlap: candidate actors vs sim keyActors. + // Drives +0.04 bonus when actorSource=affectedAssets (backwards compat). Telemetry when actorSource=stateSummary. + const simEntities = new Set((Array.isArray(bucketChannelMatch.keyActors) ? bucketChannelMatch.keyActors : []).map(normalizeActorName).filter(Boolean)); + details.keyActorsOverlapCount = candidateActors.filter((a) => simEntities.has(a)).length; + + // Bonus decision: role overlap for stateSummary path; entity overlap for affectedAssets fallback. + // Explicit third branch for actorSource='none' (no actors) so future values don't fall through silently. + const bonusOverlap = actorSrc === 'stateSummary' + ? details.roleOverlapCount + : actorSrc === 'affectedAssets' + ? details.keyActorsOverlapCount + : 0; + details.actorOverlapCount = bonusOverlap; // backwards-compat alias + if (bonusOverlap >= 2) { adjustment += +parseFloat((0.04 * simConf).toFixed(3)); } } @@ -12814,6 +12830,11 @@ function buildSimulationPackageFromDeepSnapshot(snapshot, priorWorldState = null topChannel: c.marketContext?.topChannel || '', rankingScore: c.rankingScore, criticalSignalTypes: c.criticalSignalTypes || [], + actorRoles: [...new Set( + (Array.isArray(c?.stateSummary?.actors) ? c.stateSummary.actors : []) + .map((s) => String(s || '').trim()) + .filter(Boolean), + )].slice(0, 12), })); const simulationRequirement = Object.fromEntries( @@ -14089,7 +14110,7 @@ function validateCaseNarratives(items, predictions) { } function sanitizeForPrompt(text) { - return (text || '').replace(/[\n\r]/g, ' ').replace(/[<>{}\x00-\x1f]/g, '').slice(0, 200).trim(); + return (text || '').replace(/[\n\r\u2028\u2029]/g, ' ').replace(/[<>{}\x00-\x1f]/g, '').slice(0, 200).trim(); } // Sanitizes LLM-returned text before writing to Redis as a prompt section. @@ -16173,6 +16194,11 @@ function buildSimulationRound2SystemPrompt(theater, pkg, round1) { const evalTargets = (r2EvalTargets?.requiredPaths || []) .map((p) => `- ${sanitizeForPrompt(p.pathType)}: ${sanitizeForPrompt(p.question)}`).join('\n') || '- General market and security dynamics'; + const actorRoles = Array.isArray(theater.actorRoles) ? theater.actorRoles : []; + const rolesSection = actorRoles.length > 0 + ? `\nCANDIDATE ACTOR ROLES (copy these EXACT strings into keyActorRoles; return [] if none apply):\n${actorRoles.map((r) => `- "${sanitizeForPrompt(r)}"`).join('\n')}` + : ''; + return `You are a geopolitical simulation engine. This is ROUND 2 of a 2-round theater simulation. THEATER: ${sanitizeForPrompt(theater.theaterLabel || theater.theaterId)} | Region: ${sanitizeForPrompt(theater.theaterRegion || theater.dominantRegion || '')} @@ -16183,12 +16209,13 @@ ${pathSummaries} VALID ACTOR IDs: ${entityIds || '(see round 1)'} EVALUATION TARGETS: -${evalTargets} +${evalTargets}${rolesSection} INSTRUCTIONS: For each of the 3 paths from Round 1 (escalation, containment, market_cascade), generate the EVOLVED outcome after 72 hours. -- keyActors: 2-4 actor IDs that drive this path +- keyActors: 2-4 actor IDs that drive this path (entity names) +- keyActorRoles: 0-4 strings copied verbatim from CANDIDATE ACTOR ROLES above (return [] if list is absent or none apply) - roundByRoundEvolution: 2 entries (round 1 summary, round 2 evolution) - timingMarkers: 2-4 key events with timing (T+Nh format) - stabilizers: 2-4 factors that could prevent the worst outcome @@ -16203,6 +16230,7 @@ Return ONLY a JSON object with no markdown fences: "label": "", "summary": "<≤200 char evolved summary>", "keyActors": [""], + "keyActorRoles": [""], "roundByRoundEvolution": [ { "round": 1, "summary": "<≤160 char>" }, { "round": 2, "summary": "<≤160 char>" } @@ -16210,8 +16238,8 @@ Return ONLY a JSON object with no markdown fences: "confidence": 0.35, "timingMarkers": [{ "event": "<≤80 char>", "timing": "T+Nh" }] }, - { "pathId": "containment", "label": "...", "summary": "...", "keyActors": [], "roundByRoundEvolution": [], "confidence": 0.50, "timingMarkers": [] }, - { "pathId": "market_cascade", "label": "...", "summary": "...", "keyActors": [], "roundByRoundEvolution": [], "confidence": 0.15, "timingMarkers": [] } + { "pathId": "containment", "label": "...", "summary": "...", "keyActors": [], "keyActorRoles": [], "roundByRoundEvolution": [], "confidence": 0.50, "timingMarkers": [] }, + { "pathId": "market_cascade", "label": "...", "summary": "...", "keyActors": [], "keyActorRoles": [], "roundByRoundEvolution": [], "confidence": 0.15, "timingMarkers": [] } ], "stabilizers": ["<≤100 char>"], "invalidators": ["<≤100 char>"], @@ -16220,6 +16248,11 @@ Return ONLY a JSON object with no markdown fences: }`; } +/** + * @param {string} text - raw LLM response text (JSON or JSON-with-prefix) + * @param {1 | 2} round - simulation round number + * @returns {{ paths: object[] | null, stabilizers?: string[], invalidators?: string[], globalObservations?: string, confidenceNotes?: string, dominantReactions?: string[], note?: string }} + */ function tryParseSimulationRoundPayload(text, round) { try { const parsed = JSON.parse(text); @@ -16229,7 +16262,12 @@ function tryParseSimulationRoundPayload(text, round) { if (paths.length === 0) return { paths: null }; if (round === 2) { return { - paths, + paths: paths.map((p) => ({ + ...p, + keyActorRoles: Array.isArray(p.keyActorRoles) + ? p.keyActorRoles.map((s) => sanitizeForPrompt(String(s || '')).trim()).filter(Boolean).slice(0, 10) + : [], + })), stabilizers: Array.isArray(parsed.stabilizers) ? parsed.stabilizers.map(String).slice(0, 6) : [], invalidators: Array.isArray(parsed.invalidators) ? parsed.invalidators.map(String).slice(0, 6) : [], globalObservations: String(parsed.globalObservations || '').slice(0, 300), @@ -16806,6 +16844,7 @@ async function writeSimulationOutcome(pkg, outcome, { storageConfig } = {}) { summary: p.summary, confidence: p.confidence, keyActors: (p.keyActors || []).slice(0, 4), + keyActorRoles: (p.keyActorRoles || []).slice(0, 8), })), dominantReactions: (tr.dominantReactions || []).slice(0, 3), stabilizers: (tr.stabilizers || []).slice(0, 3), @@ -16880,6 +16919,23 @@ async function listQueuedSimulationTasks(limit = 10) { return Array.isArray(response?.result) ? response.result : []; } +/** + * Sanitize and allowlist-filter LLM-returned keyActorRoles strings. + * Filters against theater.actorRoles (exact match after normalizeActorName). + * When allowedRoles is empty (old package, no actorRoles field), returns [] — no vocab = no valid roles. + * This prevents hallucinated keyActorRoles from old packages triggering the +0.04 overlap bonus. + * @param {string[] | undefined} rawRoles + * @param {string[]} allowedRoles + * @returns {string[]} + */ +function sanitizeKeyActorRoles(rawRoles, allowedRoles) { + if (!allowedRoles.length) return []; + const sanitized = (Array.isArray(rawRoles) ? rawRoles : []) + .map((s) => sanitizeForPrompt(String(s)).slice(0, 80)); + const allowedNorm = new Set(allowedRoles.map(normalizeActorName)); + return sanitized.filter((s) => allowedNorm.has(normalizeActorName(s))).slice(0, 8); +} + async function processNextSimulationTask(options = {}) { const workerId = options.workerId || `sim-worker-${process.pid}-${Date.now()}`; const queuedRunIds = options.runId ? [options.runId] : await listQueuedSimulationTasks(10); @@ -16943,6 +16999,7 @@ async function processNextSimulationTask(options = {}) { const r2Paths = result.round2?.paths || []; const r1Paths = result.round1?.paths || []; + const allowedRoles = Array.isArray(theater.actorRoles) ? theater.actorRoles : []; const mergedPaths = (r2Paths.length ? r2Paths : r1Paths).map((p) => { const r1Path = r1Paths.find((r) => r.pathId === p.pathId); return { @@ -16950,6 +17007,7 @@ async function processNextSimulationTask(options = {}) { label: sanitizeForPrompt(p.label || p.pathId).slice(0, 80), summary: sanitizeForPrompt(p.summary || '').slice(0, 200), keyActors: Array.isArray(p.keyActors) ? p.keyActors.map((s) => sanitizeForPrompt(String(s)).slice(0, 80)).slice(0, 6) : [], + keyActorRoles: sanitizeKeyActorRoles(p.keyActorRoles, allowedRoles), roundByRoundEvolution: Array.isArray(p.roundByRoundEvolution) ? p.roundByRoundEvolution.map((r) => ({ round: r.round, summary: sanitizeForPrompt(r.summary || '').slice(0, 160) })) : [{ round: 1, summary: sanitizeForPrompt((r1Path?.summary || p.summary || '')).slice(0, 160) }], @@ -17181,6 +17239,7 @@ export { writeSimulationOutcome, buildSimulationRound1SystemPrompt, buildSimulationRound2SystemPrompt, + tryParseSimulationRoundPayload, extractSimulationRoundPayload, runTheaterSimulation, enqueueSimulationTask, diff --git a/scripts/seed-forecasts.types.d.ts b/scripts/seed-forecasts.types.d.ts index f13e9fbbdd..0892381b82 100644 --- a/scripts/seed-forecasts.types.d.ts +++ b/scripts/seed-forecasts.types.d.ts @@ -124,6 +124,39 @@ interface ExpandedPath { candidate?: ExpandedPathCandidate; } +// --------------------------------------------------------------------------- +// Simulation package (buildSimulationPackageFromDeepSnapshot output) +// --------------------------------------------------------------------------- + +/** + * One theater entry in SimulationPackage.selectedTheaters. + * Distinct from TheaterResult (LLM output shape stored in SimulationOutcome). + * + * NOTE: When adding fields here, also add them to the uiTheaters projection + * in writeSimulationOutcome() or they will be invisible in the Redis snapshot. + */ +interface SimulationPackageTheater { + theaterId: string; + candidateStateId: string; + theaterLabel?: string; + theaterRegion?: string; + stateKind?: string; + dominantRegion?: string; + macroRegions?: string[]; + routeFacilityKey?: string; + commodityKey?: string; + topBucketId: string; + topChannel: string; + rankingScore?: number; + criticalSignalTypes: string[]; + /** + * Role-category strings from candidate stateSummary.actors. Theater-scoped (no cross-theater aggregation). + * Injected into Round 2 prompt as CANDIDATE ACTOR ROLES. Used as allowlist in sanitizeKeyActorRoles guardrail. + * keyActors (entity-space) and actorRoles (role-category) are intentionally disjoint vocabularies. + */ + actorRoles: string[]; +} + // --------------------------------------------------------------------------- // Theater simulation structures // --------------------------------------------------------------------------- @@ -133,7 +166,10 @@ interface SimulationTopPath { label: string; summary: string; confidence: number; + /** Entity-space actor names (geo-political). Used for narrative/audit. NOT used for overlap bonus scoring. */ keyActors: string[]; + /** Role-category actor strings from the candidate's stateSummary.actors vocabulary. Used for the +0.04 overlap bonus when actorSource=stateSummary. */ + keyActorRoles?: string[]; roundByRoundEvolution?: Array<{ round: number; summary: string }>; timingMarkers?: Array<{ event: string; timing: string }>; } @@ -179,8 +215,12 @@ interface SimulationOutcome { interface SimulationAdjustmentDetail { bucketChannelMatch: boolean; - /** Number of overlapping actors between path and simulation top paths (>=2 triggers +0.04 bonus). */ + /** Backwards-compat alias: equals roleOverlapCount when actorSource=stateSummary, else keyActorsOverlapCount. >=2 triggered the +0.04 bonus. */ actorOverlapCount: number; + /** Role-category overlap count (candidate stateSummary.actors vs sim keyActorRoles). Drives +0.04 bonus when actorSource=stateSummary. */ + roleOverlapCount: number; + /** Entity-space overlap count (candidate actors vs sim keyActors). Drives +0.04 bonus when actorSource=affectedAssets. Telemetry only when actorSource=stateSummary. */ + keyActorsOverlapCount: number; invalidatorHit: boolean; stabilizerHit: boolean; /** Number of candidate-theater actors used for overlap matching. Source is stateSummary.actors if raw list present, else affectedAssets. Never a union. */ @@ -208,14 +248,19 @@ interface SimulationAdjustmentRecord { /** Flat projection of SimulationAdjustmentDetail written into path-scorecards.json entries. simPathConfidence is omitted (already in simulationSignal). */ interface ScorecardSimDetail { - bucketChannelMatch: boolean; - actorOverlapCount: number; - candidateActorCount: number; - actorSource: 'stateSummary' | 'affectedAssets' | 'none'; - resolvedChannel: string; - channelSource: 'direct' | 'market' | 'none'; - invalidatorHit: boolean; - stabilizerHit: boolean; + bucketChannelMatch: boolean; + /** Backwards-compat alias for roleOverlapCount or keyActorsOverlapCount (whichever drove the bonus). */ + actorOverlapCount: number; + /** Role-category overlap (stateSummary path). Drives +0.04 when actorSource=stateSummary. */ + roleOverlapCount: number; + /** Entity-space overlap via keyActors (affectedAssets path). Drives +0.04 when actorSource=affectedAssets. */ + keyActorsOverlapCount: number; + candidateActorCount: number; + actorSource: 'stateSummary' | 'affectedAssets' | 'none'; + resolvedChannel: string; + channelSource: 'direct' | 'market' | 'none'; + invalidatorHit: boolean; + stabilizerHit: boolean; } interface SimulationEvidence { diff --git a/tests/forecast-trace-export.test.mjs b/tests/forecast-trace-export.test.mjs index 8af5978040..ef73ae38ae 100644 --- a/tests/forecast-trace-export.test.mjs +++ b/tests/forecast-trace-export.test.mjs @@ -84,6 +84,7 @@ import { negatesDisruption, normalizeActorName, summarizeImpactPathScore, + tryParseSimulationRoundPayload, } from '../scripts/seed-forecasts.mjs'; import { @@ -5847,6 +5848,24 @@ describe('simulation package export', () => { const result = await writeSimulationPackage(snapshot, { storageConfig: null }); assert.equal(result, null); }); + + it('T-PKG1: buildSimulationPackageFromDeepSnapshot adds actorRoles from candidate stateSummary to each theater', () => { + const candidateA = makeCandidate({ candidateStateId: 'state-pkg-a', stateSummary: { actors: ['Commodity traders', 'Policy officials'] } }); + const candidateB = makeCandidate({ + candidateStateId: 'state-pkg-b', + routeFacilityKey: 'Red Sea', + commodityKey: 'crude_oil', + stateSummary: { actors: ['Shipping operators'] }, + }); + const pkg = buildSimulationPackageFromDeepSnapshot(makeSnapshot([candidateA, candidateB])); + assert.ok(pkg, 'package must be built'); + const theaterA = pkg.selectedTheaters.find((t) => t.candidateStateId === 'state-pkg-a'); + assert.ok(theaterA, 'theater A must be found'); + assert.deepStrictEqual(theaterA.actorRoles, ['Commodity traders', 'Policy officials']); + const theaterB = pkg.selectedTheaters.find((t) => t.candidateStateId === 'state-pkg-b'); + assert.ok(theaterB, 'theater B must be found'); + assert.deepStrictEqual(theaterB.actorRoles, ['Shipping operators']); + }); }); // --------------------------------------------------------------------------- @@ -6166,6 +6185,28 @@ describe('simulation runner — prompt builders', () => { const prompt = buildSimulationRound2SystemPrompt(minimalTheater, minimalPkg, round1); assert.ok(prompt.includes('houthi-forces'), 'should include valid actor IDs'); }); + + it('T-P1: Round 2 prompt includes CANDIDATE ACTOR ROLES section when theater has actorRoles', () => { + const theaterWithRoles = { + ...minimalTheater, + actorRoles: ['Commodity traders', 'Shipping operators', 'Policy officials'], + }; + const round1 = { paths: [{ pathId: 'escalation', summary: 'Oil supply shock', initialReactions: [] }] }; + const prompt = buildSimulationRound2SystemPrompt(theaterWithRoles, minimalPkg, round1); + assert.ok(prompt.includes('copy these EXACT strings into keyActorRoles'), 'prompt should include role section header with copy instruction'); + assert.ok(prompt.includes('"Commodity traders"'), 'prompt should include role label'); + assert.ok(prompt.includes('"Shipping operators"'), 'prompt should include role label'); + assert.ok(prompt.includes('keyActorRoles'), 'prompt should reference keyActorRoles field'); + assert.ok(prompt.includes('return [] if none apply'), 'prompt should include fallback instruction'); + }); + + it('T-P2: Round 2 prompt omits CANDIDATE ACTOR ROLES list section when theater has no actorRoles', () => { + const round1 = { paths: [] }; + const prompt = buildSimulationRound2SystemPrompt(minimalTheater, minimalPkg, round1); + // The roles-list section header is "CANDIDATE ACTOR ROLES (copy these EXACT strings...)" — only injected when actorRoles is non-empty + // The INSTRUCTIONS block always mentions "CANDIDATE ACTOR ROLES" as a reference, which is OK + assert.ok(!prompt.includes('copy these EXACT strings into keyActorRoles'), 'role list section should NOT be injected when actorRoles absent'); + }); }); describe('simulation runner — extractSimulationRoundPayload', () => { @@ -6258,6 +6299,29 @@ describe('simulation runner — extractSimulationRoundPayload', () => { const result = extractSimulationRoundPayload(withPrefix, 1); assert.ok(Array.isArray(result.paths), 'should parse via extractFirstJsonObject fallback'); }); + + it('T-P3: tryParseSimulationRoundPayload extracts keyActorRoles from Round 2 paths', () => { + const raw = JSON.stringify({ + paths: [{ + pathId: 'escalation', label: 'Escalation', summary: 'Oil disruption', + keyActors: ['Iran'], keyActorRoles: ['Commodity traders', 'Shipping operators'], + roundByRoundEvolution: [], confidence: 0.6, timingMarkers: [], + }, { + pathId: 'containment', label: 'Containment', summary: 'Contained', keyActors: [], keyActorRoles: [], + roundByRoundEvolution: [], confidence: 0.3, timingMarkers: [], + }, { + pathId: 'market_cascade', label: 'Cascade', summary: 'Markets react', keyActors: [], keyActorRoles: [], + roundByRoundEvolution: [], confidence: 0.1, timingMarkers: [], + }], + stabilizers: [], invalidators: [], globalObservations: '', confidenceNotes: '', + }); + const result = tryParseSimulationRoundPayload(raw, 2); + assert.ok(result.paths, 'should parse paths'); + const esc = result.paths.find((p) => p.pathId === 'escalation'); + assert.deepStrictEqual(esc.keyActorRoles, ['Commodity traders', 'Shipping operators']); + const containment = result.paths.find((p) => p.pathId === 'containment'); + assert.deepStrictEqual(containment.keyActorRoles, []); + }); }); describe('simulation runner — outcome key builder', () => { @@ -6329,7 +6393,7 @@ describe('phase 3 simulation re-ingestion — computeSimulationAdjustment', () = const path = makePath('energy', 'energy_supply_shock', ['Iran', 'Houthi', 'Saudi Aramco']); const simResult = { theaterId: 'state-1', - topPaths: [{ label: 'Oil energy supply shock via Hormuz', summary: 'Crude supply disruption', keyActors: ['Iran', 'Houthi', 'US Navy'] }], + topPaths: [{ label: 'Oil energy supply shock via Hormuz', summary: 'Crude supply disruption', keyActors: ['Iran', 'Houthi', 'US Navy'], keyActorRoles: ['Iran', 'Houthi movement', 'US Navy'] }], invalidators: [], stabilizers: [], }; @@ -6501,7 +6565,7 @@ describe('phase 3 simulation re-ingestion — computeSimulationAdjustment', () = stateSummary: { actors: ['Iran', 'Saudi Arabia', 'Houthi movement'] }, }; const simResult = { - topPaths: [{ label: 'Oil supply shock', summary: 'energy supply disruption from Red Sea', keyActors: ['Iran', 'Saudi_Arabia', 'US'] }], + topPaths: [{ label: 'Oil supply shock', summary: 'energy supply disruption from Red Sea', keyActors: ['Iran', 'Saudi_Arabia', 'US'], keyActorRoles: ['Iran', 'Saudi Arabia'] }], invalidators: [], stabilizers: [], }; const { adjustment, details } = computeSimulationAdjustment(path, simResult, candidatePacket); @@ -6511,14 +6575,16 @@ describe('phase 3 simulation re-ingestion — computeSimulationAdjustment', () = assert.equal(details.actorSource, 'stateSummary'); }); - it('T-G: entity ID format keyActors resolve to names via normalization', () => { + it('T-G: entity ID format keyActors resolve to names via normalization (keyActorsOverlapCount telemetry)', () => { const path = makePath('energy', 'energy_supply_shock', []); const candidatePacket = { ...makeCandidatePacket(), stateSummary: { actors: ['Iran', 'Saudi Arabia'] }, }; const simResult = { - topPaths: [{ label: 'Oil supply shock', summary: 'energy supply disruption', keyActors: ['state-aa3f41bf4f:iran', 'state-aa3f41bf4f:saudi_arabia'] }], + // keyActors use entity ID format — normalized to 'iran'/'saudi arabia' for keyActorsOverlapCount telemetry + // keyActorRoles drives the role overlap bonus (stateSummary path) + topPaths: [{ label: 'Oil supply shock', summary: 'energy supply disruption', keyActors: ['state-aa3f41bf4f:iran', 'state-aa3f41bf4f:saudi_arabia'], keyActorRoles: ['Iran', 'Saudi Arabia'] }], invalidators: [], stabilizers: [], }; const { adjustment, details } = computeSimulationAdjustment(path, simResult, candidatePacket); @@ -6560,7 +6626,7 @@ describe('phase 3 simulation re-ingestion — computeSimulationAdjustment', () = const path1 = makePath('energy', 'energy_supply_shock', []); // affectedAssets empty const path2 = makePath('energy', 'energy_supply_shock', ['TTF gas futures', 'European utility stocks']); // different affectedAssets — irrelevant const simResult = { - topPaths: [{ label: 'Supply shock', summary: 'energy supply disruption', keyActors: ['Iran', 'Saudi_Arabia'] }], + topPaths: [{ label: 'Supply shock', summary: 'energy supply disruption', keyActors: ['Iran', 'Saudi_Arabia'], keyActorRoles: ['Iran', 'Saudi Arabia'] }], invalidators: [], stabilizers: [], }; const { adjustment: adj1, details: d1 } = computeSimulationAdjustment(path1, simResult, candidatePacket); @@ -6636,8 +6702,8 @@ describe('phase 3 simulation re-ingestion — computeSimulationAdjustment', () = const path = makePath('energy', 'energy_supply_shock', []); const candidatePacket = makeCandidatePacket(); // stateSummary.actors: ['Iran', 'Houthi movement', 'US Navy'] const simResult = { - // keyActors match 'iran' and 'us navy' from stateSummary → overlap=2 → actor bonus applies - topPaths: [{ label: 'Oil supply disruption', summary: 'energy supply disruption', confidence: 0.72, keyActors: ['Iran', 'US_Navy'] }], + // keyActorRoles match 'iran' and 'us navy' from stateSummary → roleOverlap=2 → actor bonus applies + topPaths: [{ label: 'Oil supply disruption', summary: 'energy supply disruption', confidence: 0.72, keyActors: ['Iran', 'US_Navy'], keyActorRoles: ['Iran', 'US Navy'] }], invalidators: [], stabilizers: [], }; const { adjustment, details } = computeSimulationAdjustment(path, simResult, candidatePacket); @@ -6745,6 +6811,62 @@ describe('phase 3 simulation re-ingestion — computeSimulationAdjustment', () = // +0.08 * 0.85 = 0.068 assert.equal(adjustment, 0.068); }); + + it('T-RO1: roleOverlapCount fires +0.04 bonus when stateSummary.actors and keyActorRoles match (role-category vocabulary)', () => { + const path = makePath('energy', 'energy_supply_shock', []); + const candidatePacket = { + ...makeCandidatePacket(), + stateSummary: { actors: ['Commodity traders', 'Shipping operators', 'Policy officials'] }, + }; + const simResult = { + topPaths: [{ + label: 'Oil supply shock', summary: 'energy supply disruption', + keyActors: ['Iran', 'Saudi_Arabia'], // entity-space — never matches role categories + keyActorRoles: ['Commodity traders', 'Shipping operators'], // role-space — 2 matches + }], + invalidators: [], stabilizers: [], + }; + const { adjustment, details } = computeSimulationAdjustment(path, simResult, candidatePacket); + assert.strictEqual(adjustment, 0.12); + assert.strictEqual(details.roleOverlapCount, 2); + assert.strictEqual(details.actorOverlapCount, 2); // backwards-compat alias + assert.strictEqual(details.keyActorsOverlapCount, 0); // entities don't match role categories + assert.strictEqual(details.actorSource, 'stateSummary'); + }); + + it('T-RO2: absent keyActorRoles with stateSummary source gives roleOverlapCount=0 (old sim output gracefully degrades)', () => { + const path = makePath('energy', 'energy_supply_shock', []); + const candidatePacket = { + ...makeCandidatePacket(), + stateSummary: { actors: ['Commodity traders', 'Policy officials'] }, + }; + const simResult = { + topPaths: [{ label: 'Oil supply shock', summary: 'energy supply disruption', keyActors: ['Iran'] }], // no keyActorRoles + invalidators: [], stabilizers: [], + }; + const { adjustment, details } = computeSimulationAdjustment(path, simResult, candidatePacket); + assert.strictEqual(adjustment, 0.08); // bucket+channel only — no role bonus (keyActorRoles absent) + assert.strictEqual(details.roleOverlapCount, 0); + assert.strictEqual(details.actorOverlapCount, 0); + }); + + it('T-RO3: affectedAssets fallback path uses keyActors for overlap (backwards compat preserved)', () => { + const path = makePath('energy', 'energy_supply_shock', ['Red Sea tankers', 'Saudi Aramco']); + const candidatePacket = { ...makeCandidatePacket(), stateSummary: { actors: [] } }; // empty → affectedAssets fallback + const simResult = { + topPaths: [{ + label: 'Oil supply shock', summary: 'energy supply disruption', + keyActors: ['Red Sea tankers', 'Saudi Aramco'], // entity overlap with affectedAssets + keyActorRoles: ['Commodity traders'], // 1 role match — ignored (actorSource=affectedAssets) + }], + invalidators: [], stabilizers: [], + }; + const { adjustment, details } = computeSimulationAdjustment(path, simResult, candidatePacket); + assert.strictEqual(adjustment, 0.12); // entity overlap fires bonus via affectedAssets path + assert.strictEqual(details.keyActorsOverlapCount, 2); + assert.strictEqual(details.actorSource, 'affectedAssets'); + assert.strictEqual(details.roleOverlapCount, 0); // role path not used (actorSource != stateSummary) + }); }); describe('normalizeActorName', () => { @@ -6899,7 +7021,7 @@ describe('phase 3 simulation re-ingestion — applySimulationMerge', () => { runId: 'sim-tj', isCurrentRun: true, theaterResults: [{ theaterId: 'theater-1', candidateStateId: stateA, - topPaths: [{ label: 'Oil supply shock', summary: 'energy supply disruption from Red Sea', keyActors: ['Iran', 'Saudi_Arabia'] }], + topPaths: [{ label: 'Oil supply shock', summary: 'energy supply disruption from Red Sea', keyActors: ['Iran', 'Saudi_Arabia'], keyActorRoles: ['Iran', 'Saudi Arabia'] }], invalidators: [], stabilizers: [], }], }; @@ -7178,7 +7300,7 @@ describe('phase 3 simulation re-ingestion — applySimulationMerge', () => { stateSummary: { actors: ['Iran', 'Saudi Arabia'] }, }; const simResult = { - topPaths: [{ label: 'Supply shock', summary: 'energy supply disruption', keyActors: ['Iran', 'Saudi_Arabia'] }], + topPaths: [{ label: 'Supply shock', summary: 'energy supply disruption', keyActors: ['Iran', 'Saudi_Arabia'], keyActorRoles: ['Iran', 'Saudi Arabia'] }], invalidators: [], stabilizers: [], }; const { adjustment, details } = computeSimulationAdjustment(path, simResult, candidatePacket); diff --git a/todos/087-complete-p1-simulation-package-theater-interface-missing.md b/todos/087-complete-p1-simulation-package-theater-interface-missing.md new file mode 100644 index 0000000000..fe8e94e21b --- /dev/null +++ b/todos/087-complete-p1-simulation-package-theater-interface-missing.md @@ -0,0 +1,69 @@ +--- +status: pending +priority: p1 +issue_id: "087" +tags: [code-review, typescript, type-safety, simulation] +dependencies: [] +--- + +# `SimulationPackageTheater` interface missing — `actorRoles` and pkg-theater shape untyped + +## Problem Statement + +The theater objects built by `buildSimulationPackageFromDeepSnapshot` and consumed by `buildSimulationRound2SystemPrompt` and `mergedPaths.map()` have no named TypeScript interface in `seed-forecasts.types.d.ts`. The new `actorRoles` field added in PR #2582 is a critical path for the role-overlap bonus — but with `@ts-check` active on a `.mjs` file, a typo on `theater.actorRoles` or any caller passing the wrong shape will fail silently at runtime rather than being caught statically. + +## Findings + +- Two agents (kieran-typescript-reviewer, architecture-strategist) flagged this independently as the highest-priority gap from PR #2582 +- `buildSimulationRound2SystemPrompt(theater, pkg, round1)` receives `theater` typed as inferred plain object — `@ts-check` cannot validate `theater.actorRoles` +- `mergedPaths.map()` IIFE accesses `theater.actorRoles` with a defensive `Array.isArray` guard but no type annotation +- `TheaterResult` (the LLM output shape) is a separate interface — a `SimulationPackageTheater` for the pkg-artifact shape does not exist +- PR #2582 already edits `seed-forecasts.types.d.ts` (adding `keyActorRoles` to `SimulationTopPath`) — the missing interface should have been added in the same PR + +## Proposed Solution + +Add to `scripts/seed-forecasts.types.d.ts`: + +```ts +/** One theater entry in SimulationPackage.selectedTheaters. Distinct from TheaterResult (LLM output shape). */ +interface SimulationPackageTheater { + theaterId: string; + candidateStateId: string; + label?: string; + stateKind?: string; + dominantRegion?: string; + macroRegions?: string[]; + routeFacilityKey?: string; + commodityKey?: string; + topBucketId: string; + topChannel: string; + rankingScore?: number; + criticalSignalTypes: string[]; + /** Role-category strings from candidate stateSummary.actors. Theater-scoped. Used in Round 2 prompt injection and keyActorRoles guardrail filter. */ + actorRoles: string[]; + theaterLabel?: string; + theaterRegion?: string; +} +``` + +Then annotate `theater` param in `buildSimulationRound2SystemPrompt` JSDoc: +```js +/** + * @param {import('./seed-forecasts.types.d.ts').SimulationPackageTheater} theater + */ +``` + +## Technical Details + +- Files: `scripts/seed-forecasts.types.d.ts`, `scripts/seed-forecasts.mjs` +- Effort: Small | Risk: Low (type-only change, no runtime behavior) + +## Acceptance Criteria + +- [ ] `SimulationPackageTheater` interface exists in `seed-forecasts.types.d.ts` +- [ ] `buildSimulationRound2SystemPrompt` parameter annotated with the interface +- [ ] `npm run typecheck` passes + +## Work Log + +- 2026-03-31: Identified by kieran-typescript-reviewer and architecture-strategist during PR #2582 review diff --git a/todos/088-complete-p1-keyactorroles-missing-from-uitheaters-redis-projection.md b/todos/088-complete-p1-keyactorroles-missing-from-uitheaters-redis-projection.md new file mode 100644 index 0000000000..1173e65702 --- /dev/null +++ b/todos/088-complete-p1-keyactorroles-missing-from-uitheaters-redis-projection.md @@ -0,0 +1,63 @@ +--- +status: pending +priority: p1 +issue_id: "088" +tags: [code-review, simulation, agent-native, observability, redis] +dependencies: [] +--- + +# `keyActorRoles` missing from `uiTheaters` Redis projection in `writeSimulationOutcome` + +## Problem Statement + +`writeSimulationOutcome` builds a `uiTheaters` array written to Redis (`SIMULATION_OUTCOME_LATEST_KEY`) and returned by `GetSimulationOutcomeResponse.theaterSummariesJson`. The path map explicitly projects only `pathId`, `label`, `summary`, `confidence`, and `keyActors` — the new `keyActorRoles` field from PR #2582 is omitted. Even if the `get-simulation-outcome` RPC is eventually unblocked for agent access, `keyActorRoles` will always be `undefined` on every path because it is stripped at the Redis write stage. + +## Findings + +- Found by agent-native-reviewer during PR #2582 review +- Location: `scripts/seed-forecasts.mjs` line ~16823 in `writeSimulationOutcome` +- The `uiTheaters` path projection: + ```js + .map((p) => ({ + pathId: p.pathId || '', + label: p.label, + summary: p.summary, + confidence: p.confidence, + keyActors: (p.keyActors || []).slice(0, 4), + // keyActorRoles is NOT here + })) + ``` +- `keyActorRoles` exists in the full R2 artifact (`simulation-outcome.json`) but never reaches the Redis snapshot +- The proto comment for `theater_summaries_json` in `get_simulation_outcome.proto` line 31 also needs updating + +## Proposed Solution + +Add `keyActorRoles` to the `uiTheaters` path projection: + +```js +.map((p) => ({ + pathId: p.pathId || '', + label: p.label, + summary: p.summary, + confidence: p.confidence, + keyActors: (p.keyActors || []).slice(0, 4), + keyActorRoles: (p.keyActorRoles || []).slice(0, 8), +})) +``` + +Update `get_simulation_outcome.proto` comment for `theater_summaries_json` to include `keyActorRoles` in the documented shape. + +## Technical Details + +- Files: `scripts/seed-forecasts.mjs` (uiTheaters map), `proto/worldmonitor/forecast/v1/get_simulation_outcome.proto` +- Effort: Small | Risk: Low + +## Acceptance Criteria + +- [ ] `keyActorRoles` appears in `uiTheaters` path projection +- [ ] `redis-cli get forecast:simulation-outcome:latest | jq '.uiTheaters[0].topPaths[0].keyActorRoles'` returns a non-null value after next sim run +- [ ] Proto comment updated + +## Work Log + +- 2026-03-31: Identified by agent-native-reviewer during PR #2582 review diff --git a/todos/089-complete-p2-keyactorroles-guardrail-iife-extract-to-named-function.md b/todos/089-complete-p2-keyactorroles-guardrail-iife-extract-to-named-function.md new file mode 100644 index 0000000000..678c07e262 --- /dev/null +++ b/todos/089-complete-p2-keyactorroles-guardrail-iife-extract-to-named-function.md @@ -0,0 +1,66 @@ +--- +status: pending +priority: p2 +issue_id: "089" +tags: [code-review, quality, simulation, simplicity] +dependencies: [] +--- + +# Extract `keyActorRoles` guardrail IIFE to named `sanitizeKeyActorRoles` function + +## Problem Statement + +The `keyActorRoles` guardrail in `mergedPaths.map()` is implemented as an 8-line IIFE. The logic is non-trivial (sanitize → check allowlist via normalizeActorName → filter), runs once per path per theater, and is currently untestable in isolation. IIFEs of this complexity violate the project's "extractable at natural boundaries" principle. Additionally, `allowed.map(normalizeActorName)` is recomputed fresh for every path, even though `theater.actorRoles` is constant across all paths in a theater. + +## Findings + +- Flagged by kieran-typescript-reviewer (HIGH) and code-simplicity-reviewer independently +- Performance: `allowedNorm` Set rebuilt per path (up to 5 paths × 12 roles = 60 redundant normalizeActorName calls per theater) +- The IIFE has no dedicated test — the guardrail filter behavior is only exercised implicitly through T-K (which covers the stateSummary path) +- The sanitize-before-filter ordering could cause a latent mismatch: `sanitizeForPrompt(s).slice(0,80)` is applied before `normalizeActorName(s)` comparison (normalizeActorName normalizes the truncated string, which is correct — but the intent is clearer with filter-first) + +## Proposed Solution + +Extract to a named function near the other sim helpers: + +```js +/** + * @param {string[] | undefined} rawRoles - LLM-returned role strings from keyActorRoles + * @param {string[]} allowedRoles - theater.actorRoles allowlist (empty = no semantic filter) + * @returns {string[]} + */ +function sanitizeKeyActorRoles(rawRoles, allowedRoles) { + const sanitized = (Array.isArray(rawRoles) ? rawRoles : []) + .map((s) => sanitizeForPrompt(String(s)).slice(0, 80)); + if (!Array.isArray(allowedRoles) || allowedRoles.length === 0) return sanitized.slice(0, 8); + const allowedNorm = new Set(allowedRoles.map(normalizeActorName)); + return sanitized.filter((s) => allowedNorm.has(normalizeActorName(s))).slice(0, 8); +} +``` + +Then in `mergedPaths.map()`: +```js +keyActorRoles: sanitizeKeyActorRoles(p.keyActorRoles, theater.actorRoles), +``` + +And hoist (optional but clean): +```js +const allowedRoles = Array.isArray(theater.actorRoles) ? theater.actorRoles : []; +// pass allowedRoles to sanitizeKeyActorRoles for all paths in this theater +``` + +## Technical Details + +- Files: `scripts/seed-forecasts.mjs` +- Effort: Small | Risk: Low + +## Acceptance Criteria + +- [ ] `sanitizeKeyActorRoles` is a named function (not IIFE) +- [ ] `allowedNorm` Set not recomputed per path when `allowed` is constant per theater +- [ ] `npm run test:data` passes +- [ ] New unit test for `sanitizeKeyActorRoles` (allowlist match, allowlist miss, empty allowed) + +## Work Log + +- 2026-03-31: Identified by kieran-typescript-reviewer and code-simplicity-reviewer during PR #2582 review diff --git a/todos/090-complete-p2-bonusoverlap-ternary-none-actorsource-fallthrough.md b/todos/090-complete-p2-bonusoverlap-ternary-none-actorsource-fallthrough.md new file mode 100644 index 0000000000..f06b47e54f --- /dev/null +++ b/todos/090-complete-p2-bonusoverlap-ternary-none-actorsource-fallthrough.md @@ -0,0 +1,56 @@ +--- +status: pending +priority: p2 +issue_id: "090" +tags: [code-review, simulation, correctness, architecture] +dependencies: [] +--- + +# `bonusOverlap` ternary silently falls through to affectedAssets path for future `actorSource` values + +## Problem Statement + +In `computeSimulationAdjustment`: + +```js +const bonusOverlap = actorSrc === 'stateSummary' ? roleOverlap.length : details.keyActorsOverlapCount; +``` + +The `else` branch catches both `actorSource='affectedAssets'` (correct) and `actorSource='none'` (incorrect — `keyActorsOverlapCount` would be 0 anyway, but the branch is semantically wrong) and any future third value. If a new `actorSource` variant is added later, it will silently use the `affectedAssets` entity-overlap count rather than failing visibly. + +## Findings + +- Flagged by architecture-strategist during PR #2582 review +- Current `actorSource` values: `'stateSummary'` | `'affectedAssets'` | `'none'` +- `actorSource='none'` currently has `candidateActors=[]` so `keyActorsOverlapCount=0`, masking the incorrect branch +- The fix is one line and makes the intent explicit + +## Proposed Solution + +```js +// BEFORE: +const bonusOverlap = actorSrc === 'stateSummary' ? roleOverlap.length : details.keyActorsOverlapCount; + +// AFTER: +const bonusOverlap = actorSrc === 'stateSummary' + ? details.roleOverlapCount + : actorSrc === 'affectedAssets' + ? details.keyActorsOverlapCount + : 0; +``` + +This also removes the need for the intermediate `roleOverlap` array (use `details.roleOverlapCount` directly as flagged by code-simplicity-reviewer). + +## Technical Details + +- Files: `scripts/seed-forecasts.mjs` (computeSimulationAdjustment, ~line 11501) +- Effort: Trivial | Risk: Very Low + +## Acceptance Criteria + +- [ ] All three `actorSource` values have explicit branches (no implicit fallthrough) +- [ ] `node --test tests/forecast-trace-export.test.mjs` passes + +## Work Log + +- 2026-03-31: Identified by architecture-strategist during PR #2582 review diff --git a/todos/091-complete-p2-sanitizeprompt-unicode-line-separators.md b/todos/091-complete-p2-sanitizeprompt-unicode-line-separators.md new file mode 100644 index 0000000000..5bf4a27a72 --- /dev/null +++ b/todos/091-complete-p2-sanitizeprompt-unicode-line-separators.md @@ -0,0 +1,51 @@ +--- +status: pending +priority: p2 +issue_id: "091" +tags: [code-review, security, simulation, sanitization] +dependencies: [] +--- + +# `sanitizeForPrompt` does not strip Unicode line-separator characters (U+2028, U+2029) + +## Problem Statement + +`sanitizeForPrompt` strips `\n` and `\r` but not Unicode LINE SEPARATOR (U+2028) or PARAGRAPH SEPARATOR (U+2029). Some LLM tokenizers treat these as line breaks. For the new `CANDIDATE ACTOR ROLES` section in the Round 2 prompt, role strings derived from `stateSummary.actors` (LLM-generated caseFile output) could theoretically contain these characters, allowing a role string to inject a structural line break into the prompt without being caught. + +## Findings + +- Flagged by security-sentinel during PR #2582 review +- Current regex: `.replace(/[\n\r]/g, ' ')` +- Proposed: `.replace(/[\n\r\u2028\u2029]/g, ' ')` +- Impact is low-risk for current pipeline (role strings are short, server-side only, no user input reaches this path), but is a defence-in-depth gap that applies to all callers of `sanitizeForPrompt` +- Fix benefits ALL LLM prompt construction across the codebase, not just the new `rolesSection` + +## Proposed Solution + +In `scripts/seed-forecasts.mjs` (line ~14040): + +```js +// BEFORE: +function sanitizeForPrompt(str) { + return String(str || '').replace(/[\n\r]/g, ' ').replace(/[<>{}]/g, '').replace(/[\x00-\x1f]/g, '').slice(0, 200); +} + +// AFTER: +function sanitizeForPrompt(str) { + return String(str || '').replace(/[\n\r\u2028\u2029]/g, ' ').replace(/[<>{}]/g, '').replace(/[\x00-\x1f]/g, '').slice(0, 200); +} +``` + +## Technical Details + +- Files: `scripts/seed-forecasts.mjs` (sanitizeForPrompt function) +- Effort: Trivial | Risk: Very Low (pure defence-in-depth, no behavior change for current inputs) + +## Acceptance Criteria + +- [ ] `sanitizeForPrompt` strips U+2028 and U+2029 +- [ ] `npm run test:data` passes + +## Work Log + +- 2026-03-31: Identified by security-sentinel during PR #2582 review diff --git a/todos/092-complete-p3-roleoverlap-intermediate-array-simplify.md b/todos/092-complete-p3-roleoverlap-intermediate-array-simplify.md new file mode 100644 index 0000000000..7371805e58 --- /dev/null +++ b/todos/092-complete-p3-roleoverlap-intermediate-array-simplify.md @@ -0,0 +1,49 @@ +--- +status: pending +priority: p3 +issue_id: "092" +tags: [code-review, simplicity, simulation] +dependencies: ["089", "090"] +--- + +# Eliminate `roleOverlap` intermediate array in `computeSimulationAdjustment` + +## Problem Statement + +`roleOverlap` is stored as a filtered array but is only ever used as `.length` (twice). The array itself is never inspected. This creates an unnecessary intermediate allocation. + +## Current Code + +```js +const roleOverlap = actorSrc === 'stateSummary' ? candidateActors.filter((a) => simRoles.has(a)) : []; +details.roleOverlapCount = roleOverlap.length; +// ... +const bonusOverlap = actorSrc === 'stateSummary' ? roleOverlap.length : details.keyActorsOverlapCount; +``` + +## Proposed Solution + +```js +details.roleOverlapCount = actorSrc === 'stateSummary' + ? candidateActors.filter((a) => simRoles.has(a)).length + : 0; +const bonusOverlap = actorSrc === 'stateSummary' + ? details.roleOverlapCount + : actorSrc === 'affectedAssets' ? details.keyActorsOverlapCount : 0; +``` + +This combines with todo #090 (explicit ternary for `bonusOverlap`). + +## Technical Details + +- Files: `scripts/seed-forecasts.mjs` +- Effort: Trivial | Risk: Very Low + +## Acceptance Criteria + +- [ ] No `roleOverlap` intermediate array +- [ ] `node --test tests/forecast-trace-export.test.mjs` passes + +## Work Log + +- 2026-03-31: Identified by code-simplicity-reviewer during PR #2582 review diff --git a/todos/093-complete-p3-tryparsesimulation-jsdoc-and-sanitize-at-boundary.md b/todos/093-complete-p3-tryparsesimulation-jsdoc-and-sanitize-at-boundary.md new file mode 100644 index 0000000000..7e1aaa403d --- /dev/null +++ b/todos/093-complete-p3-tryparsesimulation-jsdoc-and-sanitize-at-boundary.md @@ -0,0 +1,55 @@ +--- +status: pending +priority: p3 +issue_id: "093" +tags: [code-review, typescript, simulation, security] +dependencies: [] +--- + +# `tryParseSimulationRoundPayload` missing JSDoc after export + apply `sanitizeForPrompt` at parse boundary + +## Problem Statement + +Two related cleanup items on the newly-exported `tryParseSimulationRoundPayload`: + +1. **Missing JSDoc** — the function was private before PR #2582. Exporting it without JSDoc annotations means `@ts-check` callers get no type feedback on parameters. The project pattern for exported functions is to annotate `@param` and `@returns`. + +2. **`sanitizeForPrompt` deferred to merge step** — `tryParseSimulationRoundPayload` applies only `String(s).trim()` to `keyActorRoles` items. Sanitization happens later in `mergedPaths.map()`. If a future caller uses `tryParseSimulationRoundPayload` directly (e.g., in a test or a new code path) and skips the merge step, unsanitized LLM strings will escape. The fix is to apply `sanitizeForPrompt` at the parse boundary. + +## Proposed Solution + +Add JSDoc: +```js +/** + * @param {string} text - raw LLM response text (may be JSON or JSON-with-prefix) + * @param {1 | 2} round - simulation round number + * @returns {{ paths: import('./seed-forecasts.types.d.ts').SimulationTopPath[] | null, stabilizers?: string[], invalidators?: string[], globalObservations?: string, confidenceNotes?: string, dominantReactions?: string[] }} + */ +function tryParseSimulationRoundPayload(text, round) { +``` + +Apply `sanitizeForPrompt` at parse boundary: +```js +// BEFORE: +p.keyActorRoles.map((s) => String(s || '').trim()).filter(Boolean).slice(0, 10) + +// AFTER: +p.keyActorRoles.map((s) => sanitizeForPrompt(String(s || '')).trim()).filter(Boolean).slice(0, 10) +``` + +Note: `.trim()` after `sanitizeForPrompt` is fine since `sanitizeForPrompt` doesn't strip leading/trailing spaces. + +## Technical Details + +- Files: `scripts/seed-forecasts.mjs` (tryParseSimulationRoundPayload) +- Effort: Trivial | Risk: Very Low + +## Acceptance Criteria + +- [ ] `tryParseSimulationRoundPayload` has `@param` + `@returns` JSDoc +- [ ] `sanitizeForPrompt` applied to `keyActorRoles` items at parse time +- [ ] T-P3 test still passes + +## Work Log + +- 2026-03-31: Identified by kieran-typescript-reviewer and security-sentinel during PR #2582 review