feat: verify Global Ask semantic claims with public evidence - #276
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Superseded by the current exact-head GREEN integration instruction in #276 comment 5350114633. Do not use the deleted bootstrap payload/workflow or create wrapper copies of |
|
@opencode-agent Exact-head review requested for |
|
@opencode-agent Exact-head review requested for |
|
Exact-head follow-up for |
…man-v2190' into feat/global-ask-public-claim-verification-v2200
…man-v2190' into feat/global-ask-public-claim-verification-v2200 # Conflicts: # frontend/src/App.tsx # frontend/src/i18n.test.ts
|
@opencode-agent @devin-ai-integration Please review exact current head |
…man-v2190' into HEAD # Conflicts: # docker/postgres-init/migrate.sh
|
Restacked onto current #266 head 0beb110 with normal merge commit af7c0d5. Local verification on the pushed tree: backend 839 passed, 16 skipped; frontend 190 passed; Global Ask verification and project-history targeted tests 63 backend plus 13 frontend passed; lint, build, Storybook build, compileall, actionlint, and diff checks passed. Migration allowlisting preserves both the semantic-search/project-history slice and the current Event Lineage migrations. Awaiting independent review and hosted Checks on the current head. |
…man-v2190' into codex/review-pr276-current
There was a problem hiding this comment.
🔍 Project-history frontend components are not wired into the app
ProjectHistoryDisclosure, ProjectHistoryTimeline, fetchProjectHistory, and groupProjectEvidence are exercised only by tests/stories; none are referenced in App.tsx. The backend /api/project-history route is mounted (main.py), but no buyer surface calls it. This is dead-but-tested code, not a defect, though reviewers may want to confirm the intended integration point is a follow-up.
Was this helpful? React with 👍 or 👎 to provide feedback.
| except (HttpClientError, KeyError, OSError, TypeError, ValueError): | ||
| return VERIFICATION_UNAVAILABLE, () |
There was a problem hiding this comment.
🟡 Malformed verification response crashes the whole Ask answer
The except clause here catches only (HttpClientError, KeyError, OSError, TypeError, ValueError), but client.verify reads body["choices"][0] (claim_verification.py), which raises IndexError on an empty choices list. That escapes _verify_public_claims, running before persist_global_ask_turn, so the whole opt-in /api/ask request returns 500 and the already-computed answer is discarded. Other provider boundaries here fail closed with a broad except Exception.
| except (HttpClientError, KeyError, OSError, TypeError, ValueError): | |
| return VERIFICATION_UNAVAILABLE, () | |
| except Exception: # noqa: BLE001 - provider boundary is fail-closed. | |
| return VERIFICATION_UNAVAILABLE, () |
Was this helpful? React with 👍 or 👎 to provide feedback.
| list(authorized_corporate_entity_ids), | ||
| candidate_ids, | ||
| limit, | ||
| not bool(question), | ||
| ) | ||
| visible_rows = [row for row in rows if can_see_post(row)][:limit] | ||
| candidate_id_set = frozenset(candidate_ids) | ||
| visible_rows = [ | ||
| row | ||
| for row in rows | ||
| if (not question or str(row["post_id"]) in candidate_id_set) and can_see_post(row) |
There was a problem hiding this comment.
📝 Info: No-unrelated-recency fallback correctly closes on empty candidate scores
gather_global_chat_sources adds if question and not candidate_scores: return [] (post_chat_ingestion.py) plus a candidate_predicate restricting the final query to candidate_ids (post_chat_ingestion.py) and a post-filter str(row['post_id']) in candidate_id_set (post_chat_ingestion.py). Together these prevent the previous behavior where a question with few/no lexical matches fell through to recent unrelated posts. When question is falsy, the predicate is empty but $2/$3 are still bound, so the SQL remains valid. This was verified against test_global_ask_public_integration.py.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "cited_post_evidence": cited_post_evidence(sources, cited_ids), | ||
| "source_post_ids": [source.post_id for source in sources], | ||
| "timeline": global_ask_timeline(sources), | ||
| "external_verification_status": verification_status, | ||
| "external_claims": [claim.to_payload() for claim in external_claims], | ||
| "next_action": _verification_next_action(verification_status), |
There was a problem hiding this comment.
📝 Info: Main-branch Global Ask response now always carries next_action
Previously the cited-posts return path of POST /api/ask did not include next_action; it now always returns one (e.g. Enable public verification to check eligible public claims. when verify_external is false). The frontend renders answer.next_action unconditionally, so buyers will now always see a verification nudge alongside the existing 'Authorized cited posts are current...' message. This looks intentional per ADR 0106 but is a visible behavior change; confirm the double next-action messaging is desired UX.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| if post_id == lineage_anchor_id | ||
| else "keyword_match" | ||
| ), | ||
| external_claim_facts=external_facts, |
There was a problem hiding this comment.
📝 Info: public_external_claim_facts receives the full shared graph_facts list for every row
In gather_global_chat_sources, public_external_claim_facts(row, semantic_facts.get(post_id), graph_facts, public_post_ids) is called for each source with the same full graph_facts list (post_chat_ingestion.py), whereas the LLM-prompt graph_facts is only attached to index == 0. This means every public source's external_claim_facts can carry the same graph relations. It is not a correctness bug because public_claim_candidates de-duplicates by (kind, claim_text) and each graph fact remains bound to its own evidence-post ids (which must be a subset of public_post_ids), so private evidence still cannot egress. Just noting the intentional asymmetry between prompt graph facts and egress graph facts.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| def _verification_next_action( | ||
| status_code: str, | ||
| *, | ||
| has_authorized_sources: bool = True, | ||
| ) -> str: | ||
| """Give the Buyer a bounded action without treating web evidence as authority.""" | ||
|
|
||
| if not has_authorized_sources: | ||
| return "No authorized source posts are available for this question." | ||
| return { | ||
| VERIFICATION_SKIPPED: "Enable public verification to check eligible public claims.", | ||
| VERIFICATION_UNAVAILABLE: "Configure public search and contextual-orchestrator, then retry.", | ||
| VERIFICATION_NO_PUBLIC_CLAIMS: "Inspect the internal cited posts; no public claim was eligible.", | ||
| VERIFICATION_COMPLETED: "Inspect public evidence separately before any governed graph review.", | ||
| CLAIM_NOT_ENOUGH_INFORMATION: "Collect stronger authoritative evidence before accepting the claim.", | ||
| }.get(status_code, "Inspect the authorized cited posts and their evidence.") |
There was a problem hiding this comment.
📝 Info: CLAIM_NOT_ENOUGH_INFORMATION branch in next-action map is unreachable via verification_status
_verification_next_action maps CLAIM_NOT_ENOUGH_INFORMATION to a buyer message (main.py), but the status_code passed to this function is always one of the VERIFICATION_* constants (skipped/unavailable/no_public_claims/completed), never a per-claim claim_* status. The CLAIM_NOT_ENOUGH_INFORMATION entry is therefore dead in production and only exercised directly by unit tests. The ADR's buyer-next-action table distinguishes supported/refuted/not-enough-information, but the implementation collapses all completed states to the single COMPLETED message, so per-claim verdicts do not drive distinct next actions. Not a correctness bug, but the ADR intent is only partially realized.
Was this helpful? React with 👍 or 👎 to provide feedback.
| list(authorized_corporate_entity_ids), | ||
| candidate_ids, | ||
| limit, | ||
| not bool(question), | ||
| ) | ||
| visible_rows = [row for row in rows if can_see_post(row)][:limit] | ||
| candidate_id_set = frozenset(candidate_ids) | ||
| visible_rows = [ | ||
| row | ||
| for row in rows | ||
| if (not question or str(row["post_id"]) in candidate_id_set) and can_see_post(row) | ||
| ][:limit] |
There was a problem hiding this comment.
📝 Info: Lineage-neighbor expansion can crowd out semantic/lexical matches within the source limit
After ranking candidate_ids by score, the top match's direct lineage neighbors are inserted immediately after the anchor (post_chat_ingestion.py), and the final SQL orders by array_position($2, post_id) then limits to limit (default 4). When the anchor has several lineage neighbors, they occupy the top-limit window ahead of other strongly-scored semantic/lexical candidates, so those candidates may never become sources. This mirrors pre-existing behavior (the same expansion existed before this PR), so it is not introduced here, but the new semantic-nomination scoring makes the interaction more consequential since strong persisted-semantic matches can now be displaced by an anchor's lineage chain.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| export function askAgent( | ||
| accessToken: string, | ||
| question: string, | ||
| verifyExternalOrSessionId: boolean | string = false, | ||
| sessionId?: string, | ||
| ): Promise<AskAgentResponse> { | ||
| const verifyExternal = typeof verifyExternalOrSessionId === "boolean" | ||
| ? verifyExternalOrSessionId | ||
| : undefined; | ||
| const existingSessionId = typeof verifyExternalOrSessionId === "string" | ||
| ? verifyExternalOrSessionId | ||
| : sessionId; | ||
| return backendFetch("/api/ask", accessToken, { | ||
| method: "POST", | ||
| body: JSON.stringify({ question, ...(sessionId ? { session_id: sessionId } : {}) }), | ||
| body: JSON.stringify({ | ||
| question, | ||
| ...(verifyExternal !== undefined ? { verify_external: verifyExternal } : {}), | ||
| ...(existingSessionId ? { session_id: existingSessionId } : {}), | ||
| }), |
There was a problem hiding this comment.
📝 Info: askAgent overload preserves legacy 3-arg (sessionId) callers
The new askAgent signature (api.ts) accepts verifyExternalOrSessionId: boolean | string. A legacy call passing a string session id as the third argument is routed to existingSessionId and omits verify_external, while the sole app caller now passes a boolean plus the 4th sessionId. This preserves backward compatibility; verified there are no other callers passing a string third argument.
Was this helpful? React with 👍 or 👎 to provide feedback.
| authorized_ids = frozenset(str(post_id) for post_id in public_post_ids) | ||
| claims = tuple( | ||
| claim | ||
| for claim in public_claim_candidates(sources, question) | ||
| if set(claim.source_post_ids).issubset(authorized_ids) | ||
| ) | ||
| if not claims: | ||
| return VERIFICATION_NO_PUBLIC_CLAIMS, () | ||
| client = _claim_verification_client() | ||
| if not client.available: | ||
| return VERIFICATION_UNAVAILABLE, () | ||
| try: | ||
| results = tuple( | ||
| await asyncio.gather( | ||
| *(asyncio.to_thread(client.verify, claim) for claim in claims) | ||
| ) | ||
| ) | ||
| except (HttpClientError, KeyError, OSError, TypeError, ValueError): | ||
| return VERIFICATION_UNAVAILABLE, () | ||
| return VERIFICATION_COMPLETED, tuple( | ||
| result | ||
| for result in results | ||
| if set(result.source_post_ids).issubset(authorized_ids) | ||
| ) |
There was a problem hiding this comment.
📝 Info: Public-egress filter does not leak private/person data even with hallucinated cited ids
_verify_public_claims (main.py) sets authorized_ids from the caller-provided cited_ids and keeps only claims whose source_post_ids are a subset. Because public_external_claim_facts returns () for any non-public row (global_ask_retrieval.py), only public sources ever contribute external_claim_facts, and public_claim_candidates further drops person/Keyman/actor facts. Consequently, even if the LLM returns a hallucinated or private post id in cited_ids, no private-post data can enter a SearXNG query. The double subset check on returned results (main.py) is redundant but harmless.
Was this helpful? React with 👍 or 👎 to provide feedback.
| results = tuple( | ||
| await asyncio.gather( | ||
| *(asyncio.to_thread(client.verify, claim) for claim in claims) | ||
| ) |
There was a problem hiding this comment.
📝 Info: verify_external can extend /api/ask latency substantially when enabled
When verify_external=True and public claims are eligible, _verify_public_claims fans out up to maximum_claims (4) client.verify calls via asyncio.gather/to_thread (main.py). Each SearxngOrchestratedClaimVerificationClient.verify does a SearXNG GET (15s) plus an orchestrator adjudication POST with a 180s timeout (claim_verification.py). In the worst case a single opt-in Ask request can block for up to ~180s before responding. This is opt-in and bounded, but reviewers should confirm the request-level timeout/UX is acceptable.
Was this helpful? React with 👍 or 👎 to provide feedback.
* feat: search verified multilingual organization labels * fix: replay organization label indexes * fix: make organization label indexes reversible
d0ecc3d
into
feat/gnb-event-lineage-focus-keyman-v2190
| for post_id in semantic_candidate_ids: | ||
| candidate_scores[post_id] = candidate_scores.get(post_id, 0.0) + 4.0 |
There was a problem hiding this comment.
📝 Info: Semantic score outranks exact title match
Each semantic/KG candidate adds a flat +4.0, above the title weight of 3.0, so any persisted semantic match outranks an exact title match when selecting the lineage anchor and ordering sources. The inline comment only claims it outranks a weak body hit; the effect is broader.
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes #272
Buyer problem
Global Ask answers from authorized internal evidence and persisted Knowledge Graph / semantic evidence. This stack adds semantic/KG nomination without access escalation and keeps explicitly requested public corroboration separate from internal authority, citations, PII/Keyman facts, TEPP evidence, and fast-mlsirm measurement artifacts.
Current exact candidate
139cd24cbdc5282230ce0619617d113914efa7d6feat/gnb-event-lineage-focus-keyman-v2190@259b3b0d073eaa4c050ee5459a95ebb815a43f4fProduction boundaries
verify_externalis backward-compatible and defaults tofalse; SearXNG runs only after explicit opt-in and only cited public non-person claims may egress. External evidence remains separate from internal citations and never mutates Knowledge Graph or ontology authority.mode="auto", reasoning effort, schema validation, and provider boundary; this repository has no direct provider fallback.Exact-head local verification
796 passed, 16 skipped, 4 warnings.179 passed; lint, TypeScript production build, and Storybook build passed.98 passed, 5 skipped. Dangling images only were pruned to restore Docker capacity; no product data volume was deleted.git diff --checkand CodeGraph sync passed.Exact-head merge gate
Merge only when this exact head has terminal required Tests, PostgreSQL/integration, frontend, Storybook, coverage, SAST, Security, supply-chain, OpenCode/Noema/Strix organization gates, zero valid unresolved threads, and an independent qualifying formal approval. Queued, pending, skipped, cancelled, absent, predecessor-head, model-only, or author-only evidence is not success. Do not self-approve or bypass protection.
Research and standards
pg_trgmsupports indexed substring search; SearXNG remains bounded self-hosted retrieval.