chore(warehouse-sources): extract DeltaWriter from DeltaTableHelper - #76084
chore(warehouse-sources): extract DeltaWriter from DeltaTableHelper#76084estefaniarabadan wants to merge 1 commit into
Conversation
Phase 3 (PR 3) of the warehouse-sources untangling, after #75978 and #76067: the core write/merge path and commit-metadata idempotency move to core/delta/writer.py, leaving DeltaTableHelper as a pure table access and lifecycle object (~270 lines: cached open, corruption detection, reset, file listing, first-sync state). DeltaWriter is a stateless wrapper over the helper, mirroring DeltaMaintenance and Scd2DeltaWriter: write_to_deltalake becomes DeltaWriter.write, together with the deltalite canary (moved verbatim, mid-rollout), batch dedupe, schema-mismatch fallback, and the idempotency pair has_commit_with_metadata / has_batch_been_committed. Tag-write and tag-read stay together deliberately: only the terminal commit of a multi-commit write may carry the (run_uuid, batch_index) tag, and that invariant spans both halves. _commit_matches becomes the module-level commit_matches, shared with the SCD2 tagging test. Callers updated: the v2 pipeline chunk write, the v3 load processor, the CDC companion seeding in common/load.py, and the redelivery fallback in load/idempotency.py (which now wraps the helper in a DeltaWriter). Debug log prefixes change from 'write_to_deltalake:' to 'write:' with the rename; no behavior changes otherwise.
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Hey @estefaniarabadan! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from each directory's |
|
Reviews (1): Last reviewed commit: "chore(warehouse-sources): extract DeltaW..." | Re-trigger Greptile |
|
Note 🤖 stamphog reviewed Gates denied on size for a cross-team refactor of the core Delta write/merge and idempotency path in warehouse ingestion — risky territory with zero reviews and only moderate author familiarity, which isn't independent assurance here.
Gate mechanics and policy version
|
🤖 CI report
|
| File | Patch | Uncovered changed lines |
|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/pipelines/common/load.py |
0.0% | 171, 246 |
products/warehouse_sources/backend/temporal/data_imports/pipelines/core/delta/writer.py |
93.8% | 85–86, 113, 138, 222–223, 303, 388, 448–450, 452 |
🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 30659927718 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
batch_exports |
████████░░░░░░░░░░░░ 39.4% |
8,789 / 22,299 |
demo |
███████████░░░░░░░░░ 56.3% |
1,497 / 2,661 |
warehouse_sources_queue |
████████████░░░░░░░░ 59.2% |
148 / 250 |
data_tools |
██████████████░░░░░░ 70.0% |
63 / 90 |
tasks |
██████████████░░░░░░ 70.3% |
33,203 / 47,255 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
signals |
████████████████░░░░ 81.5% |
25,295 / 31,055 |
cdp |
████████████████░░░░ 82.1% |
3,285 / 3,999 |
data_modeling |
█████████████████░░░ 85.7% |
7,716 / 9,008 |
notebooks |
█████████████████░░░ 86.0% |
7,794 / 9,060 |
wizard |
█████████████████░░░ 86.4% |
1,060 / 1,227 |
actions |
█████████████████░░░ 86.6% |
717 / 828 |
cohorts |
█████████████████░░░ 87.5% |
6,393 / 7,309 |
exports |
██████████████████░░ 87.9% |
7,078 / 8,054 |
product_tours |
██████████████████░░ 87.9% |
1,303 / 1,482 |
data_warehouse |
██████████████████░░ 88.4% |
12,264 / 13,870 |
business_knowledge |
██████████████████░░ 89.0% |
4,391 / 4,936 |
engineering_analytics |
██████████████████░░ 89.3% |
6,529 / 7,309 |
dashboards |
██████████████████░░ 89.4% |
5,983 / 6,693 |
conversations |
██████████████████░░ 89.5% |
17,667 / 19,746 |
visual_review |
██████████████████░░ 89.5% |
5,870 / 6,558 |
alerts |
██████████████████░░ 90.3% |
4,482 / 4,966 |
links |
██████████████████░░ 90.6% |
183 / 202 |
streamlit_apps |
██████████████████░░ 90.7% |
2,630 / 2,901 |
mcp_analytics |
██████████████████░░ 90.9% |
3,191 / 3,511 |
error_tracking |
██████████████████░░ 91.0% |
10,928 / 12,010 |
slack_app |
██████████████████░░ 91.1% |
9,664 / 10,610 |
marketing_analytics |
██████████████████░░ 91.2% |
12,092 / 13,265 |
stamphog |
██████████████████░░ 91.3% |
4,505 / 4,936 |
mcp_store |
██████████████████░░ 92.2% |
6,525 / 7,076 |
product_analytics |
███████████████████░ 92.5% |
5,849 / 6,321 |
managed_migrations |
███████████████████░ 92.6% |
1,556 / 1,681 |
early_access_features |
███████████████████░ 92.6% |
1,287 / 1,390 |
notifications |
███████████████████░ 92.6% |
1,017 / 1,098 |
ai_observability |
███████████████████░ 92.8% |
15,618 / 16,821 |
surveys |
███████████████████░ 93.2% |
5,853 / 6,281 |
posthog_ai |
███████████████████░ 93.2% |
1,326 / 1,422 |
approvals |
███████████████████░ 93.3% |
3,437 / 3,682 |
web_analytics |
███████████████████░ 93.4% |
15,047 / 16,112 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
legal_documents |
███████████████████░ 93.8% |
1,628 / 1,736 |
workflows |
███████████████████░ 94.1% |
7,251 / 7,707 |
endpoints |
███████████████████░ 94.2% |
8,655 / 9,192 |
tracing |
███████████████████░ 94.5% |
2,671 / 2,827 |
review_hog |
███████████████████░ 94.6% |
8,101 / 8,563 |
skills |
███████████████████░ 94.6% |
3,158 / 3,337 |
messaging |
███████████████████░ 94.7% |
2,885 / 3,048 |
logs |
███████████████████░ 95.4% |
10,487 / 10,987 |
experiments |
███████████████████░ 95.5% |
26,047 / 27,287 |
growth |
███████████████████░ 96.1% |
3,245 / 3,376 |
annotations |
███████████████████░ 96.2% |
732 / 761 |
revenue_analytics |
███████████████████░ 96.3% |
1,887 / 1,960 |
feature_flags |
███████████████████░ 96.4% |
17,488 / 18,144 |
replay_vision |
███████████████████░ 96.4% |
16,286 / 16,891 |
user_interviews |
███████████████████░ 96.5% |
2,638 / 2,734 |
access_control |
███████████████████░ 96.9% |
870 / 898 |
customer_analytics |
███████████████████░ 97.1% |
10,463 / 10,777 |
warehouse_sources |
███████████████████░ 97.3% |
359,927 / 369,932 |
data_catalog |
████████████████████ 97.7% |
2,588 / 2,648 |
analytics_platform |
████████████████████ 98.0% |
2,153 / 2,197 |
metrics |
████████████████████ 98.2% |
2,491 / 2,536 |
pulse |
████████████████████ 98.4% |
2,017 / 2,049 |
live_debugger |
████████████████████ 99.2% |
613 / 618 |
field_notes |
████████████████████ 99.4% |
158 / 159 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
Problem
Third step of splitting
DeltaTableHelper(after the maintenance extraction in #75978 and the SCD2 writer in #76067). The core write/merge path and the commit-metadata idempotency checks were the last non-access concerns left in the helper, keeping it at ~750 lines and forcing every write-path change (including the active deltalite rollout) through the shared file.Changes
core/delta/writer.py:DeltaWriter, a stateless wrapper over the table helper, mirroringDeltaMaintenanceandScd2DeltaWriter.write_to_deltalakebecomesDeltaWriter.write, moving together with the deltalite canary (verbatim — it's mid-rollout), the incremental batch dedupe, the per-partition merge loop, and the schema-mismatch fallback.has_commit_with_metadata/has_batch_been_committedmoves with the writer deliberately: stamping the(run_uuid, batch_index)tag on the terminal commit only and reading it back on redelivery are two halves of one contract — splitting them is how someone later tags an intermediate commit without knowing that loses data on redelivery._commit_matchesbecomes the module-levelcommit_matches, which the SCD2 tagging test now uses too.DeltaTableHelperis now a pure table access/lifecycle object (~270 lines): cached open with corrupt-table auto-heal, corruption detection, reset, file listing, and the first-sync flag. The final PR in this series renames it to reflect that.common/load.py, andload/idempotency.py's redelivery fallback (which wraps the helper in aDeltaWriter).Note
Debug log prefixes change from
write_to_deltalake:towrite:with the rename. No behavior changes otherwise.How did you test this code?
Automated only (no manual testing):
delta/test/test_writer.py(legacy-dlt reconciliation, decimal reconciliation, schema-evolution nullability, batch dedupe, unpartitioned-table layout, misaligned-decimal end-to-end, NULL-safe merge predicate, deltalite canary, commit-metadata pass-through, and the idempotency layout matrix).test_delta_table_helper.pykeeps only the access/lifecycle suites.test_idempotency.pykeeps its full decision matrix, with the new writer boundary stubbed to pass the helper-shaped mocks through;test_processor.py's lease-loss and final-batch tests patchDeltaWriteralongside the existingScd2DeltaWriterpatch.ruffand repo-wideuv run mypy --cache-fine-grained .(clean).Automatic notifications
Docs update
No user-facing or documented-workflow changes.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored with Claude Code, continuing the agreed
DeltaTableHelpersplit plan (maintenance → SCD2 → writer → rename).Skills invoked: /writing-tests.
Decisions: idempotency lives on the writer rather than a separate module because tag-write and tag-read form one contract;
is_batch_already_processedkeeps itsDeltaTableHelperparameter (the object every signature already threads) and constructs the writer internally, so no call-site signatures change.Commit landed via GraphQL
createCommitOnBranch(server-side signing), with the remote tree verified equal to the locally tested commit.