fix(#156): report research gaps in the plan prompt - #157
Open
ken-talltree-io wants to merge 2 commits into
Open
Conversation
A failed OpenDeepWiki fetch and a search that genuinely returned nothing both yield zero architecture insights. The `## Code Architecture Context` block was guarded on `architectureInsights.length > 0`, so both cases omitted the section entirely and produced byte-identical plan prompts - the model was never told a section was missing, and after the run there was no way to tell a degraded plan from a good one. Warnings and confidence were already carried on the findings object all the way to `buildPlanPrompt`, but were only rendered inside that same guarded block, so the guard discarded the very warning that recorded the failure. Both guards now have an else branch that states what is missing and why. Warnings and confidence are reported whenever findings exist, insights or not. The healthy branches are unchanged, so only degraded runs differ. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`parsePlanResponse` caps `riskFactors` at five and `identifyRiskFactors` appended the code context risks last, so the only record that the plan's inputs were degraded was truncated away exactly when the issue was complex enough to trip the other five rules - which is when it matters most. Ordering the provenance risks first fixes this without touching the cap, so it holds whether or not the limit of five is a deliberate product decision. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Description
All three fixes from the reproduction I posted on the issue. My comment there offered this conditionally, and I've put it up rather than wait on the round-trip — but that offer stands exactly as written: if you'd rather take this yourself, or an outside PR isn't welcome on this repo right now, close it without ceremony and no hard feelings.
A failed OpenDeepWiki fetch and a search that genuinely returned nothing both yield zero architecture insights.
## Code Architecture Contextwas guarded onarchitectureInsights.length > 0, so both cases omitted the section entirely and produced byte-identical plan prompts (78,720 chars in the repro). The model was never told a section was missing, and after the run there was no way to tell a degraded plan from a good one.warningsandconfidencewere already populated correctly and carried all the way intobuildPlanPrompt— but they were only rendered inside that same guarded block, so the guard discarded the very warning that recorded the failure.What changed
warningsandconfidenceare reported whenever findings exist, insights or not.**Architecture Insights**: unavailable (Failed to fetch code context from cht-core); a genuinely empty search readsnone returned for this issue. Those two are now distinguishable, which is the whole point of the issue.Second commit — fix 3, the truncated marker
parsePlanResponsecapsriskFactorsat five, andidentifyRiskFactorsappended the code-context risks last. So the only surviving record that the plan's inputs were degraded got truncated away exactly when the issue was complex enough to trip the other five rules — which is when it matters most. Measured before the change:I asked on the issue whether the cap of 5 is deliberate, and this doesn't answer that. Ordering the provenance risks first fixes the truncation without touching the cap, so it holds either way: if five is a deliberate product decision it stays five, and if you later decide to raise it nothing here fights that. The one call I did make is that "we couldn't fetch your architecture context" should outrank "this issue is high priority" when something has to go — say the word if you'd rank them the other way.
Still deliberately not in scope
state.errorsthreading. As the issue comment notes,callOpenDeepWikicatches internally, sostate.errorsis empty on this path and threading it would emit nothing here.One wording note:
contextAnalysis.codeContextis routinelynullon normal runs, so the snippet branch saysnone available (…)rather thanunavailable (…). Calling that an outage would cry wolf on a common, healthy condition.#156
Code review checklist
Tests
11 new tests in
test/supervisors/research-supervisor.spec.ts, reusing the existingv9b2harness rather than adding a second one.8 of the 11 fail without the source change — verified by reverting each change on its own and re-running with the tests in place, rather than trusting that a passing test is a working one.
3 pass on
maintoo, by design, and I'd rather say so than let them pad the count:leaves a healthy run reporting its insights and no gap wording,still respects the five-risk cap, andleaves the heuristic risks alone when the code context is healthy. They exist to catch regressions of the healthy path and of the cap, not to demonstrate either fix.The decisive two are
distinguishes a failed fetch from a search that genuinely found nothing(the prompts are no longer equal) andkeeps the upstream-failure risk when every heuristic rule also fires(the marker survives the cap).Full CI sequence locally, on Node 22.15.0 / macOS 26.3.1 arm64:
npm run build— cleannpm run lint— cleannpm run validate-schema— 110 passed, 0 failed, 3 skippednpm test— 1012 passing, 0 failing (was 1001 on a cleanmain)I also re-ran both reproduction scripts from the issue against this branch. The first now reports
prompts byte-identical : no, and the truncation case that previously lost the marker now reads:So both fixes hold against the real graph end to end, not only in unit tests.
Not verified: I have no live OpenDeepWiki or Kapa instance. The repro drives a local HTTPS server returning 503 through the real client, which exercises the
!response.okthrow atdeepwiki-client.ts:248-249, but nothing here has been run against the real services.AI disclosure, per the guidelines: written with Claude Code (Claude Opus), acting on my request to implement the fixes I'd already scoped on the issue. It made the changes, wrote the tests, and reverted each change on its own to confirm the relevant tests actually fail without it. It also caught that the original
unavailable (…)wording would fire on the commoncodeContext === nullcase and softened it tonone available (…), and proposed the reorder-not-raise approach to the cap so the open question stays yours to answer. I reviewed the diff and I'm accountable for it.License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.