Remove unused Account and Disclosure transaction nodes from graph schema

[jfrench9]

Summary

This refactor removes unused Account and Disclosure nodes from the graph schema and associated components to simplify the codebase and eliminate dead code. The changes span across graph middleware, routing, schema definitions, and test files.

Key Accomplishments

• Cleaned up graph schema: Removed Account and Disclosure node definitions from the roboledger extension
• Updated documentation: Corrected node counts and descriptions in README files to reflect the simplified schema
• Streamlined routing: Removed unused transaction node references from graph routers and schema extensions
• Eliminated dead code: Removed 66 lines from roboledger.py and 107 lines of unused test code
• Improved clarity: Enhanced documentation to better describe remaining accounting features

Breaking Changes

⚠️ Potential Breaking Changes:

• Applications or services relying on Account or Disclosure nodes in the graph schema will need to be updated
• Any external integrations querying these specific node types will be affected
• Graph traversal logic that included these nodes may require adjustment

Testing Notes

• Removed comprehensive test suite for QuickBooks transaction processing (test_qb_transactions.py)
• Existing graph functionality should be validated to ensure no regressions
• Verify that metrics service and graph routing continue to function correctly without the removed node types

Infrastructure Considerations

• The schema simplification reduces complexity in graph operations and may improve performance
• Documentation updates ensure alignment between actual implementation and described capabilities
• Net reduction of 175 lines of code improves maintainability

This refactor prepares the codebase for future enhancements by removing legacy transaction node implementations that were no longer utilized in the current architecture.

---

🤖 Generated with Claude Code

Branch Info:

• Source: refactor/remove-unused-transaction-nodes
• Target: main
• Type: refactor

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

[jfrench9]

@claude please review this PR

[claude]

Claude finished @jfrench9's task —— View job

---

PR Review: Remove unused Account and Disclosure transaction nodes

Tasks

• Read and analyze all changed files
• Search for remaining references to removed nodes
• Review code quality and completeness
• Provide comprehensive feedback

---

Review Summary

This PR successfully removes unused Account and Disclosure nodes from the RoboLedger schema extension. The changes are well-executed and thorough, with proper documentation updates and no remaining references to the removed nodes.

✅ Strengths

1. Complete Removal: All references to Account and Disclosure nodes removed cleanly:

   • 2 node definitions removed (Account, Disclosure)
   • 4 relationships removed (ENTITY_HAS_ACCOUNT, ACCOUNT_HAS_TRANSACTION, PROCESS_HAS_DISCLOSURE, ACCOUNT_PARENT_CHILD)
   • Total: 66 lines removed from roboledger.py

2. Documentation Consistency: All documentation properly updated to reflect the simplified schema:

   • Node counts corrected (14 → 12 nodes) in main.py:484, 533 and extensions.py:108
   • README examples updated to use Element instead of Account (middleware/graph/README.md:338-340)
   • Schema diagram updated (schemas/README.md:102)
   • Context-aware loading descriptions updated

3. Code Quality Improvements:

   • Removed confusing comment about Account/Element balance conflicts in metrics_service.py:239
   • Removed 107 lines of unused QuickBooks test code
   • No breaking changes to actual functionality (nodes were unused)

4. Architectural Clarity: The PR correctly identifies that chart of accounts is now represented via the Structure/Association/Element pattern, which is more aligned with XBRL standards.

🔍 Observations

1. Schema Architecture: The removal makes sense because:

   • Account node duplicated functionality already provided by Element (XBRL taxonomy elements)
   • Disclosure node was unused and not integrated with any workflows
   • The Structure/Association/Element pattern from the reporting section provides a more flexible and standard-compliant approach to chart of accounts

2. No Remaining References: Comprehensive search confirms no dangling references:

   • No "Disclosure" references found anywhere
   • "Account" references in other files are legitimate (account_id fields, etc.)
   • All imports and type references properly cleaned up

3. Test Coverage: The removed test file (test_qb_transactions.py) was specific to QuickBooks integration and tested custom QBAccount/QBTransaction tables, not the removed schema nodes. This is appropriate cleanup.

📋 Recommendations

Minor Suggestions

1. Consider Adding Migration Guidance (Optional):

   • If any external systems were using these nodes, consider documenting the migration path
   • The PR description mentions breaking changes but could be more specific about the Element-based alternative

2. Verify Graph Creation Tests (Low Priority):

   • Ensure integration tests for graph creation with RoboLedger extension still pass
   • Verify that entity graphs with full accounting context load correctly with 12 nodes instead of 14

Code-Specific Feedback

robosystems/schemas/README.md:105 - Excellent addition of the note about chart of accounts representation:

- **Note**: Chart of accounts is represented via Structure/Association/Element pattern (from Reporting Section)

This helps users understand the alternative approach.

robosystems/routers/graphs/schema/extensions.py:104 - The excludes list is now cleaner and more accurate:

"excludes": ["Transaction", "LineItem", "Process"],  # Was 5 items, now 3

⚠️ Potential Risks (All Low)

1. Breaking Changes: As noted in PR description, any code expecting Account or Disclosure nodes will fail. However, since these were unused, this risk is minimal.

2. Data Migration: If any existing graphs have these node types populated, they'll become orphaned. Recommend verifying no production graphs have these nodes before merging.

3. Documentation References: While I didn't find any, check if external documentation (wikis, confluence, etc.) references these node types.

🎯 Verdict

APPROVED ✓

This is a clean refactoring that:

• Removes dead code (175 net lines deleted)
• Improves schema clarity and maintainability
• Aligns better with XBRL standards
• Updates all documentation consistently
• Has no impact on current functionality (nodes were unused)

The code changes are correct, complete, and well-documented. The reduction from 14 to 12 nodes in the full accounting context simplifies the schema without losing functionality.

📝 Suggested Next Steps

1. Merge after verifying no production graphs use these nodes (low risk)
2. Monitor for any unexpected issues in staging
3. Consider adding a schema validation check to warn if orphaned Account/Disclosure nodes exist in any graphs

---

Note: I cannot submit a formal GitHub PR review or approve the PR through the GitHub API, but based on code analysis, this PR is ready to merge pending final verification that no production data relies on these node types.

---

Job: View run
Branch: refactor/remove-unused-transaction-nodes