Skip to content

feat: bind financial report sources to AIS persistence - #52

Draft
seonghobae wants to merge 27 commits into
feat/financial-reporting-xbrl-exportfrom
feat/financial-report-authority-registry
Draft

feat: bind financial report sources to AIS persistence#52
seonghobae wants to merge 27 commits into
feat/financial-reporting-xbrl-exportfrom
feat/financial-report-authority-registry

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Stack

This is a Draft successor to #50 and targets feat/financial-reporting-xbrl-export. It does not bypass or replace #50. #50 remains the proposal-format/XBRL serializer boundary; this PR adds the first database-owned source-authority slice required by Issue #51.

The current branch contains the live #50 parent through an ordinary merge and remains an isolated development stack. Its 0020_financial_report_source_registry.sql identity is provisional because unreleased reconciliation/Period Close migrations own intervening forward numbers. After those prerequisites integrate into protected develop, rebuild this branch non-destructively from that exact protected authority, assign the next unclaimed migration number, and update installer/tests/docs together. Do not rewrite prerequisite history or close this PR because its provisional number becomes stale.

Problem and authority boundary

A balanced caller-supplied report package and SHA-256 digest do not prove AIS origin. An authoritative financial report needs durable evidence of the exact tenant/entity/book/period population retained by AIS, with database-owned accounting-book period state and chronology.

financial_report_source is append-only, but the protected foundation consumed by this branch does not yet freeze a referenced trial_balance_snapshot header or its trial_balance_line population. That missing invariant belongs to retained Period Close evidence authority. Reporting may bind and consume retained close evidence; it may not synthesize close control, mutate retained snapshot truth, or create a narrower report-only substitute for Period Close immutability.

Bounded delta

  • normalized accounting_reporting.financial_report_run and financial_report_source;
  • forced tenant RLS and revoked PUBLIC table authority;
  • FK-bound tenant/legal-entity/accounting-book and current/comparison book-period scope;
  • retained trial_balance_snapshot source reference;
  • database-derived reporting currency, current book-period status, knowledge cutoff and recorded_at chronology;
  • database-forced initial lifecycle collecting_sources;
  • fail-closed report-run lifecycle: raw SQL cannot change run_status_code; later supersession remains reserved for a purpose-bound command owner;
  • current/comparison source-role validation;
  • rejection of cross-entity/book/period/currency and future-cutoff snapshots;
  • append-only source evidence;
  • real PostgreSQL regression coverage for scope, chronology, RLS, lifecycle and source immutability;
  • ADR 0068 remains Proposed and records the migration collision/non-force-restack requirement.

TDD / repair lineage

Predecessor f4354a2d3c15b6270dbedb834372deb37e4be05d added two real PostgreSQL source-population dependency REDs. Foundation run 33876130106 initially stopped earlier with 736 tests / 8 failures / 0 errors: seven tests lacked a lawful accounting_book_period_control because their fixture was created after migration 0009's one-time backfill, and one docs contract promoted a provisional migration filename to canonical install truth.

The ordinary repair sequence removed those precursor defects without changing production accounting authority:

  1. 33106ed3c00aaa81170dc968fe14e0a451eb7a90 — fixture book-period control is materialized through the existing foundation owner helper _lock_book_period.
  2. fd1153fddc77edb0e7d6b81723ff82a03828cffe — population-freeze regressions consume that lawful owner-controlled fixture.
  3. 8163909a1d79b29ffd52f65be05c50589170a540 — recording-time regressions consume the same fixture boundary.
  4. a62a74d7b4b0d949b74a8c22fc181333edcf15cb — README derives the canonical foundation endpoint from the executable manifest rather than lexicographic migration filenames.
  5. ddb08b8ec5e05b3aa2cf3a3120e9b97625c341b2 — ordinary merge adopts non-overlapping live feat: generate unverified financial report and XBRL proposals #50 reporting/XBRL deltas after exact comparison.
  6. 8286b6cf0e9195eb97801a2b9cf952faa3587ddd — raw retained-snapshot test fixture supplies migration-0004-required close_idempotency_key without weakening production constraints.
  7. 1473e0c45fdb7a74967fb8cc041bea47c8203f81 — population-freeze tests avoid imported unittest.TestCase rediscovery.
  8. af95c4b098431e3c87e3dffa225de9e8c00c9d39 — recording-time tests use the same module-import boundary.
  9. c7d4ef300df6acf58bf8d5d5fd1799f82d509b23 — a real PostgreSQL RED proves direct SQL could change a report run from collecting_sources to superseded without actor, command, idempotency, source-hash or outbox evidence.
  10. 5a26eba357d09a0e65933da1e5ced99197130ecf — the database lifecycle guard now rejects raw run_status_code mutation with financial_report_run_lifecycle_immutable; no supersession command or publication authority is invented in this slice.

Steps 1–8 did not change database/migrations/0020_financial_report_source_registry.sql. Steps 9–10 then exposed and repaired one Reporting-local lifecycle-authority gap without weakening Period Close, Posting, Billing, source-population or RLS controls.

Exact current state — 2026-09-09

  • exact head: 5a26eba357d09a0e65933da1e5ced99197130ecf;
  • base: feat: generate unverified financial report and XBRL proposals #50 branch feat/financial-reporting-xbrl-export; this PR remains open / Draft / not merge-ready;
  • Accounting Foundation run 34271427012 is terminal RED on this exact head. Accounting job 102213693861 ran 520 tests in 85.603s with 2 failures / 0 errors. Exact-head dependency diff 102213694040, SAST 102213694259, and Security 102213694269 are GREEN; integrated-head attestations 102214537378 are skipped because this stack is not protected authority;
  • the lifecycle regression test_direct_sql_cannot_supersede_report_run_without_command_evidence is GREEN on this exact head and PostgreSQL emits financial_report_run_lifecycle_immutable for the hostile direct UPDATE. Initial lifecycle remains database-forced to collecting_sources; Issue [Product Gap] Complete statutory report publishing and validated XBRL round trip #51 owns any later purpose-bound supersession/withdrawal command, command/outbox evidence, approval or publication lifecycle;
  • the former book-period fixture, README endpoint, close_idempotency_key, duplicate-test-discovery and raw lifecycle-mutation defects are now GREEN. Source-registry scope, chronology, FORCE RLS, database-owned recording time, installer failure handling, and ordinary reporting contracts reach and pass their intended assertions;
  • the only two behavior failures are test_linked_snapshot_header_cannot_be_rewritten and test_linked_snapshot_cannot_gain_new_lines. Both reach the hostile PostgreSQL UPDATE/INSERT and fail because the protected parent does not yet provide retained trial_balance_snapshot / trial_balance_line population immutability;
  • those two failures remain a canonical dependency RED, not a Reporting-local production defect. PR fix(close): freeze hard-close trial balance evidence #53 owns retained snapshot immutability, one-population identity, chronology, exact scope/currency/conservation and concurrency. fix(close): freeze hard-close trial balance evidence #53 remains open/Draft and has not entered protected lineage, so its mutable implementation is not copied here;
  • current review inventory has no formal submitted review and no inline review thread. Draft status is therefore retained independently of the dependency RED;
  • the provisional 0020 installer ordering remains a post-foundation-restack/renumber requirement. Current isolated tests do not make it protected install authority.

Explicit non-claims

This PR does not create an authoritative financial-report artifact or claim IFRS/DART conformance, XBRL validation, filing readiness, approval, publication, regulator acceptance, audit, or assurance. It establishes database-owned source provenance only.

Issue #51 continues to own authenticated report-run assembly/finalization, purpose-bound supersession/withdrawal, independent XBRL/Calculations/Formula/jurisdiction validation, official released taxonomy profiles, accessible HTML/PDF/XLSX/Inline XBRL, localized verified commentary, maker-checker approval, publication receipts, recovery, and release evidence.

Merge boundary

Keep Draft and preserve the two realistic dependency REDs. Do not add financial_report_source_population_frozen or any equivalent Reporting-owned trigger/function merely to make this branch GREEN, and do not weaken/skip/xfail the tests to manufacture a pass.

Foundation first: #53 must reach protected lineage through its own normal RED→GREEN→review→merge path. Then rebuild #52 non-force from the resulting protected authority while preserving #50 deltas, renumber the provisional reporting migration, reconcile the public installer order, adapt the two regressions to assert the canonical Period Close invariant rather than a report-local marker, and reacquire real PostgreSQL behavior, exact owned production statement/branch coverage, repository contracts, security/SAST/dependency/package/SBOM/provenance, current reviews and all live ruleset gates on one unchanged head.

No self-approval, bypass, force-push, destructive rebase, synthetic status, premature Ready transition, tag or release from this provisional state.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
✨ 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 feat/financial-report-authority-registry

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review the database authority boundary rather than only SQL style. In particular verify that a caller cannot obtain stronger financial-report authority by supplying a balanced package, digest, reporting currency, period status, knowledge cutoff, run-status label, cross-scope snapshot, or future snapshot. This PR intentionally stops before source-set finalization, XBRL validation, approval, publication, filing, or regulator acceptance.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== PR metadata =='
gh pr view 52 --json number,title,state,isDraft,baseRefName,headRefName,files \
  --jq '{number,title,state,isDraft,baseRefName,headRefName,files:[.files[].path]}'

echo
echo '== Changed files with status =='
git diff --name-status "origin/$(gh pr view 52 --json baseRefName --jq .baseRefName)...HEAD"

echo
echo '== Candidate migration, installer, reporting, and test files =='
fd -t f . | rg -i '(0020|migration|financial.*report|report.*financial|trial.*balance|install|rls|source)'

Length of output: 1639


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== PR patch =='
gh pr diff 52 --patch

echo
echo '== Python structure =='
ast-grep outline src/accounting_information_platform/migration_install.py --items all
ast-grep outline tests/test_financial_report_source_install.py --items all
ast-grep outline tests/test_financial_report_source_registry_red.py --items all

echo
echo '== Changed files with line numbers =='
for f in \
  database/migrations/0020_financial_report_source_registry.sql \
  src/accounting_information_platform/migration_install.py \
  tests/test_financial_report_source_install.py \
  tests/test_financial_report_source_registry_red.py \
  docs/adr/0068-financial-report-source-authority.md
do
  echo "----- $f -----"
  cat -n "$f"
done

Length of output: 73481


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Candidate definitions and migration files =='
fd -t f . database src tests | rg '(migration|persistence|trial_balance_snapshot|fiscal_period|accounting_book|tenant)'

echo
echo '== Definition and mutation references =='
rg -n -i -C 5 \
  'CREATE TABLE accounting_reporting\.trial_balance_snapshot|CREATE TABLE accounting_core\.fiscal_period|CREATE TABLE accounting_core\.accounting_book|CREATE( OR REPLACE)? FUNCTION accounting_core\.current_tenant_account_id|apply_foundation_migration|trial_balance_snapshot.*(UPDATE|DELETE)|BEFORE (UPDATE|DELETE).*trial_balance_snapshot' \
  database src tests

echo
echo '== Migration sequence =='
fd -t f -e sql . database/migrations | sort

Length of output: 42901


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Foundation: accounting book, fiscal period, and trial-balance snapshot =='
sed -n '27,135p;226,270p;300,345p' database/migrations/0001_accounting_foundation.sql

echo
echo '== Book-period control =='
cat -n database/migrations/0009_accounting_book_period_control.sql

echo
echo '== Snapshot creation and immutability references =='
rg -n -i -C 8 \
  'trial_balance_snapshot|snapshot_generated_at|snapshot_currency_code|snapshot.*immutable|immutable.*snapshot' \
  database/migrations/0001_accounting_foundation.sql \
  database/migrations/0005_closed_period_guard.sql \
  database/migrations/0009_accounting_book_period_control.sql \
  database/migrations/0010_soft_close_command_evidence.sql \
  src/accounting_information_platform/persistence.py

echo
echo '== Installer manifest and migration execution =='
sed -n '6190,6315p' src/accounting_information_platform/persistence.py

echo
echo '== Existing PostgreSQL integration-test fixtures for scope and RLS =='
rg -n -i -C 3 \
  'ROW LEVEL SECURITY|FORCE ROW LEVEL SECURITY|cross-tenant|tenant.*deni|snapshot.*(entity|book|period|currency)|accounting_book_period_control' \
  tests

Length of output: 127500


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Status-update helper definitions and calls =='
rg -n -C 12 \
  'def _set_book_period_closed|def _lock_book_period|UPDATE accounting_core\.accounting_book_period_control|UPDATE accounting_core\.fiscal_period|period_status_code.*accounting_book_period_control' \
  src/accounting_information_platform/persistence.py

echo
echo '== Book-specific status behavior in integration tests =='
rg -n -C 10 \
  'accounting_book_period_control|management.*(open|soft_closed|hard_closed)|statutory.*(open|soft_closed|hard_closed)|different.*book|book.*independent' \
  tests/test_period_close_book_scope.py tests/test_postgres_posting.py

Length of output: 11335


@seonghobae I found one database authority-boundary defect.

  • source_period_status_code can assert a stronger status than the selected accounting book has. At Line 118 of database/migrations/0020_financial_report_source_registry.sql, bind_financial_report_run_scope() reads accounting_core.fiscal_period.period_status_code. That value is an aggregate compatibility value. The authoritative status is accounting_core.accounting_book_period_control.period_status_code, keyed by tenant, accounting book, and fiscal period. Existing behavior proves that sibling books can have different statuses for the same period in tests/test_period_close_book_scope.py::test_soft_close_one_book_does_not_block_sibling_book_posting. For example, a statutory book can be soft_closed while a management book remains open. A caller can select the management book and obtain a run that records the aggregate soft_closed status. This gives the run a stronger period-status claim than its selected book supports. Derive period_status_value from accounting_book_period_control using NEW.tenant_account_id, NEW.accounting_book_id, and NEW.fiscal_period_id. Reject the insert if that row does not exist.

The remaining reviewed controls have the intended design: the insert trigger overwrites caller currency, cutoff, and initial status; the source trigger checks tenant/entity/book/period/currency and cutoff; and the new tables use forced tenant RLS. The PR still needs PostgreSQL integration tests for these controls. The current new tests inspect SQL text or use mocks, so they cannot prove runtime enforcement.

⚠️ Action not completed

Head commit changed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

The authority-boundary finding from comment 5533718322 is now repaired.

  • RED: 54944f238c29fa856bf3226dad610c2d74935eef
  • book-period fix: fe7fc984b12797cb45709489e4b4af5ea3f12eeb
  • ADR correction: d4752b8c9f534a22316ec3f6b4080201e78311fc
  • real PostgreSQL regression: d5b90240004e24fc2b40a01326fb9c747a4c2df1
  • current head: 8afbf102bfaeccd8ffacee83df8f8863a3676a03

financial_report_run now composite-FKs current and comparison periods to accounting_book_period_control, and the insert trigger derives source_period_status_code from that selected book-period row. The PostgreSQL regression applies migration 0020 in a rollback-only real DB transaction and checks caller authority-field overwrite, cross-book source rejection, future snapshot rejection, source immutability, and FORCE RLS catalog state.

Please re-review the repaired authority boundary and the test isolation/transaction semantics. The PR remains Draft; queued CI is not treated as passing.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Current HEAD and PR metadata =='
git rev-parse HEAD
gh pr view 52 --json number,isDraft,baseRefName,headRefName,commits,files \
  --jq '{number,isDraft,baseRefName,headRefName,headCommit:.commits[-1].oid,files:[.files[].path]}'

echo
echo '== Migration and test candidate files =='
fd -t f . | rg '(^|/)(0020_financial_report_source_registry\.sql|test_.*financial.*report.*|migration_install\.py|0068-financial-report-source-authority\.md|0009_accounting_book_period_control\.sql)$'

echo
echo '== Outlines =='
ast-grep outline src/accounting_information_platform/migration_install.py --items all || true
fd -t f -e py tests | rg 'financial.*report|report.*financial|postgres.*report' | while read -r f; do
  echo "----- $f -----"
  ast-grep outline "$f" --items all || true
done

echo
echo '== Migration 0020 =='
cat -n database/migrations/0020_financial_report_source_registry.sql

echo
echo '== Book-period control migration =='
cat -n database/migrations/0009_accounting_book_period_control.sql

echo
echo '== Installer references =='
rg -n -C 8 '0020_financial_report_source_registry|apply_foundation_migration|migration' \
  src/accounting_information_platform/migration_install.py \
  src/accounting_information_platform/persistence.py

echo
echo '== Regression test implementations =='
fd -t f -e py tests | rg 'financial.*report|report.*financial|postgres.*report' | while read -r f; do
  echo "----- $f -----"
  cat -n "$f"
done

echo
echo '== ADR 0068 =='
cat -n docs/adr/0068-financial-report-source-authority.md

echo
echo '== Relevant recent commits =='
git show --stat --oneline --no-renames \
  54944f238c29fa856bf3226dad610c2d74935eef \
  fe7fc984b12797cb45709489e4b4af5ea3f12eeb \
  d4752b8c9f534a22316ec3f6b4080201e78311fc \
  d5b90240004e24fc2b40a01326fb9c747a4c2df1 \
  8afbf102bfaeccd8ffacee83df8f8863a3676a03

Length of output: 3919


🏁 Script executed:

#!/bin/bash
set -u

echo '== Migration 0020 =='
cat -n database/migrations/0020_financial_report_source_registry.sql || true

echo
echo '== Book-period control =='
cat -n database/migrations/0009_accounting_book_period_control.sql || true

echo
echo '== PostgreSQL source-registry regression =='
cat -n tests/test_postgres_financial_report_source_registry.py || true

echo
echo '== Book-period authority regression =='
cat -n tests/test_financial_report_book_period_authority.py || true

echo
echo '== Migration installer =='
cat -n src/accounting_information_platform/migration_install.py || true

echo
echo '== Source installation test =='
cat -n tests/test_financial_report_source_install.py || true

echo
echo '== Snapshot and tenant helper definitions referenced by 0020 =='
rg -n -C 12 \
  'CREATE TABLE accounting_reporting\.trial_balance_snapshot|CREATE( OR REPLACE)? FUNCTION accounting_core\.current_tenant_account_id|CREATE TABLE accounting_core\.accounting_book|CREATE TABLE accounting_core\.fiscal_period' \
  database/migrations || true

echo
echo '== Other migrations that modify trial_balance_snapshot or account_book_period_control =='
rg -n -C 5 \
  '(trial_balance_snapshot|accounting_book_period_control)' \
  database/migrations/0001_accounting_foundation.sql \
  database/migrations/0005_closed_period_guard.sql \
  database/migrations/0009_accounting_book_period_control.sql \
  database/migrations/0010_soft_close_command_evidence.sql || true

echo
echo '== Relevant PR diff only =='
git diff --no-ext-diff --unified=30 \
  origin/feat/financial-reporting-xbrl-export...HEAD -- \
  database/migrations/0020_financial_report_source_registry.sql \
  src/accounting_information_platform/migration_install.py \
  tests/test_postgres_financial_report_source_registry.py \
  tests/test_financial_report_book_period_authority.py \
  tests/test_financial_report_source_install.py \
  docs/adr/0068-financial-report-source-authority.md || true

Length of output: 81273


🏁 Script executed:

#!/bin/bash
set -u

echo '== PostgreSQL fixture lifecycle and cleanup =='
rg -n -C 25 \
  'class PostgresPostingTests|def setUpClass|def tearDownClass|def setUp\(|def tearDown\(|def doCleanups|DATABASE_URL|psycopg\.connect|rollback\(|commit\(' \
  tests/test_postgres_posting.py || true

echo
echo '== Fixture helper implementations used by the source-registry regression =='
ast-grep outline tests/test_postgres_posting.py --items all || true
rg -n -C 18 \
  'tenant_id|policy|accounting_book_reference|INSERT INTO accounting_core\.fiscal_period|INSERT INTO accounting_core\.accounting_book_period_control|SET app\.tenant_account_id' \
  tests/test_postgres_posting.py || true

echo
echo '== Period-state synchronization and fiscal-period mutation paths =='
rg -n -i -C 12 \
  'UPDATE accounting_core\.fiscal_period|period_status_code.*fiscal_period|fiscal_period.*period_status_code|accounting_book_period_control.*period_status_code|CREATE( OR REPLACE)? FUNCTION.*period' \
  database/migrations src/accounting_information_platform tests || true

echo
echo '== Base migration installer transaction behavior =='
rg -n -C 20 \
  'def apply_foundation_migration|psycopg\.connect|autocommit|execute\(.*migration|read_text\(.*encoding' \
  src/accounting_information_platform/persistence.py || true

echo
echo '== Registry test commits and transaction control =='
rg -n -C 5 \
  '\.commit\(|\.rollback\(|autocommit|psycopg\.connect|_apply_registry_inside_transaction' \
  tests/test_postgres_financial_report_source_registry.py || true

echo
echo '== Exact migration chain available in the checkout =='
fd -t f -e sql . database/migrations | sort || true

Length of output: 155443


@seonghobae I verified that the original authority-boundary finding is repaired.

bind_financial_report_run_scope() now derives source_period_status_code from the selected accounting_book_period_control row. The composite foreign keys also require both current and comparison periods to exist for the selected accounting book.

The PostgreSQL regression creates a conflicting state. It sets the selected book-period row to hard_closed while the initial tenant-global fiscal_period state remains open. The asserted report value is hard_closed. This proves that the report run uses the book-scoped value.

The rollback-only test design is correct. The test removes migration 0020's outer transaction statements, runs the DDL in psycopg's default transaction, and calls rollback() on every connection. The expected trigger failures abort their transactions, and the test recreates the connection before further statements.

I found one remaining installer-order defect.

  • src/accounting_information_platform/persistence.py::apply_foundation_migration() installs only migrations 0001 through 0014.
  • src/accounting_information_platform/migration_install.py::apply_foundation_migration() then installs migration 0020.
  • The stated integration contract requires migration 0020 to run after the concurrent migrations 0015 through 0019.

When the stacks integrate, the public installer will skip migrations 0015 through 0019 and apply 0020 directly after 0014. Update the installer manifest and its failure-closed checks to require and execute migrations 0015 through 0019 before 0020_financial_report_source_registry.sql. Add an installer regression that verifies this execution order.

I did not execute the PostgreSQL test suite. Queued CI remains non-passing evidence.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

Fresh foundation review promotes this branch's source-population freeze finding to the canonical Period Close owner boundary. Protected/inherited close schema currently documents trial_balance_snapshot as immutable but does not independently freeze the committed snapshot/header/line population. Draft PR #53 (fix(close): freeze hard-close trial balance evidence) is now stacked on exact #47 and owns the test-first foundation repair.

Keep this PR and its RED. At the later non-force restack after the reconciliation/close foundation integrates, consume the protected snapshot-immutability invariant and rerun this report-source RED as consumer acceptance. If #53 fully makes hard-close snapshot header/line populations immutable and concurrency-safe, do not duplicate a narrower report-source-only mutation mechanism here; retain only linkage/scope/cutoff checks that are specific to financial_report_source. The report registry remains provisional and must still be renumbered from its branch-local 0020 after the reconciliation chain integrates.

seonghobae commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Owner handoff from Period Close foundation PR #53: the underlying retained trial_balance_snapshot / trial_balance_line population now has a database-bound repair candidate at current exact #53@5cdcbd1cd6bc74fb2cfa988f454934b1e5be17a5 (0029_trial_balance_snapshot_immutability.sql). It rejects header/line UPDATE+DELETE and serializes line admission on accounting_book_period_control, rejecting append after hard_closed with trial_balance_snapshot_immutable.

Do not duplicate that invariant here as a private financial_report_source_population_frozen mechanism. Keep this PR Draft on its current parent. After the reconciliation/Period Close foundation integrates into protected develop, non-force restack #52 onto that exact protected head, renumber its provisional reporting migration, then adapt the current source-population RED so the report registry proves it consumes the foundation immutability boundary rather than owning a second mutation authority. #53 itself is not GREEN yet; exact-head Accounting Foundation run 33888350597 remains queued, so this is a dependency handoff, not released authority.

Copy link
Copy Markdown
Contributor Author

#53 Period Close source-authority handoff: after the foundation is normally integrated and this Reporting lane non-force restacks, consume the exact book-period retained-snapshot pair rather than reimplementing it. The foundation now requires both directions at commit: retained snapshot => exact accounting_book_period_control is hard_closed, and transition to hard_closed => exact retained trial_balance_snapshot exists for the same tenant/book/period. The inverse RED starts at bea21ed65d9c9e8a79cb48a102f7688032baae0a; migration 0036 starts at 26e71eb4e5a8450159f5ced482de43176c80e0f6; current doctoring lineage reaches 780b66c0fe6b641fcaee15d76c16d78e138ab230. Reporting must not infer hard-close state from shared fiscal_period, create missing close controls/snapshots, or weaken retained snapshot immutability to satisfy report-source binding. Your provisional migration number remains branch-local until the reconciliation/Period Close chain integrates.

Copy link
Copy Markdown
Contributor Author

Follow-up Reporting boundary from #53: migration 0036 now also refuses a legacy one-sided hard-close/snapshot population before its future-write trigger is installed. Do not make Reporting “repair” either marker by synthesizing a snapshot, flipping book-period status, falling back to shared fiscal_period, or weakening retained-evidence immutability. A report-source registry should consume only a certified exact tenant/book/period retained snapshot after the Period Close foundation has integrated. Current upgrade lineage is f1935c8515a74967355cfc5ffd96ef4c134500a2 (real PostgreSQL RED) -> 08c7b0f25bcdae7e5b223108eb68d06a6cf3a29a (FORCE-RLS-safe preflight) -> 4536a84c35371926fb6e128fa92e5c3d109d7e07 (static ratchet), with doctoring/operability through 6cc80d7... / 9923941.... Keep your migration number provisional until normal non-force restack.

Copy link
Copy Markdown
Contributor Author

Foundation handoff from #53: when this reporting branch is eventually non-force restacked onto the integrated Period Close foundation, consume existing accounting_book_period_control only. Do not synthesize missing controls, do not infer per-book close state from shared fiscal_period, and do not introduce a reporting-owned writer for book-period authority. Raw/direct control creation is now intended to fail explicitly with database check_violation; report-source binding must surface missing authority as a prerequisite/error instead of reconstructing it. The retained snapshot <-> hard-closed control pair and 0036 legacy-preflight remain upstream invariants, not reporting-owned logic.

Copy link
Copy Markdown
Contributor Author

Reporting-Export handoff from #53: current-role catalog state must not be used to reinterpret historical posted journal semantics. Period Close now has RED evidence showing why account_role_mapping ... valid_to IS NULL is unsafe for historical P&L classification when the immutable posted line already carries account_role_code. Audit reporting paths such as operational-income projections for the same pattern and consume posted line roles or immutable close snapshots after the foundation is integrated. Do not create report-owned close state or a second historical-classification authority; restack non-force after the canonical #53 foundation is green/integrated.

Copy link
Copy Markdown
Contributor Author

Reporting-Export handoff from #53: when this lane is eventually non-force restacked onto the integrated Period Close foundation, do not reconstruct historical journal semantics from only the current effective account_role_mapping. An immutable posted line already carries journal_entry_line.account_role_code, and a hard-close snapshot is retained accounting evidence. Effective-dated mapping changes after posting must not make historical report-source semantics disappear or change. Keep the reporting registry read-only with respect to close/posting authority; use integrated immutable posted/snapshot evidence and keep current mapping as current policy/presentation input only where that distinction is explicit and tested.

Copy link
Copy Markdown
Contributor Author

Reporting-Export handoff from #53: when this lane eventually consumes integrated Period Close evidence, preserve exact historical chart_account_id/posted-role identity from AIS evidence. Do not reconstruct a historical source account solely from the current active chart-account code or current account_role_mapping, and do not make report-catalog completeness a second authority over hard close. The retained-earnings destination is different: it is a new close-time decision and may use the current effective mapping. Keep this PR Draft/provisional until the foundation integrates and you can non-force restack/renumber normally.

Copy link
Copy Markdown
Contributor Author

Reporting-Export handoff from #53 current RED 06342053f2937e94748b40ed9182b20cfbf0ef74: consume historical account identity from immutable AIP journal/snapshot evidence after the Period Close foundation integrates. Do not reconstruct historical chart_account_id/role from the current effective catalog, and do not make report-source/catalog completeness a second hard-close authority. A Reporting projection can fail closed with its own next action, but it must not veto an otherwise valid ledger/trial-balance hard close. Keep the provisional reporting migration isolated until normal non-force restack onto the integrated accounting foundation.

Copy link
Copy Markdown
Contributor Author

#53 Period Close downstream boundary update — current exact successor RED f8d199b222f2ceaf1ed83e2ce6664abd8ca32861 on #47@a089ea41608e669db74430d12511909484b5ac55.

A new realistic code-reuse RED (c5266ce29c181474331e8a4b035f6d57d185ed2d) proves why Reporting must consume immutable posted/snapshot identity rather than reconstruct historical accounts from the current catalog: after the original usage_revenue account/mapping ends and a successor Entity reuses 410100, Period Close must clear the original journal_entry_line.chart_account_id, not the successor. Separately, hard-close authorization must not depend on successful construction of the buyer Reporting-Export package; report catalog completeness can fail as a reporting read without acquiring veto authority over an otherwise balanced immutable ledger population.

Keep #52 Draft and non-force restack only after the Period Close foundation integrates. Do not add a report-owned close writer, current-code historical identity fallback, shared fiscal_period authority, or a second snapshot/freshness/currency/calculation authority.

Copy link
Copy Markdown
Contributor Author

#53 foundation handoff, current traceability head 0602fc599e067e3c046ab84135fc0edf7c423472: Reporting must continue to consume book-period state and retained evidence; it must not recreate Period Close transition authority. In particular, do not perform close-state mutation at weak isolation, do not create/reseed journal-population fences from Reporting, and do not infer a missing book-period control from shared fiscal_period. The canonical close lane now fails weak transition isolation closed and keeps runtime FORCE-RLS fence seeding tenant-bound subject to PostgreSQL effective-role bypass semantics. The separate #53 REDs remain important for this PR: current-catalog Reporting completeness must not veto an otherwise ledger-valid hard close, and historical account identity must come from immutable journal/snapshot evidence rather than current catalog reconstruction. Consume only after normal #53 integration/non-force restack; do not copy its mutable branch source.

Copy link
Copy Markdown
Contributor Author

Period Close foundation handoff from #53: Reporting-Export must consume an authoritative soft_closed book-period only when the ledger control has its durable original soft-close command evidence. Do not infer or bless missing evidence from shared fiscal_period, current report/catalog state, or later balances, and do not create a second soft-close/period-close authority. If the foundation migration reports legacy incomplete soft-close evidence, surface the fail-closed/audited-remediation state rather than synthesizing report-side authority. Consume only after normal #53 integration and non-force restack.

seonghobae commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

/opencode

Exact-log/source RCA for current RED head f4354a2d3c15b6270dbedb834372deb37e4be05d. Preserve this as the single active #52 source-writer instruction; do not create a second writer.

Foundation run 33876130106 is terminal failure in Run behavior and repository tests. Raw hosted PostgreSQL job 101033427848 ran 736 tests with 8 failures / 0 errors. The intended source-population freeze tests did not reach a missing financial_report_source_population_frozen guard.

Verified causal precursor:

  • PostgresPostingTests.setUpClass() installs the migration chain once; migration 0009_accounting_book_period_control.sql backfills controls only for books/periods that exist at migration time.
  • each PostgresPostingTests.setUp() then creates the isolated tenant/entity/book/fiscal-period fixture after that backfill, so the primary book has no accounting_book_period_control row;
  • both population-freeze tests, both recording-time tests, and the source-registry/authority cases then stop in _accounting_scope() because its INNER JOIN correctly requires that authoritative book-period row;
  • _insert_sibling_book() already demonstrates the lawful test-fixture shape by inserting a book-period-control row for the sibling book. Repair the primary fixture equivalently in test setup; do not copy mutable fix(close): freeze hard-close trial balance evidence #53 production materialization or create a second Period Close authority merely to make this branch pass.
  • test_readme_names_latest_checked_in_migration is a separate repository-contract defect: it lexicographically treats every checked-in *.sql as the installed endpoint. This branch explicitly declares 0020_financial_report_source_registry.sql provisional/colliding, so scope the contract to the canonical executable manifest/installer authority (or otherwise represent the proposal outside that installed set). Do not falsify README by advertising provisional 0020 as integrated.

Repair order remains TDD-strict: (1) repair only fixture/README precursor contracts; (2) rerun real PostgreSQL and require the population-freeze cases to reach their hostile UPDATE/INSERT and fail specifically because financial_report_source_population_frozen is absent; (3) only then add the smallest snapshot-header/line population serialization/freeze control, with rollback, RLS, concurrency and exact-head 100% statement/branch evidence.

Dependency refresh: stored PR base remains #50@38c43eae...; live #50 is now 50cf7c5fb15ca65272748e165ab7351f5925dc2a with exact-head Foundation GREEN. Do not inherit that evidence. Adopt current #50 only by ordinary/non-force restack while preserving this RED lineage. Final reporting migration identity still waits for reconciliation/Period Close integration and renumbering.

seonghobae commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Exact-head evidence correction (2026-09-07): f4354a2d3c15b6270dbedb834372deb37e4be05d is no longer queued. Accounting Foundation run 33876130106 reached a hosted PostgreSQL runner and completed failure; job 101033427848 passed checkout/dependency setup and failed at step 8 Run behavior and repository tests, so coverage/repository-contract/compile/package stages were skipped. Exact-head dependency diff 101033427949, SAST 101033428213, and security 101033428505 succeeded.

The raw job log is now recoverable and fixes the earlier uncertainty: 736 tests ran with 8 failures, 0 errors. Seven failures are precursor fixture/authority failures rather than the intended snapshot-freeze RED: _accounting_scope() cannot find the tenant/entity/book/period because its INNER JOIN requires accounting_core.accounting_book_period_control, while this #50-based PostgreSQL seed path does not create that control. The affected cases include both FinancialReportSourcePopulationFreezeTests, both recording-time tests, the source-registry test, and FinancialReportAuthorityRegistryTests. The eighth failure is test_readme_names_latest_checked_in_migration: it expects branch-local provisional 0020_financial_report_source_registry.sql while README still truthfully names executable foundation migration 0014.

An existing /opencode owner comment 5563324859 already owns the bounded RED-harness repair, so do not create a competing source writer. Its correct first task is to make the real PostgreSQL tests establish lawful book-period control through the owning foundation path and reach the intended report-source mutation; only after the exact mutation fails for the intended freeze invariant should production freeze code be added. Do not make README falsely advertise a provisional/colliding migration as installed; distinguish checked-in proposal from canonical installer authority. Keep #52 Draft on #50 and do not copy mutable #53/#47 source before normal foundation integration/restack/renumber.

seonghobae commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Parent handoff refresh: #50 has advanced normally to exact 50cf7c5fb15ca65272748e165ab7351f5925dc2a; its Accounting Foundation run 34083432534 is terminal GREEN with 496 behavior/repository tests, 5,508/5,508 production statements, 2,000/2,000 branches, repository contracts, compile/import, reproducible wheel and SBOM/provenance verification. #52 remains exact f4354a2d3c15b6270dbedb834372deb37e4be05d on stored parent #50@38c43eae... and must not claim that newer evidence by inheritance. Preserve the existing branch single-writer: first repair the already-proven 736 tests / 8 failures / 0 errors precursor REDs as specified, then adopt the current #50 delta by ordinary/non-force integration when doing the dependency restack; do not overwrite #50's new JSON context-type regressions or its evidence. Reconciliation migration-number integration remains a separate prerequisite before assigning #52's final forward migration identity.

Copy link
Copy Markdown
Contributor Author

Correction to the earlier fixture wording in comment 5563324859: do not repair the seven precursor failures by copying _insert_sibling_book()'s raw accounting_book_period_control INSERT for the primary book. That would let Reporting tests synthesize Period Close-owned authority.

The existing owner path is PostgresPostingLedger._lock_book_period(): it resolves the canonical fiscal period, performs the foundation-owned item UPSERT for active books, and locks/returns the selected book-period control without changing it from open. The bounded fixture repair should first resolve tenant/legal-entity/book/fiscal-period without joining accounting_book_period_control, then call self.case.ledger._lock_book_period(connection, tenant_id, book_id, period_code) and require the returned period identity/status to match the seeded scope. Reuse that helper from the registry, recording-time, authority, and population-freeze tests rather than adding report-owned SQL or a second control initializer.

Keep the README contract repair separate: derive the executable endpoint from migration_install.py's canonical manifest, not lexicographic database/migrations/*.sql, because this branch's reporting 0020 remains provisional/colliding. Then rerun the same hosted PostgreSQL path and require the two population-freeze cases to reach the hostile snapshot UPDATE/line INSERT and fail specifically for the absent financial_report_source_population_frozen invariant before production freeze code is added.

seonghobae commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

/opencode single-source-writer continuation — exact 5a26eba357d09a0e65933da1e5ced99197130ecf.

Do not create another competing lifecycle fix. Real-PostgreSQL RED c7d4ef300df6acf58bf8d5d5fd1799f82d509b23 proved raw SQL could mutate financial_report_run.run_status_code to superseded without purpose-bound command evidence. Production descendant 5a26eba357d09a0e65933da1e5ced99197130ecf now rejects that direct mutation with stable marker financial_report_run_lifecycle_immutable; hosted Foundation 34271427012 shows test_direct_sql_cannot_supersede_report_run_without_command_evidence GREEN.

Current exact-head behavior is 520 tests / 2 failures / 0 errors. The only failures are the existing canonical Period Close dependency REDs: test_linked_snapshot_header_cannot_be_rewritten and test_linked_snapshot_cannot_gain_new_lines. Do not implement financial_report_source_population_frozen or an equivalent Reporting-owned snapshot-freeze trigger/function. Preserve #52 as Draft and preserve those REDs until #53 enters protected lineage.

After #53 normal protected integration, non-force rebuild/restack #52 from the resulting exact protected authority, preserve #50 valid deltas, renumber provisional migration 0020 to the next unclaimed forward number, reconcile installer/docs/tests, adapt the two dependency regressions to the canonical Period Close invariant, then reacquire real PostgreSQL behavior, 100% owned production statement/branch coverage, repository contracts, security/SAST/dependency/package/SBOM/provenance and current review gates on one unchanged exact head. No self-approval, bypass, force-push, destructive rebase, synthetic status or premature Ready transition.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Exact-head review on af95c4b098431e3c87e3dffa225de9e8c00c9d39 after hosted PostgreSQL Foundation 34268553044 completed.

The precursor #52-local defects are repaired: lawful book-period fixture materialization, README executable-endpoint contract, required close_idempotency_key, and imported-TestCase rediscovery no longer block the suite. Dependency diff, SAST and Security are GREEN.

The remaining two behavior failures are both valid and reach the intended hostile database mutation: the linked retained snapshot header can still be rewritten and the linked snapshot can still gain a line because no Period Close-owned immutability control rejects them. This is not permission to add a Reporting-local financial_report_source_population_frozen authority. Retained trial_balance_snapshot / trial_balance_line immutability belongs to Period Close #53, which is still Draft and outside protected lineage.

Keep #52 Draft and preserve these as dependency REDs. Do not skip/xfail/delete the tests, weaken constraints, copy mutable #53 SQL, or create a second Reporting-owned snapshot freeze. After #53 integrates normally, rebuild/restack #52 non-force on the resulting protected authority, renumber the provisional reporting migration, and adapt the regressions to assert the canonical Period Close invariant before reacquiring exact-head GREEN.

This COMMENT is review evidence only; it is not an approval or merge authorization.

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

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

1 participant