Skip to content

feat: add official event entity schema foundation - #23

Closed
Pigbibi wants to merge 2 commits into
mainfrom
codex/p0-entity-schema-foundation-v2
Closed

feat: add official event entity schema foundation#23
Pigbibi wants to merge 2 commits into
mainfrom
codex/p0-entity-schema-foundation-v2

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add entity_match_type, match_evidence, and relationship_type to OfficialRecord
  • default legacy CSV inputs to unverified/empty values
  • serialize the three fields with stable normalized CSV columns

Validation

  • focused tests: 11 passed
  • full tests: 26 passed
  • ruff: passed
  • git diff --check: passed

Scope is limited to official_event_import and its direct unit tests. No merge requested.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

🤖 Codex PR Review

🚫 Merge blocked: 2 serious issue(s) found in high-risk files

⚖️ Codex Review Arbitration

🚫 block: The current diff still proves at least one blocking regression. In src/political_event_tracking_research/official_event_import.py, normalize_records() now always adds entity_match_type, match_evidence, and relationship_type, and import_official_events() always returns those expanded row dicts even when include_entity_metadata is left at its default False; only CSV header selection is gated. The prior/base behavior returned the legacy 8-field rows, so this remains a real API compatibility break for Python callers rather than a wording-only disagreement with the prior finding. The new test test_legacy_input_defaults_to_legacy_eight_column_serialization even codifies that returned rows still contain the three metadata keys under the default path, so the issue is not fixed. The second finding also remains supported by the code: load_official_records() uses row.get("entity_match_type") or "unverified", which converts an explicitly empty new-schema value into "unverified" before validation, even though the contract elsewhere only recognizes the enumerated match types and treats legacy absence as the defaulting case. No current source-of-truth evidence shows that silently accepting an empty provided value is intended. The current findings do not conflict with the prior one; the first current finding narrows the same compatibility requirement to the returned-Python-data path rather than reversing it.

🚫 Blocking Issues

These issues must be fixed before this PR can be merged:

1. 🟠 [HIGH] Logic in src/political_event_tracking_research/official_event_import.py

normalize_records() now always injects entity_match_type, match_evidence, and relationship_type into every returned row, and import_official_events() returns those expanded dicts even when include_entity_metadata=False. That changes the function's effective API for existing Python callers that relied on the legacy 8-column event schema, despite the opt-in flag only gating CSV serialization. (line 155)

Suggestion: When include_entity_metadata is false, strip the three new keys before returning rows, or add a separate code path so both the serialized CSV and the returned data preserve the legacy 8-field contract unless the caller explicitly opts in.

2. 🟠 [HIGH] Bug in src/political_event_tracking_research/official_event_import.py

load_official_records() uses row.get("entity_match_type") or "unverified", so any present-but-empty entity_match_type value is silently rewritten to unverified. That masks malformed new-schema inputs and can emit records that look intentionally unverified instead of failing validation, which is a data corruption risk once producers start sending the new columns. (line 102)

Suggestion: Only default to unverified when the column is absent (legacy input). If the column exists but is empty or whitespace, raise a validation error so broken new-schema rows are not silently normalized into a different meaning.


Review by Codex PR Review bot • PR

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Superseded after the single allowed remediation. Re-slicing from latest main into an OfficialRecord input-schema/validation-only contract; normalization, returned-row shape, CSV serialization, CLI, and consumers are explicitly deferred. No further changes will be made on this PR.

@Pigbibi Pigbibi closed this Jul 13, 2026
@Pigbibi
Pigbibi deleted the codex/p0-entity-schema-foundation-v2 branch July 13, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant