Skip to content

ENG-155#239

Merged
Connorbelez merged 2 commits intomainfrom
stagger-phase15-cron-times-eng155
Mar 21, 2026
Merged

ENG-155#239
Connorbelez merged 2 commits intomainfrom
stagger-phase15-cron-times-eng155

Conversation

@Connorbelez
Copy link
Copy Markdown
Owner

@Connorbelez Connorbelez commented Mar 21, 2026

Summary by CodeRabbit

  • Chores

    • Adjusted daily reconciliation check scheduling from 06:00 UTC to 07:00 UTC.
    • Removed obsolete internal TODO comment.
  • Tests

    • Added TypeScript interfaces for typed result shapes in reconciliation query testing.
  • Documentation

    • Updated technical documentation clarifying that validation is actively enforced and reflecting current reconciliation scheduling.

@linear
Copy link
Copy Markdown

linear bot commented Mar 21, 2026

Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 21, 2026

Warning

Rate limit exceeded

@Connorbelez has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 43 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 18e72fbc-d7b2-47f2-a07a-b0d3b837df70

📥 Commits

Reviewing files that changed from the base of the PR and between 4fdfcaa and 9368964.

📒 Files selected for processing (1)
  • docs/technical-design/unified-payment-rails.md
📝 Walkthrough

Walkthrough

This PR updates the reconciliation cron schedule from 06:00 UTC to 07:00 UTC, adds TypeScript interface definitions for reconciliation query result shapes in the test suite, removes a stale TODO comment, and updates documentation to reflect the enforced validation state and revised cron schedule.

Changes

Cohort / File(s) Summary
Cron Schedule Update
convex/crons.ts
Changed daily reconciliation check scheduled run time from 06:00 UTC to 07:00 UTC via hourUTC and minuteUTC arguments.
Test Type Definitions
convex/dispersal/__tests__/reconciliation.test.ts
Added seven TypeScript interfaces (DispersalSummaryByLender, DispersalHistoryEntry, DispersalHistoryResult, DispersalsByMortgageResult, DispersalsByObligationResult, ServicingFeeHistoryResult, UndisbursedBalanceResult) to provide typed result shapes for reconciliation queries wired to makeFunctionReference generics.
Code Cleanup
convex/ledger/types.ts
Removed stale TODO comment referencing ENTRY_TYPE_ACCOUNT_MAP validation pipeline wiring.
Documentation Update
docs/technical-design/unified-payment-rails.md
Updated ledger validation narrative to reflect that ENTRY_TYPE_ACCOUNT_MAP is actively enforced in postEntry via typeCheck(). Revised F1 registry status from critical (TODO) to resolved. Updated operational foot gun O1 from concurrent 06:00 UTC cron overlap to resolved schedule with obligations at 06:00 UTC and reconciliation at 07:00 UTC. Updated Phase 1 action item from "wire validation" to "confirm validation remains enforced.""

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related issues

  • Issue #168: Addresses the same code-level changes—moving reconciliation cron from 06:00 to 07:00 UTC and removing the stale TODO in convex/ledger/types.ts.

Possibly related PRs

  • PR #235: Edits convex/dispersal/__tests__/reconciliation.test.ts type declarations and imports, introducing TypeScript interfaces for query result shapes.
  • PR #234: Modifies the same reconciliation test file, adding typed query result interfaces and switching to typed function references.
  • PR #145: Modifies convex/dispersal/__tests__/reconciliation.test.ts with this PR adding TypeScript interface definitions that are consumed by the retrieved PR's new cross-check reconciliation test.

Poem

🐰 Hopping through the hours, we reconcile,
Seven o'clock now keeps us all in style!
Types now enforce what once was just a dream,
The ledger flows with validating gleam.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'ENG-155' is just a ticket identifier with no meaningful description of the actual changes made. Provide a descriptive title that summarizes the main change, such as 'Stagger cron times for daily reconciliation and obligations' or 'Update cron schedules to prevent overlapping jobs'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch stagger-phase15-cron-times-eng155

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Owner Author

Connorbelez commented Mar 21, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Connorbelez Connorbelez force-pushed the stagger-phase15-cron-times-eng155 branch from 66f9198 to 1b9b4b0 Compare March 21, 2026 14:59
@Connorbelez Connorbelez marked this pull request as ready for review March 21, 2026 15:19
Copilot AI review requested due to automatic review settings March 21, 2026 15:19
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Connorbelez has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates ledger/payment-rails documentation and small code/config cleanups under ENG-155, primarily to align written guidance with the already-implemented ledger validation pipeline and to stagger financial cron execution.

Changes:

  • Document that ENTRY_TYPE_ACCOUNT_MAP is already enforced in the postEntry type-check step; remove the stale TODO in convex/ledger/types.ts.
  • Stagger financial crons by moving daily reconciliation from 06:00 UTC to 07:00 UTC (obligation transitions remain at 06:00 UTC).
  • Documentation/reference updates (Notion canonical spec pointer, CLAUDE guidance) plus minor test refactors/formatting.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
planning/notion-goals/cash-and-obligations-ledger/SPEC-ledger-cash-and-obligations.md Adds a pointer noting Notion as the canonical spec source.
docs/technical-design/unified-payment-rails.md Replaces outdated “missing enforcement” notes with current-state enforcement details; updates foot-gun registry items accordingly.
convex/ledger/types.ts Removes stale TODO; keeps comments aligned with the actual postEntry pipeline.
convex/dispersal/tests/reconciliation.test.ts Minor import/type refactor and formatting adjustments.
convex/crons.ts Moves reconciliation cron to 07:00 UTC to avoid running in the same window as obligation transitions.
CLAUDE.md Adds “context-mode” routing/tooling guidance for contributors using that workflow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/technical-design/unified-payment-rails.md Outdated
@Connorbelez Connorbelez changed the base branch from 03-20-ft_money_ledger to graphite-base/239 March 21, 2026 16:46
@Connorbelez Connorbelez force-pushed the stagger-phase15-cron-times-eng155 branch from 1b9b4b0 to f90d014 Compare March 21, 2026 18:41
@Connorbelez Connorbelez changed the base branch from graphite-base/239 to main March 21, 2026 18:41
@Connorbelez Connorbelez force-pushed the stagger-phase15-cron-times-eng155 branch from f90d014 to 4fdfcaa Compare March 21, 2026 18:52
@Connorbelez Connorbelez merged commit 3cf200e into main Mar 21, 2026
1 of 3 checks passed
Connorbelez added a commit that referenced this pull request Apr 20, 2026
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

* **Chores**
  * Adjusted daily reconciliation check scheduling from 06:00 UTC to 07:00 UTC.
  * Removed obsolete internal TODO comment.

* **Tests**
  * Added TypeScript interfaces for typed result shapes in reconciliation query testing.

* **Documentation**
  * Updated technical documentation clarifying that validation is actively enforced and reflecting current reconciliation scheduling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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.

2 participants