You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Tracking issue for Stage 2 (Python row storage) of the shared SDK persistence proposal, adcontextprotocol/adcp#7996 (specs/shared-sdk-persistence-proposal.md, §3, §4, §7 and §8).
The JS SDK finished Stage 1 in adcontextprotocol/adcp-client #3145–#3175 (published in @adcp/sdk 15.4–15.6). This issue brings the Python SDK onto the same row-storage tables and encoding so that both SDKs can share one database and one bucket.
Scope
Canonical JSONL row encoding (JCS(row)+"\n", RFC 8785 via rfc8785). Segments of 500 rows, and chunks of at most 10,000 rows or 8 MiB, each with segment, chunk and manifest digests. Python must match the JS golden fixtures byte for byte.
The same row-storage DDL as JS (REPORTING_ROW_STORAGE_MIGRATION): bindings, chunks, bodies, write intents, and a row-set side table until Stage 3.
The postgres row binding with byte-verifying readers, and object bindings (filesystem, then GCS/S3/Azure as extras) with create-only writes and version-pinned reads and deletes.
Migrating reporting_revision_rows with verification. The legacy table is renamed or dropped, never truncated. A mismatch quarantines the revision as ROWS_INTEGRITY_FAILED and is never rewritten.
Protocol-shape control totals ({name, value, value_type}) in the content hash.
Retention and the change feed, in line with JS.
Acceptance gate: chunks written by JS verify in Python and vice versa, against one database and one bucket.
Work lands as a sequence of small PRs that reference this issue.
ad-tech-protocol-expert: rfc8785 is already a core dep; chunk/segment/manifest digest design aligns with the adcp#7996 spec; bidirectional verification gate (JS writes → Python verifies and vice versa) is the correct acceptance criterion for cross-SDK byte-level correctness.
code-reviewer: psycopg[binary] and rfc8785 deps are already present; existing SQL migration infra in reporting/ledger/*.sql is the right extension point; migration quarantine path (ROWS_INTEGRITY_FAILED, rename/drop never truncate) is sound; scope is appropriately multi-PR.
My take: This is a correctly-structured tracking issue for Stage 2 persistence work. Work should land as the planned sequence of small PRs, each using Refs #1457 (not Closes); no single auto-PR is appropriate for a tracking issue of this scope.
Proposed PR sequence for Stage 2. Each PR is small, targets main, has tests, and merges before the next starts. I read #7996 and its review thread, the JS implementation (src/lib/reporting/ledger/row-*.ts, change-feed.ts, the cloud providers) and the golden fixture test/fixtures/reporting-row-storage-v1.json.
Canonical encoding, no DB.row_encoding module: JCS(row)+"\n" via rfc8785, 500-row segments, chunks of at most 10,000 rows / 8 MiB, segment/chunk/manifest digests, row_manifest_sha256, streaming envelope and rows-only hashers, verifiers. The write-side number domain follows the spec (finite numbers, integers within +-(2^53-1), no lone surrogates). The JS golden fixture is copied into tests/fixtures/ and every chunk, manifest and digest is asserted byte for byte.
Protocol-shape control totals. New Core revisions hash {name, value, value_type[, unit]}. Legacy revisions are never rehashed; the verifier recognises them (non-empty totals and no typed evidence) as legacy_py_core_pairs_v0 and recomputes the old digest.
Row-storage DDL, postgres binding, dual-path readers. The tables are the JS ones (adcp_persistence_installation, adcp_reporting_row_bindings, adcp_reporting_row_sets, adcp_reporting_row_chunks, adcp_reporting_chunk_bodies, adcp_reporting_row_write_intents) with the same columns, checks and immutability triggers, so one database can serve both SDKs. One deliberate difference: Python's obligation_id has no FK, because until Stage 3 Python's obligations live in reporting_obligations rather than adcp_reporting_obligations. Also: the postgres row binding, verified paged read_revision_rows that handles both inline (reporting_revision_rows) and chunked revisions, the materializer re-verification switched to stored bytes, and a catalog fingerprint file for the new objects.
Verified migration. Resumable, bounded batches. Each legacy revision's rows are re-canonicalized and must reproduce its committed digest before the chunks are written; a mismatch quarantines the revision as ROWS_INTEGRITY_FAILED and is never rewritten. The legacy reporting_revision_rows table is renamed after verification, not truncated, so pre-chunk binaries fail closed.
External write path + filesystem provider. Lease-budget check, fenced write intents, create-only writes that adopt identical bytes, version-pinned reads and deletes, and the provider conformance suite (including write-unique-version).
GCS, S3 and Azure providers as optional extras (adcp[reporting-gcs|reporting-s3|reporting-azure]). S3 and Azure require bucket/container versioning and pin the version id; GCS pins the generation. Never ETag-only.
Retention. Row pruning, period retirement with tombstones, intent sweeps, and alignment of the change feed and registered consumers with JS.
Interop and docs. A cross-language test with JS-written chunks verified by Python and Python-written chunks verified by JS against one local database and one filesystem bucket, with cross-generated fixtures committed so Python CI does not need Node, plus the adopter guide and migration notes.
Open interpretation, flagged for the Python maintainer: #7996 stage 2 says row location lives in a side table until Stage 3. I am using the JS row_sets/row_chunks tables as that side table, keyed by reporting_revision_id. For legacy pair-hashed revisions the row set records digest profile rows_v1 (the only other profile the shared CHECK allows), and the pair-hashed envelope is verified against the Python header.
Progress and any divergence from the spec will be recorded in each PR description.
Per the plan each PR merges before the next one is opened, so the remaining PRs (row-storage DDL and postgres kind, verified migration, object write path and filesystem provider, cloud providers, retention, interop and docs) are prepared locally and will be opened in order as these merge. I am not using admin bypass for these.
@rachitm022, status and three questions on Stage 2.
Ready for your review:#1458 (canonical row encoding) and #1459 (protocol-shape control totals). CI passes on both, and code-owner approval is the only thing blocking them.
Built locally, opened one PR at a time once those merge:
Postgres row storage, verified migration of reporting_revision_rows, and the object write path with the filesystem provider
GCS, S3 and Azure providers
Sweeps and pruning, plus a JS↔Python interop test (one database, one bucket, both directions)
REFERENCE_NOT_FOUND / SERVICE_UNAVAILABLE per adcp#8085 / #8099
The shared change feed, current-revision reads, full period retirement, and maintenance scheduling in the production worker
Questions
Frozen records during retirement. Python's append-only guard triggers protect provisional observations and reconciliation records. Retirement therefore deletes a period's row bytes but leaves its records, and marks the period protected. Is that acceptable for Stage 2, or should a separate PR relax those triggers for retirement?
Unconfigured bindings. The intent sweep now skips bindings the host doesn't configure, so they can't starve the others. Should the sweep result count the skipped intents?
Python's existing reporting_ledger_changes feed. It stays as it is until Stage 3, and the shared adcp_reporting_changes feed is added next to it. Are you OK with that?
@rachitm022 Stage 2 (Python row storage) of adcontextprotocol/adcp#7996 is merged. Everything below is on main; nothing is published to PyPI (a release still needs its own environment approval). You asked to test the finished stack, so this is what landed, how to test it, and the questions I would like your call on.
Scheduling of sweeps, retirement and change pruning in the production worker
All of these were merged on the repo owner's instruction without waiting for code-owner review. The review bots' findings were fixed and resolved on each PR; the design questions below are the part that needs a human.
How to test
Setup:
pip install -e '.[dev,pg,reporting-gcs,reporting-s3,reporting-azure]'
export ADCP_PG_TEST_URL=postgresql://postgres@localhost:5432/adcp_test # PostgreSQL 13+; each test uses its own schema
Binding configuration is in docs/reporting-row-storage.md:
Enable: PgReportingLedgerStore(pool=pool, row_storage=ReportingRowStorageOptionsV1(deployment_namespace="prod-us")), then await store.create_schema().
GCS: create_gcs_reporting_row_object_provider_v1(storage=storage.Client()), binding ReportingObjectRowBindingV1(provider="gcs", location={"bucket": "..."}, prefix="adcp-rows"), providers={"gcs": gcs}. The bucket needs uniform bucket-level access and enforced public access prevention.
S3: create_s3_reporting_row_object_provider_v1(client=boto3.client("s3", region_name=...)), location {"bucket": ..., "region": ...}. The bucket must have versioning enabled and the client finite connect and read timeouts (botocore defaults pass).
Azure: create_azure_blob_reporting_row_object_provider_v1(client=BlobServiceClient(url, credential=DefaultAzureCredential())), location {"account": ..., "container": ...}, provider azure_blob. Blob versioning must be on.
Select a binding per revision with select_row_binding=lambda s: .... Every provider's probe refuses unsafe destinations (public access, lifecycle deletes that can match live objects, stores that ignore create-only) with UNSAFE_BINDING.
The GCS, S3 and Azure providers are tested against fakes (S3 against moto), not real buckets. A manual pass against real buckets would be the most useful thing you can add.
JS to Python interop:
cd <adcp-client> && npm ci && npm run build
ADCP_JS_SDK_DIR=<adcp-client> pytest -n0 tests/conformance/reporting/test_reporting_row_storage_interop_pg.py
Without ADCP_JS_SDK_DIR the test replays the rows each SDK wrote, committed under tests/fixtures/reporting-row-storage-interop-v1/, so Python CI needs no Node.
Locks, shared with the JS SDK (full table in #1467): the object lifetime lock is pg_advisory_xact_lock(hashtextextended('adcp-reporting-object:' || binding_id || ':' || object_key, 0)), identical in both SDKs. The account locks have different key text (Python hashtext('adcp.reporting:' || account_id), JS hashtextextended('adcp-reporting-account:' || ...)); they guard each SDK's own Core tables, so aligning them waits for the shared Core layout (Stage 3).
Decisions that interpret the spec
Python keeps its own Core headers until Stage 3. The shared row tables have no foreign key from adcp_reporting_row_sets.obligation_id, and Python's buyer-facing reporting_ledger_changes feed is unchanged.
Legacy pair-hashed revisions are recognised by "non-empty totals and no typed evidence", not by a stored marker, because a new column would change the pinned catalog fingerprint.
The retention window is max(status_retention_days, ledger_record_retention_days) from the latest of period end, newest revision, newest adjustment (adcp#8100); a shorter configured value is ignored.
Emitting the REVISION_UNREADABLE status issue waits for the Python schemas to include it; unreadable rows answer SERVICE_UNAVAILABLE meanwhile.
A JS-found data-loss bug (seq::text ... ORDER BY seq sorting as text) was present in an earlier draft of the Python feed. It never reached main; the merged feed qualifies every sort key and a regression test crosses 99 to 100 on both feeds.
Open design questions for you
protected periods. Period retirement stays retiring and reports protected for a period whose records sit behind frozen append-only guards (provisional observations, reconciliation records, notification events): the row bytes are gone, the records wait. Acceptable until Stage 3, or should those triggers get a retirement exemption in a separate PR?
Skipped intents. The intent sweep only considers bindings the host configures; other expired intents are counted in skipped_unconfigured and left alone. Is a count enough, or should an unconfigured binding with expired intents be an error?
Two change feeds. The new shared feed (adcp_reporting_changes, cursors compatible with JS) coexists with Python's buyer-facing reporting_ledger_changes feed until Stage 3. Do you want the two merged earlier, or is documenting the split enough?
Account lock key text. Should Python adopt JS's key text for the account lock now (taking both locks in one fixed order), or wait for Stage 3?
Unlocked just-created object. A writer's own just-created object is not covered by the object lock in the instant between the create and the intent record (and _discard_created when recording fails). JS leaves the same window. Worth fencing?
statx birth time. The filesystem provider reads birth time via statx through ctypes, to match Node. Is a ctypes dependency acceptable in this package, or would you rather have a pure fallback?
@bokelley thanks for the writeup. I ran the stack and read the code behind each question. Answers first, then the issues I hit while testing, which I've kept separate so the design answers stand on their own.
Some context for how I'm weighing these: on our side the plan is to put the agentic-api social adapters (Meta first, then TikTok, Snap and the rest) on the JS SDK's ledger. That deployment is JS-only, in its own database. The embedded sales agent, our Python seller, has a separate Postgres.
The six questions
1. Protected periods. I think this is fine until Stage 3, and I wouldn't relax the triggers in a separate PR. The append-only guards are audit evidence, so a retirement exemption is a decision about the audit model. It is better designed once, for both SDKs, in the shared Core layout. Changing frozen catalog objects now would also move the pinned fingerprint. One thing should be said plainly, though. In a run of the production service against Postgres, every production period carried reconciliation records, so the hold means retirement never picks those periods up at all. In practice, record deletion is off for production deployments until Stage 3. Could the release notes and docs/reporting-row-storage.md say that?
2. Skipped intents. A count is enough. Making it an error would fire all the time in a shared database where another host legitimately owns the binding. The case worth catching is a binding that no host configures any more, because its orphaned objects are never cleaned up. I'd warn when skipped intents have been expired for much longer than any sweep interval, since that means nobody is handling them. (The worker currently drops the count; see issue B below.)
3. Two change feeds. Documenting the split is enough. They serve different readers, and merging now would change cursors buyers already hold, only for Stage 3 to rework it again. Worth noting in the docs that retirement events appear only in the shared feed.
4. Account lock key. For us, waiting for Stage 3 is fine, because we won't have Python and JS writing the same account in one database before then. But the limitation should be stated rather than implied. Account readers of the shared feed order by seq only, so they rely on the account lock to keep seq in commit order. I reproduced a Python append (seq 1) and a JS append (seq 2) on the same account, each under its own SDK's lock key, with JS committing first. An account reader then skipped seq 1 permanently. So until Stage 3, "mixed-SDK writers on one account are not supported" should be in the docs. Longer term, giving account readers the same xmin guard the deployment-wide feed uses would remove the dependence on the lock in both SDKs.
5. The unlocked just-created object. The window itself seems acceptable to me, as long as a failure in it only leaks storage. That is what JS does: if recording the created object fails, upload throws and leaves the object behind. Holding the object lock across a network upload would cost far more than an occasional orphaned object. The problem is Python's failure path, which deletes instead of leaking (issue D below). With that fixed to match JS, I'd leave the window alone.
6. statx via ctypes. Fine. It's in the standard library, imported lazily, with a fallback. I'd document the filesystem provider as dev and single-host only.
Issues noticed while testing
Test results: 114 unit and provider tests passed. Of 76 PostgreSQL conformance tests, 75 passed and 1 was skipped: the live JS interop test, which needs ADCP_JS_SDK_DIR. No CI workflow sets that, so the cross-SDK gate in #7996 has not run in CI. I haven't done the real-bucket pass yet.
Setup. The install command in your comment leaves out the pg extra, so psycopg isn't installed. It should be pip install -e '.[dev,pg,reporting-gcs,reporting-s3,reporting-azure]'.
A. Stuck periods block retirement for everyone._candidate_query always puts retiring periods first, under LIMIT, and a protected period stays retiring forever. In a scratch test with limit=1, one protected period kept an eligible period from ever being retired. With the worker's limit=20, twenty protected periods stop retirement for the whole deployment.
B. The worker hides what retention didn't do._maintain only logs when failed is non-empty. Protected periods, held periods and skipped_unconfigured are all discarded, so none of the above shows up anywhere.
C. Production row bytes are never pruned. The worker schedules intent sweeps, period retirement and change pruning, but not prune_expired_row_sets. That function's only hold is materializer work, so it should free row bytes for production periods even while their records stay. I haven't run it against a production period.
D. A failed upload can delete another writer's committed object. If on_created fails because the intent was taken over, _discard_created deletes the object version with no object lock and no reference check. If the writer that took over has already adopted that version and committed, the delete removes a committed revision's bytes. Because the delete is version-pinned, they're gone for good. It needs a stall longer than the intent expiry, for example waiting on the pool inside record(), so it's rare. Either drop the discard to match JS, or send it through delete_unreferenced.
E. One broken binding can block the intent sweep. The sweep selects oldest first under LIMIT 100. A hundred intents that keep failing on one binding are picked first on every pass, so no other binding's intents are swept.
I have scratch tests for A, the production retirement run and the feed skip, and can share them. Happy to pick up fixes for any of these.
@rachitm022 thank you. Running the stack and reading the code behind each question is exactly what we needed. Answers, then what we're doing about A–E.
A direction change first. Following Konstantin's review on adcp#7996, Stage 3 is dropped. There will be no shared Core layout, dual-key locks or shared idempotency store. The deployment model is one writer SDK per ledger database, which matches your plan: the social adapters run JS-only in their own database, and the Python seller has a separate Postgres. So "wait for Stage 3" is no longer the answer to anything below.
On your six answers:
Protected periods. Agreed, don't relax the triggers in a drive-by PR; that's an audit-model decision for the Python maintainers. With Stage 3 gone, it's now a Python-only decision, whenever you want to make it. Meanwhile we'll say plainly in the docs and release notes that record deletion is effectively off for production deployments whose periods carry protected records. We'll also make sure row bytes are still pruned for them (your C).
Skipped intents. A count, plus a warning when intents have been expired far longer than any sweep interval. Agreed.
Two feeds. We'll document the split, including that retirement events appear only in the shared feed.
Account lock. "One writer SDK per database" and "mixed-SDK writers on one account are unsupported" will be stated outright, with your seq-skip reproduction as the reason. We're also adding the xmin guard to account readers in both SDKs, so ordering no longer depends on lock discipline. Good suggestion.
The unlocked just-created object. Agreed. The fix is your D: leak, never delete.
statx. Fine, and the filesystem provider will be documented as dev and single-host only.
Issues A–E: we're fixing all five, with regression tests.
D first._discard_created will leak, not delete, matching JS.
A. Stuck periods get attempt backoff and can't occupy candidate slots.
B. The worker reports protected, held and skipped_unconfigured, not just failed.
C. The production worker schedules prune_expired_row_sets.
E. The intent sweep gets per-intent backoff and skips unconfigured bindings.
A and E exist in the JS SDK too, so we're fixing them there as well. If you can share your scratch tests for A, the production retirement run and the feed skip, we'll fold them in as regressions. Your review on those PRs would be very welcome.
Interop gate. You're right that it has never run in CI. Python CI will now build JS main and run the interop test, and it fails rather than skips when the JS side is missing.
Setup. Thanks for catching the missing pg extra. The command is pip install -e '.[dev,pg,reporting-gcs,reporting-s3,reporting-azure]', and the docs will say so.
Activity
bokelley commentedon Oct 9, 2026
Triage
Classification: Feature request (tracking)
Bucket(s): validation, client
Status: deferred
What the experts said:
rfc8785is already a core dep; chunk/segment/manifest digest design aligns with the adcp#7996 spec; bidirectional verification gate (JS writes → Python verifies and vice versa) is the correct acceptance criterion for cross-SDK byte-level correctness.psycopg[binary]andrfc8785deps are already present; existing SQL migration infra inreporting/ledger/*.sqlis the right extension point; migration quarantine path (ROWS_INTEGRITY_FAILED, rename/drop never truncate) is sound; scope is appropriately multi-PR.My take: This is a correctly-structured tracking issue for Stage 2 persistence work. Work should land as the planned sequence of small PRs, each using
Refs #1457(notCloses); no single auto-PR is appropriate for a tracking issue of this scope.Triaged by Claude Code. Session: https://claude.ai/code/session_017Twdu39NAV5byxdgdqLoRC
Generated by Claude Code
bokelley commentedon Oct 9, 2026
Proposed PR sequence for Stage 2. Each PR is small, targets
main, has tests, and merges before the next starts. I read #7996 and its review thread, the JS implementation (src/lib/reporting/ledger/row-*.ts,change-feed.ts, the cloud providers) and the golden fixturetest/fixtures/reporting-row-storage-v1.json.row_encodingmodule:JCS(row)+"\n"viarfc8785, 500-row segments, chunks of at most 10,000 rows / 8 MiB, segment/chunk/manifest digests,row_manifest_sha256, streaming envelope and rows-only hashers, verifiers. The write-side number domain follows the spec (finite numbers, integers within +-(2^53-1), no lone surrogates). The JS golden fixture is copied intotests/fixtures/and every chunk, manifest and digest is asserted byte for byte.{name, value, value_type[, unit]}. Legacy revisions are never rehashed; the verifier recognises them (non-empty totals and no typed evidence) aslegacy_py_core_pairs_v0and recomputes the old digest.adcp_persistence_installation,adcp_reporting_row_bindings,adcp_reporting_row_sets,adcp_reporting_row_chunks,adcp_reporting_chunk_bodies,adcp_reporting_row_write_intents) with the same columns, checks and immutability triggers, so one database can serve both SDKs. One deliberate difference: Python'sobligation_idhas no FK, because until Stage 3 Python's obligations live inreporting_obligationsrather thanadcp_reporting_obligations. Also: thepostgresrow binding, verified pagedread_revision_rowsthat handles both inline (reporting_revision_rows) and chunked revisions, the materializer re-verification switched to stored bytes, and a catalog fingerprint file for the new objects.ROWS_INTEGRITY_FAILEDand is never rewritten. The legacyreporting_revision_rowstable is renamed after verification, not truncated, so pre-chunk binaries fail closed.write-unique-version).adcp[reporting-gcs|reporting-s3|reporting-azure]). S3 and Azure require bucket/container versioning and pin the version id; GCS pins the generation. Never ETag-only.Open interpretation, flagged for the Python maintainer: #7996 stage 2 says row location lives in a side table until Stage 3. I am using the JS
row_sets/row_chunkstables as that side table, keyed byreporting_revision_id. For legacy pair-hashed revisions the row set records digest profilerows_v1(the only other profile the shared CHECK allows), and the pair-hashed envelope is verified against the Python header.Progress and any divergence from the spec will be recorded in each PR description.
bokelley commentedon Oct 9, 2026
Status update for the Stage 2 sequence.
main, which the bot does not satisfy, so it is waiting on a human review (@KonstantinMirin or @bokelley).Per the plan each PR merges before the next one is opened, so the remaining PRs (row-storage DDL and postgres kind, verified migration, object write path and filesystem provider, cloud providers, retention, interop and docs) are prepared locally and will be opened in order as these merge. I am not using admin bypass for these.
bokelley commentedon Oct 9, 2026
@rachitm022, status and three questions on Stage 2.
Ready for your review: #1458 (canonical row encoding) and #1459 (protocol-shape control totals). CI passes on both, and code-owner approval is the only thing blocking them.
Built locally, opened one PR at a time once those merge:
reporting_revision_rows, and the object write path with the filesystem providerREFERENCE_NOT_FOUND/SERVICE_UNAVAILABLEper adcp#8085 / #8099Questions
protected. Is that acceptable for Stage 2, or should a separate PR relax those triggers for retirement?reporting_ledger_changesfeed. It stays as it is until Stage 3, and the sharedadcp_reporting_changesfeed is added next to it. Are you OK with that?🤖 Generated with Claude Code
feat(reporting): add canonical JSONL row encoding for shared row stor…
feat(reporting): bind protocol-shape control totals in new revisions (#…
feat(reporting): store revision rows as verified chunks in PostgreSQL (…
feat(reporting): migrate legacy inline rows to verified chunks (#1466)
feat(reporting): write row chunks to object storage with fenced uploa…
feat(reporting): add GCS, S3 and Azure Blob row providers (#1468)
feat(reporting): sweep abandoned uploads and prune expired row sets (#…
test(reporting): prove JS and Python chunks verify in one database an…
fix(reporting): answer unreadable exact revision reads with SERVICE_U…
feat(reporting): add the shared host change feed and registered consu…
feat(reporting): add current-revision host reads (#1473)
feat(reporting): retire expired periods with tombstones and holds (#1474
feat(reporting): schedule retention in the production worker (#1476)
bokelley commentedon Oct 11, 2026
@rachitm022 Stage 2 (Python row storage) of adcontextprotocol/adcp#7996 is merged. Everything below is on
main; nothing is published to PyPI (a release still needs its own environment approval). You asked to test the finished stack, so this is what landed, how to test it, and the questions I would like your call on.What landed
{name, value}pair-hash conformance bug)reporting_row_storage.sql,read_revision_rows,read_verified_revision_rows)reporting_revision_rowsREFERENCE_NOT_FOUND/SERVICE_UNAVAILABLEalignment for exact reads (adcp#8085, #8099)adcp_reporting_changes, horizons, registered consumers,CURSOR_EXPIRED)get_current_revision,list_current_revisionsAll of these were merged on the repo owner's instruction without waiting for code-owner review. The review bots' findings were fixed and resolved on each PR; the design questions below are the part that needs a human.
How to test
Setup:
Unit and provider tests (no database):
PostgreSQL conformance (run serially,
-n0; xdist produces spurious failures in these suites):Binding configuration is in
docs/reporting-row-storage.md:PgReportingLedgerStore(pool=pool, row_storage=ReportingRowStorageOptionsV1(deployment_namespace="prod-us")), thenawait store.create_schema().create_gcs_reporting_row_object_provider_v1(storage=storage.Client()), bindingReportingObjectRowBindingV1(provider="gcs", location={"bucket": "..."}, prefix="adcp-rows"),providers={"gcs": gcs}. The bucket needs uniform bucket-level access and enforced public access prevention.create_s3_reporting_row_object_provider_v1(client=boto3.client("s3", region_name=...)), location{"bucket": ..., "region": ...}. The bucket must have versioning enabled and the client finite connect and read timeouts (botocore defaults pass).create_azure_blob_reporting_row_object_provider_v1(client=BlobServiceClient(url, credential=DefaultAzureCredential())), location{"account": ..., "container": ...}, providerazure_blob. Blob versioning must be on.select_row_binding=lambda s: .... Every provider's probe refuses unsafe destinations (public access, lifecycle deletes that can match live objects, stores that ignore create-only) withUNSAFE_BINDING.JS to Python interop:
Without
ADCP_JS_SDK_DIRthe test replays the rows each SDK wrote, committed undertests/fixtures/reporting-row-storage-interop-v1/, so Python CI needs no Node.Locks, shared with the JS SDK (full table in #1467): the object lifetime lock is
pg_advisory_xact_lock(hashtextextended('adcp-reporting-object:' || binding_id || ':' || object_key, 0)), identical in both SDKs. The account locks have different key text (Pythonhashtext('adcp.reporting:' || account_id), JShashtextextended('adcp-reporting-account:' || ...)); they guard each SDK's own Core tables, so aligning them waits for the shared Core layout (Stage 3).Decisions that interpret the spec
adcp_reporting_row_sets.obligation_id, and Python's buyer-facingreporting_ledger_changesfeed is unchanged.max(status_retention_days, ledger_record_retention_days)from the latest of period end, newest revision, newest adjustment (adcp#8100); a shorter configured value is ignored.REVISION_UNREADABLEstatus issue waits for the Python schemas to include it; unreadable rows answerSERVICE_UNAVAILABLEmeanwhile.seq::text ... ORDER BY seqsorting as text) was present in an earlier draft of the Python feed. It never reachedmain; the merged feed qualifies every sort key and a regression test crosses 99 to 100 on both feeds.Open design questions for you
protectedperiods. Period retirement staysretiringand reportsprotectedfor a period whose records sit behind frozen append-only guards (provisional observations, reconciliation records, notification events): the row bytes are gone, the records wait. Acceptable until Stage 3, or should those triggers get a retirement exemption in a separate PR?skipped_unconfiguredand left alone. Is a count enough, or should an unconfigured binding with expired intents be an error?adcp_reporting_changes, cursors compatible with JS) coexists with Python's buyer-facingreporting_ledger_changesfeed until Stage 3. Do you want the two merged earlier, or is documenting the split enough?_discard_createdwhen recording fails). JS leaves the same window. Worth fencing?statxbirth time. The filesystem provider reads birth time viastatxthroughctypes, to match Node. Is a ctypes dependency acceptable in this package, or would you rather have a pure fallback?rachitm022 commentedon Oct 12, 2026
@bokelley thanks for the writeup. I ran the stack and read the code behind each question. Answers first, then the issues I hit while testing, which I've kept separate so the design answers stand on their own.
Some context for how I'm weighing these: on our side the plan is to put the agentic-api social adapters (Meta first, then TikTok, Snap and the rest) on the JS SDK's ledger. That deployment is JS-only, in its own database. The embedded sales agent, our Python seller, has a separate Postgres.
The six questions
1. Protected periods. I think this is fine until Stage 3, and I wouldn't relax the triggers in a separate PR. The append-only guards are audit evidence, so a retirement exemption is a decision about the audit model. It is better designed once, for both SDKs, in the shared Core layout. Changing frozen catalog objects now would also move the pinned fingerprint. One thing should be said plainly, though. In a run of the production service against Postgres, every production period carried reconciliation records, so the hold means retirement never picks those periods up at all. In practice, record deletion is off for production deployments until Stage 3. Could the release notes and
docs/reporting-row-storage.mdsay that?2. Skipped intents. A count is enough. Making it an error would fire all the time in a shared database where another host legitimately owns the binding. The case worth catching is a binding that no host configures any more, because its orphaned objects are never cleaned up. I'd warn when skipped intents have been expired for much longer than any sweep interval, since that means nobody is handling them. (The worker currently drops the count; see issue B below.)
3. Two change feeds. Documenting the split is enough. They serve different readers, and merging now would change cursors buyers already hold, only for Stage 3 to rework it again. Worth noting in the docs that retirement events appear only in the shared feed.
4. Account lock key. For us, waiting for Stage 3 is fine, because we won't have Python and JS writing the same account in one database before then. But the limitation should be stated rather than implied. Account readers of the shared feed order by
seqonly, so they rely on the account lock to keepseqin commit order. I reproduced a Python append (seq 1) and a JS append (seq 2) on the same account, each under its own SDK's lock key, with JS committing first. An account reader then skipped seq 1 permanently. So until Stage 3, "mixed-SDK writers on one account are not supported" should be in the docs. Longer term, giving account readers the samexminguard the deployment-wide feed uses would remove the dependence on the lock in both SDKs.5. The unlocked just-created object. The window itself seems acceptable to me, as long as a failure in it only leaks storage. That is what JS does: if recording the created object fails,
uploadthrows and leaves the object behind. Holding the object lock across a network upload would cost far more than an occasional orphaned object. The problem is Python's failure path, which deletes instead of leaking (issue D below). With that fixed to match JS, I'd leave the window alone.6.
statxvia ctypes. Fine. It's in the standard library, imported lazily, with a fallback. I'd document the filesystem provider as dev and single-host only.Issues noticed while testing
Test results: 114 unit and provider tests passed. Of 76 PostgreSQL conformance tests, 75 passed and 1 was skipped: the live JS interop test, which needs
ADCP_JS_SDK_DIR. No CI workflow sets that, so the cross-SDK gate in #7996 has not run in CI. I haven't done the real-bucket pass yet.pgextra, so psycopg isn't installed. It should bepip install -e '.[dev,pg,reporting-gcs,reporting-s3,reporting-azure]'._candidate_queryalways putsretiringperiods first, underLIMIT, and a protected period staysretiringforever. In a scratch test withlimit=1, one protected period kept an eligible period from ever being retired. With the worker'slimit=20, twenty protected periods stop retirement for the whole deployment._maintainonly logs whenfailedis non-empty. Protected periods, held periods andskipped_unconfiguredare all discarded, so none of the above shows up anywhere.prune_expired_row_sets. That function's only hold is materializer work, so it should free row bytes for production periods even while their records stay. I haven't run it against a production period.on_createdfails because the intent was taken over,_discard_createddeletes the object version with no object lock and no reference check. If the writer that took over has already adopted that version and committed, the delete removes a committed revision's bytes. Because the delete is version-pinned, they're gone for good. It needs a stall longer than the intent expiry, for example waiting on the pool insiderecord(), so it's rare. Either drop the discard to match JS, or send it throughdelete_unreferenced.LIMIT 100. A hundred intents that keep failing on one binding are picked first on every pass, so no other binding's intents are swept.I have scratch tests for A, the production retirement run and the feed skip, and can share them. Happy to pick up fixes for any of these.
fix(reporting): keep retention moving past stuck work and make it vis…
fix(reporting): truncate account feed pages at the first in-flight ch…
bokelley commentedon Oct 13, 2026
@rachitm022 thank you. Running the stack and reading the code behind each question is exactly what we needed. Answers, then what we're doing about A–E.
A direction change first. Following Konstantin's review on adcp#7996, Stage 3 is dropped. There will be no shared Core layout, dual-key locks or shared idempotency store. The deployment model is one writer SDK per ledger database, which matches your plan: the social adapters run JS-only in their own database, and the Python seller has a separate Postgres. So "wait for Stage 3" is no longer the answer to anything below.
On your six answers:
xminguard to account readers in both SDKs, so ordering no longer depends on lock discipline. Good suggestion.statx. Fine, and the filesystem provider will be documented as dev and single-host only.Issues A–E: we're fixing all five, with regression tests.
_discard_createdwill leak, not delete, matching JS.skipped_unconfigured, not justfailed.prune_expired_row_sets.A and E exist in the JS SDK too, so we're fixing them there as well. If you can share your scratch tests for A, the production retirement run and the feed skip, we'll fold them in as regressions. Your review on those PRs would be very welcome.
Interop gate. You're right that it has never run in CI. Python CI will now build JS
mainand run the interop test, and it fails rather than skips when the JS side is missing.Setup. Thanks for catching the missing
pgextra. The command ispip install -e '.[dev,pg,reporting-gcs,reporting-s3,reporting-azure]', and the docs will say so.🤖 Generated with Claude Code
feat(reporting)!: align row storage with AdCP stored names and the pe…