fix: restore evidence integrity in V2 pipeline - #2
Conversation
📝 WalkthroughWalkthroughThe change replaces pre-adjudicated evidence cards with provider candidates and centralized validation, adds sequence-safe event persistence and replay, and updates side-panel rendering, settings persistence, authentication polling, and political-context projection. ChangesEvidence pipeline
Event delivery
Side-panel reliability
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟡 Not ready to approve
Several confirmed logic issues remain (context subject matching rejects short identities, compass confidence labeling inconsistency, and scheduler cancellation behavior not actually implemented) that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR restores evidence integrity and durability in the V2 analysis pipeline by preventing provider discovery results from becoming reader-facing assertions until a bounded adjudication step maps exact candidate IDs to explicit claims/relationships, followed by centralized validation and ledger acceptance. It also strengthens event journaling/replay semantics and refines UI/state handling around progress, retries, and preference persistence.
Changes:
- Reworked retrieval → ledger flow to store provider results as
EvidenceCandidates and introduce adjudication + centralized validation before creating ledger assertions/sources. - Restored grounding/validation for article compass/bias signals and tightened Works Cited projection using
ArticleIndexlink classifications. - Made event journaling “durable-first” with idempotent retries, gap rejection, paginated replay, and more section-isolated UI subscriptions.
File summaries
| File | Description |
|---|---|
| README.md | Updates V2 narrative to reflect candidate-only providers and adjudication/validation boundary. |
| packages/intelligence/src/synthesis/perspective.ts | Adds political context synthesis from validated ledger assertions and applies context-aware compass projection. |
| packages/intelligence/src/retrieval/types.ts | Switches retrieval surface export from legacy cards to EvidenceCandidate. |
| packages/intelligence/src/retrieval/scheduler.ts | Introduces streaming priority scheduler API used by providers. |
| packages/intelligence/src/retrieval/coordinator.ts | Adds candidate-count progress and adjusts retry logic to target failed missions. |
| packages/intelligence/src/report/projector.ts | Projects Works Cited from ArticleIndex with canonical URL normalization + filtering/deduping. |
| packages/intelligence/src/report/projector.test.ts | Adds tests for Works Cited link filtering behavior. |
| packages/intelligence/src/planning/lens.ts | Grounds article compass/bias signals against actual index paragraph/sentence anchors. |
| packages/intelligence/src/planning/lens.test.ts | Adds tests for article signal grounding and excerpt substring requirements. |
| packages/intelligence/src/pipeline.ts | Adds adjudication phase, accumulates candidates during retrieval, and updates retry/partial-status semantics. |
| packages/intelligence/src/pipeline.test.ts | Updates pipeline tests for candidates/adjudication boundary and targeted retry behavior. |
| packages/intelligence/src/index.ts | Exposes new evidence normalization and adjudication APIs. |
| packages/intelligence/src/evidence/validation.ts | Adds adjudication-level semantic validation (mission/claim boundaries, anchors, context identity, excerpt mechanics). |
| packages/intelligence/src/evidence/validation.test.ts | Adds tests covering candidate→adjudication→ledger acceptance and rejection cases. |
| packages/intelligence/src/evidence/sufficiency.ts | Tracks candidate counts in sufficiency snapshots. |
| packages/intelligence/src/evidence/source-ledger.ts | Separates candidate acceptance from ledger assertion/source creation via validated adjudications; tracks failed missions. |
| packages/intelligence/src/evidence/normalization.ts | Updates source ID generation to be namespace-based rather than provider-based. |
| packages/intelligence/src/evidence/adjudication.ts | Adds model-driven adjudicator + prompt shaping and a no-op adjudicate wrapper when absent. |
| packages/intelligence/src/compass/calculate.ts | Improves article signal excerpt selection and adds context-assisted compass projection. |
| packages/contracts/src/report.ts | Introduces an enum schema for bias technique values to constrain report signal shape. |
| packages/contracts/src/index.ts | Tightens documentation around search-summary limitations (no support/contradiction/qualification). |
| packages/contracts/src/evidence.ts | Adds EvidenceCandidate + EvidenceAdjudication types and enriches ledger snapshot/validation fields. |
| packages/contracts/src/events.ts | Adds candidateCount to research.progress payload schema. |
| docs/architecture-v2.md | Updates architecture doc to reflect candidate/adjudication boundary and durable-first journaling/replay. |
| apps/extension/src/storage/stores.test.ts | Adds durability/idempotency tests for job cursor vs journal row ordering and gap rejection. |
| apps/extension/src/storage/job-store.ts | Replaces appendEvent with commitEvent to journal-before-cursor with idempotent same-sequence retries. |
| apps/extension/src/storage/job-journal.ts | Persists events more reliably and adds point lookup for idempotent commit repair. |
| apps/extension/src/runtime/background-controller.ts | Uses commitEvent, adds hasMore pagination signal, and removes unsafe cursor advancement on failures. |
| apps/extension/src/providers/exa-evidence.ts | Emits candidates (not assertions), streams batches, adds deadline-based abort handling, and caches validated candidate shapes. |
| apps/extension/src/providers/evidence.test.ts | Updates provider tests to assert candidate-only outputs and absence of assertion fields. |
| apps/extension/src/providers/chatgpt-evidence.ts | Emits a global-search candidate batch, validates cached batch shape, and yields a failed batch instead of throwing on non-abort errors. |
| apps/extension/entrypoints/sidepanel/SettingsScreen.tsx | Makes preference updates awaitable and shows explicit save/saved/error status messaging. |
| apps/extension/entrypoints/sidepanel/report-state.ts | Extends research progress state with candidateCount. |
| apps/extension/entrypoints/sidepanel/report-state.test.ts | Updates reducer tests for candidateCount. |
| apps/extension/entrypoints/sidepanel/ChatGptConnection.tsx | Makes ChatGPT polling resilient to transient errors and handles device expiration explicitly. |
| apps/extension/entrypoints/sidepanel/App.tsx | Adds section-level store subscriptions to reduce rerenders and makes preference saving transactional. |
| apps/extension/entrypoints/sidepanel/api.ts | Implements paginated contiguous replay with out-of-order buffering and explicit gap detection. |
| apps/extension/entrypoints/sidepanel/api.test.ts | Adds tests for missing-range replay before applying out-of-order live events. |
| apps/extension/entrypoints/sidepanel/AnalysisProgress.test.ts | Updates progress UI tests to include candidateCount. |
| apps/extension/entrypoints/offscreen/main.ts | Wires model adjudicator into job + retry runs and updates provider test plumbing for candidates. |
Review details
- Files reviewed: 40/40 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| function containsSubject(haystack: string, subject: string): boolean { | ||
| const source = normalizeEvidenceText(haystack).toLocaleLowerCase("en-US"); | ||
| const expected = words(subject); | ||
| return ( | ||
| expected.length > 0 && | ||
| expected.filter((word) => source.includes(word)).length >= expected.length | ||
| ); | ||
| } |
| * Start bounded work in priority order and yield each completed task as soon | ||
| * as it settles. Returning from the consumer aborts the scheduler so a | ||
| * provider can cancel in-flight network work instead of waiting for the | ||
| * entire queue to drain. | ||
| */ |
| const total = Math.max(1, context.signals.length); | ||
| return CompassResultSchema.parse({ | ||
| ...article, | ||
| label: placement, | ||
| displayLabel: display[placement], | ||
| score, | ||
| confidenceScore: Math.min(0.86, Math.round((article.confidenceScore * 0.7 + 0.16) * 100) / 100), | ||
| confidence: | ||
| article.confidenceScore >= 0.7 ? "high" : article.confidenceScore >= 0.42 ? "medium" : "low", |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/intelligence/src/pipeline.ts (1)
167-181: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftThe pipeline keeps a second candidate collection that diverges from the ledger.
The ledger is already the authoritative candidate store.
EvidenceLedger.acceptdeduplicates bycandidate.idand caps storage atMath.max(24, budget.maxSources * 4). The pipeline ignores that store and builds a parallel array withcandidates.push(...progress.batch.candidates), which applies neither the deduplication nor the cap. Every downstream inconsistency follows from this duplication.Three concrete consequences:
- The array passed to
adjudicateEvidenceis unbounded.EvidenceCandidatepermitscontentup to 20,000 characters anddiscoveryContextup to 20,000 characters, and each batch carries up to 32 candidates. Across many missions the adjudicator prompt grows without limit, which risks a token-limit failure and uncontrolled cost.- Candidates that the ledger dropped at its cap are still adjudicated.
acceptAdjudicationsthen rejects those decisions with the reason "The adjudicator referenced a candidate that was not retrieved." The reason is wrong. The candidate was retrieved and then dropped by the ledger cap. The model work is wasted and the diagnostic misleads.- Duplicate candidate ids appearing in more than one batch are adjudicated more than once.
Reading
ledger.getCandidates()at the adjudication step removes all three, and it makes the emittedcandidateCountmatch the set that is actually adjudicated.Per-site changes:
packages/intelligence/src/pipeline.ts#L167-L181: delete the localcandidatesarray and thecandidates.push(...)call. Passcandidates: ledger.getCandidates()toadjudicateEvidenceat line 201. Skip the adjudication phase when that array is empty, so an honestly empty run does not issue a model call with zero candidates. If you keep a local array instead, annotate it asconst candidates: EvidenceCandidate[] = []; the bare[]relies on evolving-any inference and provides no type safety at thepushsite.packages/intelligence/src/pipeline.ts#L315-L329: apply the same change in the retry path. Delete the local array and passartifacts.ledger.getCandidates()toadjudicateEvidenceat line 349.packages/intelligence/src/retrieval/coordinator.ts#L55-L62: rename the field toledgerCandidateCount, or set it frombatch.candidates.length, so a per-batch progress record does not carry a cumulative ledger total under a per-batch name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/intelligence/src/pipeline.ts` around lines 167 - 181, The pipeline should use the ledger as the sole candidate source. In packages/intelligence/src/pipeline.ts#L167-L181, remove the local candidates array and push, pass ledger.getCandidates() to adjudicateEvidence, and skip adjudication when empty; apply the same change in the retry path at packages/intelligence/src/pipeline.ts#L315-L329 using artifacts.ledger.getCandidates(). In packages/intelligence/src/retrieval/coordinator.ts#L55-L62, rename the progress field to ledgerCandidateCount or populate it from batch.candidates.length so its meaning matches the per-batch record.
🟡 Minor comments (6)
apps/extension/entrypoints/sidepanel/SettingsScreen.tsx-120-127 (1)
120-127: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle synchronous
onChangeexceptions.
onChangepermits a synchronous implementation, butonChange(next)executes beforePromise.resolveis created. If it throws, the error handler does not run andsaveStatusremains"saving".Proposed fix
- void Promise.resolve(onChange(next)).then( + void Promise.resolve() + .then(() => onChange(next)) + .then( () => setSaveStatus("saved"), () => setSaveStatus("error"), - ); + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/entrypoints/sidepanel/SettingsScreen.tsx` around lines 120 - 127, Update savePreferences so synchronous exceptions from onChange(next) are captured by the same failure path as rejected promises, ensuring setSaveStatus("error") runs instead of leaving the status at "saving". Wrap the onChange invocation in a promise-safe boundary while preserving the existing saved and error status transitions.apps/extension/entrypoints/sidepanel/ChatGptConnection.tsx-88-96 (1)
88-96: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear the stale local error when the authorization expires.
If a transient poll failure already set
error, Line 92 updates onlyauth.error. The returned value useserror ?? auth.error, so the stale transient message masks the expiration message. Clear the local error in this branch.Proposed fix
if (Date.now() >= device.expiresAt) { + setError(null); setAuth((current) => ({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/entrypoints/sidepanel/ChatGptConnection.tsx` around lines 88 - 96, In the device expiration branch of the authorization polling flow, clear the local transient error state before returning so the expiration message is displayed. Update the branch around setAuth, setDevice, and setConnecting to reset the local error alongside the existing auth status and cleanup.packages/contracts/src/evidence.ts-60-68 (1)
60-68: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd a refinement to
EvidenceContextSignalSchemasoscoreanddirectioncannot disagree.
EvidenceContextSignalis stored on adjudication assertions, but itsscoreanddirectionfields are independent. Production validation rejects mismatched context signals, while compass projection usesscorefor weighting and labeling. Add a single refinement so an accepted signal cannot havescore: -2withdirection: "right"or other inconsistent values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/contracts/src/evidence.ts` around lines 60 - 68, The EvidenceContextSignalSchema currently permits inconsistent score and direction values. Add a single refinement to EvidenceContextSignalSchema that validates direction against the sign of score, rejecting mismatches while preserving the existing field constraints and accepting only semantically aligned signals.apps/extension/src/providers/chatgpt-evidence.ts-110-110 (1)
110-110: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winA passed deadline now produces a reported provider failure.
The minimum of
1_000ms means the call still runs whenplan.deadlineAthas already passed. The web-search request cannot complete in one second, so it times out. The catch block at Lines 161-180 then emitsstatus: "failed"with the timeout message and records afaileddiagnostic.The PR objectives distinguish completed-empty from failed states, and restrict targeted retries to failed provider lanes. A deadline overrun is not a provider failure. It marks this lane as retryable and shows an error to the reader.
Return early with a completed-empty batch when the remaining budget is too small to be usable.
🐛 Proposed guard
if (plan.missions.length === 0) return; + const remainingMs = plan.deadlineAt - Date.now(); + if (remainingMs < 5_000) { + yield { + missionId: "global-search", + provider: "chatgpt", + candidates: [], + coveredMissionIds: plan.missions.map((mission) => mission.id), + status: "completed", + error: null, + searched: false, + cacheHit: false, + durationMs: Date.now() - startedAt, + }; + return; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/src/providers/chatgpt-evidence.ts` at line 110, Update the deadline handling in the provider flow around totalMs so an already passed deadline or unusably small remaining budget returns a completed-empty batch immediately, without starting the web-search request or entering the failure catch path. Preserve normal execution when sufficient time remains and use the existing batch/status conventions for completed-empty results.apps/extension/src/providers/chatgpt-evidence.ts-152-152 (1)
152-152: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winA cache write failure discards a successful search.
this.persistentCache?.setruns inside thetryblock and before the batch is yielded. If the write rejects, for example on a storage quota error, control moves to thecatchat Line 161. The provider then reportsoutcome: "failed"and yields an empty failed batch, and every candidate from the completed search is lost.Isolate the cache write so a persistence error cannot fail the retrieval.
🐛 Proposed fix
- await this.persistentCache?.set(cacheKey, batches, 30 * 60_000); + await this.persistentCache?.set(cacheKey, batches, 30 * 60_000).catch(() => undefined);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/src/providers/chatgpt-evidence.ts` at line 152, In the retrieval flow around the cache write in the provider method, isolate the optional `persistentCache.set` failure from the surrounding search `try`/`catch` so a rejected cache write cannot enter the failure path. Preserve the completed batches and continue yielding them even when persistence fails, while retaining the existing search-error handling.docs/architecture-v2.md-68-68 (1)
68-68: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the spelling of "quoteable".
Use "quotable" instead of "quoteable".
✏️ Proposed fix
-`search-summary` candidates with no quoteable excerpt; they can only become clearly labeled +`search-summary` candidates with no quotable excerpt; they can only become clearly labeled🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/architecture-v2.md` at line 68, In the “search-summary” documentation text, replace the misspelled word “quoteable” with “quotable” while leaving the surrounding wording unchanged.Source: Linters/SAST tools
🧹 Nitpick comments (20)
apps/extension/entrypoints/sidepanel/App.tsx (1)
711-714: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the leftover commented-out legacy markup.
This block is dead code. It also references field-access patterns (
state.compass.status,finding.excerpt) that no longer match the new section-based access pattern (section.status,finding.excerptviaConnectedBias). Keeping it risks confusing a future reader into thinking these paths are still valid.♻️ Proposed removal
- {/* Legacy section markup removed in favor of connected sections. - {state.compass.status === "error" ? "Unavailable" : "Finding placement…"} - <ProgressiveText text={`"${finding.excerpt}"`} /> - */} -🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/entrypoints/sidepanel/App.tsx` around lines 711 - 714, Remove the leftover commented-out legacy section markup near the connected sections in App, including the obsolete state.compass.status and ProgressiveText/finding.excerpt references; leave the active connected-section implementation unchanged.apps/extension/src/storage/job-store.ts (1)
230-232: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
terminalStatusinsetUnsafe.
setUnsafeat Line 65 repeats the same four-status list inline. CallterminalStatus(parsed.status)there so the terminal definition has one source.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/src/storage/job-store.ts` around lines 230 - 232, Update setUnsafe to replace its inline terminal-status list with a call to terminalStatus(parsed.status), reusing the existing helper as the single source for terminal status definitions.apps/extension/src/runtime/background-controller.ts (1)
725-733: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPublish
analysis.eventDeltabeforeanalysis.jobChanged.For a terminal event, the current order publishes the terminal
jobChangedfirst. Inapi.ts, a terminaljobChangedstarts a replay. That replay runs before the delta for the same event arrives, so the side panel fetches the terminal event from the journal and then discards the delta as stale.The result is correct but adds a replay round trip for every terminal event. Publish the delta first so the in-band event settles the stream and the terminal
jobChangedreplay finds nothing to do.♻️ Proposed reordering
- await this.publish({ type: "analysis.jobChanged", job: committed.job }); await this.publish({ type: "analysis.eventDelta", jobId: request.jobId, runToken: request.runToken, revision: committed.job.revision, sequence: request.sequence, event: request.event, }); + await this.publish({ type: "analysis.jobChanged", job: committed.job });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/src/runtime/background-controller.ts` around lines 725 - 733, In the terminal-event publish flow, reorder the calls so analysis.eventDelta is published before analysis.jobChanged. Keep the existing payloads and await behavior unchanged, ensuring the in-band delta settles before the terminal jobChanged can trigger replay.apps/extension/src/storage/stores.test.ts (2)
26-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the job key from the shared prefix.
The double hardcodes
"perspectica.jobs.v1.job-1". Match the key by suffix, or import the job key prefix from the storage module, so a prefix change does not silently change what this test exercises.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/src/storage/stores.test.ts` around lines 26 - 32, Update the override set method’s failNextJobWrite condition to derive the job key from the shared storage prefix or match the job suffix instead of hardcoding "perspectica.jobs.v1.job-1"; preserve the existing one-shot failure behavior for the targeted job write.
241-267: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the run-token and payload mismatch rejections.
This test covers the new-row gap branch. Two rejection branches in
commitEventremain uncovered: a wrongrunTokenfor the job, and a stored journal row whose payload differs from the resubmitted event. Both silently drop events if they regress. Add one case for each.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/src/storage/stores.test.ts` around lines 241 - 267, Add coverage in the stores tests for the two uncovered rejection paths in JobStore.commitEvent: reject an event with a runToken that differs from the job’s run token, and reject a resubmission whose payload differs from the stored journal row. Assert each event is not accepted and is not silently appended or altered, using the existing event setup and journal inspection patterns.apps/extension/entrypoints/sidepanel/api.ts (2)
219-232: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winFlatten the drain loop and prune superseded buffer entries.
Two points in this block:
applyEnvelope(next, true)recurses, and the nested call runs its own drain loop. The outerwhilethen finds nothing left. The drain is therefore redundant, and stack depth grows with the buffered run length.- Entries buffered in
pendingEnvelopesare removed only when the drain reaches them. Whenreplaydelivers those sequences instead, the stale entries stay in the map for the lifetime of the stream.An iterative drain that deletes every entry at or below
deliveredSequenceresolves both.♻️ Proposed iterative drain
- deliveredSequence = envelope.sequence; - onEvent(envelope.event); - finishFromEvent(envelope.event); - while (!settled) { - const next = pendingEnvelopes.get(deliveredSequence + 1); - if (!next) break; - pendingEnvelopes.delete(next.sequence); - applyEnvelope(next, true); - } + let current: StreamEnvelope | undefined = envelope; + while (current && !settled) { + pendingEnvelopes.delete(current.sequence); + deliveredSequence = current.sequence; + onEvent(current.event); + finishFromEvent(current.event); + current = pendingEnvelopes.get(deliveredSequence + 1); + } + for (const sequence of [...pendingEnvelopes.keys()]) + if (sequence <= deliveredSequence) pendingEnvelopes.delete(sequence);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/entrypoints/sidepanel/api.ts` around lines 219 - 232, Refactor the envelope handling around applyEnvelope so buffered envelopes are drained iteratively without recursively calling applyEnvelope from the drain loop. After advancing deliveredSequence, repeatedly remove and process the next pending envelope, and prune any pendingEnvelopes entries whose sequence is at or below deliveredSequence, including entries superseded by replay delivery.
164-171: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winReuse the contract envelope type instead of defining
StreamEnvelopelocally.
@perspectica/contracts/eventsalready definesAnalysisEnvelopewithprotocol,jobId,runToken,sequence,revision, andevent. The sidepanel only uses the envelope fields after strippingprotocol, so derive the local type fromAnalysisEnveloperather than duplicating the shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/entrypoints/sidepanel/api.ts` around lines 164 - 171, Replace the local StreamEnvelope definition near pendingEnvelopes with a type derived from the existing AnalysisEnvelope contract in `@perspectica/contracts/events`, omitting the protocol field while retaining jobId, runToken, sequence, revision, and event. Update references as needed so pendingEnvelopes uses this derived contract type without duplicating the envelope shape.apps/extension/src/storage/job-journal.ts (1)
46-58: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueBound the write-through event cache.
appendEventnow caches every persisted envelope, not only the no-IndexedDB fallback rows.eventMemoryis pruned only byclearEventsfor a job id. A long run keeps every envelope resident in addition to the IndexedDB copy.
getEventreads only a single recent sequence during commit repair. A small bounded cache, or caching only the most recent sequences, gives the same repair behavior with a fixed memory ceiling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/src/storage/job-journal.ts` around lines 46 - 58, Update appendEvent in the eventMemory write-through path so it retains only a bounded set of recent envelopes instead of caching every persisted event. Enforce the fixed-size limit after both IndexedDB writes and fallback writes, while preserving getEvent’s ability to repair recent sequences and existing clearEvents behavior.apps/extension/entrypoints/offscreen/main.ts (1)
203-206: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
sourceCountnow counts candidates, not sources.The rename to
first.value.candidates.lengthis correct for the newEvidenceBatchshape. The surrounding vocabulary no longer matches. This PR establishes that a candidate is provider discovery and a source is adjudicated evidence. The returned field is stillsourceCount, and the error text at line 205 still says "did not return a web source."
testSearchProvideris a connectivity probe, so the behavior is correct. Rename the field tocandidateCountand adjust the message so the probe uses the same terms as the rest of the pipeline. The return type is declared at line 166 and consumed by the command caller, so update both.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/entrypoints/offscreen/main.ts` around lines 203 - 206, Update testSearchProvider and its declared return type to use candidateCount instead of sourceCount, and update the command caller to consume the renamed field. Change the zero-candidate error message to refer to a web candidate rather than a source, preserving the existing connectivity-probe behavior.packages/intelligence/src/evidence/source-ledger.ts (2)
130-130: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the injected clock for
retrievedAt.
new Date().toISOString()bypasses thenowfunction thatAnalysisInputandTargetedRetryInputalready thread through the pipeline. Snapshots become nondeterministic, so tests cannot assert onretrievedAt. Accept a clock on the ledger and use it here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/intelligence/src/evidence/source-ledger.ts` at line 130, Update the ledger construction and snapshot creation flow around retrievedAt to accept the injected clock already provided by AnalysisInput and TargetedRetryInput, then call that clock instead of new Date().toISOString(). Ensure retrievedAt remains an ISO timestamp while using the shared now function for deterministic snapshots.
222-230: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
hasEvidenceForSectionandisSufficientbuild a full validated snapshot.
snapshot()runsevaluateSufficiency,buildEvidenceGraph, and a completeSourceLedgerSnapshotSchema.parseover up to 64 sources, 96 assertions, 256 nodes, and 512 edges. Both accessors call it to read one value.If a caller checks all seven report sections, this performs seven graph builds and seven full zod parses. Read the underlying maps directly instead.
♻️ Proposed refactor: avoid snapshot construction in accessors
hasEvidenceForSection(section: ReportSection): boolean { - return this.snapshot().servedSections[section]?.length > 0; + return [...this.assertions.values()].some((assertion) => { + const mission = this.plan.missions.find((value) => value.id === assertion.missionId); + return mission?.canServeSections.includes(section) ?? false; + }); }
isSufficientcan callevaluateSufficiencydirectly rather than throughsnapshot():isSufficient(): boolean { - return this.snapshot().sufficiency.stop; + return evaluateSufficiency( + this.plan, + [...this.assertions.values()], + this.completedMissionCount(), + this.budget, + this.candidates.size, + ).stop; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/intelligence/src/evidence/source-ledger.ts` around lines 222 - 230, Update isSufficient and hasEvidenceForSection to avoid constructing the full validated snapshot for single-value reads: have isSufficient call evaluateSufficiency directly, and have hasEvidenceForSection read the underlying served-sections map or equivalent ledger state directly. Preserve their existing boolean results while avoiding repeated buildEvidenceGraph and SourceLedgerSnapshotSchema.parse work; leave hasEvidenceForClaim unchanged.packages/intelligence/src/pipeline.ts (1)
194-211: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdjudication rejections are discarded and produce no telemetry.
ledger.acceptAdjudications(decisions)returns aLedgerAcceptResultcarryingrejectedandreasons. Line 206 ignores it. Line 354 in the retry path does the same.This PR adds adjudication as the gate that prevents provider URLs from reaching the reader. When that gate drops a decision, nothing records it. An operator cannot distinguish "the providers found nothing" from "the adjudicator produced 20 decisions and validation rejected all 20". The
ledger.updatedevent emitted at lines 207-211 reports only the surviving counts.Emit the rejection count and reasons through
onTelemetry.📊 Proposed change: record the adjudication outcome
- ledger.acceptAdjudications(decisions); + const adjudicationResult = ledger.acceptAdjudications(decisions); + input.onTelemetry?.({ + level: adjudicationResult.rejected > 0 ? "warn" : "info", + scope: "evidence.adjudication", + event: "decisions.accepted", + message: `Adjudication accepted ${adjudicationResult.acceptedAssertions} of ${decisions.length} decisions.`, + payload: { + decisionCount: decisions.length, + rejected: adjudicationResult.rejected, + reasons: adjudicationResult.reasons, + }, + });Adjust the telemetry object to match the
PipelineTelemetryshape in this package.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/intelligence/src/pipeline.ts` around lines 194 - 211, Capture the LedgerAcceptResult returned by ledger.acceptAdjudications in both the main adjudication flow and the retry path, then report its rejected count and reasons through onTelemetry using the package’s PipelineTelemetry shape. Update the related ledger.updated telemetry so adjudication rejections are observable alongside the surviving source and assertion counts.packages/intelligence/src/retrieval/coordinator.ts (1)
55-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
candidateCountreports a ledger total, not a batch count.The field sits on a per-batch progress record next to
batch, but it holdsoptions.ledger.getCandidates().length, which is the cumulative deduplicated and capped ledger total.packages/intelligence/src/pipeline.tsforwards this value intoresearch.progresswhile separately accumulatingprogress.batch.candidatesinto an unbounded local array. The two counts diverge.Rename the field to
ledgerCandidateCount, or derive it frombatch.candidates.length, so the emitted progress number matches the set that is actually adjudicated. This shares a root cause with the pipeline accumulator; see the consolidated comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/intelligence/src/retrieval/coordinator.ts` around lines 55 - 62, Update the per-batch progress record around the generator yielding batch data so the candidate count reflects the current adjudicated batch, either by renaming the ledger total field to ledgerCandidateCount or by deriving it from batch.candidates.length; keep the ledger total distinct from the batch candidate count and align pipeline consumers accordingly.packages/intelligence/src/pipeline.test.ts (1)
97-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the rejection paths this PR introduces.
The adjudicator double always produces an accepted decision. Four new behaviors stay untested.
- Excerpt mismatch. Line 110 sets
excerpt: candidate.content, and line 78 makescontentidentical to the candidate content. Any excerpt-containment check invalidateEvidenceAdjudicationpasses trivially. The rejection branch never runs.- Unknown candidate id.
acceptAdjudicationsrejects a decision whosecandidateIdis not in the ledger. No test emits a decision with an unrecognized id.- Context signals. Line 113 always sets
context: null. The newEvidenceContextSignalSchemaand thecontextfield onEvidenceAssertionare never populated.- Failed-mission retry. The empty-run test at lines 224-263 asserts that a completed-empty run performs no retry retrieval. The positive counterpart is missing: a run whose missions are marked failed must perform retry retrieval.
Item 1 matters most. Preventing an unverifiable excerpt from reaching the reader is the stated purpose of this change.
#!/bin/bash # Description: Check existing coverage for adjudication rejection and retry-after-failure. set -euo pipefail echo "== validation tests ==" fd -t f 'validation.test.ts' packages/intelligence/src | xargs -r rg -n -C 5 'excerpt|claimRelevant|relationshipChecked|candidateId' echo "== retry-after-failure coverage ==" rg -n -C 8 'retryArticleSections' packages/intelligence/src --glob '*.test.ts' echo "== candidateCount assertions ==" rg -n -C 4 'candidateCount' packages/intelligence/src packages/contracts/src apps/extension --glob '*.test.ts'Also applies to: 224-263
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/intelligence/src/pipeline.test.ts` around lines 97 - 118, Extend the tests around the EvidenceAdjudicator double and the empty-run coverage to exercise all new rejection and retry paths: return a non-contained excerpt to verify validateEvidenceAdjudication rejects it, emit an unknown candidateId to verify acceptAdjudications rejects it, populate context with a valid EvidenceContextSignalSchema value, and add a failed-mission run asserting retryArticleSections performs retrieval. Keep the existing accepted-decision and completed-empty no-retry assertions intact.packages/contracts/src/evidence.ts (1)
42-57: 📐 Maintainability & Code Quality | 🔵 TrivialMigrate these schemas to Zod 4 top-level string formats.
packages/contractsuseszod@4, so replace the deprecated method-style formats consistently across the file:z.string().url()→z.url(), andz.string().datetime({ offset: true })→z.iso.datetime({ offset: true }).Confirm that
z.url()is acceptable forsourceUrl,canonicalUrl, andurlbecause its WHATWG URL validation behavior can differ from the old regex-based validation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/contracts/src/evidence.ts` around lines 42 - 57, Update all affected schemas in packages/contracts to use Zod 4 top-level formats: replace z.string().url() with z.url() and z.string().datetime({ offset: true }) with z.iso.datetime({ offset: true }). Apply this consistently to sourceUrl, canonicalUrl, and url fields, and verify z.url() preserves the intended WHATWG URL validation behavior for each.packages/intelligence/src/planning/lens.ts (1)
168-173: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReplace the constructed regular expression with a case-insensitive substring test.
The excerpt is escaped, so the pattern is a literal string and no backtracking risk exists. The regular expression adds no matching power over a substring check, and it allocates a new
RegExpfor every bias signal. A substring test is simpler, faster, and removes the dependency on keeping the escape set correct. It also clears theregexp-non-literal-typescriptstatic analysis warning.♻️ Proposed refactor
const excerpt = signal.excerpt.trim(); - if ( - !excerpt || - !new RegExp(excerpt.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), "i").test(paragraph.text) - ) - return []; + if ( + !excerpt || + !paragraph.text + .toLocaleLowerCase("en-US") + .includes(excerpt.toLocaleLowerCase("en-US")) + ) + return [];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/intelligence/src/planning/lens.ts` around lines 168 - 173, In the paragraph filtering logic around signal.excerpt, replace the escaped RegExp construction and test with a case-insensitive substring check. Preserve the existing empty-excerpt behavior and return [] when paragraph.text does not contain the excerpt, without allocating a RegExp.Source: Linters/SAST tools
packages/intelligence/src/evidence/adjudication.ts (1)
50-56: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the candidate section by characters, not only by count.
Each candidate contributes up to about 6,300 characters (4,000 content + 2,000 discovery + title). The cap
Math.max(input.budget.maxSources * 4, 24)bounds only the number of candidates. With 24 candidates the prompt already reaches roughly 150,000 characters, and it grows linearly withbudget.maxSources. A singlegenerateTextcall with a 45-second deadline can then exceed the model context window or time out.Add a running character budget so the prompt stops adding candidates after a fixed total.
♻️ Proposed refactor to add a total character budget
- const candidates = input.candidates - .slice(0, Math.max(input.budget.maxSources * 4, 24)) - .map( - (candidate) => - `CANDIDATE ${candidate.id} mission=${candidate.missionId ?? "global-search"} url=${candidate.sourceUrl} title=${compact(candidate.title, 300)} kind=${candidate.contentKind} sourceType=${candidate.sourceType}\nCONTENT: ${compact(candidate.content, 4_000)}\nDISCOVERY: ${compact(candidate.discoveryContext ?? "", 2_000)}`, - ) - .join("\n\n"); + const MAX_CANDIDATE_CHARACTERS = 60_000; + const renderedCandidates: string[] = []; + let usedCharacters = 0; + for (const candidate of input.candidates.slice( + 0, + Math.max(input.budget.maxSources * 4, 24), + )) { + const rendered = `CANDIDATE ${candidate.id} mission=${candidate.missionId ?? "global-search"} url=${candidate.sourceUrl} title=${compact(candidate.title, 300)} kind=${candidate.contentKind} sourceType=${candidate.sourceType}\nCONTENT: ${compact(candidate.content, 4_000)}\nDISCOVERY: ${compact(candidate.discoveryContext ?? "", 2_000)}`; + if (usedCharacters + rendered.length > MAX_CANDIDATE_CHARACTERS) break; + usedCharacters += rendered.length; + renderedCandidates.push(rendered); + } + const candidates = renderedCandidates.join("\n\n");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/intelligence/src/evidence/adjudication.ts` around lines 50 - 56, Update the candidate-building flow around the candidates map so it enforces a fixed total character budget in addition to the existing candidate-count limit. Track accumulated rendered candidate length, stop adding candidates once the budget is reached, and preserve the current formatting and per-candidate compact limits for included candidates.packages/intelligence/src/evidence/validation.test.ts (1)
251-272: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for accepted context missions.
Every context test asserts rejection. No test drives a
journalist-context,publication-context, ormissing-backgroundmission to an accepted result. Those are the paths where the subject matching and the mandatory-context rules apply.Two defects raised on
packages/intelligence/src/evidence/validation.tssit on exactly those paths, and the current suite would not detect either one:
containsSubjectreturnsfalsefor any subject whose words are all shorter than 4 characters. Add a case withpublication: "NPR".- A
missing-backgroundmission rejects every decision that carries nocontextsignal. Add a case withpurpose: "missing-background"andcontext: null.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/intelligence/src/evidence/validation.test.ts` around lines 251 - 272, Add accepted-result coverage in validation.test.ts for journalist-context or publication-context using publication "NPR", ensuring subject matching accepts the short subject, and for a missing-background mission with context: null, ensuring mandatory-context validation accepts it. Assert both decisions are accepted and exercise the relevant mission-specific paths in validateEvidenceAdjudication.packages/intelligence/src/planning/lens.test.ts (1)
92-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test that retains a bias signal.
Both tests assert that
grounded.biasis empty. A regression that drops every bias signal would still pass. Add a case with a bias signal whose excerpt is an exact substring of its own paragraph, and assert that the signal survives.The fixture also supports a positive attribution case. A signal on
p2/s2should produceattributed: true, becauses2has a speaker, an attribution verb, andisQuoted: true. That locks in thesignalAttributionbehavior added inpackages/intelligence/src/planning/lens.ts.💚 Proposed test
+ it("retains a grounded bias signal and recomputes attribution from the sentence", () => { + const grounded = groundedArticleSignals( + { + compass: [], + bias: [ + { + id: "bias-3", + technique: "word-choice", + paragraphId: "p2", + sentenceId: "s2", + excerpt: "the proposal is affordable", + explanation: "Attributed evaluative wording.", + confidence: 0.8, + attributed: false, + }, + ], + }, + article(), + ); + expect(grounded.bias).toHaveLength(1); + expect(grounded.bias[0]).toMatchObject({ + paragraphId: "p2", + sentenceId: "s2", + attributed: true, + }); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/intelligence/src/planning/lens.test.ts` around lines 92 - 146, Add a positive case to the “article signal grounding” tests using a bias signal on p2/s2 whose excerpt exactly matches text in that paragraph, then assert grounded.bias retains the signal and sets attributed to true. Keep the existing rejection cases unchanged and exercise the signalAttribution behavior in lens.ts.packages/intelligence/src/synthesis/perspective.ts (1)
57-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the duplicate
contextSourcesmap.
contextSources(line 58) rebuilds the exact sameMapasjournalistSources(line 30):new Map(ledger.getSources().map((source) => [source.id, source])). ReusejournalistSourcesinstead of callingledger.getSources()and constructing a second identical map.♻️ Proposed fix
const contextAssertions = ledger.getAssertions().filter((assertion) => assertion.context); - const contextSources = new Map(ledger.getSources().map((source) => [source.id, source])); + const contextSources = journalistSources;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/intelligence/src/synthesis/perspective.ts` around lines 57 - 58, Remove the duplicate contextSources Map construction and reuse the existing journalistSources map from the synthesis flow when resolving context sources. Update references to contextSources accordingly, while preserving the current source lookup behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ce0f9ed-f853-4656-8904-b0074454bab3
📒 Files selected for processing (40)
README.mdapps/extension/entrypoints/offscreen/main.tsapps/extension/entrypoints/sidepanel/AnalysisProgress.test.tsapps/extension/entrypoints/sidepanel/App.tsxapps/extension/entrypoints/sidepanel/ChatGptConnection.tsxapps/extension/entrypoints/sidepanel/SettingsScreen.tsxapps/extension/entrypoints/sidepanel/api.test.tsapps/extension/entrypoints/sidepanel/api.tsapps/extension/entrypoints/sidepanel/report-state.test.tsapps/extension/entrypoints/sidepanel/report-state.tsapps/extension/src/providers/chatgpt-evidence.tsapps/extension/src/providers/evidence.test.tsapps/extension/src/providers/exa-evidence.tsapps/extension/src/runtime/background-controller.tsapps/extension/src/storage/job-journal.tsapps/extension/src/storage/job-store.tsapps/extension/src/storage/stores.test.tsdocs/architecture-v2.mdpackages/contracts/src/events.tspackages/contracts/src/evidence.tspackages/contracts/src/index.tspackages/contracts/src/report.tspackages/intelligence/src/compass/calculate.tspackages/intelligence/src/evidence/adjudication.tspackages/intelligence/src/evidence/normalization.tspackages/intelligence/src/evidence/source-ledger.tspackages/intelligence/src/evidence/sufficiency.tspackages/intelligence/src/evidence/validation.test.tspackages/intelligence/src/evidence/validation.tspackages/intelligence/src/index.tspackages/intelligence/src/pipeline.test.tspackages/intelligence/src/pipeline.tspackages/intelligence/src/planning/lens.test.tspackages/intelligence/src/planning/lens.tspackages/intelligence/src/report/projector.test.tspackages/intelligence/src/report/projector.tspackages/intelligence/src/retrieval/coordinator.tspackages/intelligence/src/retrieval/scheduler.tspackages/intelligence/src/retrieval/types.tspackages/intelligence/src/synthesis/perspective.ts
| while (!settled && hasMore) { | ||
| const response = await sendRuntimeRequest<{ | ||
| jobId: string; | ||
| lastSequence: number; | ||
| hasMore?: boolean; | ||
| events: StreamEnvelope[]; | ||
| complete: boolean; | ||
| }>({ type: "analysis.getEventsSince", jobId: job.id, lastSequence: deliveredSequence }); | ||
| const events = [...response.events].sort((left, right) => left.sequence - right.sequence); | ||
| if ( | ||
| events.length === 0 && | ||
| response.lastSequence > deliveredSequence && | ||
| !response.complete | ||
| ) | ||
| throw new Error( | ||
| "The analysis journal has a missing event gap. Reconnect and try again.", | ||
| ); | ||
| for (const envelope of events) applyEnvelope(envelope, true); | ||
| const lastReturned = events.at(-1)?.sequence ?? deliveredSequence; | ||
| hasMore = Boolean(response.hasMore) || lastReturned < response.lastSequence; | ||
| if (hasMore && events.length === 0) | ||
| throw new Error("The analysis journal could not replay its next event."); | ||
| if (!hasMore && response.lastSequence > deliveredSequence) | ||
| throw new Error( | ||
| "The analysis journal has a missing event gap. Reconnect and try again.", | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a no-progress guard to the pagination loop.
The loop repeats while hasMore is true. hasMore becomes true when lastReturned < response.lastSequence. The throw at Line 264 only covers the case where the page is empty.
A non-empty page whose events are all discarded by applyEnvelope leaves deliveredSequence unchanged. The next request sends the same lastSequence, so the server returns the same page, and the loop repeats without delay or bound. applyEnvelope discards an envelope when envelope.runToken !== (job.runToken ?? ""), so a journal that still holds rows from an earlier run token for the same job id reaches this state.
Track deliveredSequence across iterations and stop when a page produces no advance.
🐛 Proposed progress guard
let hasMore = true;
while (!settled && hasMore) {
+ const before = deliveredSequence;
const response = await sendRuntimeRequest<{ for (const envelope of events) applyEnvelope(envelope, true);
+ if (!settled && deliveredSequence === before)
+ throw new Error(
+ "The analysis journal returned no usable events. Reconnect and try again.",
+ );
const lastReturned = events.at(-1)?.sequence ?? deliveredSequence;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| while (!settled && hasMore) { | |
| const response = await sendRuntimeRequest<{ | |
| jobId: string; | |
| lastSequence: number; | |
| hasMore?: boolean; | |
| events: StreamEnvelope[]; | |
| complete: boolean; | |
| }>({ type: "analysis.getEventsSince", jobId: job.id, lastSequence: deliveredSequence }); | |
| const events = [...response.events].sort((left, right) => left.sequence - right.sequence); | |
| if ( | |
| events.length === 0 && | |
| response.lastSequence > deliveredSequence && | |
| !response.complete | |
| ) | |
| throw new Error( | |
| "The analysis journal has a missing event gap. Reconnect and try again.", | |
| ); | |
| for (const envelope of events) applyEnvelope(envelope, true); | |
| const lastReturned = events.at(-1)?.sequence ?? deliveredSequence; | |
| hasMore = Boolean(response.hasMore) || lastReturned < response.lastSequence; | |
| if (hasMore && events.length === 0) | |
| throw new Error("The analysis journal could not replay its next event."); | |
| if (!hasMore && response.lastSequence > deliveredSequence) | |
| throw new Error( | |
| "The analysis journal has a missing event gap. Reconnect and try again.", | |
| ); | |
| } | |
| let hasMore = true; | |
| while (!settled && hasMore) { | |
| const before = deliveredSequence; | |
| const response = await sendRuntimeRequest<{ | |
| jobId: string; | |
| lastSequence: number; | |
| hasMore?: boolean; | |
| events: StreamEnvelope[]; | |
| complete: boolean; | |
| }>({ type: "analysis.getEventsSince", jobId: job.id, lastSequence: deliveredSequence }); | |
| const events = [...response.events].sort((left, right) => left.sequence - right.sequence); | |
| if ( | |
| events.length === 0 && | |
| response.lastSequence > deliveredSequence && | |
| !response.complete | |
| ) | |
| throw new Error( | |
| "The analysis journal has a missing event gap. Reconnect and try again.", | |
| ); | |
| for (const envelope of events) applyEnvelope(envelope, true); | |
| if (!settled && deliveredSequence === before) | |
| throw new Error( | |
| "The analysis journal returned no usable events. Reconnect and try again.", | |
| ); | |
| const lastReturned = events.at(-1)?.sequence ?? deliveredSequence; | |
| hasMore = Boolean(response.hasMore) || lastReturned < response.lastSequence; | |
| if (hasMore && events.length === 0) | |
| throw new Error("The analysis journal could not replay its next event."); | |
| if (!hasMore && response.lastSequence > deliveredSequence) | |
| throw new Error( | |
| "The analysis journal has a missing event gap. Reconnect and try again.", | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/entrypoints/sidepanel/api.ts` around lines 244 - 270, Update
the pagination loop around sendRuntimeRequest and applyEnvelope to track
deliveredSequence before processing each page, then detect when the page
produces no sequence advance. Stop or throw according to the existing replay
error behavior when hasMore remains true without progress, including non-empty
pages whose envelopes are discarded by applyEnvelope.
| const updatePreferences = async (next: SettingsPreferences): Promise<void> => { | ||
| if (!runtime) throw new Error("Perspectica settings are still loading."); | ||
| const previous = runtime; | ||
| const updated: ExtensionPreferences = { | ||
| ...runtime.preferences, | ||
| ...next, | ||
| }; | ||
| setRuntime({ ...runtime, preferences: updated }); | ||
| void updateExtensionPreferences(updated); | ||
| try { | ||
| const saved = await updateExtensionPreferences(updated); | ||
| setRuntime((current) => (current ? { ...current, preferences: saved } : current)); | ||
| } catch (error) { | ||
| setRuntime(previous); | ||
| throw error; | ||
| } | ||
| }; | ||
|
|
||
| const updateSearchProvider = async (provider: SearchProviderKind) => { | ||
| if (!runtime) throw new Error("Perspectica settings are still loading."); | ||
| const previous = runtime; | ||
| const test = await testSearchProvider(provider); | ||
| if (!test.available) throw new Error(`${provider} search is not available.`); | ||
| const updated = { ...runtime.preferences, searchProvider: provider }; | ||
| setRuntime({ ...runtime, preferences: updated }); | ||
| setProviderReady(true); | ||
| await updateExtensionPreferences(updated); | ||
| try { | ||
| const saved = await updateExtensionPreferences(updated); | ||
| setRuntime((current) => (current ? { ...current, preferences: saved } : current)); | ||
| } catch (error) { | ||
| setRuntime(previous); | ||
| setProviderReady(previous.preferences.searchProvider === "chatgpt" || previous.hasExaKey); | ||
| throw error; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Guard updatePreferences and updateSearchProvider against overlapping calls.
Both functions capture runtime as previous at call time and later call setRuntime(previous) on failure, or setRuntime with the fetched saved value on success, with no sequence check. If two calls overlap, for example a preference toggle and a search-provider change fired close together, or one call resolving slower than a later one, the later-resolving call determines the final state regardless of which call started more recently:
- On success: an older, slower call can overwrite the state committed by a newer, faster call with stale
saveddata. - On failure: an older call's
previousrollback can undo a newer call's already-applied optimistic or committed state.
Both paths silently revert or overwrite the user's most recent settings change without any visible error. The file already has prior art for this exact problem: analyze() (Line 523) guards stale completions with a monotonic runRef counter. Apply the same pattern here, shared across both functions since they both mutate runtime.preferences.
🔒 Proposed fix using a shared sequence guard
+ const preferencesRequestRef = useRef(0);
+
const updatePreferences = async (next: SettingsPreferences): Promise<void> => {
if (!runtime) throw new Error("Perspectica settings are still loading.");
+ const requestId = ++preferencesRequestRef.current;
const previous = runtime;
const updated: ExtensionPreferences = {
...runtime.preferences,
...next,
};
setRuntime({ ...runtime, preferences: updated });
try {
const saved = await updateExtensionPreferences(updated);
+ if (preferencesRequestRef.current !== requestId) return;
setRuntime((current) => (current ? { ...current, preferences: saved } : current));
} catch (error) {
- setRuntime(previous);
+ if (preferencesRequestRef.current === requestId) setRuntime(previous);
throw error;
}
};
const updateSearchProvider = async (provider: SearchProviderKind) => {
if (!runtime) throw new Error("Perspectica settings are still loading.");
+ const requestId = ++preferencesRequestRef.current;
const previous = runtime;
const test = await testSearchProvider(provider);
if (!test.available) throw new Error(`${provider} search is not available.`);
const updated = { ...runtime.preferences, searchProvider: provider };
setRuntime({ ...runtime, preferences: updated });
setProviderReady(true);
try {
const saved = await updateExtensionPreferences(updated);
+ if (preferencesRequestRef.current !== requestId) return;
setRuntime((current) => (current ? { ...current, preferences: saved } : current));
} catch (error) {
- setRuntime(previous);
- setProviderReady(previous.preferences.searchProvider === "chatgpt" || previous.hasExaKey);
+ if (preferencesRequestRef.current === requestId) {
+ setRuntime(previous);
+ setProviderReady(previous.preferences.searchProvider === "chatgpt" || previous.hasExaKey);
+ }
throw error;
}
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/entrypoints/sidepanel/App.tsx` around lines 835 - 868, Guard
both updatePreferences and updateSearchProvider with a shared monotonic sequence
ref, following the existing analyze() runRef pattern. Increment the shared
sequence when either function starts, and only apply optimistic updates,
successful saved preferences, rollback state, or provider readiness changes if
that call still owns the latest sequence; stale completions must not overwrite
newer preference changes.
| } catch (cause) { | ||
| if (cancelled) return; | ||
| setError(cause instanceof Error ? cause.message : "ChatGPT sign-in did not finish."); | ||
| setConnecting(false); | ||
| if (Date.now() >= device.expiresAt) { | ||
| setAuth((current) => ({ | ||
| ...current, | ||
| status: "expired", | ||
| error: "The one-time sign-in code expired. Start a new Login with ChatGPT flow.", | ||
| })); | ||
| setDevice(null); | ||
| setConnecting(false); | ||
| return; | ||
| } | ||
| // Polling errors are transient (the browser may briefly lose access | ||
| // to OpenAI). Keep the device authorization alive and schedule the | ||
| // next poll instead of leaving the UI permanently pending. | ||
| setError(cause instanceof Error ? cause.message : "ChatGPT sign-in is still pending."); | ||
| timer = setTimeout(poll, Math.max(1_500, device.intervalMs)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Stop retrying terminal authentication failures.
The catch block treats every error as transient until device.expiresAt. The upstream apps/extension/src/auth/chatgpt-session.ts poll can throw the terminal "ChatGPT did not return a refreshable session. Start sign-in again." error before that time. This code then keeps the UI pending and schedules more polls. Return a typed terminal outcome or classify non-retryable authentication errors before scheduling another poll.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/entrypoints/sidepanel/ChatGptConnection.tsx` around lines 86 -
102, The polling catch block in the ChatGPT connection flow must stop retrying
terminal authentication failures from the session poll. Classify the known
non-retryable refresh-session error (or use a typed terminal outcome from the
poll) before the transient-error path, update authentication state to require a
new sign-in, clear the device/connection state as appropriate, and return
without scheduling another timeout; preserve retries for genuinely transient
errors.
| const [exaKey, setExaKey] = useState(""); | ||
| const [providerStatus, setProviderStatus] = useState<string | null>(null); | ||
| const [cacheStatus, setCacheStatus] = useState<string | null>(null); | ||
| const [saveStatus, setSaveStatus] = useState<"idle" | "saving" | "saved" | "error">("idle"); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Keep a local draft and serialize preference saves.
Because App.tsx:933-940 updates preferences only after persistence resolves, rapid changes can build multiple payloads from the same old snapshot. The second change can drop the first change. Older requests can also overwrite the latest saveStatus.
Maintain a local draft, serialize or coalesce writes, roll back the draft on the latest failure, and ignore stale completion statuses. Add a test that resolves two saves out of order.
Also applies to: 120-127, 161-161, 206-206, 331-331, 374-382
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/entrypoints/sidepanel/SettingsScreen.tsx` at line 49, Update
SettingsScreen’s preference editing and save flow around saveStatus and the
referenced handlers to maintain a local draft instead of deriving rapid updates
from the persisted preferences snapshot. Serialize or coalesce writes so each
change includes prior draft changes, roll back the draft only when the latest
save fails, and ignore stale request completions when updating saveStatus. Add a
test that resolves two saves out of order and verifies the latest draft and
status are preserved.
| const discoveryContext = result.text.trim().slice(0, 20_000) || null; | ||
| const candidates: EvidenceCandidate[] = unique.map(({ source, url }) => ({ | ||
| id: sourceIdFor(url, "candidate"), | ||
| missionId: null, | ||
| sourceUrl: url, | ||
| title: source.title?.trim() || hostname(url), | ||
| publication: hostname(url), | ||
| publishedAt: null, | ||
| content: discoveryContext ?? `Native web search returned ${hostname(url)}.`, | ||
| contentKind: "search-summary", | ||
| sourceType: sourceType(url), | ||
| discoveryContext, | ||
| discoveryExcerpt: null, | ||
| providerScore: null, | ||
| provider: "chatgpt", | ||
| })); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Every candidate shares the same aggregated content, which defeats the per-source claim anchor check.
discoveryContext is the full model response text for the whole search. Lines 131 and 134 assign that same text to content and discoveryContext for every candidate in unique. The text describes all discovered sources, not the individual URL.
claimAnchorMatches in packages/intelligence/src/evidence/validation.ts Lines 52-54 builds its haystack from candidate.content, candidate.discoveryContext, and candidate.title. Because all candidates carry identical aggregated text, a claim anchor found anywhere in the response makes every URL in the batch pass the anchor check, including URLs the model never connected to that claim.
The search-summary rule at validation.ts Line 223 blocks supports, contradicts, and qualifies, so the impact is limited to adds-context and context signals. It is still a false-grounding path, and it is the pattern this PR removes elsewhere.
Attribute per-source text where the response provides it. If the response cannot be split per source, keep content source-specific and pass the aggregated text separately so the anchor check does not treat it as source content.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/src/providers/chatgpt-evidence.ts` around lines 123 - 138,
Update the candidate construction in the ChatGPT evidence discovery flow so each
candidate’s content and discoveryContext contain only text attributable to its
own source URL, rather than the shared aggregated result.text. Reuse the
response’s per-source attribution when available; otherwise keep content
source-specific using the existing fallback and pass the full aggregated
response through a separate non-source-content field that claimAnchorMatches
will not inspect.
| const previous = this.sources.get(sourceId); | ||
| if (!previous && this.sources.size >= this.budget.maxSources) continue; | ||
| if (!previous) acceptedSources += 1; | ||
| this.sources.set(sourceId, mergeSource(previous, source)); | ||
| if (this.assertions.size >= MAX_EVIDENCE_ASSERTIONS) continue; | ||
| const assertionId = assertionIdFor(sourceId, card.missionId, card.statement); | ||
| const assertionId = assertionIdFor(sourceId, decision.missionId, decision.statement); | ||
| if (this.assertions.has(assertionId)) continue; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The assertion cap leaves orphan sources in the ledger.
The source is written at line 136 before the assertion cap is tested at line 137. When this.assertions.size >= MAX_EVIDENCE_ASSERTIONS, the method writes the SourceRecord and increments acceptedSources, then skips the assertion with continue.
The result is a SourceRecord in the snapshot that no assertion references. That record still flows into buildEvidenceGraph and into the Works Cited projection, so the reader sees a cited source with no evidence attached to it. acceptedSources also over-reports.
Test the assertion cap before writing the source.
🐛 Proposed fix: check both caps before mutating state
const previous = this.sources.get(sourceId);
if (!previous && this.sources.size >= this.budget.maxSources) continue;
- if (!previous) acceptedSources += 1;
- this.sources.set(sourceId, mergeSource(previous, source));
if (this.assertions.size >= MAX_EVIDENCE_ASSERTIONS) continue;
const assertionId = assertionIdFor(sourceId, decision.missionId, decision.statement);
if (this.assertions.has(assertionId)) continue;
+ if (!previous) acceptedSources += 1;
+ this.sources.set(sourceId, mergeSource(previous, source));
const claim = decision.claimId📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const previous = this.sources.get(sourceId); | |
| if (!previous && this.sources.size >= this.budget.maxSources) continue; | |
| if (!previous) acceptedSources += 1; | |
| this.sources.set(sourceId, mergeSource(previous, source)); | |
| if (this.assertions.size >= MAX_EVIDENCE_ASSERTIONS) continue; | |
| const assertionId = assertionIdFor(sourceId, card.missionId, card.statement); | |
| const assertionId = assertionIdFor(sourceId, decision.missionId, decision.statement); | |
| if (this.assertions.has(assertionId)) continue; | |
| const previous = this.sources.get(sourceId); | |
| if (!previous && this.sources.size >= this.budget.maxSources) continue; | |
| if (this.assertions.size >= MAX_EVIDENCE_ASSERTIONS) continue; | |
| const assertionId = assertionIdFor(sourceId, decision.missionId, decision.statement); | |
| if (this.assertions.has(assertionId)) continue; | |
| if (!previous) acceptedSources += 1; | |
| this.sources.set(sourceId, mergeSource(previous, source)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intelligence/src/evidence/source-ledger.ts` around lines 133 - 139,
Check the MAX_EVIDENCE_ASSERTIONS limit before mutating state in the
source-processing flow, alongside the existing source-cap validation. Ensure
sources skipped because the assertion cap is reached are not written to
this.sources and do not increment acceptedSources; preserve existing behavior
for sources that can still accept an assertion.
| function containsSubject(haystack: string, subject: string): boolean { | ||
| const source = normalizeEvidenceText(haystack).toLocaleLowerCase("en-US"); | ||
| const expected = words(subject); | ||
| return ( | ||
| expected.length > 0 && | ||
| expected.filter((word) => source.includes(word)).length >= expected.length | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
containsSubject returns false for every subject with only short words.
words() keeps only tokens of length 4 or more. For a subject such as "NPR", "Vox", "BBC", or an author named "Li Wu", expected is empty, and the guard expected.length > 0 makes containsSubject return false.
This propagates to Lines 317-324. A journalist-context or publication-context mission with such a subject is always rejected with "The candidate does not identify the requested context subject", even when the candidate names the subject verbatim. The same failure applies at Lines 303-316 for the context subject check.
Add a fallback that matches the normalized full subject when no long tokens exist.
🐛 Proposed fix for short subjects
function containsSubject(haystack: string, subject: string): boolean {
const source = normalizeEvidenceText(haystack).toLocaleLowerCase("en-US");
+ const normalizedSubject = normalizeEvidenceText(subject).toLocaleLowerCase("en-US").trim();
+ if (!normalizedSubject) return false;
const expected = words(subject);
+ if (expected.length === 0) return source.includes(normalizedSubject);
return (
expected.length > 0 &&
expected.filter((word) => source.includes(word)).length >= expected.length
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function containsSubject(haystack: string, subject: string): boolean { | |
| const source = normalizeEvidenceText(haystack).toLocaleLowerCase("en-US"); | |
| const expected = words(subject); | |
| return ( | |
| expected.length > 0 && | |
| expected.filter((word) => source.includes(word)).length >= expected.length | |
| ); | |
| } | |
| function containsSubject(haystack: string, subject: string): boolean { | |
| const source = normalizeEvidenceText(haystack).toLocaleLowerCase("en-US"); | |
| const normalizedSubject = normalizeEvidenceText(subject).toLocaleLowerCase("en-US").trim(); | |
| if (!normalizedSubject) return false; | |
| const expected = words(subject); | |
| if (expected.length === 0) return source.includes(normalizedSubject); | |
| return ( | |
| expected.length > 0 && | |
| expected.filter((word) => source.includes(word)).length >= expected.length | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intelligence/src/evidence/validation.ts` around lines 38 - 45,
Update containsSubject to fall back to matching the normalized full subject when
words(subject) produces no long tokens, while preserving the existing
token-based matching for subjects with eligible words. Ensure short subjects
such as “NPR”, “Vox”, “BBC”, and “Li Wu” are recognized when present verbatim.
| if (decision.context && !expectedKind) | ||
| return { | ||
| ...result( | ||
| false, | ||
| "verified-url", | ||
| basic.validation.excerptMatches, | ||
| "Political context signals are restricted to context missions.", | ||
| ), | ||
| canonicalUrl: basic.canonicalUrl, | ||
| }; | ||
| if (expectedKind && !decision.context) | ||
| return { | ||
| ...result( | ||
| false, | ||
| "verified-url", | ||
| basic.validation.excerptMatches, | ||
| "Context missions require a source-backed context signal.", | ||
| ), | ||
| canonicalUrl: basic.canonicalUrl, | ||
| }; | ||
| if (decision.context && decision.context.sourceKind !== expectedKind && expectedKind) | ||
| return { | ||
| ...result( | ||
| false, | ||
| "verified-url", | ||
| basic.validation.excerptMatches, | ||
| "The context signal does not match the mission purpose.", | ||
| ), | ||
| canonicalUrl: basic.canonicalUrl, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A missing-background mission cannot produce evidence without a political context signal.
expectedContextKind returns "topic-context" for missing-background. Line 269 then rejects every decision from such a mission when decision.context is null.
This conflicts with the adjudicator instruction in packages/intelligence/src/evidence/adjudication.ts Line 74: the model must emit context only when it is explicitly present in the candidate. A missing-background candidate that provides plain factual background, with no political lean, produces a decision without context. Validation drops it.
The result inverts the intent of the change. The model must fabricate a score for the decision to survive, or all background evidence is lost.
Require a context signal only for the identity-bound kinds, and allow topic-context and comparable-coverage decisions without one.
🐛 Proposed fix
- if (expectedKind && !decision.context)
+ if (
+ expectedKind &&
+ ["journalist-work", "publication-history"].includes(expectedKind) &&
+ !decision.context
+ )
return {
...result(
false,
"verified-url",
basic.validation.excerptMatches,
"Context missions require a source-backed context signal.",
),
canonicalUrl: basic.canonicalUrl,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (decision.context && !expectedKind) | |
| return { | |
| ...result( | |
| false, | |
| "verified-url", | |
| basic.validation.excerptMatches, | |
| "Political context signals are restricted to context missions.", | |
| ), | |
| canonicalUrl: basic.canonicalUrl, | |
| }; | |
| if (expectedKind && !decision.context) | |
| return { | |
| ...result( | |
| false, | |
| "verified-url", | |
| basic.validation.excerptMatches, | |
| "Context missions require a source-backed context signal.", | |
| ), | |
| canonicalUrl: basic.canonicalUrl, | |
| }; | |
| if (decision.context && decision.context.sourceKind !== expectedKind && expectedKind) | |
| return { | |
| ...result( | |
| false, | |
| "verified-url", | |
| basic.validation.excerptMatches, | |
| "The context signal does not match the mission purpose.", | |
| ), | |
| canonicalUrl: basic.canonicalUrl, | |
| }; | |
| if (decision.context && !expectedKind) | |
| return { | |
| ...result( | |
| false, | |
| "verified-url", | |
| basic.validation.excerptMatches, | |
| "Political context signals are restricted to context missions.", | |
| ), | |
| canonicalUrl: basic.canonicalUrl, | |
| }; | |
| if ( | |
| expectedKind && | |
| ["journalist-work", "publication-history"].includes(expectedKind) && | |
| !decision.context | |
| ) | |
| return { | |
| ...result( | |
| false, | |
| "verified-url", | |
| basic.validation.excerptMatches, | |
| "Context missions require a source-backed context signal.", | |
| ), | |
| canonicalUrl: basic.canonicalUrl, | |
| }; | |
| if (decision.context && decision.context.sourceKind !== expectedKind && expectedKind) | |
| return { | |
| ...result( | |
| false, | |
| "verified-url", | |
| basic.validation.excerptMatches, | |
| "The context signal does not match the mission purpose.", | |
| ), | |
| canonicalUrl: basic.canonicalUrl, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intelligence/src/evidence/validation.ts` around lines 259 - 288,
Update the validation checks around expectedKind and decision.context so a
missing context signal is required only for identity-bound context kinds.
Preserve rejection for identity-bound missions lacking decision.context, but
allow topic-context and comparable-coverage decisions to proceed without one;
keep existing sourceKind mismatch validation for supplied context signals.
| const missions = options.plan.missions.filter( | ||
| (mission) => | ||
| mission.canServeSections.some((section) => requested.has(section)) && | ||
| !coveredMissionIds.has(mission.id), | ||
| options.ledger.isMissionFailed(mission.id), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
A mission whose candidates are all rejected can never be retried.
The targeted retry filter now requires options.ledger.isMissionFailed(mission.id). failedMissions is set only in EvidenceLedger.accept when batch.status === "failed". Adjudication rejections in acceptAdjudications do not mark a mission failed.
Consider this sequence:
- A provider returns candidates for mission
m1and reportsstatus: "completed".m1enterscompletedMissions. - Every candidate for
m1is rejected byvalidateEvidenceAdjudication(unverifiable URL, excerpt mismatch, claim not relevant). - The section served by
m1renders empty. failedSectionsForReportinpackages/intelligence/src/pipeline.tsreports no failure, so the report status iscomplete.- If the reader requests a retry for that section, this filter excludes
m1, so the retry performs no retrieval.
The section is permanently empty with no recovery path and no signal to the reader. The previous snapshot-based check covered this case because it inspected whether the mission produced assertions.
Distinguishing completed-empty from failed is correct for a provider that honestly returned nothing. A mission whose every candidate was rejected during adjudication is a different state. Track it, for example by marking a mission as failed or as exhausted-rejected when it contributed candidates but produced zero accepted assertions.
#!/bin/bash
# Description: Confirm failedMissions is only written on batch failure and never on adjudication rejection.
set -euo pipefail
echo "== writes to failedMissions =="
rg -n -C 4 'failedMissions' packages/intelligence/src
echo "== acceptAdjudications rejection paths =="
ast-grep run --lang typescript --pattern 'acceptAdjudications($$$) { $$$ }' packages/intelligence/src/evidence/source-ledger.ts
echo "== retry eligibility filter =="
rg -n -C 10 'isMissionFailed' packages/intelligence/src🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intelligence/src/retrieval/coordinator.ts` around lines 84 - 87,
Update the mission outcome tracking used by the retrieval coordinator so a
mission that contributes candidates but has all of them rejected during
acceptAdjudications is marked retry-eligible, such as failed or
exhausted-rejected. Preserve the distinction for completed missions that
honestly return no candidates, and ensure the filter around
options.ledger.isMissionFailed(mission.id) includes adjudication-exhausted
missions when retrying the affected section.
| while (inFlight.size > 0) { | ||
| if (controller.signal.aborted) throw abortError(controller.signal); | ||
| const result = await Promise.race(inFlight.values()); | ||
| inFlight.delete(result.id); | ||
| if (result.error !== undefined) throw result.error; | ||
| if (result.value !== undefined) yield result.value; | ||
| while (inFlight.size < concurrency && cursor < queue.length) start(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
undefined is used as a sentinel for both the value and the error, so results and failures can be dropped.
Two cases fail:
- Line 49: a task that resolves to
undefinedis not yielded.runPriorityTasksthen returns fewer results than tasks. The previousPromise.allimplementation preserved every slot, includingundefined, so this is a silent behavior change for anySchedulerTask<T>whereTadmitsundefinedorvoid. - Line 48: a task that rejects with
undefinedproduces{ id, error: undefined }, which is indistinguishable from success. The rejection is swallowed and the loop continues.
Use an explicit discriminant instead of undefined checks.
🐛 Proposed fix
- const inFlight = new Map<number, Promise<{ id: number; value?: T; error?: unknown }>>();
+ type Settled<V> = { id: number; ok: true; value: V } | { id: number; ok: false; error: unknown };
+ const inFlight = new Map<number, Promise<Settled<T>>>();
let taskId = 0;
const start = () => {
const task = queue[cursor++];
if (!task) return;
const id = taskId++;
const promise = task
.run()
- .then((value) => ({ id, value }))
- .catch((error) => ({ id, error }));
+ .then((value): Settled<T> => ({ id, ok: true, value }))
+ .catch((error): Settled<T> => ({ id, ok: false, error }));
inFlight.set(id, promise);
};
@@
const result = await Promise.race(inFlight.values());
inFlight.delete(result.id);
- if (result.error !== undefined) throw result.error;
- if (result.value !== undefined) yield result.value;
+ if (!result.ok) throw result.error;
+ yield result.value;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intelligence/src/retrieval/scheduler.ts` around lines 44 - 51,
Update the result handling in the scheduler loop around Promise.race and the
inFlight task wrapper to use an explicit success/error discriminant rather than
checking whether result.value or result.error is undefined. Always yield
successful values, including undefined, and propagate rejected errors, including
undefined, while preserving task ordering and concurrency behavior.
What changed
Root causes addressed
The merged V2 pipeline promoted provider URLs into assertions based on mission purpose and result position. This corrective PR keeps discovery records out of the ledger until a bounded adjudicator maps a known candidate to an exact claim and relationship; deterministic validation then checks provenance, claim anchors, excerpt mechanics, mission boundaries, context identity, and score direction.
The event path previously advanced the job cursor before the IndexedDB journal write. It now journals first and repairs the cursor on same-sequence retry. Replay requires contiguous sequences and paginates beyond the old 256-event ceiling.
Validation
pnpm verify:releasepnpm bench:v2pnpm audit --prod --audit-level=highgit diff --checkResult: 107 tests across 25 files, typecheck/build/package checks passing, 15 build files, 1,142,549 unpacked build bytes, 1,037,000 JavaScript bytes, 25,146 CSS bytes, and no known production audit vulnerabilities.
Limitation
No live authenticated Exa or ChatGPT analysis was performed in this environment. The benchmark is structural/package evidence, not proof of live provider quality, latency, token use, or reader-facing correctness. Those authenticated runs remain the final release gate.
Summary by CodeRabbit
New Features
Bug Fixes