feat: Enhance reporting and scheduling with rule evaluation improvements#635
Conversation
- Updated README to reflect changes in PostgreSQL usage for IAM and extensions. - Added 'SumEquals' rule pattern to taxonomy library and rule evaluation logic. - Introduced rule summary in report and schedule responses to aggregate rule evaluation results. - Implemented disposal of schedules with a new DisposeScheduleRequest and response model. - Enhanced ScheduleService to create FactSet rows and auto-generate SumEquals rules during schedule creation. - Added tests for report command helpers and schedule service to ensure correct FactSet handling and rule evaluations.
|
@claude please review this PR |
|
Claude finished @jfrench9's task in 3m 40s —— View job PR Review — feat: Enhance reporting and scheduling with rule evaluation improvements
This is a solid, well-structured PR. The architecture is consistent with the existing CQRS/OperationSpec patterns, the new tests are comprehensive and well-organized, and the accounting logic (dispose, truncate, manual entries) is careful about invariants. A few issues worth fixing before merge are listed below. Bugs1. Dead computation in
|
|
@claude have we fixed all issues? |
|
Claude finished @jfrench9's task in 1m 35s —— View job Issue Status Check — PR #635
3 of 7 issues are fixed. 4 remain. ✅ FixedBug #1 — Dead computation in Minor #5 — Minor #6 — Router docstring stale ❌ Not FixedBug #2 — Both functions still end with Logic #3 —
SELECT ROUND(SUM(value)::numeric, 2) AS total
FROM facts
WHERE element_id = :eid AND structure_id = :sid AND period_type = 'duration'This will still fail for truncated schedules — the sum of remaining facts won't equal The fix is either:
Security #4 — Raw exception leaked in The first except Exception as e:
return ShareResultItem(
target_graph_id=target_graph_id,
status="error",
error=f"Failed to validate target graph: {e!s}", # ← raw str(e), no log
)The second Minor #7 — Still no None-filtering: element_ids = {a.from_element_id for a in associations} | {
a.to_element_id for a in associations
}If any association has a null element_ids = {
x for x in (
{a.from_element_id for a in associations} |
{a.to_element_id for a in associations}
)
if x is not None
} |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| for gid, sid in self._cleanup: | ||
| try: | ||
| client.delete_schedule(gid, sid) | ||
| except Exception: |
| dr, cr = et.get("debit_element_id", ""), et.get("credit_element_id", "") | ||
| if dr and cr: | ||
| return dr, cr | ||
| except Exception: |
1 participant
Summary
This PR introduces significant enhancements to the reporting and scheduling functionalities across the platform, including expanded rule evaluation capabilities, new schedule management APIs, and comprehensive report command improvements.
Key Accomplishments
Reporting Enhancements
+238 lines), adding new reporting capabilities and workflows within the roboledger subsystemledgerGraphQL type field to support downstream reporting queriesScheduling Improvements
+70 lines) with new scheduling constructs and data structures+161 lines) for managing schedule lifecycle+73 lines) with additional business logic and integrity checks, aligning with the branch goal of schedule integrityRule Engine & Evaluation
+55 lines) with expanded evaluation logic+30 lines) to support additional rule types or conditionsAPI & Routing
+24 lines) for roboledger operations, exposing the new scheduling and reporting functionality via the API surfaceInfrastructure & Migrations
Breaking Changes
0002_taxonomy_library.pyhas been modified — ensure that environments already past this migration are assessed for compatibility or that the change is backward-compatible (e.g., a cosmetic/comment fix).Testing
test_reports.py,+190 lines) providing comprehensive coverage of the new reporting logictest_service.py,+37 lines) to cover the new scheduling integrity featuresInfrastructure Considerations
🤖 Generated with Claude Code
Branch Info:
feature/schedule-integritymainCo-Authored-By: Claude noreply@anthropic.com