Conversation
…lossary Records the decision (from a grilling session comparing pr-review v1 vs unic-pr-review v2 on ADO PR #5570) that existing Human Threads are read-only review context: surfaced post-fan-out as a Notice (unresolved) and Finding annotations (matched by filePath+line), never a Finding-suppression signal (Confidence < 60 stays the sole filter, ADR-0002), and never written to in any Mode. Implementation is tracked in issue #248. Why: v2 fetches PR Threads but only classifies bot-vs-not (Iteration Marker), so human review discussion is invisible to a Review. Letting resolved human threads suppress Findings would mirror the work-item false-negative bug (#247) in reverse — under-reporting code issues — so suppression is explicitly rejected in favour of annotation. - docs/adr/0016-human-threads-as-read-only-context.md (new) - CONTEXT.md: new terms Human Thread, System Thread + relationship line - docs/adr/README.md + AGENTS.md: index wiring, doctrine bullet, count -> sixteen Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The pullrequests endpoint never includes workItemRefs; Work Item links live at a separate pullrequestworkitems endpoint that the fetcher was not calling. This caused discoverWorkItems() to always return [] and silently skip the Intent Check on every PR with linked Work Items. - Add Step 1.5 to ado-fetcher.md: fetch pullrequestworkitems via az devops invoke and set PR_METADATA.workItemRefs from the result - Update Step 6 output schema to document workItemRefs in prMetadata - Fix misleading 'native Work Item field' comment in provider.mjs - Add pullrequestworkitems to ado-cli-inventory.json - Add fetcher-contract tests asserting workItemRefs is always present Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ADR-0016 was committed directly to develop with non-conforming emphasis markers (`*x*` → `_x_`), turning the root Prettier check red on every PR. Formatting-only; unblocks #247 CI and repairs develop on merge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixed: - Widen JSDoc @param type to id: string | number to match real ADO wire format - Update ADR-0001 amendment to correctly name pullrequestworkitems endpoint - Add comment to fetcher-contract test noting synthetic string ids vs real ADO integers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…vider README, lock id normalisation The provider README still documented discoverWorkItems as reading the PR's "native workItemRefs field" — the exact root-cause misconception #247 fixes — and claimed fixtures mirror the raw pullrequests response (which omits Work Item links). Both were reintroduction vectors for the bug this PR closes. Reword to match the corrected provider.mjs JSDoc / ADR-0001 amendment. Add a discoverWorkItems test feeding an integer wire-format id to lock the String(ref.id) coercion the JSDoc `id: string | number` widening opened; the existing fixture-shape tests only exercise string ids. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lformed responses The original Step 1.5 used `WI_LIST.value ?? []`, which collapses a zero-exit response missing `.value` (auth-token race, proxy/HTML body, api-version drift) into an empty list with no warning — reintroducing the #247 silent false negative through a different door. Discriminate three outcomes: zero-exit + valid `value` (use it; empty is legitimate), non-zero exit (warn + []), and zero-exit-malformed (treat as fetch failure, warn + []). Also state explicitly that the warning joins the same `warnings` array emitted in Step 6, matching Steps 5a–5c. Markdown-only agent-prompt change; the ado-cli smoke test still passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…247-fetch-linked-work-items fix(unic-pr-review): Work Item discovery always returns empty (fetcher never populates workItemRefs)
…iew context Wire the already-built Human Thread classification, matcher, and Notice renderer into the orchestrator so reviewer discussion becomes read-only review context (ADR-0016). Human Threads were classified and emitted by the Fetcher but never consumed: the orchestrator never matched them to Findings, never injected the Notice, and never forwarded them to the Re-review Coordinator. Changes: - review-pr.md: new Step 1.8b (first-review) + Step 1.8b extension (re-review) run human-thread-matcher.mjs after the fan-out, annotate matched Findings, and add humanThreadsNotice to NOTICES_CONTEXT - review-pr.md: pass read-only humanThreads to the Re-review Coordinator - README.md: How-it-works prose + mermaid flowchart (classify step, Step 7b matcher, Coordinator humanThreads input, Notice at Step 8, w2 box clarified to bot threads only — never human) - matcher never suppresses or down-ranks a Finding; Confidence < 60 stays the sole filter; the ADO Writer never posts to a Human Thread Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Promote the Human Threads read-only context entry (#248) from Unreleased. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…bad input - Validate findings/humanThreads are arrays after JSON.parse at CLI entry (non-array valid JSON now exits 1 with a clear message, not a cryptic TypeError) - Guard finding.startLine type before Math.abs comparison (missing startLine previously silently dropped the annotation via NaN) - Guard t.excerpt with null coalescing in notices.mjs (null/undefined excerpt from ADO data boundary previously threw TypeError) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace endsWith isMain guard with canonical import.meta.url === pathToFileURL pattern - Add symmetric Array.isArray(findings) guard to matchHumanThreadsToFindings - Fix notices.mjs filePath null check: !== null → != null (covers undefined) - Add CLI subprocess test suite for human-thread-matcher.mjs (HIGH gap) - Add notices.test: exactly-80-char excerpt boundary and undefined excerpt tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ailure diagnostics Two findings from the PR #250 review of the Human Thread feature (ADR-0016): - notices.mjs pluralized the noun but not the verb, rendering "1 unresolved reviewer comment have no matching Finding" for a single thread. Make the verb agree (has/have) and assert both forms in notices.test.mjs. - review-pr.md Step 1.8b (first-review and re-review) substituted a generic "skipping thread annotations" message and discarded the matcher's own stderr, hiding whether the orchestrator serialized invalid JSON — a real bug that would silently disable Human-Thread context on every run. Relay the matcher's stderr verbatim, matching the file's convention elsewhere. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…248-surface-human-threads feat(unic-pr-review): surface existing Human Threads as read-only review context
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.
No description provided.