Skip to content

Fix EDGAR ingest idempotency and watermarking - #1319

Merged
stranske merged 3 commits into
mainfrom
codex/issue-1298-edgar-idempotent-watermark
Jun 29, 2026
Merged

Fix EDGAR ingest idempotency and watermarking#1319
stranske merged 3 commits into
mainfrom
codex/issue-1298-edgar-idempotent-watermark

Conversation

@stranske

@stranske stranske commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Closes #1298

Summary

  • replace EDGAR holdings for an upserted filing before reinserting parsed rows
  • preserve generic ingest SQLite filing ids with conflict updates instead of INSERT OR REPLACE
  • resolve scheduled EDGAR runs from latest stored filed_date before falling back to 1970-01-01
  • lazy-load embeddings storage to avoid import-time torch crashes in ingest tests

Validation

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest -p pytest_asyncio.plugin tests/test_edgar_flow.py tests/test_ingest_flow.py -q
  • black --fast --check --line-length 100 --exclude '(\.workflows-lib|node_modules)' etl/edgar_flow.py etl/ingest_flow.py tests/test_edgar_flow.py tests/test_ingest_flow.py\n- deliberate-break: removing the legacy holdings replacement made tests/test_edgar_flow.py::test_fetch_and_store_idempotent_rerun fail with holding_count 2\n

Summary by CodeRabbit

  • Bug Fixes
    • Re-running ingestion now replaces existing holdings for the same filing using a single transactional delete-then-replace flow (prevents duplicates).
    • When no start date is provided, EDGAR ingestion now resumes from the latest stored filed date for the relevant CIK.
    • UK filings now clear prior holdings before saving refreshed holdings.
    • Improved resilience: document storage now safely degrades when optional embedding support isn’t available.
  • Tests
    • Extended coverage for EDGAR watermark behavior, holdings replacement transactions, and updated idempotency expectations.
    • Added assertions around safe persistence behavior and fetcher wiring.

@stranske stranske added agent:codex autofix Triggers autofix on PR agents:keepalive Enables keepalive automation for PR labels Jun 29, 2026
@stranske
stranske temporarily deployed to agent-standard June 29, 2026 02:10 — with GitHub Actions Inactive
@stranske stranske added the agent:retry Add to trigger agent retry after rate limit or pause label Jun 29, 2026
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ecee4335-5d7c-432b-926d-4ab024e877ec

📥 Commits

Reviewing files that changed from the base of the PR and between bad562c and b21a4b1.

📒 Files selected for processing (1)
  • tests/test_edgar_flow.py
📜 Recent review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Python CI / python 3.13
  • GitHub Check: Python CI / python 3.12
⚠️ CI failures not shown inline (4)

GitHub Actions: Health 45 Agents Guard / guard: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

GitHub Actions: Health 45 Agents Guard / 0_guard.txt: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

GitHub Actions: Claude Code Review (Opt-in) / Resolve review target: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

GitHub Actions: Claude Code Review (Opt-in) / 5_Resolve review target.txt: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Manager-Database repository uses Prefect 2.x - import schedules from prefect.client.schemas.schedules

Files:

  • tests/test_edgar_flow.py
🔇 Additional comments (1)
tests/test_edgar_flow.py (1)

452-452: LGTM!

Also applies to: 468-470


📝 Walkthrough

Walkthrough

EDGAR and generic ingest now replace existing holdings for a filing, SQLite filing inserts use ON CONFLICT ... RETURNING, and EDGAR fetches derive since from the latest stored filing date when it is omitted. store_document() also imports embeddings.store_document at runtime.

Changes

Idempotent EDGAR Ingest & Watermark

Layer / File(s) Summary
store_document runtime-import wrapper
etl/ingest_flow.py, etl/edgar_flow.py
Both files now wrap embeddings.store_document in a runtime import and return 0 if import resolution fails.
SQLite filing upsert: ON CONFLICT RETURNING
etl/ingest_flow.py
_insert_filing() now uses ON CONFLICT ... DO UPDATE ... RETURNING for SQLite filing identity handling.
Holdings replace helpers and call sites
etl/ingest_flow.py, etl/edgar_flow.py
Delete-before-insert helpers and transactional replacement paths are added for holdings tied to a filing_id.
CIK watermark resolution and fetch wiring
etl/edgar_flow.py
_latest_filed_date_for_cik() reads the latest stored filed_date, and edgar_flow() passes that value into fetch_and_store when since is missing.
Test coverage for replacement and watermark behavior
tests/test_edgar_flow.py, tests/test_ingest_flow.py
Tests cover transaction-backed holdings replacement, duplicate holdings removal, watermark lookup, and the updated fetcher wiring.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

  • #1298 – The changes implement idempotent holdings replacement and watermark-based since resolution for EDGAR ingest.
  • #1310 – The SQLite filing upsert change aligns with the filing identity stability issue described there.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: EDGAR ingest idempotency and watermarking.
Linked Issues check ✅ Passed The changes replace duplicate holdings, add a real EDGAR watermark, and cover both SQLite and Postgres paths as required by #1298.
Out of Scope Changes check ✅ Passed No clear out-of-scope code changes are evident; the extra lazy import is described as part of the PR's stated objectives.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-1298-edgar-idempotent-watermark

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Runner dispatch state for codex on PR #1319. Do not edit.

@stranske-keepalive

stranske-keepalive Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

🤖 Keepalive Loop Status

PR #1319 | Agent: Codex | Iteration 0/12

Current State

Metric Value
Iteration progress [----------] 0/12
Action run (agent-run-failed)
Agent status ❌ AGENT FAILED
Gate unknown
Tasks 0/6 complete
Timeout 45 min (default)
Timeout usage 5m elapsed (12%, 40m remaining)
Keepalive ✅ enabled
Autofix ❌ disabled

Last Codex Run

Result Value
Status ❌ AGENT FAILED
Reason agent-run-failed
Exit code 0
Failures 1/3 before pause

To retry immediately:

  • Add the agent:retry label to this PR

Or wait for the next successful Gate run to automatically retry.

Codex output:

Blocked by the execution environment before I could make or verify changes. What happened: - Every local shell command failed before execution with: bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted - Local apply_patch also failed with the same sandbox helper error. - I inspected file...

🔍 Failure Classification

| Error type | infrastructure |
| Error category | transient |
| Suggested recovery | Capture logs and context; retry once and escalate if the issue persists. |

🧠 Task Analysis

| Provider | ✅ GitHub Models (primary) |
| Confidence | 35% |

⚠️ Failure Tracking

| Consecutive failures | 1/3 |
| Reason | agent-run-failed |

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
etl/edgar_flow.py (1)

333-343: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make legacy holdings replacement transactional.

On Postgres, the delete at Line 333 can autocommit before the per-row inserts complete. A failure in _insert_holding_legacy then leaves the filing with deleted or partially replaced holdings. Wrap the delete and insert loop in one transaction.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@etl/edgar_flow.py` around lines 333 - 343, The legacy holdings replacement in
the filing flow is not atomic because `_delete_holdings_for_filing` and the
`_insert_holding_legacy` loop can commit separately. Update the logic in
`etl/edgar_flow.py` around the holdings replacement path to run the delete and
all inserts inside a single transaction on `conn`, so a failure in
`_insert_holding_legacy` rolls back the deletion and prevents partially replaced
holdings.
etl/ingest_flow.py (1)

448-458: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make holdings replacement atomic for Postgres.

connect_db opens Postgres with autocommit=True, so Line 448 can commit the delete before _insert_holdings_rows finishes. If parsing/casting/insertion fails mid-loop, the previous holdings are lost or partially replaced. Wrap delete + all inserts in one transaction for the Postgres path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@etl/ingest_flow.py` around lines 448 - 458, The holdings replace flow in the
insert path is not atomic for Postgres because `connect_db` uses autocommit, so
`_delete_holdings_rows` can commit before `_insert_holdings_rows` completes.
Update the holdings replacement logic in `ingest_flow.py` to run the delete and
all insert work in a single transaction for the Postgres path, ideally around
the code that calls `_delete_holdings_rows` and `_insert_holdings_rows`, so a
failure during parsing/casting/insertion rolls back the whole replacement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@etl/edgar_flow.py`:
- Around line 282-285: The EDGAR watermark query in the filings lookup is too
broad because it computes MAX(filed_date) across all sources for a manager,
which can be advanced by non-EDGAR rows. Update the query in the EDGAR flow’s
watermark logic to filter on the EDGAR source column when selecting the maximum
filed_date, using the existing EDGAR filing query path so only EDGAR rows affect
the watermark.

---

Outside diff comments:
In `@etl/edgar_flow.py`:
- Around line 333-343: The legacy holdings replacement in the filing flow is not
atomic because `_delete_holdings_for_filing` and the `_insert_holding_legacy`
loop can commit separately. Update the logic in `etl/edgar_flow.py` around the
holdings replacement path to run the delete and all inserts inside a single
transaction on `conn`, so a failure in `_insert_holding_legacy` rolls back the
deletion and prevents partially replaced holdings.

In `@etl/ingest_flow.py`:
- Around line 448-458: The holdings replace flow in the insert path is not
atomic for Postgres because `connect_db` uses autocommit, so
`_delete_holdings_rows` can commit before `_insert_holdings_rows` completes.
Update the holdings replacement logic in `ingest_flow.py` to run the delete and
all insert work in a single transaction for the Postgres path, ideally around
the code that calls `_delete_holdings_rows` and `_insert_holdings_rows`, so a
failure during parsing/casting/insertion rolls back the whole replacement.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7c5d0002-f5fe-4d39-92b7-e9087fd1fc05

📥 Commits

Reviewing files that changed from the base of the PR and between 9b210f6 and 7789f19.

📒 Files selected for processing (4)
  • etl/edgar_flow.py
  • etl/ingest_flow.py
  • tests/test_edgar_flow.py
  • tests/test_ingest_flow.py
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: Python CI / python 3.12
  • GitHub Check: Python CI / python 3.13
  • GitHub Check: Python CI / lint-ruff
⚠️ CI failures not shown inline (7)

GitHub Actions: Agents Verifier / 2_check.txt: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

GitHub Actions: Agents Verifier / check: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

GitHub Actions: Claude Code Review (Opt-in) / 5_Resolve review target.txt: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

GitHub Actions: Claude Code Review (Opt-in) / Resolve review target: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

GitHub Actions: Health 45 Agents Guard / 0_guard.txt: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

GitHub Actions: Health 45 Agents Guard / guard: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

Commit Status: Gate / gate: Gate / gate

Conclusion: failure

Path classification was cancelled
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Manager-Database repository uses Prefect 2.x - import schedules from prefect.client.schemas.schedules

Files:

  • tests/test_ingest_flow.py
  • etl/ingest_flow.py
  • tests/test_edgar_flow.py
  • etl/edgar_flow.py
+(adapters|api|etl|embeddings.py)/**

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Manager-Database repository has modules with type issues (adapters/, api/, etl/, embeddings.py) - use mypy overrides with ignore_errors for these

Files:

  • etl/ingest_flow.py
  • etl/edgar_flow.py
🪛 OpenGrep (1.23.0)
etl/ingest_flow.py

[ERROR] 388-388: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.

(coderabbit.sql-injection.python-fstring-execute)

etl/edgar_flow.py

[ERROR] 269-269: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.

(coderabbit.sql-injection.python-fstring-execute)


[ERROR] 282-285: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.

(coderabbit.sql-injection.python-fstring-execute)

🔇 Additional comments (9)
tests/test_edgar_flow.py (2)

118-120: LGTM!

Also applies to: 486-486


423-438: 🎯 Functional Correctness

No change needed _setup_relational_schema(...) already inserts the matching managers row (manager_id=100, cik="0"), so the watermark tests do exercise the real cik -> manager_id path.

			> Likely an incorrect or invalid review comment.
tests/test_ingest_flow.py (1)

125-161: LGTM!

Also applies to: 367-367

etl/ingest_flow.py (3)

20-38: LGTM!


383-388: LGTM!


264-290: 🎯 Functional Correctness

No action needed

etl/edgar_flow.py (3)

20-38: LGTM!


265-269: LGTM!


369-377: LGTM!

Comment thread etl/edgar_flow.py
@agents-workflows-bot

Copy link
Copy Markdown
Contributor
Keepalive Work Log (click to expand)
# Time (UTC) Agent Action Result Files Tasks Progress Commit Gate
0 2026-06-29 02:16:22 Codex run (agent-run-failed) failure 6 file(s) 0 0/6

@stranske

Copy link
Copy Markdown
Owner Author

Closer review fix pushed in bad562c.

Addressed CodeRabbit data-integrity findings:

  • EDGAR watermark now filters latest filed_date to source = "edgar" when the shared filings.source column exists, so non-EDGAR filings cannot advance the EDGAR fetch watermark.
  • EDGAR legacy holdings replacement now runs delete + all inserts inside conn.transaction() when the connection supports it.
  • Generic ingest holdings replacement now uses the same transaction wrapper for Postgres-style connections.

Validation:

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest -p pytest_asyncio.plugin tests/test_edgar_flow.py tests/test_ingest_flow.py -q -> 31 passed, 1 skipped
  • python -m ruff check etl/edgar_flow.py etl/ingest_flow.py tests/test_edgar_flow.py tests/test_ingest_flow.py -> passed
  • black --fast --check --line-length 100 --exclude '(\\.workflows-lib|node_modules)' etl/edgar_flow.py etl/ingest_flow.py tests/test_edgar_flow.py tests/test_ingest_flow.py -> passed
  • git diff --check -> passed

Fresh GitHub checks are pending after the push; next closer pass should re-check #1319 and merge/apply verify:compare if clean.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_edgar_flow.py`:
- Around line 450-471: The transaction test for _replace_holdings_for_filing()
is too weak because conn.holdings starts empty, so it won’t catch a broken
delete path. Seed a stale holding for filing_id=9001 before calling
flow._replace_holdings_for_filing(), and tighten the assertions to verify the
DELETE statement and its bound parameter(s) in StrictPostgresConnection. Keep
the existing checks on conn.transactions and final holdings so the test proves
replacement behavior through the Postgres path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c2deb4dd-7167-430d-aed5-34ff56fc31b9

📥 Commits

Reviewing files that changed from the base of the PR and between 7789f19 and bad562c.

📒 Files selected for processing (4)
  • etl/edgar_flow.py
  • etl/ingest_flow.py
  • tests/test_edgar_flow.py
  • tests/test_ingest_flow.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Python CI / python 3.12
  • GitHub Check: Python CI / python 3.13
⚠️ CI failures not shown inline (4)

GitHub Actions: Health 45 Agents Guard / 0_guard.txt: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

GitHub Actions: Health 45 Agents Guard / guard: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

GitHub Actions: Claude Code Review (Opt-in) / Resolve review target: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...

GitHub Actions: Claude Code Review (Opt-in) / 5_Resolve review target.txt: Fix EDGAR ingest idempotency and watermarking

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m�[0m
 �[36;1m# Use dedicated scripts dir, create if needed�[0m
 �[36;1mINSTALL_DIR=""�[0m
 �[36;1mif [ -z "$INSTALL_DIR" ]; then�[0m
 �[36;1m  INSTALL_DIR="$***GITHUB_WORKSPACE***/.github/scripts"�[0m
 �[36;1mfi�[0m
 �[36;1mmkdir -p "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mecho "📦 Installing `@octokit` dependencies in $INSTALL_DIR..."�[0m
 �[36;1mcd "$INSTALL_DIR"�[0m
 �[36;1m�[0m
 �[36;1mdeclare -a VENDORED_ALIAS_DIRS=()�[0m
 �[36;1m�[0m
 �[36;1mcleanup_vendor_aliases() ***�[0m
 �[36;1m  if [ "$***`#VENDORED_ALIAS_DIRS`[@]***" -eq 0 ]; then�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  local cleanup_dir="$INSTALL_DIR"�[0m
 �[36;1m  if [ -z "$cleanup_dir" ]; then�[0m
 �[36;1m    echo "::warning::Install dir is empty, skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  if ! pushd "$cleanup_dir" >/dev/null 2>&1; then�[0m
 �[36;1m    echo "::warning::Failed to enter install dir \"$cleanup_dir\"; skipping vendored alias cleanup"�[0m
 �[36;1m    return 0�[0m
 �[36;1m  fi�[0m
 �[36;1m�[0m
 �[36;1m  for vendored_alias in "$***VENDORED_ALIAS_DIRS[@]***"; do�[0m
 �[36;1m    if [ -z "$vendored_alias" ]; then�[0m
 �[36;1m      continue�[0m
 �[36;1m    fi�[0m
 �[36;1m    rm -rf -- "$vendored_alias" || true�[0m
 �[36;1m    local parent_dir�[0m
 �[36;1m    parent_dir=$(dirname "$vendored_alias")�[0m
 �[36;1m    # Remove empty parent directories that may have been created for scoped packages�[0m
 �[36;1m    while [ "$parent_dir" != "." ] && [ "$parent_dir" != "/" ]; do�[0m
 �[36;1m      rmdir -- "$parent_dir" 2>/dev/null || break�[0m
 �[36;1m      parent_dir=$(dirname "$parent_dir")�[0m
 �[36;1m    done�[0m
 �[36;1m  done�[0m
 �[36;1m�[0m
 �[36;1m  popd >/dev/null 2>&1 || true�[0m
 �[36;1m***�[0m
 �[36;1m�[0m
 �[36;1mtrap cleanup_vendor_aliases EXIT�[0m
 �[36;1m�[0m
 �[36;1mcreate_vendor_aliases() ***�[0m
 �[36;1m  if [ ! -f "package.json" ];...
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Manager-Database repository uses Prefect 2.x - import schedules from prefect.client.schemas.schedules

Files:

  • tests/test_ingest_flow.py
  • etl/ingest_flow.py
  • tests/test_edgar_flow.py
  • etl/edgar_flow.py
+(adapters|api|etl|embeddings.py)/**

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Manager-Database repository has modules with type issues (adapters/, api/, etl/, embeddings.py) - use mypy overrides with ignore_errors for these

Files:

  • etl/ingest_flow.py
  • etl/edgar_flow.py
🔇 Additional comments (5)
etl/ingest_flow.py (2)

19-38: LGTM!

Also applies to: 391-423, 483-483


264-290: 🎯 Functional Correctness

SQLite RETURNING needs a 3.35+ runtime
This path relies on INSERT ... ON CONFLICT ... DO UPDATE ... RETURNING, so the deployed SQLite version must be 3.35 or newer.

etl/edgar_flow.py (1)

19-38: LGTM!

Also applies to: 272-303, 316-323, 372-380, 406-408

tests/test_edgar_flow.py (1)

5-5: LGTM!

Also applies to: 97-127, 408-447

tests/test_ingest_flow.py (1)

3-3: LGTM!

Also applies to: 91-105, 271-296

Comment thread tests/test_edgar_flow.py
@stranske

Copy link
Copy Markdown
Owner Author

Closer review-thread fix pushed in b21a4b1.

Addressed the remaining CodeRabbit test-strength thread by seeding a stale holding for filing_id=9001 and asserting the exact Postgres DELETE statement plus bound parameter before checking replacement rows.

Validation:

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest -p pytest_asyncio.plugin tests/test_edgar_flow.py tests/test_ingest_flow.py -q -> 31 passed, 1 skipped
  • python -m ruff check etl/edgar_flow.py etl/ingest_flow.py tests/test_edgar_flow.py tests/test_ingest_flow.py -> passed
  • black --fast --check --line-length 100 --exclude '(\\.workflows-lib|node_modules)' etl/edgar_flow.py etl/ingest_flow.py tests/test_edgar_flow.py tests/test_ingest_flow.py -> passed\n- git diff --check -> passed\n\nFresh GitHub checks are pending after the push; next closer pass should re-check Fix EDGAR ingest idempotency and watermarking #1319 and merge/apply verify:compare if clean.

@stranske
stranske merged commit c7ec55f into main Jun 29, 2026
33 checks passed
@stranske stranske added the verify:compare Runs verifier comparison mode after merge label Jun 29, 2026
@stranske
stranske deleted the codex/issue-1298-edgar-idempotent-watermark branch June 29, 2026 04:23
@stranske
stranske temporarily deployed to agent-standard June 29, 2026 04:23 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown
Contributor

Provider Comparison Report

Provider Summary

Provider Model Verdict Confidence Summary
openai gpt-5.4 PASS 86% The merged changes appear to satisfy the documented acceptance criteria. In edgar_flow, holdings are now explicitly replaced for an existing filing via delete-then-reinsert logic wrapped in a trans...
anthropic claude-sonnet-4-6 PASS 78% The PR addresses all four stated tasks: holdings idempotency via delete-then-reinsert in a transaction, watermarking via _latest_filed_date_for_cik(), ingest_flow conflict updates, and lazy-load em...
📋 Full Provider Details (click to expand)

openai

  • Model: gpt-5.4
  • Verdict: PASS
  • Confidence: 86%
  • Scores:
    • Correctness: 8.0/10
    • Completeness: 9.0/10
    • Quality: 8.0/10
    • Testing: 9.0/10
    • Risks: 7.0/10
  • Summary: The merged changes appear to satisfy the documented acceptance criteria. In edgar_flow, holdings are now explicitly replaced for an existing filing via delete-then-reinsert logic wrapped in a transaction helper, addressing the duplicate-holdings/idempotency bug. The generic ingest path was also updated to preserve filing IDs on conflict instead of using INSERT OR REPLACE semantics, matching the requirement to keep SQLite filing identity stable. Watermarking was addressed by deriving the scheduled EDGAR since date from the latest stored filed_date for the CIK before falling back to 1970-01-01. The embeddings import was converted from import-time optional import to a lazy runtime load, which directly addresses test crashes from torch-related import failures. Tests were expanded in both test_edgar_flow.py and test_ingest_flow.py and, based on the diff summary and described additions, cover the new idempotency rerun behavior and watermark derivation expected by the issue/PR acceptance criteria. Overall this is a solid, targeted fix with only minor maintainability risk from broad exception handling in the embeddings shim.
  • Concerns:
    • The new lazy-loading wrapper for embeddings catches broad Exception rather than only import/runtime errors expected from optional torch dependencies, which may mask unrelated bugs during document storage.
    • The holdings replacement approach deletes and reinserts child rows, which is appropriate for idempotency but may drop row-level metadata if additional columns are later added outside the parsed source model.

anthropic

  • Model: claude-sonnet-4-6
  • Verdict: PASS
  • Confidence: 78%
  • Scores:
    • Correctness: 8.0/10
    • Completeness: 8.0/10
    • Quality: 7.0/10
    • Testing: 8.0/10
    • Risks: 6.0/10
  • Summary: The PR addresses all four stated tasks: holdings idempotency via delete-then-reinsert in a transaction, watermarking via _latest_filed_date_for_cik(), ingest_flow conflict updates, and lazy-load embeddings. The gate CI passed cleanly (8 success). The implementation logic is sound for the core idempotency fix. Minor concerns exist around transaction duck-typing reliability and per-call schema inspection overhead, but these don't break correctness for the primary use case. The diff is truncated so full test coverage cannot be confirmed, but the file change counts (+95 test lines for edgar_flow, +80 for ingest_flow) are consistent with adequate test additions including the required idempotency test.
  • Concerns:
    • ci.yml shows failure:1 in one job - while likely a non-test artifact job (UI screenshot), this warrants attention post-merge
    • _run_in_transaction uses duck-typing (getattr) which may silently skip transaction wrapping on adapters that don't expose a .transaction() method, potentially leaving partial deletes on failure
    • _delete_holdings_for_filing calls _columns() on every invocation which adds schema-inspection overhead per filing; should be cached
    • The truncated diff prevents full verification of ingest_flow.py changes and test implementations - cannot confirm all acceptance criteria are fully implemented from visible code alone
    • Lazy-load embeddings catches bare Exception which may mask real import errors unrelated to torch

Agreement

  • Verdict: PASS (all providers)
  • Correctness: scores within 1 point (avg 8.0/10, range 8.0-8.0)
  • Completeness: scores within 1 point (avg 8.5/10, range 8.0-9.0)
  • Quality: scores within 1 point (avg 7.5/10, range 7.0-8.0)
  • Testing: scores within 1 point (avg 8.5/10, range 8.0-9.0)
  • Risks: scores within 1 point (avg 6.5/10, range 6.0-7.0)

Disagreement

No major disagreements detected.

Unique Insights

  • openai: The new lazy-loading wrapper for embeddings catches broad Exception rather than only import/runtime errors expected from optional torch dependencies, which may mask unrelated bugs during document storage.; The holdings replacement approach deletes and reinserts child rows, which is appropriate for idempotency but may drop row-level metadata if additional columns are later added outside the parsed source model.
  • anthropic: ci.yml shows failure:1 in one job - while likely a non-test artifact job (UI screenshot), this warrants attention post-merge; _run_in_transaction uses duck-typing (getattr) which may silently skip transaction wrapping on adapters that don't expose a .transaction() method, potentially leaving partial deletes on failure; _delete_holdings_for_filing calls _columns() on every invocation which adds schema-inspection overhead per filing; should be cached; The truncated diff prevents full verification of ingest_flow.py changes and test implementations - cannot confirm all acceptance criteria are fully implemented from visible code alone; Lazy-load embeddings catches bare Exception which may mask real import errors unrelated to torch

🔍 LangSmith Traces

@github-actions

Copy link
Copy Markdown
Contributor

Workflow state fingerprint for Agents Verifier. Do not edit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:codex agent:retry Add to trigger agent retry after rate limit or pause agents:keepalive Enables keepalive automation for PR autofix Triggers autofix on PR verify:compare Runs verifier comparison mode after merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Audit] EDGAR nightly ingest re-runs from 1970 and appends duplicate holdings

1 participant