Fix repository credits allocation with new billing operation

[jfrench9]

Summary

Adds a new user repository credit allocation operation to the billing pipeline to fix an issue where repository credits were not being properly allocated to users.

Key Accomplishments

• New billing operation: Introduced a dedicated operation for handling user repository credit allocation within the Dagster billing jobs module
• Credit allocation logic: Implemented the core logic to correctly compute and assign credits at the repository level, addressing the bug where allocations were missing or incorrect
• Integration with existing billing pipeline: The new operation is additive (66 lines added, 1 line modified) and integrates cleanly with the existing billing job infrastructure

What Was Fixed

The prior implementation lacked a discrete step for allocating credits to user repositories, which could result in users not receiving their expected credit balances tied to specific repositories. This change ensures that repository-level credit allocation is explicitly handled as part of the billing workflow.

Breaking Changes

None. This is an additive change that introduces a new operation without modifying existing operation signatures or pipeline contracts.

Testing Notes

• Verify that the new credit allocation operation executes successfully within the Dagster job pipeline
• Confirm that user repository credits are correctly computed and persisted after a billing run
• Test edge cases such as users with no repositories, repositories with zero usage, and users with multiple repositories
• Validate that existing billing operations remain unaffected and continue to produce correct results
• Run the billing pipeline in a staging/development environment before promoting to production

Infrastructure Considerations

• Ensure the Dagster deployment is updated to pick up the new operation definition
• Monitor initial runs for any unexpected latency or resource usage introduced by the additional operation
• Verify that any downstream assets or sensors that depend on billing job outputs continue to function correctly with the new operation in the execution graph

---

🤖 Generated with Claude Code

Branch Info:

• Source: bugfix/repo-credits-allocation
• Target: main
• Type: bugfix

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

[jfrench9]

@claude please review this PR

[claude]

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

---

PR Review: Fix repository credits allocation with new billing operation

• Gather context and read changed files
• Review code for correctness, patterns, and issues
• Post review feedback

---

Overall this is a clean, additive change that follows the existing billing job patterns. The pipeline wiring (monthly_credit_allocation_job) is correct, and the logic for querying due pools and delegating to the model method is sound. A few issues worth addressing before merging:

---

Bug: Missing session.rollback() on exception — dirty ORM state can be committed

robosystems/dagster/jobs/billing.py (new op, inside the loop)

When pool.allocate_monthly_credits(session) raises a non-SQLAlchemy exception (e.g. AttributeError, TypeError), the except Exception block catches it but never calls session.rollback(). The ORM has already applied attribute changes to the session's identity map (e.g. self.current_balance, self.last_allocation_date). Those dirty attributes are then committed by the outer session.commit() at line ~1038, producing a credit balance change with no corresponding transaction record.

for pool in due_pools:
    try:
        if pool.allocate_monthly_credits(session):
            ...
    except Exception as e:
        errors.append(...)          # ← session not rolled back here
        context.log.error(...)

session.commit()                    # ← can commit partial/inconsistent state

Fix: add session.rollback() (or session.expunge(pool)) in the except block before continuing. Fix this →

---

Issue: Redundant outer session.commit() — create_transaction already commits

UserRepositoryCreditTransaction.create_transaction calls session.commit() internally for every allocation. By the time the loop exits successfully, all changes are already persisted. The outer session.commit() after the loop is harmless in the happy path, but it interacts poorly with the bug above (it's what makes partial dirty state dangerous). It also makes the control flow harder to reason about — the reader has to know that the commit already happened inside allocate_monthly_credits.

Consider either: (a) removing the outer commit and documenting that create_transaction commits inline, or (b) refactoring to defer all commits to the outer commit by having create_transaction not auto-commit when a session is passed (a larger change, but more consistent with how the rest of the session management works).

---

Issue: Late import of UserRepositoryCredits is inconsistent with module-level imports

All other models (Graph, GraphCredits, GraphCreditTransaction) are imported at the top of the file (lines 20–24). The new op imports UserRepositoryCredits inline:

def allocate_user_repository_credits(...):
    from robosystems.models.core import UserRepositoryCredits   # ← inline

This is inconsistent and makes the dependency harder to track. Unless there's a circular import concern (which isn't apparent here since other models.core imports work fine at module level), this should be moved to the top-level imports. Fix this →

---

Minor: Partial failures are silent to Dagster

When errors occur, the op returns {"errors": [...], ...} and logs a warning, but the op itself succeeds. Dagster will not retry or alert. This means allocation failures for some pools can go unnoticed unless someone is actively reading logs.

If errors is non-empty, consider raising a dagster.Failure (or at minimum a structured warning via context.log.warning) with an appropriate message so monitoring surfaces these. This is a design decision — the current approach (fail-open) may be intentional — but it's worth being explicit.

---

Nit: float(total_credits) in return value loses precision

total_credits is accumulated as Decimal, which is correct for currency. Converting to float in the return dict loses precision. Consider using str(total_credits) or keeping it as Decimal if downstream consumers can handle it.

---

Summary: The core logic is correct and the pipeline wiring is sound. The session rollback gap is the most important fix before merging — the rest are lower priority but worth tracking.