feat: REI Hub general ledger import endpoint#107
Conversation
POST /api/import/reihub — ingests REI Hub (TurboTenant accounting hub) general ledger exports. Handles pipe-delimited markdown table format. - 14-column double-entry ledger (Account, Date, Type, Description, Debit, Credit, Split Account, Scope, Property, Unit, Sub-Portfolio, Vendor, Fixed Asset, Additional Notes) - Account category → COA code mapping (Rent→4010, Repairs→5030, etc.) - Property address + Sub-Portfolio → tenant resolution - TurboTenant Payment ID extraction from descriptions for dedup - Per-category import statistics Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
chittyfinance | 512d191 | Apr 24 2026, 12:21 PM |
|
@coderabbitai review Please evaluate:
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 49 minutes and 32 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Code Review: REI Hub General Ledger ImportOverall: Solid addition that follows the established import pattern in Bugs / Correctness1. 2. Silent default fallback is too aggressive return slugToTenantId['aribia-mgmt'] || null;Any row that doesn't match a property address or sub-portfolio silently falls under 3. const amount = isIncome ? credit : -debit;
// ...
amount: String(Math.abs(amount)),The sign on 4. Rows with both debit and credit non-zero are not handled Performance5. No bulk dedup or batch insert — 7.8K sequential round-trips Security / Auth6. No explicit auth check in the handler Hardcoded Entity Data7. 8. Minor
Test CoverageThe test plan has the automated deploy + data checks unchecked. Given the hardcoded tenant maps and the category-header state machine, a unit test covering:
...would significantly reduce regression risk on the parsing logic before the 7.8K-row live test. Summary: Items 1–4 are worth fixing before merge. Items 5–6 are acceptable tech debt with a note. The rest are minor. Good work on the format auto-detection and TurboTenant Payment ID cross-source dedup — those are non-obvious and well-executed. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 512d191b7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const tenantId = resolveTenantId(property, subPortfolio, scope); | ||
| if (!tenantId) { skipped++; continue; } |
There was a problem hiding this comment.
This route derives the target tenantId from CSV content (property/subPortfolio) and then writes transactions with that resolved ID, rather than constraining writes to c.get('tenantId'). In practice, a caller who is authorized for one tenant can submit rows that match another tenant mapping and create transactions in that other tenant, which breaks tenant isolation and can corrupt another entity’s ledger.
Useful? React with 👍 / 👎.
| const accounts = await storage.getAccounts(tenantId); | ||
| const active = accounts.find((a) => a.isActive); | ||
| if (active) { |
There was a problem hiding this comment.
The importer picks accounts.find((a) => a.isActive) as the “default” account, but getAccounts is ordered by updatedAt, so this effectively means “most recently updated active account.” For tenants with multiple active accounts, imports can land in different (and incorrect) ledgers over time after unrelated account edits, causing account-level balances and reconciliation to be wrong.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a new protected import route for ingesting REI Hub (TurboTenant accounting hub) general-ledger exports into the transactions system, with deduplication and per-category import statistics.
Changes:
- Adds
POST /api/import/reihubto parse REI Hub exports (CSV or pipe-delimited table) and create transactions. - Implements tenant resolution from property/sub-portfolio heuristics and TurboTenant Payment ID extraction for dedup.
- Returns per-category imported/skipped stats and writes an audit entry via
ledgerLog.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const portfolioLower = subPortfolio.toLowerCase(); | ||
| for (const [pattern, slug] of Object.entries(PORTFOLIO_TENANT_MAP)) { | ||
| if (portfolioLower.includes(pattern)) return slugToTenantId[slug] || null; | ||
| } |
There was a problem hiding this comment.
resolveTenantId(property, subPortfolio, scope) takes scope but never uses it. Either incorporate scope into resolution (if it carries needed routing info) or remove the parameter to avoid implying behavior that isn't implemented.
| } | |
| } | |
| // Try scope as an additional routing hint | |
| const scopeLower = scope.toLowerCase(); | |
| for (const [pattern, slug] of Object.entries(PORTFOLIO_TENANT_MAP)) { | |
| if (scopeLower.includes(pattern)) return slugToTenantId[slug] || null; | |
| } |
| const allTenants = await storage.getTenants(); | ||
| const slugToTenantId: Record<string, string> = {}; | ||
| for (const t of allTenants) slugToTenantId[t.slug] = t.id; | ||
|
|
||
| // Get a default account per tenant for transaction storage | ||
| const tenantDefaultAccount: Record<string, string> = {}; | ||
| async function getDefaultAccount(tenantId: string): Promise<string | null> { | ||
| if (tenantDefaultAccount[tenantId]) return tenantDefaultAccount[tenantId]; | ||
| const accounts = await storage.getAccounts(tenantId); | ||
| const active = accounts.find((a) => a.isActive); | ||
| if (active) { | ||
| tenantDefaultAccount[tenantId] = active.id; | ||
| return active.id; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| function resolveTenantId(property: string, subPortfolio: string, scope: string): string | null { | ||
| // Try property address first | ||
| const propLower = property.toLowerCase(); | ||
| for (const [pattern, slug] of Object.entries(PROPERTY_TENANT_MAP)) { | ||
| if (propLower.includes(pattern)) return slugToTenantId[slug] || null; | ||
| } | ||
| // Try sub-portfolio | ||
| const portfolioLower = subPortfolio.toLowerCase(); | ||
| for (const [pattern, slug] of Object.entries(PORTFOLIO_TENANT_MAP)) { | ||
| if (portfolioLower.includes(pattern)) return slugToTenantId[slug] || null; | ||
| } | ||
| // Default to ARIBIA MGMT | ||
| return slugToTenantId['aribia-mgmt'] || null; | ||
| } |
There was a problem hiding this comment.
This endpoint resolves tenantId per row using storage.getTenants() + hard-coded slug mappings, instead of using the request-scoped c.get('tenantId') / tenantMiddleware membership check. As written, any authenticated caller for one tenant could import transactions into other tenants they don't belong to. Please restrict imports to the current tenant (and treat property/sub-portfolio as metadata only), or explicitly authorize cross-tenant imports (e.g., verify storage.getUserTenants(c.get('userId')) includes the resolved tenantId, or require service-token-only access for multi-tenant writes).
| // REI Hub Account category → COA code mapping | ||
| const ACCOUNT_COA_MAP: Record<string, string> = { | ||
| 'rent': '4010', // Rental income | ||
| 'management fees': '6110', // Management expenses | ||
| 'cleaning and maintenance': '5040', // Maintenance | ||
| 'repairs': '5030', // Repairs | ||
| 'insurance': '5060', // Insurance | ||
| 'mortgage interest': '5080', // Mortgage interest | ||
| 'property taxes': '5050', // Property taxes | ||
| 'utilities': '5070', // Utilities | ||
| 'supplies': '5040', // Supplies (under maintenance) | ||
| 'travel': '6050', // Travel | ||
| 'legal and professional': '6060', // Legal/professional | ||
| 'advertising': '6020', // Advertising | ||
| 'auto balance': '9010', // Opening balances → suspense | ||
| 'owner funds': '3010', // Owner equity | ||
| 'depreciation': '5090', // Depreciation | ||
| 'capital expenditures': '1500', // Capital improvements | ||
| }; | ||
|
|
There was a problem hiding this comment.
ACCOUNT_COA_MAP appears to use COA codes that don't match the repo’s canonical chart/mappings in database/chart-of-accounts.ts (e.g., Rent is 4000 there, Repairs is 5070, Mortgage Interest is 5300, and codes like 6110/6050/6060 are not defined). This will produce systematically incorrect suggestedCoaCode values. Please align these mappings with REI_CHART_OF_ACCOUNTS/TURBOTENANT_CATEGORY_MAP (or validate codes via getAccountByCode before storing) so suggestions are always valid and consistent.
| // REI Hub Account category → COA code mapping | |
| const ACCOUNT_COA_MAP: Record<string, string> = { | |
| 'rent': '4010', // Rental income | |
| 'management fees': '6110', // Management expenses | |
| 'cleaning and maintenance': '5040', // Maintenance | |
| 'repairs': '5030', // Repairs | |
| 'insurance': '5060', // Insurance | |
| 'mortgage interest': '5080', // Mortgage interest | |
| 'property taxes': '5050', // Property taxes | |
| 'utilities': '5070', // Utilities | |
| 'supplies': '5040', // Supplies (under maintenance) | |
| 'travel': '6050', // Travel | |
| 'legal and professional': '6060', // Legal/professional | |
| 'advertising': '6020', // Advertising | |
| 'auto balance': '9010', // Opening balances → suspense | |
| 'owner funds': '3010', // Owner equity | |
| 'depreciation': '5090', // Depreciation | |
| 'capital expenditures': '1500', // Capital improvements | |
| }; | |
| // REI Hub Account category → canonical chart account name mapping. | |
| // Resolve codes via `findAccountCode` so suggested COA codes always come | |
| // from the repo's canonical chart of accounts instead of stale literals. | |
| const ACCOUNT_CATEGORY_TO_CANONICAL_NAME: Record<string, string> = { | |
| 'rent': 'Rent', | |
| 'management fees': 'Management Fees', | |
| 'cleaning and maintenance': 'Cleaning and Maintenance', | |
| 'repairs': 'Repairs', | |
| 'insurance': 'Insurance', | |
| 'mortgage interest': 'Mortgage Interest', | |
| 'property taxes': 'Property Taxes', | |
| 'utilities': 'Utilities', | |
| 'supplies': 'Supplies', | |
| 'travel': 'Travel', | |
| 'legal and professional': 'Legal and Professional', | |
| 'advertising': 'Advertising', | |
| 'auto balance': 'Auto Balance', | |
| 'owner funds': 'Owner Funds', | |
| 'depreciation': 'Depreciation', | |
| 'capital expenditures': 'Capital Expenditures', | |
| }; | |
| const ACCOUNT_COA_MAP: Record<string, string> = Object.fromEntries( | |
| Object.entries(ACCOUNT_CATEGORY_TO_CANONICAL_NAME).flatMap(([category, accountName]) => { | |
| const code = findAccountCode(accountName); | |
| return code ? [[category, code]] : []; | |
| }) | |
| ); |
| const active = accounts.find((a) => a.isActive); | ||
| if (active) { | ||
| tenantDefaultAccount[tenantId] = active.id; | ||
| return active.id; |
There was a problem hiding this comment.
Unlike other import endpoints in this file, this route doesn't require an explicit target accountId (X-Account-ID or query param) and instead picks the first active account returned by storage.getAccounts(tenantId). That choice depends on account ordering and can silently import into the wrong account when a tenant has multiple active accounts. Consider requiring an account ID (or a deterministic account selection strategy per tenant) and validating it belongs to the resolved tenant.
| const active = accounts.find((a) => a.isActive); | |
| if (active) { | |
| tenantDefaultAccount[tenantId] = active.id; | |
| return active.id; | |
| const activeAccounts = accounts.filter((a) => a.isActive); | |
| if (activeAccounts.length === 1) { | |
| tenantDefaultAccount[tenantId] = activeAccounts[0].id; | |
| return activeAccounts[0].id; |
| const isPipeDelimited = lines[0].includes('|'); | ||
| function parseLine(line: string): string[] { | ||
| if (isPipeDelimited) { | ||
| return line.split('|').map((cell) => cell.trim()).filter((_, i, arr) => i > 0 && i < arr.length); |
There was a problem hiding this comment.
Pipe-delimited parsing has an off-by-one issue: filter((_, i, arr) => i > 0 && i < arr.length) doesn't drop the trailing empty cell when a line ends with |, and it will drop the first real column if a line doesn't start with a leading |. A safer approach is to trim() then strip a single leading/trailing pipe (if present) before splitting, or to filter with i < arr.length - 1.
| return line.split('|').map((cell) => cell.trim()).filter((_, i, arr) => i > 0 && i < arr.length); | |
| const trimmedLine = line.trim().replace(/^\|/, '').replace(/\|$/, ''); | |
| return trimmedLine.split('|').map((cell) => cell.trim()); |
| // REI Hub exports as pipe-delimited markdown table or CSV — detect format | ||
| const lines = body.split('\n').filter((l) => l.trim() && !l.trim().startsWith('| :-')); | ||
| if (lines.length < 2) { | ||
| return c.json({ error: 'No data rows found' }, 400); | ||
| } | ||
|
|
||
| // Parse header — handle both pipe-delimited (markdown table) and CSV | ||
| const isPipeDelimited = lines[0].includes('|'); | ||
| function parseLine(line: string): string[] { | ||
| if (isPipeDelimited) { | ||
| return line.split('|').map((cell) => cell.trim()).filter((_, i, arr) => i > 0 && i < arr.length); | ||
| } |
There was a problem hiding this comment.
The route header comment says it accepts "tab-delimited" input, but the implementation only supports pipe-delimited markdown (|) or comma-delimited CSV via parseCSVLine (no tab parsing). Please either add tab-delimited parsing support or update the comment so the documented contract matches behavior.
| // REI Hub exports as pipe-delimited markdown table or CSV — detect format | |
| const lines = body.split('\n').filter((l) => l.trim() && !l.trim().startsWith('| :-')); | |
| if (lines.length < 2) { | |
| return c.json({ error: 'No data rows found' }, 400); | |
| } | |
| // Parse header — handle both pipe-delimited (markdown table) and CSV | |
| const isPipeDelimited = lines[0].includes('|'); | |
| function parseLine(line: string): string[] { | |
| if (isPipeDelimited) { | |
| return line.split('|').map((cell) => cell.trim()).filter((_, i, arr) => i > 0 && i < arr.length); | |
| } | |
| // REI Hub exports as pipe-delimited markdown table, tab-delimited text, or CSV — detect format | |
| const lines = body.split('\n').filter((l) => l.trim() && !l.trim().startsWith('| :-')); | |
| if (lines.length < 2) { | |
| return c.json({ error: 'No data rows found' }, 400); | |
| } | |
| // Parse header — handle pipe-delimited (markdown table), tab-delimited text, and CSV | |
| const isPipeDelimited = lines[0].includes('|'); | |
| const isTabDelimited = !isPipeDelimited && lines[0].includes('\t'); | |
| function parseLine(line: string): string[] { | |
| if (isPipeDelimited) { | |
| return line.split('|').map((cell) => cell.trim()).filter((_, i, arr) => i > 0 && i < arr.length); | |
| } | |
| if (isTabDelimited) { | |
| return line.split('\t').map((cell) => cell.trim()); | |
| } |
| const debit = parseFloat(debitStr.replace(/[$,]/g, '')) || 0; | ||
| const credit = parseFloat(creditStr.replace(/[$,]/g, '')) || 0; | ||
| if (debit === 0 && credit === 0) { skipped++; continue; } | ||
|
|
||
| // Net amount: credits are income (positive), debits are expenses (negative for P&L) | ||
| const isIncome = credit > 0 && debit === 0; | ||
| const amount = isIncome ? credit : -debit; | ||
|
|
||
| // Parse date (YYYY-MM-DD or MM/DD/YYYY) | ||
| let date: Date; | ||
| if (dateStr.includes('-') && dateStr.length === 10) { | ||
| date = new Date(dateStr); | ||
| } else { | ||
| const parts = dateStr.split('/'); | ||
| if (parts.length === 3) { | ||
| date = new Date(`${parts[2]}-${parts[0].padStart(2, '0')}-${parts[1].padStart(2, '0')}`); | ||
| } else { | ||
| date = new Date(dateStr); | ||
| } | ||
| } | ||
| if (isNaN(date.getTime())) { skipped++; continue; } | ||
|
|
||
| // Extract TurboTenant Payment ID from description if present | ||
| const paymentMatch = description.match(/PAYMENT\s*#(\d+)/i); | ||
| const ttPaymentId = paymentMatch ? paymentMatch[1] : null; | ||
|
|
||
| // Build external ID for dedup | ||
| const descHash = description.slice(0, 40).replace(/[^a-zA-Z0-9]/g, '').toLowerCase(); | ||
| const externalId = ttPaymentId | ||
| ? `reihub:tt:${ttPaymentId}` | ||
| : `reihub:${dateStr}:${amount}:${descHash}`; | ||
|
|
There was a problem hiding this comment.
externalId uses ${amount} where amount is a JS floating-point value derived from parseFloat. This can introduce unstable IDs due to floating precision (e.g., 10.1 becoming 10.0999999998), breaking deduplication across runs. Consider normalizing to cents (integer) or using a fixed decimal string (e.g., 2dp) for both externalId and stored amount.
Summary
POST /api/import/reihub— ingests REI Hub (TurboTenant accounting hub) general ledger exportsTest plan
npx tsc --noEmitpasses🤖 Generated with Claude Code