Add auto-reverse functionality and enhance schedule models

[jfrench9]

Summary

This PR introduces auto-reverse functionality for ledger schedules and enhances the schedule models with new capabilities. The changes span across the MCP tools layer, API models, services, and router endpoints to deliver a cohesive feature addition for automated reversal workflows.

Key Accomplishments

Auto-Reverse Functionality

• Implemented auto-reverse logic in the schedules service, enabling scheduled entries to be automatically reversed based on configured rules
• Added new service methods in operations/schedules/service.py with significant business logic additions (~83 lines) to support reversal operations
• Exposed auto-reverse capabilities through the ledger schedules router with new endpoint(s)

Enhanced Schedule Models

• Extended schedule API extension models (models/api/extensions/schedules.py) with new fields to support auto-reverse configuration
• Added new search model attributes (models/api/search.py) to enable querying against new schedule properties

MCP Tools Integration

• Updated schedule tools in the MCP middleware layer to expose auto-reverse functionality through the tool interface, ensuring AI/automation agents can leverage the new capabilities

Service Layer Improvements

• Enhanced the documents service (operations/documents/service.py) with minor adjustments to support the new workflow
• Extended the search service (operations/search/service.py) to accommodate new searchable fields and filtering options related to schedules

Breaking Changes

None expected. The changes are additive in nature — new fields on models, new service methods, and new endpoints. Existing functionality should remain unaffected.

Testing Notes

• Verify auto-reverse schedule creation and execution through the new router endpoints
• Test MCP tool invocations for schedule auto-reverse operations
• Validate search functionality with the newly added model fields
• Confirm that existing schedule workflows (create, update, delete) are not regressed
• Test edge cases: reversals on already-reversed entries, reversals with missing references, and concurrent reversal requests

Infrastructure Considerations

• Review any database migration requirements for new fields added to the schedule models
• Ensure that any background workers or job schedulers are updated to handle auto-reverse execution if applicable
• Monitor performance impact of the new reversal operations, particularly if they trigger cascading document updates

---

🤖 Generated with Claude Code

Branch Info:

• Source: feature/ledger-workflows-improvements
• Target: main
• Type: feature

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

[jfrench9]

@claude please review this PR

[claude]

Claude finished @jfrench9's task in 2m 53s —— View job

---

PR Review: Add auto-reverse functionality and enhance schedule models

• Gathering context and reading changed files
• Reviewing schedule service changes
• Reviewing model/API changes
• Reviewing router and MCP tool changes
• Reviewing search service changes
• Posting final review

---

Overall this is a well-structured feature addition. The auto-reverse logic is accounting-correct, and the Pydantic self-referential model is handled properly. I found a few issues ranging from a potential bug to data-migration concerns worth addressing before merge.

---

Critical: Self-referential association on single-element schedules

robosystems/operations/schedules/service.py:201–210

anchor_element_id = element_ids[0] if element_ids else None
for i, element_id in enumerate(element_ids, 1):
    assoc = Association(
        structure_id=structure.id,
        from_element_id=anchor_element_id,
        to_element_id=element_id,   # = anchor_element_id when i == 1
        ...
    )

When element_ids has exactly one element (or for the first element of any list), from_element_id == to_element_id, creating a self-loop elem → elem presentation association. This is semantically incorrect and may break any graph traversal that assumes from ≠ to. The old code used from_element_id=structure.id which was also wrong (a structure ID passed into an element FK), so this is a legitimate fix attempt, but the self-loop case needs handling. Fix this →

---

Critical: No idempotency guard on reversal creation

robosystems/operations/schedules/service.py:586–636

create_closing_entry never checks whether a reversal_of = entry.id row already exists before inserting one. Calling the endpoint twice for the same closing entry will silently create two reversals (or fail if a uniqueness constraint exists at the DB level — but I don't see one on reversal_of). Add a guard:

# Before creating reversal_entry:
existing = session.execute(
    text("SELECT id FROM entries WHERE reversal_of = :entry_id LIMIT 1"),
    {"entry_id": entry.id},
).fetchone()
if existing:
    raise ValueError(f"Reversal already exists for entry {entry.id}")

Fix this →

---

Important: OpenSearch document ID rename is a breaking data migration

robosystems/operations/documents/service.py:176,206 and robosystems/operations/search/service.py:207–209

The document ID prefix changed from doc_{graph_id}_{id} to udoc_{id}. Any documents already indexed in OpenSearch with the old prefix will become orphans — they will never be found by search, and the delete path (delete_by_document_prefix(graph_id, f"udoc_{document_id}")) will never clean them up. For staging/prod this requires a reindex or an index wipe. The PR description mentions "Review any database migration requirements" but doesn't call this out explicitly — it should. Fix this →

---

Minor: parent_document_id extracted for all udoc_ docs, including hash-based ones

robosystems/operations/search/service.py:109–113

if doc_id.startswith("udoc_"):
    parent_doc_id = doc_id.rsplit("_", 1)[0].removeprefix("udoc_")

When external_id is not set, base_id = f"udoc_{content_hash(request.content)}", so parent_doc_id would be the content hash — not a real PG document ID. The comment in search/service.py:204 says "User docs use udoc_ prefix so the PG document ID is trivially extractable", but this only holds when the doc was indexed via external_id. In practice this may never matter (since documents/service.py always passes doc.id as external_id), but the extraction logic is fragile: if external_id ever comes in as a content hash or UUID, the parent extraction gives a nonsensical result silently.

---

Minor: Manual reversal field mapping in router

robosystems/routers/ledger/schedules.py:300–316

reversal_resp is built field-by-field from result.reversal. If ClosingEntryResponse gains a new field later, this mapping silently drops it. Consider:

reversal_resp = (
    ClosingEntryResponse.model_validate(result.reversal.__dict__)
    if result.reversal else None
)

or add a to_response() method to ClosingEntryResult. Fix this →

---

Positive observations

• reversal_of and type='reversing' are pre-existing columns/values in the Entry model — no migration needed for those fields.
• auto_reverse is stored in the metadata JSONB column — correct, no migration needed.
• from __future__ import annotations + ClosingEntryResponse.model_rebuild() is the correct Pydantic v2 pattern for self-referential models.
• Excluding type != 'reversing' from best_entry CTE is a correct fix to prevent reversals from masking the actual closing entry status.
• The DR/CR flip in the reversal line items (credit_element_id → debit, debit_element_id → credit) is accounting-correct.
• DISTINCT ON (reversal_of) with status-priority ORDER BY is an elegant way to pick the canonical reversal status.
• The December month-boundary check for reversal_date is handled correctly.

---