feat: disclose verified organization labels on Global Ask (v2.21.0) - #318
Conversation
Show search-corroborated raw-to-canonical organization labels on cited-post evidence after ABAC, and name opening Event Lineage as the next action. Pending and uncorroborated aliases stay excluded (ADR 0107).
|
Exact head 63ebb11. Independent exact-head APPROVE required before squash-merge. Copilot/model reviews are not qualifying approval. I will not self-approve. Local suite on this slice: 44 passed / 2 skipped (retrieval, sources, post_chat evidence, public-verification next-action) plus frontend AskAgentPanel / i18n / Ask cited-post next-action vitest. Pending and uncorroborated aliases remain fail-closed. |
|
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 |
|
Supersede earlier review requests and review exact current HEAD b50e51f. The failed Full test suite was reproduced locally; the test double now explicitly returns no corroborated organization-label rows for the post-ABAC lookup. Fresh evidence: Python 781 passed, 16 skipped; frontend lint, 168 tests, build, and Storybook build passed. Publish a formal review for this exact SHA. |
|
Review exact current HEAD 79b2bd3. The CI repair and its exact-head evidence note are now pushed; the code change remains limited to the integration test double accepting the post-ABAC matched_organization_label lookup. Please supersede prior requests and publish a formal review for this SHA. |
…abel-search' into HEAD
|
Remote work on this PR advanced the branch to exact current HEAD f025eb4. I rechecked the combined head locally: Python 781 passed, 16 skipped; frontend 169 passed, lint, build, and Storybook build passed. The earlier test-double repair is an ancestor, and the merge also carries the current stacked Global Ask/UI changes. Supersede all earlier review targets and publish a formal review for this exact SHA. |
…abel-search' into codex/fix-pr318-stable-entity-join # Conflicts: # docker/postgres-init/migrate.sh # tests/test_migration_replay.py
|
Review exact current HEAD |
|
Exact current HEAD is now |
|
Updated and reviewed against exact current head
Formal independent approval and terminal required Checks are still pending; no bypass used. |
…abel-search' into repair/pr318-current-parent-2
…rganization-label-search' into repair/pr318-current-parent-2
|
Exact head 69b6052 now includes the organization-evidence test lint correction. Revalidated 44 focused Global Ask/organization/tie-safety tests, targeted Ruff, and diff-check; raw provider error paths remain stable through the existing generic boundary. Hosted checks and formal approval remain pending; no merge attempted. |
There was a problem hiding this comment.
📝 Info: next_action override coexists with the verify_external skip status
_ask_next_action (main.py) overrides the verification-derived next action with the verified-label message whenever any cited post carries a verified_organization_label fact, regardless of external_verification_status. This means even with verify_external=False (status SKIPPED), a corroborated label match will surface the Event-Lineage next action rather than 'Enable public verification...'. This appears intentional per ADR 0107 ('Public-verification next actions remain for answers that have no such label'), but worth confirming that suppressing the verification-enable hint when a label is present is the desired precedence.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| if corporate_entity_id is not None: | ||
| await link_verified_organization_entity( | ||
| conn, | ||
| organization_name, | ||
| post_body, | ||
| corporate_entity_id, | ||
| ) |
There was a problem hiding this comment.
📝 Info: link_verified_organization_entity is a no-op for non-corroborated resolutions
link_verified_organization_entity (organization_name_resolution_ingestion.py) is called from the Keyman loop whenever corporate_entity_id is not None (keyman_ingestion.py). get_or_create_corporate_entity can return a non-null id even when the organization name was never corroborated (a plain similarity match on an unchanged raw name), so the UPDATE runs with a WHERE clause requiring verification_status_code = 'verify_corroborated' and simply matches zero rows. This is harmless (no wrong link is created) but issues an extra query per uncorroborated affiliation. Worth noting only if ingestion throughput matters; not a correctness issue.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if corporate_entity_id is not None: | ||
| await link_verified_organization_entity( | ||
| conn, | ||
| organization_name, | ||
| post_body, | ||
| corporate_entity_id, | ||
| ) |
There was a problem hiding this comment.
🔍 Verified-label nomination now requires the ingestion-time entity link
The verified_organization CTE join changed from a display-name match (entity.entity_name = resolution.resolved_organization_name) to a stable-identity match (resolution.resolved_corporate_entity_id = entity.corporate_entity_id) in both global_ask_retrieval.py (nomination) and global_ask_retrieval.py (label disclosure). resolved_corporate_entity_id is only ever populated by link_verified_organization_entity, which is wired solely into keyman_ingestion.py (the Keyman affiliation loop). Consequently: (1) historical organization_name_resolution rows with a NULL entity id no longer nominate or disclose anything (documented/intended per ADR 0122), and (2) R&R organization actors resolved in backend/app/post_summary_ingestion.py are NOT linked, so a post_organization_mention whose organization was resolved only through the R&R path will not surface a verified label unless the same entity was independently linked via the Keyman path. ADR 0107 lists "mentioned on ... those visible posts" as a supported path; verify whether omitting the link call from the R&R ingestion path is intentional scope or a coverage gap.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ), matched_organization_label as ( | ||
| select distinct | ||
| mention.post_id, | ||
| organization.raw_organization_name, | ||
| organization.resolved_organization_name | ||
| from post_organization_mention mention | ||
| join verified_organization organization | ||
| on organization.corporate_entity_id = mention.corporate_entity_id | ||
| join nominated_post nominated | ||
| on nominated.post_id = mention.post_id | ||
| union | ||
| select distinct | ||
| mention.post_id, | ||
| organization.raw_organization_name, | ||
| organization.resolved_organization_name | ||
| from post_person_mention mention | ||
| join person_affiliation affiliation | ||
| on affiliation.person_id = mention.person_id | ||
| join verified_organization organization | ||
| on organization.corporate_entity_id = affiliation.affiliated_corporate_entity_id | ||
| join nominated_post nominated | ||
| on nominated.post_id = mention.post_id |
There was a problem hiding this comment.
📝 Info: Label disclosure is context-agnostic once a resolution is entity-linked
link_verified_organization_entity (organization_name_resolution_ingestion.py) links a resolution keyed by raw-name + context digest to a stable entity id, but verified_organization_label_facts then discloses that alias for ANY ABAC-visible post mentioning the linked corporate_entity (or affiliating a mentioned person to it) whose raw/resolved label matches a query term — it does not re-check the post's own context digest. This is intentional per ADR 0107/0122 (disclosure joins by catalog identity, not per-post context), so a homonym-safe entity id gates it; noting it because it means a corroboration established in one post's context surfaces the alias across other posts about the same entity.
Was this helpful? React with 👍 or 👎 to provide feedback.
| graph_facts=graph_facts if index == 0 else (), | ||
| evidence_facts=_source_hint_facts(row) | ||
| + semantic_facts.get(post_id, ()) | ||
| + label_facts.get(post_id, ()) |
There was a problem hiding this comment.
📝 Info: Label facts kept out of external claims but fed to the LLM prompt
label_facts are concatenated into evidence_facts at post_chat_ingestion.py, which means they appear both in the buyer-facing cited_post_evidence and in the LLM source block (evidence_facts is rendered into the prompt). Critically, external_claim_facts is computed separately from semantic_facts only (post_chat_ingestion.py), and the label prefix 'verified organization label:' does not start with 'project:', so label facts are correctly excluded from public_external_claim_facts. This matches ADR 0107's requirement that the label is internal SKOS evidence, not a public-search claim. Verified as correct, not a bug.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if fact.startswith("verified organization label:"): | ||
| return "verified_organization_label" |
There was a problem hiding this comment.
📝 Info: Label prefix string is duplicated between modules
_buyer_evidence_kind in post_chat.py hardcodes the literal "verified organization label:" to classify a fact as verified_organization_label, while global_ask_retrieval.py defines the same value as VERIFIED_ORGANIZATION_LABEL_PREFIX and uses it to build the fact. The two strings currently match, so classification works, but they are not linked — changing the constant would silently break kind classification (and therefore the _ask_next_action / frontend next-action logic that keys off kind == "verified_organization_label"). A shared import would remove this coupling risk.
Was this helpful? React with 👍 or 👎 to provide feedback.
| async def link_verified_organization_entity( | ||
| conn: asyncpg.Connection, | ||
| raw_name: str, | ||
| context_text: str, | ||
| corporate_entity_id: str, | ||
| ) -> None: | ||
| """Attach a corroborated resolution to its stable catalog entity id. | ||
|
|
||
| Display names are not identity keys: two catalog entities may legitimately | ||
| share one name. Only the exact raw-name/context cache row is linked, and | ||
| only after external verification has corroborated the resolution. | ||
| """ | ||
| await conn.execute( | ||
| """ | ||
| update organization_name_resolution | ||
| set resolved_corporate_entity_id = $1, | ||
| resolved_at = now() | ||
| where raw_organization_name = $2 | ||
| and context_sha256 = $3 | ||
| and verification_status_code = 'verify_corroborated' | ||
| """, | ||
| corporate_entity_id, | ||
| raw_name, | ||
| _context_sha256(context_text), | ||
| ) |
There was a problem hiding this comment.
📝 Info: Resolution row committed before the transaction, linked inside it
In keyman_ingestion.py, _resolve_affiliated_organization (which calls resolve_organization_name, potentially INSERTing the organization_name_resolution cache row) runs in the pre-transaction enrichment loop at keyman_ingestion.py, so that row auto-commits. link_verified_organization_entity then runs inside the async with conn.transaction() block at line 294-300 and updates that already-committed row. The cache key is consistent on both sides (raw name = organization_name, context_sha256 = sha256(post_body)), and the verification_status_code = 'verify_corroborated' guard means an uncorroborated resolution is never linked even though get_or_create_corporate_entity may still return a non-null entity id. If the Keyman write transaction rolls back, the resolution row survives with a NULL entity id while its intended link is discarded — a benign transient state that self-heals on the next re-ingestion. No correctness issue found.
Was this helpful? React with 👍 or 👎 to provide feedback.
| <> | ||
| <p className="board-next-action" role="status" aria-label={t("Next action")}> | ||
| {t("Authorized cited posts are current. Open a cited post to read Event Lineage.")} | ||
| {t(askCitedPostsNextAction(answer))} |
There was a problem hiding this comment.
📝 Info: Corroborated label next action is rendered in two places for label answers
For a cited-posts answer, App.tsx renders answer.next_action (post-meta) and App.tsx renders askCitedPostsNextAction(answer) (board-next-action). When a verified organization label matched, the backend _ask_next_action sets answer.next_action to the corroborated-label sentence, and askCitedPostsNextAction returns answer.next_action || fallback — i.e. the same sentence. The buyer therefore sees the identical 'Corroborated organization labels are current. Open a cited post to read Event Lineage.' string twice. For non-label answers the two lines differ (verification action vs. the static 'Authorized cited posts are current...'), so this duplication is new behavior for the label case. The dual-render structure itself is pre-existing; this is a minor cosmetic redundancy, not a correctness bug.
Was this helpful? React with 👍 or 👎 to provide feedback.
| cur.execute(_SUMMARY_FIVE_W1H_MIGRATION.read_text()) | ||
| cur.execute(_POST_CONTENT_QUEUE_MIGRATION.read_text()) | ||
| cur.execute(_ORGANIZATION_CONTEXT_MIGRATION.read_text()) | ||
| cur.execute(_ORGANIZATION_ENTITY_ID_MIGRATION.read_text()) |
There was a problem hiding this comment.
📝 Info: 0103 applied out of numeric order in the API integration fixture
In test_api.py the identity migration 0103 is executed between 0051 and 0052. This is functionally fine because 0103 only depends on organization_name_resolution (present after 0051) and corporate_entity, but it diverges from both the natural glob order used by migrate.sh and the ADR 0122 statement about replaying 0055_* then 0103_*. If a future migration between 0052 and 0103 becomes a dependency, this hand-ordered fixture could mask an ordering problem that production would hit.
Was this helpful? React with 👍 or 👎 to provide feedback.
| async def verified_organization_label_facts( | ||
| conn: asyncpg.Connection, | ||
| question: str | None, | ||
| post_ids: list[str], | ||
| *, | ||
| maximum_terms: int = 8, | ||
| ) -> dict[str, tuple[str, ...]]: | ||
| """Disclose corroborated raw→canonical labels for already-visible posts. | ||
|
|
||
| Pending and uncorroborated aliases never appear. Nomination remains | ||
| identifier-only; this query runs after ABAC-visible post ids are known. | ||
| """ | ||
|
|
||
| if not post_ids: | ||
| return {} | ||
| terms = global_ask_query_terms(question, maximum_terms=maximum_terms) | ||
| if not terms: | ||
| return {} | ||
| rows = await conn.fetch( | ||
| """ | ||
| with query_terms as ( | ||
| select unnest($1::text[]) as term | ||
| ), nominated_post as ( | ||
| select unnest($2::uuid[]) as post_id | ||
| ), verified_organization as ( | ||
| select distinct | ||
| entity.corporate_entity_id, | ||
| resolution.raw_organization_name, | ||
| resolution.resolved_organization_name | ||
| from organization_name_resolution resolution | ||
| join corporate_entity entity | ||
| on resolution.resolved_corporate_entity_id = entity.corporate_entity_id | ||
| join query_terms term | ||
| on resolution.raw_organization_name ilike '%' || term.term || '%' | ||
| or resolution.resolved_organization_name ilike '%' || term.term || '%' | ||
| where resolution.verification_status_code = 'verify_corroborated' | ||
| ), matched_organization_label as ( | ||
| select distinct | ||
| mention.post_id, | ||
| organization.raw_organization_name, | ||
| organization.resolved_organization_name | ||
| from post_organization_mention mention | ||
| join verified_organization organization | ||
| on organization.corporate_entity_id = mention.corporate_entity_id | ||
| join nominated_post nominated | ||
| on nominated.post_id = mention.post_id | ||
| union | ||
| select distinct | ||
| mention.post_id, | ||
| organization.raw_organization_name, | ||
| organization.resolved_organization_name | ||
| from post_person_mention mention | ||
| join person_affiliation affiliation | ||
| on affiliation.person_id = mention.person_id | ||
| join verified_organization organization | ||
| on organization.corporate_entity_id = affiliation.affiliated_corporate_entity_id | ||
| join nominated_post nominated | ||
| on nominated.post_id = mention.post_id | ||
| ) | ||
| select post_id::text as post_id, | ||
| raw_organization_name, | ||
| resolved_organization_name | ||
| from matched_organization_label | ||
| order by post_id, raw_organization_name, resolved_organization_name | ||
| """, | ||
| list(terms), | ||
| list(post_ids), | ||
| ) | ||
| facts: dict[str, list[str]] = {} | ||
| for row in rows: | ||
| facts.setdefault(str(row["post_id"]), []).append( | ||
| verified_organization_label_fact( | ||
| row["raw_organization_name"], | ||
| row["resolved_organization_name"], | ||
| ) | ||
| ) | ||
| return {post_id: tuple(dict.fromkeys(values)) for post_id, values in facts.items()} |
There was a problem hiding this comment.
📝 Info: Disclosure query recomputes terms independently of nomination
verified_organization_label_facts re-derives query terms via global_ask_query_terms(question, maximum_terms=8) (global_ask_retrieval.py), the same default used by semantic_candidate_post_ids's global_ask_query_terms(question). Because both cap at 8 terms and use the same stopword/token rules, the disclosed labels stay consistent with what nomination matched. Disclosure runs strictly on the ABAC-filtered visible_ids, and pending/uncorroborated aliases are excluded by the verify_corroborated filter — matching ADR 0107's 'disclosure cannot precede ABAC' constraint. No divergence found.
Was this helpful? React with 👍 or 👎 to provide feedback.
b5d36fe
into
feat/multilingual-organization-label-search
Buyer outcome
Global Ask discloses search-corroborated raw-to-canonical organization labels on cited-post evidence after ABAC. Pending and uncorroborated aliases remain excluded from nomination and disclosure; a corroborated label keeps opening the cited post to read Event Lineage as the next action.
Integrity repair
resolved_corporate_entity_idand join the catalog by foreign key, preventing same-named organizations from cross-matching through display labels.Stack and verification
1d4e65707abbdea2d0d131cbc03742fe9cfb8ab0; restacked with a normal merge after the parent advanced.8e67050b4cf6ea0a205f7cbafb5492f0a6c164fe.actionlintandgit diff --checkpassed.Independent exact-head approval and terminal Checks remain required before merge.