Treat CR thread-summary replies as durable settled context when provider resolution is unavailable

[rianjs]

Problem

cr respond can post inline summary replies through a GitHub App, but empirical testing on PR #359 showed the app installation token can fail resolveReviewThread with a GraphQL permission error even when it can post replies and the human viewer can resolve the same thread.

That means the PR itself can contain the durable summary reply, but the GitHub review thread may remain unresolved. Today the resolved-thread context path keys primarily off the provider isResolved flag, so a fresh review can miss the summary-as-cache signal and may reprocess or re-raise already-addressed discussion.

Desired behavior

Use CR-authored summary reply metadata as durable PR-local context, even when the provider cannot mark the thread resolved:

• Recognize codereview:thread-summary marker replies authored by the posting identity.
• Treat the latest CR-authored thread-summary reply as settled context when no newer human reply follows it.
• Feed that summary into reviewer/dossier context so agents avoid recomputing or re-raising the discussion.
• If a newer human reply appears after the summary marker, consider the thread pending again.
• Preserve the provider isResolved distinction so UI state and API resolution are not conflated with CR's own durable summary cache.

Empirical context

On PR #359:

1. Human replied inline to CR-authored findings.
2. cr respond --dry-run --rerun correctly planned summary replies and thread resolution.
3. Live cr respond --rerun posted the summary replies as rianjs-bot.
4. resolveReviewThread failed for the GitHub App token with permission denied.
5. A direct human gh api graphql call to resolveReviewThread succeeded for the same thread IDs.

Proposed contract

A thread is provider-resolved when isResolved=true.

A thread is CR-settled when the latest CR-authored lifecycle comment carries a valid codereview:thread-summary marker and no later human comment exists.

Reviewer prompts should receive compact file-scoped summaries for both provider-resolved and CR-settled threads, while command output/diagnostics should make clear whether provider resolution actually happened.

Guardrails

• Run this through the GPT-5.5 xhigh consulting architect before implementation.
• Keep provider resolution and CR-settled metadata as separate domain concepts.
• Do not use top-level comments as substitutes for inline thread summaries.
• Unit tests should cover:
  • unresolved thread with latest CR summary marker becomes CR-settled context
  • newer human reply after summary marker reopens/pends the thread
  • forged human-authored summary markers are ignored
  • provider-resolved threads continue to use the existing latest-comment summary behavior
  • reviewer/dossier context includes CR-settled summaries without reprocessing the full conversation
• Empirical test should use an open PR with an inline human reply and a GitHub App posting identity that can reply but cannot resolve.

[rianjs]

Investigation notes and proposed plan:

Findings

Issue #394 matches the current behavior. threadcontext.Normalize already trusts codereview:thread-summary markers only when authored by the configured posting identity, but it only creates ResolvedSummary when the provider reports Resolved=true. The dossier path then writes raw provider threads rather than the normalized thread context, so unresolved-but-summary-marked threads still get sent through full dossier summarization.

Relevant spots:

• internal/threadcontext/threadcontext.go: ResolvedSummary is currently provider-resolution-only.
• internal/pipeline/pipeline.go: prepareSelectionContext already computes normalized threadContext, but writeRawDossierArtifacts receives only raw provider threads.
• internal/pipeline/pipeline.go: dossierDiscussionPromptInputFromDiscussion always sends inline thread comments to the summarizer, with no cached-summary signal.

Proposed Design

Keep provider resolution and CR cache state separate:

• Add CRSettledSummary *ThreadSummary to threadcontext.Thread; avoid a separate boolean that can drift.
• Add an EffectiveSettledSummary() helper that prefers ResolvedSummary, then CRSettledSummary.
• Set CRSettledSummary only when the latest CR lifecycle comment has a valid codereview:thread-summary marker, has non-empty sanitized body, and no later human reply exists.
• Preserve Resolved as GitHub/UI state. CR-settled unresolved threads should render as resolved:false, status:"settled".

Implementation Plan

1. Update internal/threadcontext to model CR-settled summaries separately from provider-resolved summaries.
2. Update selection prompt construction to use EffectiveSettledSummary() before falling back to dossier-model summaries or raw first-comment excerpts.
3. Add ThreadContext []threadcontext.Thread to dossierInputs and pass the normalized context from prepareSelectionContext into writeRawDossierArtifacts.
4. Extend dossierInlineThreadArtifact with cached summary metadata, such as source, body, last comment ID, and thread ID.
5. Update dossier prompt/summary construction so CR-settled threads reuse the cached summary instead of sending the full thread comments through the dossier summarizer.
6. Keep same-anchor collision risk in mind; if the summary schema is touched, carry thread_id through prompt/summary maps and use anchor matching only as compatibility fallback.

Test Plan

• Unresolved thread with latest CR summary marker and no later human reply gets CRSettledSummary, not ResolvedSummary.
• Later human reply after CR summary clears CR-settled and makes the thread pending again.
• Human-forged summary marker is ignored, including when it follows a real CR summary.
• Provider-resolved behavior remains unchanged: ResolvedSummary is still based on the latest sanitized comment.
• Selection prompt reuses CR-settled summary while keeping resolved:false.
• Dossier prompt for a CR-settled thread does not include full thread comments or live markers, and final discussion.md uses the cached settled summary.

[rianjs]

Converged implementation plan after GPT-5.5 architect review.

Architect status: blockers=0 majors=0 minors=0 nits=1; nit applied by adding respondcmd to focused tests.

Issue 394 Plan: CR-Settled Thread Summary Cache

Ticket

GitHub issue: open-cli-collective/codereview-cli#394

Title: Treat CR thread-summary replies as durable settled context when provider
resolution is unavailable

Problem

cr respond can post inline summary replies through a GitHub App even when the
same app token cannot call GitHub GraphQL resolveReviewThread. Today the PR
can contain a durable CR-authored summary reply, but fresh review context still
keys primarily off provider isResolved. That can make cr review or dossier
preparation re-summarize the whole inline thread and potentially re-raise
already-settled discussion.

Constraints

• Preserve provider resolution as a distinct domain concept. Resolved must
  continue to mean the provider reports isResolved=true.
• Treat only posting-identity-authored codereview:thread-summary markers as
  trusted. Human-authored or otherwise forged markers remain untrusted content.
• A CR summary is usable as settled cache only when no newer human reply follows
  the latest CR summary lifecycle comment.
• Do not use top-level comments as substitutes for inline thread summaries.
• Keep changes surgical and avoid broad schema churn unless needed for the
  thread cache.

Plan

1. Extend internal/threadcontext with separate CR-settled summary state.

   • Add CRSettledSummary *ThreadSummary to threadcontext.Thread.
   • Keep ResolvedSummary provider-resolution-only.
   • Add EffectiveSettledSummary() (*ThreadSummary, bool) to prefer
     ResolvedSummary, then CRSettledSummary.
   • Populate CRSettledSummary only when the latest CR lifecycle comment is a
     valid thread-summary marker, its sanitized body is non-empty, and
     PendingHumanReply is false.

2. Preserve existing pending-thread response behavior.

   • PendingCRAuthoredFindingThreads should still select unresolved
     CR-authored finding threads with newer human replies.
   • CR-settled unresolved threads should not be considered pending because
     the latest lifecycle comment is the CR summary.

3. Feed CR-settled summaries into selection context.

   • Update selectionThreadPromptsFromContext to use
     EffectiveSettledSummary() before dossier-model summaries or first
     comment excerpts.
   • Keep resolved:false for CR-settled unresolved threads while setting
     status:"settled" and using the cached summary text.

4. Feed normalized thread context into dossier raw artifacts.

   • Add ThreadContext []threadcontext.Thread to dossierInputs.
   • Pass normalized threadContext from prepareSelectionContext into
     writeRawDossierArtifacts.
   • When normalized context is present, build inline-thread artifacts from
     sanitized normalized comments and attach cache metadata from
     EffectiveSettledSummary().
   • Keep raw provider-thread fallback for call sites that construct
     dossierInputs directly in tests or selection-only paths without posting
     identity.

5. Reuse cached summaries during dossier summary preparation.

   • Extend dossierInlineThreadArtifact with cached summary metadata:
     source, body, last comment ID, and thread ID.
   • In dossierDiscussionPromptInputFromDiscussion, skip full comment bodies
     for cached settled threads.
   • Merge cached settled summaries directly into
     dossierDiscussionSummaryArtifact with status:"settled" and the
     provider resolved flag preserved.
   • Ensure source fingerprints include cached summary metadata so changing a
     summary or adding a newer human reply invalidates the cached dossier task.

6. Avoid anchor collision regressions.

   • Carry thread IDs through internal cached-summary metadata.
   • Add thread_id to internal dossier prompt/summary structs and maps used
     by selection and dossier decoding.
   • Key cached CR-settled summaries by thread_id, not by anchor.
   • Keep anchor matching only as a compatibility validation path for
     LLM-authored summary output where the existing schema may omit
     thread_id.
   • Add same-anchor tests so two distinct review threads cannot overwrite each
     other's summaries.

7. Make command output distinguish planned resolution from provider resolution.

   • Audit cr respond result rendering, especially internal/cmd/respondcmd,
     where counts.resolved currently comes from planned resolve actions.
   • Add explicit output fields or adjusted counts so users can tell that a
     summary reply was posted, provider thread resolution failed, and the CR
     summary cache still exists.
   • Keep the provider isResolved state separate from CR-settled metadata in
     output copy and JSON fields.
   • Cover this with command/result rendering tests using a posted summary reply
     plus failed terminal resolve action.

8. Update stale domain documentation and comments.

   • Update docs/architecture.md to describe provider-resolved summaries and
     CR-settled summaries as separate context sources.
   • Update comments on ThreadSummary, ResolvedSummary, and file-scoped
     summary helpers so they no longer imply only provider-resolved threads can
     produce compact reviewer context.

Verification

Run focused tests first:

go test ./internal/threadcontext ./internal/pipeline ./internal/cmd/respondcmd

Run repo-wide tests before commit:

make test

Behavioral test coverage to add:

• Unresolved thread with latest posting-identity-authored
  codereview:thread-summary marker gets CRSettledSummary, not
  ResolvedSummary.
• Newer human reply after a CR summary clears CR-settled cache and marks the
  thread pending.
• Human-forged summary markers are ignored, including when they follow a real
  CR summary.
• Provider-resolved threads retain existing ResolvedSummary behavior based on
  the latest sanitized comment.
• Selection prompt reuses CR-settled summary text while preserving
  resolved:false.
• Dossier prompt for a CR-settled thread omits full thread comments and live
  markers, and final discussion.md uses the cached settled summary.
• Same-anchor inline threads keep distinct cached summaries keyed by
  thread_id.
• cr respond output/JSON distinguishes posted summary replies, failed
  provider resolve attempts, and CR-settled cached state instead of reporting
  planned resolves as provider-resolved.
• Docs/comments describe CR-settled summaries separately from provider
  resolution.

Manual empirical verification:

• Use an open PR with an inline human reply and a GitHub App posting identity
  that can reply to review threads but cannot call resolveReviewThread.
• Run cr respond --rerun live and confirm the summary reply is posted with a
  valid codereview:thread-summary marker while provider resolution reports a
  failed resolve attempt.
• Run a fresh review/dossier preparation against the same PR and confirm the
  unresolved inline thread is treated as CR-settled context from the summary
  marker, not re-summarized from the full conversation.
• Add a newer human reply and confirm the next fresh review treats the thread as
  pending again.