fix: fail closed on generic political entity matches - #21
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review🚫 Merge blocked: 2 serious issue(s) found in high-risk files ⚖️ Codex Review Arbitration🚫 block: The current source of truth disproves the event-study finding but supports the source-mention finding. In 🚫 Blocking IssuesThese issues must be fixed before this PR can be merged: 1. 🟠 [HIGH] Logic in
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e984c087a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if event.entity_match_type not in {"issuer", "direct_beneficiary"}: | ||
| continue |
There was a problem hiding this comment.
This guard only fail-closes priority_score; the same symbol_events are still passed to trigger_status, latest_event, and event_count in build_tracker_rows. In the RSS/source pipelines that write source_tracker.csv, an industry_context row such as a generic CHIPS/cybersecurity/tokenization mention still marks a watchlist item as policy_triggered or mentioned, even though the new evidence columns say it is not company-level evidence.
Useful? React with 👍 / 👎.
| if _is_generic_alias(alias_record, alias): | ||
| return "industry_context", normalize_match_text(alias) |
There was a problem hiding this comment.
When an alias CSV lists an industry term before the issuer name or ticker, this early return classifies the whole item as industry_context even if the same text also contains a company-level alias later in the tuple, e.g. aliases tokenization|Coinbase|COIN with text mentioning both Coinbase and tokenization. Since alias order is operator-controlled input, match_evidence should scan all matches and prefer issuer/direct_beneficiary over generic evidence rather than returning on the first generic hit.
Useful? React with 👍 / 👎.
| if alias_record.name: | ||
| return True |
There was a problem hiding this comment.
When an alias row has a name, this branch treats every other non-ticker alias as industry_context, including explicit issuer aliases in the checked config such as MicroStrategy for MSTR and General Electric for GE. A source item that mentions only one of those company identifiers is therefore retained but never scored as company-level evidence downstream, even though it is a direct issuer match rather than generic industry vocabulary.
Useful? React with 👍 / 👎.
| confidence: str | ||
| source_url: str | ||
| notes: str | ||
| entity_match_type: str = "unverified" |
There was a problem hiding this comment.
Adding the evidence type here does not stop run_event_study from processing context-only rows; compute_event_returns still iterates every loaded event. Since the source workflows copy extractor output, which now retains industry_context rows, into political_events.csv, running the event-study script on that output will produce returns for generic CHIPS/cybersecurity/tokenization hits that are explicitly not company-level evidence.
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 953660f6b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "crypto assets", | ||
| "cybersecurity", | ||
| "digital assets", | ||
| "energy-related infrastructure", |
There was a problem hiding this comment.
The checked alias config includes the unhyphenated energy related infrastructure variant for CVX/NEE/VRT/XOM, but this denylist only contains the hyphenated spelling and _is_generic_alias does not collapse spaces versus hyphens. In RSS/source pipeline runs where an item uses the unhyphenated phrase, match_evidence classifies all four symbols as issuer, so the new tracker/event-study filters still score a generic industry mention as company-level evidence; add the unhyphenated variant or canonicalize punctuation/spacing before the denylist lookup.
Useful? React with 👍 / 👎.
| matches.append((relationship, normalize_match_text(alias))) | ||
| if not matches: | ||
| return None | ||
| return max(matches, key=lambda match: (RELATIONSHIP_PRIORITY[match[0]], match[1].casefold())) |
There was a problem hiding this comment.
When a source item contains only the generic phrase Bitcoin treasury, the loop records that alias as industry_context but also matches the shorter Bitcoin alias in the same span; this max then chooses the higher-priority issuer match. Because the core MSTR aliases include both phrases, generic Bitcoin-treasury policy/news items still pass the new company-level filters and get scored as MSTR issuer events.
Useful? React with 👍 / 👎.
| "cybersecurity", | ||
| "digital assets", | ||
| "energy-related infrastructure", | ||
| "foundry", |
There was a problem hiding this comment.
The core alias config uses Foundry as a PLTR product alias, but this global denylist labels every foundry match as industry_context. For articles or official records that mention Palantir's Foundry product without also saying Palantir, the extractor emits only a context row for PLTR and the tracker/event-study filters discard it; the denylist needs symbol-specific handling or an exemption for product aliases.
Useful? React with 👍 / 👎.
| "crypto assets", | ||
| "cybersecurity", | ||
| "digital assets", |
There was a problem hiding this comment.
The core COIN aliases include crypto exchange and crypto asset transactions, but only crypto assets/digital assets are denied here. A regulatory or RSS item that says only crypto exchanges or crypto asset transactions therefore emits an issuer relationship for Coinbase and passes the new tracker/event-study company filters, even though it is the same kind of industry vocabulary this change is trying to retain only for auditability.
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8fa16dbfa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def _is_generic_alias(alias_record: MentionAlias, alias: str) -> bool: | ||
| normalized = normalize_match_text(alias).strip() | ||
| if normalized.upper() == alias_record.symbol.upper(): |
There was a problem hiding this comment.
SMR is both a shipped symbol alias (config/core_us_equity_aliases.csv:35) and a common abbreviation for small modular reactor. With this blanket symbol exemption, a government-policy item that says only SMR and not NuScale is classified as issuer, so it passes the new verified-event filters and gets scored/studied as NuScale company evidence; add symbol-specific ambiguity handling, such as requiring a cashtag/company name or classifying this acronym as context, before returning false here.
Useful? React with 👍 / 👎.
| normalized = normalize_match_text(alias).strip() | ||
| if normalized.upper() == alias_record.symbol.upper(): | ||
| return False | ||
| return normalized.casefold() in GENERIC_ALIAS_DENYLIST |
There was a problem hiding this comment.
The shipped INTC aliases include advanced packaging (config/core_us_equity_aliases.csv:16), and the checked live NIST NAPMP item is about the National Advanced Packaging Manufacturing Program without naming Intel (data/live/source_items.csv:25). Since this denylist membership is the only generic check, that row extracts as relationship_type=issuer and build_tracker scores INTC as policy_triggered; add this industry phrase, or normalize all config context aliases, so generic CHIPS R&D items stay audit-only.
Useful? React with 👍 / 👎.
| `source_events.csv` keeps the original columns and adds the following compatible | ||
| evidence columns: `entity_match_type`, `match_evidence`, and | ||
| `relationship_type`. Values are `issuer`, `direct_beneficiary`, |
There was a problem hiding this comment.
This new runbook says source_events.csv has the evidence columns, but the committed data/live/source_events.csv and data/live/political_events.csv in this same commit still have the legacy header and data/live/source_tracker.csv still reflects the pre-filter generic scores. In a fresh checkout, the new loader defaults those live events to unverified, so recomputing tracker/event-study drops all existing live events, while users reading the checked tracker see stale triggers; regenerate or migrate the live artifacts with the schema change.
Useful? React with 👍 / 👎.
|
Closing this accumulated remediation branch after repeated semantic review findings and contract arbitration. The P0 work will be re-sliced from current main into smaller vertical PRs with isolated contracts and tests. No code from this branch is being merged as-is. |
1 participant
Summary
issuer,direct_beneficiary, orindustry_contextwith retained evidenceStrategyfrom becoming issuer evidenceunverifiedand exclude non-company relations from tracker scoringValidation
PYTHONPATH=src /Users/lisiyi/Projects/PoliticalEventTrackingResearch/.venv/bin/python -m pytest -qgit diff --checkNo execution, allocation, broker, or production dependency changes.