Skip to content

feat: Add report package lifecycle and statement envelope rendering#643

Merged
jfrench9 merged 7 commits intomainfrom
feature/financial-viewer-envelope
Apr 26, 2026
Merged

feat: Add report package lifecycle and statement envelope rendering#643
jfrench9 merged 7 commits intomainfrom
feature/financial-viewer-envelope

Conversation

@jfrench9
Copy link
Copy Markdown
Member

Summary

Introduces report package handling, filing lifecycle operations, and enhanced statement envelope rendering projections for the financial viewer. This feature builds out the core infrastructure for managing report packages through their lifecycle — from ingestion to materialization — while improving how financial facts are retrieved and rendered within statement envelopes.

Key Accomplishments

Report Package Lifecycle

  • Added new database migration (0006_report_package_lifecycle) to support report package state management
  • Implemented CQRS-style commands for report filing lifecycle operations (create, transition, manage state)
  • Added read-side query operations for retrieving report packages with filtering and projection support
  • Exposed new REST API routes under the roboledger extensions router for report package operations
  • Extended the roboledger report model with lifecycle and FactSet relationship capabilities

Statement Envelope & Rendering

  • Significantly expanded statement rendering logic (~378 lines of new/modified code) to support richer rendering projections
  • Updated fact retrieval logic within statement envelopes to improve accuracy and completeness
  • Enhanced envelope operations with new projection and composition capabilities
  • Refined metric and schedule operations to align with the updated statement rendering pipeline

API & GraphQL Layer

  • Added new report_package GraphQL type with full resolver support
  • Extended information_block GraphQL type and API model with new fields/projections
  • Added new API models for report package extensions
  • Updated ledger GraphQL resolvers to support new report package queries

SEC Adapter Enhancements

  • Improved XBRL graph processing with additional relationship handling
  • Enhanced SEC filing classification logic to better support report package ingestion

Infrastructure

  • Updated feature flag resolution to use boto3 directly instead of awscli for staging/prod environments, improving reliability and reducing external CLI dependencies

Breaking Changes

  • The database migration 0006_report_package_lifecycle introduces new schema elements that must be applied before deployment
  • Changes to information_block API models and types may affect existing consumers if they rely on the previous response shape — review models/api/information_block.py and types.py for details
  • Statement handler logic has been substantially reworked; downstream consumers of statement rendering should verify compatibility

Testing

  • New test suites added:
    • test_reports_filing.py — 182 lines covering report filing lifecycle commands (create, state transitions, error cases)
    • test_reports_package.py — 212 lines covering report package read operations (filtering, projections, edge cases)
    • test_envelope_fact_set.py — Extended with new test cases for envelope fact set rendering
  • Existing tests updated:
    • test_reads.py — Expanded to cover new read operation paths
    • test_statement_handlers.py — Updated for revised statement rendering logic
    • test_materialize.py — Minor update to accommodate new materialization flow

Infrastructure Considerations

  • A new database migration must be executed against the extensions database prior to deploying this change
  • The switch from awscli to boto3 for feature flag resolution requires that the runtime environment has appropriate IAM permissions and boto3 available (standard in most deployment contexts)
  • New API routes are additive and follow existing router conventions under the roboledger extensions namespace

🤖 Generated with Claude Code

Branch Info:

  • Source: feature/financial-viewer-envelope
  • Target: main
  • Type: feature

Co-Authored-By: Claude noreply@anthropic.com

@jfrench9
Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude
Copy link
Copy Markdown

claude Bot commented Apr 26, 2026

Claude finished @jfrench9's task in 4m 54s —— View job


PR Review — feat: Add report package lifecycle and statement envelope rendering

  • Read key changed files
  • Review migration and data model changes
  • Review operations layer (commands, reads, statement rendering)
  • Review API/GraphQL layer
  • Review tests
  • Review SEC adapter and infrastructure changes
  • Compile and post final review

Overall this is a well-structured feature. The two-lifecycle model (generation vs. filing), the FactSet-pinned read path, and the server-side rendering projection are all solid architectural decisions. The lazy imports to break circular-dependency cycles are applied consistently. A few issues worth fixing before merge are called out below.


Critical

Filed reports can be deleted

delete_report() in robosystems/operations/roboledger/commands/reports.py:536 checks ownership but not filing_status. A user can delete a filed or archived report, bypassing the immutability guarantee described in the model docstring. Regeneration may have the same problem — worth checking. Fix this →

# robosystems/operations/roboledger/commands/reports.py:542-546
report_def = session.get(Report, report_id)
if report_def is None:
    return False
if report_def.created_by != acting_user_id:
    raise NotAuthorizedError("Not authorized to delete this report.")
# ← no check for filing_status here — filed reports can be deleted

Medium

filed_at stored in timezone-naive column

filed_at is declared as Column(DateTime, nullable=True) in robosystems/models/extensions/roboledger/report.py:86 and as bare TIMESTAMP in migrations/extensions/versions/0006_report_package_lifecycle.py:49. The rest of the codebase uses DateTime(timezone=True) for audit timestamps (see models/core/user/user_repository_credits.py), and the README models also specify DateTime(timezone=True). Meanwhile file_report() correctly stamps datetime.now(UTC) — a timezone-aware value stored in a naive column. This works for PostgreSQL but will cause subtle off-by-one reads if the column is ever compared with naive datetimes. Fix this →

TransitionFilingStatusRequest.target_status has no input validation

target_status: str in robosystems/models/api/extensions/report_package.py:52 accepts any string. Invalid values only fail at the operations layer inside transition_filing_status(). A Pydantic Literal["draft", "under_review", "archived"] (or field_validator) at the request boundary gives better OpenAPI docs and a 422 with a clearer message before hitting the DB. Fix this →

Importing private symbols from fact_grid

statement.py:64-66 imports _Balance, _build_rows, and _HierarchyNode across module boundaries with # type: ignore[reportPrivateUsage]. The type: ignore suppresses the linter but the coupling is real. If these types are meant to be shared, they should be promoted to public exports in fact_grid.py. If not, the envelope rendering path should own its own copies (which _facts_to_balance_dict_for_period already does — inconsistently). The dual docstring comment "Mirrors fact_grid._facts_to_balance_dict" hints that the right call is to extract a shared public helper.


Minor / Nit

ReportNotFiledError is dead code

robosystems/operations/roboledger/commands/reports.py:457 defines ReportNotFiledError but it is never raised or imported anywhere in the codebase. Either wire it up (e.g. as a guard inside share_report before sharing a non-filed package) or remove it.

Step numbering off-by-one in _build_statement_rendering

statement.py has step # 3) appear twice — once for period derivation and again for building report_facts. The second occurrence through to # 7) is therefore off-by-one. Cosmetic but confusing to read.

unmapped_count is always hardcoded to 0

RenderingLite(... unmapped_count=0) in the rendering path never actually counts unmapped elements. If the field exists to surface hierarchy mismatches to the frontend, it needs to be computed (count elements with facts but no matching hierarchy node). If it's intentionally deferred, a TODO comment would signal that. Fix this →

archived terminal state gives a misleading error message

When transition_filing_status is called on an archived report, _LEGAL_NON_FILE_TRANSITIONS.get("archived", set()) returns set(), so the error reads: "Legal targets from here: []". A sentinel key "archived": set() with a distinct error message ("archived is a terminal state, no transitions possible") would be clearer.

except Exception in SEC classifier silently swallows errors

robosystems/adapters/sec/processors/classify.py in _build_structure_factsets catches Exception and logs at DEBUG when the Report lookup fails. Since this affects whether REPORT_HAS_FACT_SET edges land in the graph, a WARNING log with the full exception traceback would make SEC ingestion failures easier to detect.


Positive Observations

  • The orthogonal lifecycle model is clean and well-documented — the docstring in report.py is exactly the level of explanation this complexity warrants.
  • Replacing the "latest Report touching these elements" heuristic with the explicit FactSet pin is a clear simplification; the old heuristic had a silent failure mode.
  • The STRUCTURE_HAS_FACT_SET query fix in materialize.py (scanning fact_sets instead of facts) is more correct and removes the DISTINCT band-aid — good catch.
  • The lazy imports in file_report / transition_filing_status to break the command→reads circular dependency are applied consistently and clearly commented.
  • Test coverage for the lifecycle state machine is thorough — happy paths + error paths for all legal and illegal transitions are present.
  • The ReportPackage.from_pydantic delegation pattern keeps the GraphQL resolver thin and consistent with the rest of the resolver layer.

@jfrench9
Copy link
Copy Markdown
Member Author

@claude can you review the latest changes

@claude
Copy link
Copy Markdown

claude Bot commented Apr 26, 2026

Claude finished @jfrench9's task in 4m 18s —— View job


PR Review — feat: Add report package lifecycle and statement envelope rendering

  • Review git diff vs origin/main to see all changes
  • Read key changed files (migration, model, commands, reads, statement, API layer)
  • Review tests
  • Compile and post final review

The latest commit (11d6757) adds router + GraphQL tests, fixes the report_package.py date serialization regression (removed manual .isoformat() calls in favour of native datetime objects — good catch), switches the get_report_package outer join to an inner join with a clear comment, and improves the dedup loop in _build_structure_factsets. Solid incremental polish.

None of the items from the prior review have been addressed. They're reproduced below, with a few new observations from this pass.


Critical

Filed reports can be deleted (commands/reports.py:~542)

delete_report() checks ownership but not filing_status. A user can delete a filed or archived report, bypassing the immutability guarantee described in the model docstring. Fix this →


Medium

filed_at stored in timezone-naive column

filed_at = Column(DateTime, nullable=True) in models/extensions/roboledger/report.py:86 and sa.Column("filed_at", sa.DateTime(), nullable=True) in the migration at line 103 — both missing timezone=True. file_report() correctly stamps datetime.now(UTC), a timezone-aware value, into a naive column. Works in PostgreSQL but causes subtle issues on cross-timezone comparisons. The rest of the audit columns use DateTime(timezone=True). Fix this →

TransitionFilingStatusRequest.target_status accepts any string

target_status: str in models/api/extensions/report_package.py:52 has no Pydantic validation. Invalid values only fail at the ops layer. A Literal["draft", "under_review", "archived"] here gives better OpenAPI docs and a clean 422 at the request boundary. Fix this →

Importing private symbols from fact_grid

statement.py:64–66 imports _Balance, _build_rows, _HierarchyNode with # type: ignore[reportPrivateUsage]. The linter suppression hides the coupling but doesn't eliminate it. If these are meant to be shared they should be exported from fact_grid.py; otherwise statement.py should own its own copies (as _facts_to_balance_dict_for_period already effectively does — a duplicate that started life as "mirrors fact_grid._facts_to_balance_dict"). The dual-docstring comment confirms the right long-term fix is extracting a shared public helper.

No server-side guard: file_report() allows filing an unfinished report

file_report() allows filing from draft or under_review regardless of generation_status. The docstring says "a UI normally gates the action on completion" but there's no server-side enforcement — a client that skips the UI can file a pending or generating report. Either add a guard or make the docstring a policy note that this is a deliberate flexibility (and accept that the filed snapshot may be empty). Fix this →

get_report_package silently drops unregistered block types with no log

When get_information_block_for_fact_set returns None (FactSet's structure_type isn't a registered block type), the item is silently skipped. The caller receives a shorter-than-expected package with no indication of the gap. Add a logger.warning(...) before continue. Fix this →


Minor / Nit

ReportNotFiledError is dead code — defined in commands/reports.py:457 but never raised or imported anywhere. Wire it up (e.g. inside share_report to guard sharing a non-filed package) or remove it.

Step # 3) appears twice in _build_statement_rendering — period derivation and the ReportFact conversion both carry # 3). The second occurrence through # 7) is off-by-one. Fix this →%22%20(Convert%20envelope%20Fact%20ORM%20rows)%20should%20be%20%22%23%204)%22%2C%20and%20subsequent%20steps%20should%20be%20renumbered%205-8%20accordingly.%20Branch%3A%20feature%2Ffinancial-viewer-envelope&repo=RoboFinSystems/robosystems)

unmapped_count is hardcoded to 0RenderingLite(... unmapped_count=0) in _build_statement_rendering never computes the actual count. If the field exists to surface hierarchy mismatches, it needs to count elements in report_facts with no matching node in rows. If intentionally deferred, add a # TODO comment. Fix this →

archived gives a misleading error messagetransition_filing_status on an archived report returns "Legal targets from here: []". A sentinel message ("archived is a terminal state, no transitions possible") would be clearer.

except Exception in _build_structure_factsets silently swallows errors at DEBUG — the Report lookup failure in classify.py is logged at DEBUG. Since a miss here means REPORT_HAS_FACT_SET edges never land, a WARNING with the exception string is more appropriate. Fix this →

del fact_set_id idiom in metric.py — using del to silence an unused-variable warning is idiomatic but surprising. The conventional approach is to rename the parameter _fact_set_id (or keep the name and add a # noqa: ARG002 suppressor).


Positive observations

  • The GraphQL date/datetime fix in report_package.py (11d6757) is the right call — passing native Python datetime / date objects to Strawberry rather than manually calling .isoformat() keeps schema types consistent with auto-derived Report.
  • The inner-join change in get_report_package is correct and well-commented.
  • get_information_block_for_fact_set and the fact_set_id pin flowing cleanly through load_base_envelope_atoms is a neat integration — the pin validation fails fast before the heavy atom loads.
  • Router test coverage for file-report and transition-filing-status (happy path + 404 + 422) is comprehensive.
  • Schedule fact_set_id filtering is correct: the in_scope + fact_set_id compound filter cleanly restricts the pinned snapshot to the facts that were in scope when the report was filed.
  • Deduplication of report_ids in _build_structure_factsets is a good defensive addition.

…olicies

- Implemented `oltp_writer.py` to write demo data into a DuckDB file for OLTPLoader.
- Created `policies.py` containing accounting policy documents for Cascade Advisory Group LLC.
- Added `prompt.md` for AI month-end close workflow instructions.
- Developed `validate.py` for Information Block validation, ensuring integrity of schedules and rules.
- Updated `justfile` to include a new demo command for RoboLedger.
- Refactored taxonomy command documentation to reflect changes in demo structure.
- Modified test cases to use the new session context for RoboLedger operations.
@jfrench9 jfrench9 merged commit 50da33b into main Apr 26, 2026
7 checks passed
@jfrench9 jfrench9 deleted the feature/financial-viewer-envelope branch April 26, 2026 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant