Post-review cleanup + 3 contained bugfixes (disclosure/rules follow-ups)#887
Merged
Conversation
… the CAP vocabulary Post-review cleanup (no behavior change) across the disclosure/rules surface: - Single-source the 15-value concept_arrangement vocabulary as CONCEPT_ARRANGEMENT_VALUES in the Structure model; build the CHECK from it (byte-identical SQL, no migration) and tie the API Literal to it with a drift test, replacing three inline copies. - Extract structure_from_request into taxonomy_block/_helpers.py; call from the reporting_extension / custom_ontology / update_apply handlers (was triplicated). - Extract rule_tolerance() and replace the four duplicated tolerance-from-metadata snippets. - Define DISCLOSURE_BLOCK_TYPE once (envelope.py) and reuse it, dropping the hardcoded string literals. - disclosure.py uses the public make_statement_handlers factory instead of reaching for a private statement internal. - XBRL disclosure strip also prunes now-unreferenced units, matching the facts/concepts/period_nodes pruning.
_load_period_balances with no fact_set_id filtered only by structure_id, summing every fact per element+period across all FactSets ever stamped on the structure. Two live reports covering the same period both stamp the structure, so the evaluate-rules op (unpinned) doubled every balance and could report a false RollUp failure attributed to a stale coexisting report. Resolve to the latest FactSet per structure instead — the FactSet is the summing unit, so multiple facts per element+period within one report still sum as intended. Also shares the rs-gaap-calculations DAG across a report's structures (loaded once, copied per structure so the local-arc overlay can't leak) instead of re-querying per structure, and adopts the DISCLOSURE_BLOCK_TYPE constant and rule_tolerance helper in these files.
Schedule fact scope (historical vs in_scope) is stamped at generation from closed_through. reopen-period retreated closed_through but never re-stamped existing facts, so a reopened month's facts stayed historical: its movement dropped out of the roll-forward (the carry-in rendered that month's ending balance as the opening balance) and the re-close skipped it. reopen-period now promotes the now-open window's schedule facts back to in_scope so every reader agrees with the calendar.
Registering regulatory_disclosure admitted the ~17 arc-less disclosures:* identity rows (rs-gaap-disclosures seeds one per tenant) into list-information-blocks pages. Their envelopes build to None but the SQL LIMIT/OFFSET is applied before that filter, so pages came back short and offset pagination misaligned. Guard the listing query so a regulatory_disclosure structure is only selected when it owns a presentation arc.
The DISCLOSURE_BLOCK_TYPE refactor moved the picker's block_type from an inlined SQL literal to a bound :disclosure_block_type param, so the existing assertion that 'regulatory_disclosure' appears in the compiled SQL no longer holds. Assert the bound param and its value instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups from a deep review of the v1.6.5→main range (PRs #881–#886). The larger disclosure-hardening findings (only reachable under authoring patterns no tenant does yet) are queued as a gate for the next disclosure increment; this PR ships the safe cleanup plus three bugfixes that affect shipped behavior today. No schema migration (the CAP-vocabulary refactor emits byte-identical CHECK SQL).
Bugfixes
evaluate-rules(nofact_set_id) summed facts across every coexisting report that stamped a structure, doubling balances and risking a false RollUp failure attributed to a stale report. Now scopes to the latest FactSet per structure (the FactSet is the summing unit, so within-report multi-facts still sum).reopen-periodretreatedclosed_throughbut never re-stamped schedule fact scopes, so a reopened month's movement dropped out of the roll-forward (its ending balance rendered as the opening balance) and the re-close skipped it. Reopen now promotes the now-open window's facts back toin_scope.disclosures:*identity rows enteredlist-information-blockspages (built toNoneafter LIMIT/OFFSET), returning short pages. The listing query now excludes disclosure structures with no presentation arc.Cleanup (no behavior change)
Single-sourced the
concept_arrangementvocabulary (+ drift test), extractedstructure_from_requestandrule_tolerancehelpers, oneDISCLOSURE_BLOCK_TYPEconstant, public-factory use indisclosure.py, XBRL unit pruning, and a per-report shared calc-DAG load.Verification
Full unit suite green (only the known pre-existing
test_incremental_equals_full_loadlocal-ordering flake, which passes in isolation); ruff/format/basedpyright/cf-lint clean; the reopen re-stamp SQL validated against the live schema; new unit tests for each fix.