refactor(server): split routes into books vs accounting concerns

[chitcommit]

Summary

Folds the retired ChittyBooks concern into ChittyFinance as a structural books vs accounting split (no behavior change). Boundary: Books writes facts, Accounting derives meaning.

• server/books/ ← transactions.ts, import.ts, webhooks.ts (ingest / categorize / journal)
• server/accounting/ ← accounts.ts, reports.ts, tax.ts, allocations.ts (COA / reporting / tax / allocations)
• classification.ts left in routes/ (straddles both) with a boundary comment
• READMEs in each dir name the owning agent (chittybooks-agent / chittyaccounting-agent)

Move-only: all 7 modules are 100% git renames, imports rewired in app.ts + 8 tests. npm run check passes.

Context

Standalone ChittyBooks (Flask, non-persistent demo) is retired; its capability is a strict subset of ChittyFinance. This lands the concern boundary as code. Agent registration into the orchestrator follows once the fractal tree is built+deployed — not registered as phantoms.

Test plan

• npm run check (tsc) clean
• CI green

Summary by CodeRabbit

• Chores

  • Reorganized internal module structure to improve code organization across accounting and books subsystems.
  • Updated test imports to reference new module paths.

• Documentation

  • Added internal documentation for the accounting and books subsystems.

[github-actions]

@coderabbitai review

Please evaluate:

• Security implications
• Credential exposure risk
• Dependency supply chain concerns
• Breaking API changes

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fdbb0217-e4bf-4835-8301-afd9e3bef890

📥 Commits

Reviewing files that changed from the base of the PR and between 652917a and 1538ebe.

📒 Files selected for processing (19)

• server/__tests__/reports-and-cloudflare-integration.test.ts
• server/__tests__/routes-consumer-contract.test.ts
• server/__tests__/scenario.test.ts
• server/__tests__/tax-reporting.test.ts
• server/__tests__/valuation.test.ts
• server/__tests__/webhooks-mercury.test.ts
• server/__tests__/webhooks-wave-receiver.test.ts
• server/__tests__/webhooks-wave-secret.test.ts
• server/accounting/README.md
• server/accounting/accounts.ts
• server/accounting/allocations.ts
• server/accounting/reports.ts
• server/accounting/tax.ts
• server/app.ts
• server/books/README.md
• server/books/import.ts
• server/books/transactions.ts
• server/books/webhooks.ts
• server/routes/classification.ts

---

📝 Walkthrough

Walkthrough

Routes are reorganized from a single server/routes/ directory into domain-organized modules under server/accounting/ and server/books/. The main app entry point and all consuming tests update their imports to reflect the new structure, with new README files documenting subsystem boundaries and responsibilities.

Changes

Route Module Organization

Layer / File(s)	Summary
Subsystem documentation and boundary clarification server/accounting/README.md, server/books/README.md, server/routes/classification.ts	New documentation describes Books (ingest/categorize/journal) and Accounting (derive meaning/reporting/tax) boundaries, and a comment explains why classification code remains in routes/ until split.
Central app route reorganization server/app.ts	Route imports are reorganized to use new locations: accounting routes from ./accounting/*, books routes from ./books/*, with same exported identifiers used for route mounting.
Test imports realignment server/__tests__/reports-and-cloudflare-integration.test.ts, server/__tests__/routes-consumer-contract.test.ts, server/__tests__/scenario.test.ts, server/__tests__/tax-reporting.test.ts, server/__tests__/valuation.test.ts, server/__tests__/webhooks-mercury.test.ts, server/__tests__/webhooks-wave-receiver.test.ts, server/__tests__/webhooks-wave-secret.test.ts	Integration and unit tests update route imports to match reorganized module paths; valuation test also adds coverage for header-only CSV inputs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• chittyapps/chittyfinance#73: Introduced tax reporting endpoints that are now relocated to the new accounting/tax module structure.
• chittyapps/chittyfinance#113: Added Wave webhook receiver and secret storage behavior that tests now import from the relocated books/webhooks module.
• chittyapps/chittyfinance#90: Introduced Mercury webhook routing that is now imported from the relocated books/webhooks module in tests.

Poem

🐰 Routes shuffled from chaos to sense,
Books scribble, Accounting's immense,
Tests aligned with the new domain dance,
Boundaries clear—a refactor romance!
Organization blooms, the system's advance! 🌱

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: a structural reorganization of server routes into books (write facts) and accounting (derive meaning) concerns, which is the primary focus of the pull request.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch books-accounting-split

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Code Review — PR #120: refactor(server): split routes into books vs accounting concerns

Overview

Clean structural refactor that promotes the Books/Accounting boundary from a conceptual note into directory-level enforcement. All 7 moves are pure git renames with no logic changes; app.ts and 8 test files get the rewired imports. This is exactly the kind of refactor that pays dividends when the agent fractal tree grows — low risk, high long-term clarity.

---

What's Good

• Boundary semantics are precise. "Books writes facts / Accounting derives meaning" is a crisp invariant. The READMEs codify it with owner agents, which will be valuable once chittybooks-agent / chittyaccounting-agent are registered.
• Move-only discipline held. Every renamed file shows similarity index 100% — no logic was accidentally edited in the shuffle.
• classification.ts handling is correct. Leaving it in routes/ with a boundary comment rather than forcing a premature split is the right call. The comment itself is appropriately narrow — it names why it's there without over-explaining.
• Test imports updated consistently across all 8 affected files.

---

Issues / Suggestions

1. No barrel (index.ts) in server/books/ or server/accounting/

Currently consumers import directly from file paths (../books/import, ../accounting/reports). This works fine today, but as both dirs grow it becomes harder to enforce the boundary and track the public surface. Consider adding a thin index.ts to each dir that re-exports the route factories — this also makes future moves within the dirs non-breaking to callers.

// server/books/index.ts
export { transactionRoutes } from './transactions';
export { importRoutes } from './import';
export { webhookRoutes } from './webhooks';

Not a blocker, but worth doing before the dirs accumulate more files.

2. classification.ts needs a tracking issue

The boundary comment is good, but without a linked issue the split will drift into permanent "leave it for later" territory. The PR description doesn't reference a ticket. Recommend opening an issue (e.g. "split classification.ts into books/categorize + accounting/coa") and linking it from the comment so it doesn't go stale.

3. CI green is unchecked in the test plan

CLAUDE.md notes that CF Workers Builds (issue #111) is permanently red and auto-merge ignores it. Confirming that "real CI elsewhere" is actually green before merge would close the loop — even just a note in the PR description saying "vitest suite green on branch" would suffice.

---

Conventions Check (CLAUDE.md)

Rule	Status
All DB access through SystemStorage	Not touched — N/A
No raw Drizzle in routes	Not touched — N/A
No mocks/fake data in commits	Move-only — N/A
npm run check passes	✅ confirmed in PR description
No DoorLoop reintroduction	N/A

---

Security / Performance

No concerns. This is purely a build-time module path change with no runtime surface area.

---

Verdict

Approve with minor suggestions. The refactor is correct, disciplined, and well-scoped. The two suggestions above (barrel exports, tracking issue for classification.ts) are improvements worth making but not blockers. The CI note is a sanity check worth ticking before merge.

[coderabbitai]

Actionable comments posted: 0