fix(forecast): fix impact expansion LLM prompt + pre-filter validation diagnostic#2169
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ation diagnostic Problem: LLM generates 8 hypotheses but 6 get rejected post-hoc because the prompt gave 3 flat unlinked enum lists without per-variable constraints, causing the LLM to hallucinate plausible but invalid channel/bucket combinations. Only 2 direct-only hypotheses survived, producing no expanded paths -> completed_no_material_change on every deep run. Also fixes two related bugs: - Duplicate canonical state unit labels (same region + stateKind) blocked the deep worker. Added label dedup filter in buildCanonicalStateUnits. - When all hypotheses failed state-ID filtering in materializeImpactExpansion, impactExpansionSummary showed 0 hypotheses with no rejection reasons, losing all diagnostic data. Changes: - Bump IMPACT_EXPANSION_REGISTRY_VERSION v1 -> v2 to invalidate cached LLM responses so the model re-generates with new constraint guidance - Add buildRegistryConstraintTable() that serializes IMPACT_VARIABLE_REGISTRY and MARKET_BUCKET_ALLOWED_CHANNELS into a compact constraint block - Rewrite buildImpactExpansionSystemPrompt() to replace 3 flat enum lists with the structured constraint table, bucket-channel dual gate rule, and explicit MiroFish causal chain guidance (direct -> second_order -> third_order example) - Add validation field to all 3 return paths in evaluateDeepForecastPaths so pre-state-filter rejection data is always preserved - Add hypothesisValidation to buildImpactExpansionDebugPayload capturing totalHypotheses, validatedCount, mappedCount, rejectionReasonCounts, and per-hypothesis rejection identity (candidateIndex, candidateStateId, variableKey, channel, targetBucket, order, rejectionReason) - Filter duplicate state unit labels post-finalization in buildCanonicalStateUnits - Export buildRegistryConstraintTable, IMPACT_VARIABLE_REGISTRY, MARKET_BUCKET_ALLOWED_CHANNELS for testability - Add 4 new tests: mapped=0 early return has validation, no-expanded-accepted path has validation, hypothesisValidation flows through buildForecastTraceArtifacts, buildRegistryConstraintTable matches registry
…ighten test A2 - Extract IMPACT_EXPANSION_REGISTRY_CONSTRAINT_TABLE const so the constraint table is built once instead of on every prompt invocation - Replace IIFE dedup filter in buildCanonicalStateUnits with an explicit seenLabels Set for readability - Add format-assumption comment to buildRegistryConstraintTable - Rename test A2 and add mapped > 0 assertion to pin it past the mapped=0 early-return path
…opping units The previous dedup filter silently dropped state units whose labels collided after finalizeStateUnit(), losing their id, forecastIds, signals, and deep-candidate eligibility. Two distinct clusters can score below the merge threshold (< 5.5) while still resolving to the same formatStateUnitLabel() output when they share the same leadRegion and stateKind. The filter kept the first and discarded the rest, suppressing valid deep paths. Fix: replace the .filter() with a .map() that disambiguates collision labels using dominantDomain suffix, falling back to the last 4 chars of the unit id if domain-based disambiguation would itself collide. The seenLabels Set tracks all assigned labels to prevent any secondary collision. The snapshot validator (and deep worker) no longer sees duplicate labels, and no units are dropped. Also export buildCanonicalStateUnits for direct test coverage.
Fixes cascade of three scoring gates that caused every deep run to return completed_no_material_change despite valid LLM hypothesis generation. Changes: - Lower second_order validation floors (mapped: 0.66→0.58, internal: 0.58→0.50) and raise multiplier (0.85→0.88) so typical LLM quality (strength~0.75, conf~0.75, 2 refs, specificityScore=0.2) now reaches mapped status - Binary evidenceSupport: refs >= 2 → 1, else → 0 (enforces 2-ref requirement at scoring layer, not just prompt text; 1-ref hypotheses stay trace_only) - Parent-must-be-mapped invariant: post-validation pass downgrades mapped second_order/third_order whose dependsOnKey has no mapped parent - Lower pathScore threshold from 0.66 to 0.50 to allow barely-mapped pairs through to expanded path generation - Add 6 missing maritime chokepoints to CHOKEPOINT_MARKET_REGIONS - Bump IMPACT_EXPANSION_REGISTRY_VERSION v2 → v3 (invalidates stale LLM cache) - Prompt v3: explicit dependsOnKey pairing, 2-evidence citation rule, confidence calibration guidance, direct+second_order pair structure - Add scoringBreakdown (all hypotheses with scoring factors) and gateDetails (active thresholds) to debug artifact for observability feedback loop - Export buildImpactExpansionSystemPrompt, extractImpactRouteFacilityKey, extractImpactCommodityKey for testability - 8 new tests (T1-T8) covering all phase 2 changes; 111/111 pass
koala73
force-pushed
the
worktree-peppy-mixing-pike
branch
from
March 24, 2026 04:13
7c61f6f to
1c0359a
Compare
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.
Why this PR?
Two-phase fix for deep forecasts always returning `completed_no_material_change`.
Phase 1 (commits 1-3): Fixed structural bugs — Gemini JSON fence parsing, constraint table in LLM prompt, validation flow-through to debug artifacts, label disambiguation. The pipeline now reaches the LLM and gets back syntactically valid hypotheses.
Phase 2 (this commit): Three cascading scoring gates were mathematically impossible to clear with typical LLM output. Every second_order hypothesis scored as `trace_only` → no expanded paths were ever built.
Root causes:
Changes
`scripts/seed-forecasts.mjs`
0.75, conf0.75, 2 refs, specificityScore=0.2) now reaches `mapped``tests/forecast-trace-export.test.mjs`
Test plan