Add parser ingestion run-history schema and PostgreSQL repository contract#644
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3adb989bd
ℹ️ 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".
| %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, | ||
| %s, %s, %s, %s, %s, %s, %s::jsonb, NOW(), NOW() | ||
| ) | ||
| ON CONFLICT (run_id, source_family, target_year) DO UPDATE SET |
There was a problem hiding this comment.
Make null-year source upserts hit an idempotent conflict target
When a source result has target_year=None (validation permits it and the schema declares the column nullable), PostgreSQL will not treat the nullable (run_id, source_family, target_year) unique key as a conflict for repeated inserts. Re-persisting the same null-year result therefore bypasses this ON CONFLICT target and collides with the deterministic primary key instead, causing the repository to return FAILED_DATABASE rather than being idempotent.
Useful? React with 👍 / 👎.
| record.code, | ||
| sanitized_message, |
There was a problem hiding this comment.
Include field name in the stable issue identity
If one command reports two validation issues with the same run/source/year/stage/code/message but different field_name values, both records generate the same parser_ingestion_issue_id; the second insert then hits ON CONFLICT ... DO NOTHING and is silently dropped. Since field_name is part of the issue record and persisted column, the deterministic identity needs to include it (or another differentiator) to avoid losing per-field issues.
Useful? React with 👍 / 👎.
Motivation
Description
ParserIngestionRunRecord,ParserIngestionSourceResultRecord,ParserIngestionIssueRecord,ParserIngestionRunHistoryCommand,ParserIngestionRunHistoryPersistResult,ParserIngestionRunHistoryIssue, and theParserIngestionRunHistoryRepositoryprotocol (src/carbonfactor_parser/persistence/ingestion_run_history.py).parser_ingestion_runsbyrun_id, upsertsparser_ingestion_source_resultsby the natural key(run_id, source_family, target_year), and inserts issues idempotently using deterministic stable UUIDs derived from sanitized issue identity (src/carbonfactor_parser/persistence/postgresql_ingestion_run_history_repository.py).parser_ingestion_runs,parser_ingestion_source_results, andparser_ingestion_issues, including defaults, foreign keys, unique constraint and indexes; extend DDL renderer to renderDEFAULTclauses and emit additiveIF NOT EXISTSstatements for idempotent bootstrap (src/carbonfactor_parser/persistence/postgresql_schema_catalog.py,postgresql_schema_ddl.py,postgresql_ddl_renderer.py, andpostgresql_runtime_schema_bootstrap.py).tests/test_ingestion_run_history_repository.py,tests/test_postgresql_phase1_schema_contract.py, updated DDL/planner/catalog tests and snapshot fixture).Testing
python -m pytest tests/test_ingestion_run_history_repository.py tests/test_postgresql_phase1_schema_contract.py— passed.python -m pytest tests/test_diagnostics_runtime_output.py tests/test_configured_cycle_runner.py tests/test_postgresql_source_family_repository.py— passed (one opt-in PostgreSQL integration test remains skipped unless opted-in).python -m pytest— passed with opt-in integration tests skipped (final run: 2122 passed, 8 skipped).git diff --checkas part of validation — no whitespace or check failures reported.Codex Task