Skip to content

fix(#156): report research gaps in the plan prompt - #157

Open
ken-talltree-io wants to merge 2 commits into
medic:mainfrom
ken-talltree-io:fix/156-research-gap-provenance
Open

fix(#156): report research gaps in the plan prompt#157
ken-talltree-io wants to merge 2 commits into
medic:mainfrom
ken-talltree-io:fix/156-research-gap-provenance

Conversation

@ken-talltree-io

@ken-talltree-io ken-talltree-io commented Aug 15, 2026

Copy link
Copy Markdown

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 Context was guarded on architectureInsights.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.

warnings and confidence were already populated correctly and carried all the way into buildPlanPrompt — but they were only rendered inside that same guarded block, so the guard discarded the very warning that recorded the failure.

What changed

  • Both guards now have an else branch that says what is missing and why, instead of omitting the section.
  • warnings and confidence are reported whenever findings exist, insights or not.
  • A failed fetch reads **Architecture Insights**: unavailable (Failed to fetch code context from cht-core); a genuinely empty search reads none returned for this issue. Those two are now distinguishable, which is the whole point of the issue.

Second commit — fix 3, the truncated marker

parsePlanResponse caps riskFactors at five, and identifyRiskFactors appended 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:

issue shape heuristic risks failure marker present?
calm 1 yes, position 2 of 3
risky (5 components, 4 constraints, high) 4 yes, position 5 of 5
risky + documentation confidence < 0.5 5 no, truncated away

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.errors threading. As the issue comment notes, callOpenDeepWiki catches internally, so state.errors is empty on this path and threading it would emit nothing here.
  • The healthy branches are untouched. I kept them byte-identical rather than refactoring the shared provenance lines into one helper for both, so this PR cannot regress a good run. Happy to do that tidy-up separately if you'd prefer it.

One wording note: contextAnalysis.codeContext is routinely null on normal runs, so the snippet branch says none available (…) rather than unavailable (…). Calling that an outage would cry wolf on a common, healthy condition.

#156

Code review checklist

  • Readable: Concise, well named, follows the style guide, documented if necessary.
  • Tested: Unit and/or e2e where appropriate
  • Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.

Tests

11 new tests in test/supervisors/research-supervisor.spec.ts, reusing the existing v9b2 harness 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 main too, 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, and leaves 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) and keeps 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 — clean
  • npm run lint — clean
  • npm run validate-schema — 110 passed, 0 failed, 3 skipped
  • npm test1012 passing, 0 failing (was 1001 on a clean main)

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:

riskFactors (5, cap is 5):
  1. Code context warnings: Failed to fetch code context from cht-core
  2. Low confidence in code architecture analysis - manual review recommended
  ...
--> upstream-failure marker present: YES

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.ok throw at deepwiki-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 common codeContext === null case and softened it to none 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.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant