Phase 3a (1/3): CRM services + API, schema-parity instrument - #3

Merged
corrin merged 2 commits into
mainfrom
phase3a-1-crm
Aug 2, 2026
Merged

Phase 3a (1/3): CRM services + API, schema-parity instrument#3
corrin merged 2 commits into
mainfrom
phase3a-1-crm

Conversation

@corrin

Copy link
Copy Markdown
Owner

CRM: 18/18 v1 operations with exact-URL pins for the external phone-provider portal; celery tasks + beat-in-code; 48 tests. Infra: schema_parity_diff.py CI gate (wire-level, ledger-aware). Part 1 of the 3a stack.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222

CRM: 18/18 v1 operations (phone calls, recordings with ETag/304, endpoints
CRUD, provider settings), exact-URL pins for the external provider portal,
requests-based client, celery tasks + beat-in-code, 48 tests.
Infrastructure: scripts/schema_parity_diff.py (live OpenAPI vs v1's frozen
contract, ledger-aware, wire-level) wired into CI.
Stacked 1/3; company impl and tests follow.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@coderabbitai

coderabbitaiBot commented Aug 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 95664726-a6bb-43bf-b5a1-b729ea799a5b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f129cb and 42b51fe.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • .env.example
  • .github/workflows/ci.yml
  • .gitignore
  • apps/crm/api.py
  • apps/crm/schemas.py
  • apps/crm/services/__init__.py
  • apps/crm/services/phone_call_service.py
  • apps/crm/tasks.py
  • apps/crm/tests/__init__.py
  • apps/crm/tests/helpers.py
  • apps/crm/tests/test_phone_call_api.py
  • apps/crm/tests/test_phone_call_service.py
  • apps/crm/tests/test_phone_endpoint_guard.py
  • apps/crm/tests/urls.py
  • config/api.py
  • config/celery.py
  • config/settings.py
  • config/settings_test.py
  • docs/accepted-api-differences.yml
  • pyproject.toml
  • scripts/schema-parity-baseline.txt
  • scripts/schema_parity_diff.py

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.

- Recording-download params: null origin/destination stays empty (v1
omit-on-None semantics), not the literal string None (provider-boundary
parity).
- Phone-endpoint number stripped as v1's serializer did.
- schema_parity_diff.py: ratcheting baseline of ever-matched operationIds
(scripts/schema-parity-baseline.txt) - a ported operation vanishing is
now DRIFT, not 'not yet ported'; malformed ledger entries hard-fail;
--update-baseline grows the file deliberately. (Reviewer caught the
instrument's regression blindness.)
- Ledger: loose v1 query values (page=last, yes-booleans) now 422.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@corrin
corrin merged commit b5b6c4d into mainAug 2, 2026
3 checks passed
@corrin
corrin deleted the phase3a-1-crm branch August 2, 2026 03:51
corrin added a commit that referenced this pull request Aug 11, 2026
* Purchasing PO create/list/detail: po-created-by green
PoCreatePage (CompanyLookup + reference + save -> 201 redirect),
PoListPage, PoDetailPage with PoSummaryCard (created-by input,
reference autosave, status select), usePoLines over the single PATCH
endpoint, and the missing 'po' concurrency invalidator so 412/428
recovery actually refetches. Ports createTestPurchaseOrder +
waitForPoAutosave and the po-created-by spec.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* PO lines grid: create-purchase-order green
PoLinesTable as the third grid on the useReactTable + useDraftRows
pattern (phantom row, no add-line button, row-exit draft commit —
unit-cost stays the row's last focusable cell so the spec's Tab exits
the row). ItemSelect generalised for stock-only consumers (optional
jobId/line, label + wrapper overrides; labour-rates query gated on
jobId presence only, since textOnly labels need rate names). Inline
JobSelect over purchasing_all_jobs_retrieve — the unfiltered endpoint
v1's PO page uses, because fresh jobs are draft and the filtered
sibling excludes them. Spec ported with the autosave waiter armed
before the pick/status clicks (v2 saves immediately; v1's debounce is
what made arm-after work).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Stock page: stock-search green
Spec-lean StockPage: full active-stock list on load, 300ms-debounced
server FTS from 3 characters, enabled-gated so clearing the box
renders the cached list with no /search/ request.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Rewrite-status: purchasing slice recorded (29 of 40)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unit tests for PO line draft rules and JobSelect filtering
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unify the table layer: shared DataTable/ListTable, no cross-domain imports
The purchasing slice's PoLinesTable was about to become a third
hand-rolled grid emitting the DataTable-row-N/data-grid-* contract
inline. features/shared/DataTable.tsx is now the one owner of that
contract; SmartTimesheetTable, CostLineGrid and PoLinesTable all
render through it.
Auditing further found the same pathology in the plain list pages:
PoListPage and StockPage (both new this slice) had copied
CompaniesListPage's table shell and loading/error/retry block
verbatim, taking an existing duplicate (also in WipReportPage) from
2 instances to 4. features/shared/ListTable.tsx is the one owner of
that block instead -- deliberately separate from DataTable, since it
has no react-table dependency and forcing static lists through
column-def machinery would be indirection, not rigor. A hand-rolled
debounce-into-query-state pattern in CompaniesListPage and StockPage
is now features/shared/useDebouncedValue.ts (KanbanSearchInput keeps
its own URL-driven debounce -- a different concept).
ItemSelect, the decimal helpers and the Save-failed badge move to
features/shared/ because purchasing consumed them cross-domain -- a
domain feature is not a library.
ADR 0039 strengthened: unification is never deferred, shared concepts
get shared homes, the bar is reference quality.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Review fixes: QueryState primitive, item-label bug, company relocated
Adversarial review on the branch found the ListTable audit had stopped
short: JobMovementReportPage and CompanyDetailPage hand-rolled the same
loading/error/retry block ListTable was built to own (6 real instances,
only 4 fixed). Split the block itself out as
features/shared/QueryState.tsx -- the pending/error gate alone, no
table -- so it fits pages that show something other than a table too.
ListTable now composes QueryState instead of duplicating it. PoDetailPage,
CostLineGrid, JobMovementReportPage and CompanyDetailPage all render
through it.
Fixed: PoLinesTable's item-picker label read `item_code ?? 'Select Item'`
with no description fallback, so a bound stock item with a null code
(nullable, v1 parity) misread as unbound. Now poLineItemLabel() in
lines.ts, unit-tested.
Moved features/company to features/shared/company: it was already
cross-imported by features/job (JobCreatePage, JobSettingsTab) before
this slice added a third importer (purchasing/PoSummaryCard) -- it never
had a route of its own, a shared widget library in a domain-shaped box.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* ADR 0039: name the deadline-vs-duplicate resolution explicitly
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Docs: Slice 3 (SSE + serving model) is MUST before cutover, not deferred
Reversed 2026-08-11. The 2026-08-10 record said SSE ships with the
production-serving decision, filed under "Post-cutover -- decided,
deliberately NOT before 15 August." That was overturned: racing bad
architecture into production defeats the point of the rewrite
(non-negotiable #3 in the Cutover section), and the interim polling
shape plus the un-runnable apps/xero/sync_stream.py view are exactly
that. Slice 3 -- live updates done properly (serving model fix + SSE
ticker + discard the interim shortcuts) moves to its own MUST-tier
section with a milestone checkbox, and every stale "deferred
post-cutover" cross-reference in the file is corrected. The
purchasing-slice PR is unaffected -- it is unrelated and merges as
planned; this correction only fixes the durable record for the next
session.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Third review pass: 4 more QueryState conversions; rewrite-status cleanup
QueryState grew optional loadingNode/errorNode override props so a
spinner-based caller keeps its visual shell instead of losing it to
the plain-text default. JobFinishTab, DailyOverviewPage, and both
gates in TimesheetEntryPage's EntryWorkspace now render through it.
XeroQuoteCard, JobInvoiceCard and JobSettingsTab's pay-item field stay
excluded -- richer branching than binary success/fail, not the
page-level gate QueryState owns. TimesheetEntryPage's own outer gate
stays as guard-clause `if` returns, matching CLAUDE.md's stated
preference rather than converting to a shape that would abandon it.
Also: rewrite-status.md's narration of "review round found X, we
fixed Y" replaced with what the outcomes actually are -- which shared
component owns which contract, which sites are deliberately excluded
and why, which constraints a future change must respect. And the
Cutover section now states the actual two-question go/no-go criteria
(functional parity, proxied by MUST-tier E2E; materially better
architecture, judged directly) rather than only the E2E proxy, with
the honest fallback (abort and stay on v1) stated explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Address CodeRabbit review: real bugs and comment cleanup
- CompaniesListPage/JobFinishTab: QueryState was passed the raw
isError, breaking the first-load-only rule the other converted
sites already follow -- a background refetch failure was
unmounting already-rendered content instead of leaving it on
screen.
- JobSelect: closing the picker without a selection (blur or
Escape) left `editing` true forever, so the input kept showing
the abandoned search term instead of reverting to the bound job;
the blur timer was untracked and could fire a state update after
unmount. Also added keyboard selection (arrow keys + Enter,
role=listbox/option, aria-activedescendant) -- the dropdown was
mouse-only.
- PoListPage: the row was reachable only by mouse click; added a
real Link on the PO number cell, matching CompaniesListPage's
existing pattern.
- decimal.ts: parseDecimalInput accepted anything Number() parses,
including hex/octal/binary literals and bare exponents, and sent
them to the wire verbatim as garbage Decimal strings. Validates
fixed-point syntax explicitly now.
- Stale features/company/* paths in rewrite-status.md's build-order
table, left behind by the features/shared/company move.
- Comments narrating port history or test intent rewritten to state
the rejected alternative and the constraint (ADR 0043) in
ItemSelect, SaveFailedBadge, CreateCompanyModal, CompanyLookup;
two flagged sites (the E2E-repair-loop comment, the sonner test
comment) were already compliant and left as-is.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix uv.lock/pyproject.toml desync on the django constraint
PR #54 (main) widened pyproject.toml's django specifier to
>=6.0,<6.2 but never regenerated the matching uv.lock, which still
recorded <6.1 in its requires-dist metadata. Every `uv run` this
session silently self-corrected the lock locally; committing that
fix rather than leaving a lockfile permanently out of sync with the
manifest it's supposed to pin.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Gitignore the root-level test-results/ Playwright writes too
frontend/test-results/ was covered; run_e2e.sh's Playwright
invocation also writes artifacts relative to the repo root in some
invocations, leaving an untracked test-results/ dangling after every
E2E run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix the actual bug behind the stray root test-results/: cwd-relative config
playwright.config.ts resolved .env.test, testDir, outputDir and the
html reporter's folder relative to process.cwd(). Any invocation
whose cwd wasn't frontend/ (npm --prefix from the repo root, a bare
npx playwright test) silently missed .env.test -- dropping
E2E_TEST_USERNAME/PASSWORD with no error -- or wrote artifacts to
the wrong location instead of erroring loudly, which is how a
root-level test-results/ kept reappearing. Anchored every one of
these to import.meta.dirname instead: verified from an unrelated
cwd, .env/.env.test still resolve correctly and no stray directory
gets created anywhere. The root-level gitignore entry from the
previous commit stays as a defensive backstop, but this is the real
fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Phase 3a (1/3): CRM services + API, schema-parity instrument - #3

Merged
corrin merged 2 commits into
mainfrom
phase3a-1-crm
Aug 2, 2026
Merged

Phase 3a (1/3): CRM services + API, schema-parity instrument#3
corrin merged 2 commits into
mainfrom
phase3a-1-crm

Conversation

@corrin

Copy link
Copy Markdown
Owner

CRM: 18/18 v1 operations with exact-URL pins for the external phone-provider portal; celery tasks + beat-in-code; 48 tests. Infra: schema_parity_diff.py CI gate (wire-level, ledger-aware). Part 1 of the 3a stack.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222

CRM: 18/18 v1 operations (phone calls, recordings with ETag/304, endpoints
CRUD, provider settings), exact-URL pins for the external provider portal,
requests-based client, celery tasks + beat-in-code, 48 tests.
Infrastructure: scripts/schema_parity_diff.py (live OpenAPI vs v1's frozen
contract, ledger-aware, wire-level) wired into CI.
Stacked 1/3; company impl and tests follow.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@coderabbitai

coderabbitaiBot commented Aug 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 95664726-a6bb-43bf-b5a1-b729ea799a5b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f129cb and 42b51fe.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • .env.example
  • .github/workflows/ci.yml
  • .gitignore
  • apps/crm/api.py
  • apps/crm/schemas.py
  • apps/crm/services/__init__.py
  • apps/crm/services/phone_call_service.py
  • apps/crm/tasks.py
  • apps/crm/tests/__init__.py
  • apps/crm/tests/helpers.py
  • apps/crm/tests/test_phone_call_api.py
  • apps/crm/tests/test_phone_call_service.py
  • apps/crm/tests/test_phone_endpoint_guard.py
  • apps/crm/tests/urls.py
  • config/api.py
  • config/celery.py
  • config/settings.py
  • config/settings_test.py
  • docs/accepted-api-differences.yml
  • pyproject.toml
  • scripts/schema-parity-baseline.txt
  • scripts/schema_parity_diff.py

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.

- Recording-download params: null origin/destination stays empty (v1
omit-on-None semantics), not the literal string None (provider-boundary
parity).
- Phone-endpoint number stripped as v1's serializer did.
- schema_parity_diff.py: ratcheting baseline of ever-matched operationIds
(scripts/schema-parity-baseline.txt) - a ported operation vanishing is
now DRIFT, not 'not yet ported'; malformed ledger entries hard-fail;
--update-baseline grows the file deliberately. (Reviewer caught the
instrument's regression blindness.)
- Ledger: loose v1 query values (page=last, yes-booleans) now 422.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@corrin
corrin merged commit b5b6c4d into mainAug 2, 2026
3 checks passed
@corrin
corrin deleted the phase3a-1-crm branch August 2, 2026 03:51
corrin added a commit that referenced this pull request Aug 11, 2026
* Purchasing PO create/list/detail: po-created-by green
PoCreatePage (CompanyLookup + reference + save -> 201 redirect),
PoListPage, PoDetailPage with PoSummaryCard (created-by input,
reference autosave, status select), usePoLines over the single PATCH
endpoint, and the missing 'po' concurrency invalidator so 412/428
recovery actually refetches. Ports createTestPurchaseOrder +
waitForPoAutosave and the po-created-by spec.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* PO lines grid: create-purchase-order green
PoLinesTable as the third grid on the useReactTable + useDraftRows
pattern (phantom row, no add-line button, row-exit draft commit —
unit-cost stays the row's last focusable cell so the spec's Tab exits
the row). ItemSelect generalised for stock-only consumers (optional
jobId/line, label + wrapper overrides; labour-rates query gated on
jobId presence only, since textOnly labels need rate names). Inline
JobSelect over purchasing_all_jobs_retrieve — the unfiltered endpoint
v1's PO page uses, because fresh jobs are draft and the filtered
sibling excludes them. Spec ported with the autosave waiter armed
before the pick/status clicks (v2 saves immediately; v1's debounce is
what made arm-after work).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Stock page: stock-search green
Spec-lean StockPage: full active-stock list on load, 300ms-debounced
server FTS from 3 characters, enabled-gated so clearing the box
renders the cached list with no /search/ request.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Rewrite-status: purchasing slice recorded (29 of 40)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unit tests for PO line draft rules and JobSelect filtering
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unify the table layer: shared DataTable/ListTable, no cross-domain imports
The purchasing slice's PoLinesTable was about to become a third
hand-rolled grid emitting the DataTable-row-N/data-grid-* contract
inline. features/shared/DataTable.tsx is now the one owner of that
contract; SmartTimesheetTable, CostLineGrid and PoLinesTable all
render through it.
Auditing further found the same pathology in the plain list pages:
PoListPage and StockPage (both new this slice) had copied
CompaniesListPage's table shell and loading/error/retry block
verbatim, taking an existing duplicate (also in WipReportPage) from
2 instances to 4. features/shared/ListTable.tsx is the one owner of
that block instead -- deliberately separate from DataTable, since it
has no react-table dependency and forcing static lists through
column-def machinery would be indirection, not rigor. A hand-rolled
debounce-into-query-state pattern in CompaniesListPage and StockPage
is now features/shared/useDebouncedValue.ts (KanbanSearchInput keeps
its own URL-driven debounce -- a different concept).
ItemSelect, the decimal helpers and the Save-failed badge move to
features/shared/ because purchasing consumed them cross-domain -- a
domain feature is not a library.
ADR 0039 strengthened: unification is never deferred, shared concepts
get shared homes, the bar is reference quality.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Review fixes: QueryState primitive, item-label bug, company relocated
Adversarial review on the branch found the ListTable audit had stopped
short: JobMovementReportPage and CompanyDetailPage hand-rolled the same
loading/error/retry block ListTable was built to own (6 real instances,
only 4 fixed). Split the block itself out as
features/shared/QueryState.tsx -- the pending/error gate alone, no
table -- so it fits pages that show something other than a table too.
ListTable now composes QueryState instead of duplicating it. PoDetailPage,
CostLineGrid, JobMovementReportPage and CompanyDetailPage all render
through it.
Fixed: PoLinesTable's item-picker label read `item_code ?? 'Select Item'`
with no description fallback, so a bound stock item with a null code
(nullable, v1 parity) misread as unbound. Now poLineItemLabel() in
lines.ts, unit-tested.
Moved features/company to features/shared/company: it was already
cross-imported by features/job (JobCreatePage, JobSettingsTab) before
this slice added a third importer (purchasing/PoSummaryCard) -- it never
had a route of its own, a shared widget library in a domain-shaped box.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* ADR 0039: name the deadline-vs-duplicate resolution explicitly
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Docs: Slice 3 (SSE + serving model) is MUST before cutover, not deferred
Reversed 2026-08-11. The 2026-08-10 record said SSE ships with the
production-serving decision, filed under "Post-cutover -- decided,
deliberately NOT before 15 August." That was overturned: racing bad
architecture into production defeats the point of the rewrite
(non-negotiable #3 in the Cutover section), and the interim polling
shape plus the un-runnable apps/xero/sync_stream.py view are exactly
that. Slice 3 -- live updates done properly (serving model fix + SSE
ticker + discard the interim shortcuts) moves to its own MUST-tier
section with a milestone checkbox, and every stale "deferred
post-cutover" cross-reference in the file is corrected. The
purchasing-slice PR is unaffected -- it is unrelated and merges as
planned; this correction only fixes the durable record for the next
session.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Third review pass: 4 more QueryState conversions; rewrite-status cleanup
QueryState grew optional loadingNode/errorNode override props so a
spinner-based caller keeps its visual shell instead of losing it to
the plain-text default. JobFinishTab, DailyOverviewPage, and both
gates in TimesheetEntryPage's EntryWorkspace now render through it.
XeroQuoteCard, JobInvoiceCard and JobSettingsTab's pay-item field stay
excluded -- richer branching than binary success/fail, not the
page-level gate QueryState owns. TimesheetEntryPage's own outer gate
stays as guard-clause `if` returns, matching CLAUDE.md's stated
preference rather than converting to a shape that would abandon it.
Also: rewrite-status.md's narration of "review round found X, we
fixed Y" replaced with what the outcomes actually are -- which shared
component owns which contract, which sites are deliberately excluded
and why, which constraints a future change must respect. And the
Cutover section now states the actual two-question go/no-go criteria
(functional parity, proxied by MUST-tier E2E; materially better
architecture, judged directly) rather than only the E2E proxy, with
the honest fallback (abort and stay on v1) stated explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Address CodeRabbit review: real bugs and comment cleanup
- CompaniesListPage/JobFinishTab: QueryState was passed the raw
isError, breaking the first-load-only rule the other converted
sites already follow -- a background refetch failure was
unmounting already-rendered content instead of leaving it on
screen.
- JobSelect: closing the picker without a selection (blur or
Escape) left `editing` true forever, so the input kept showing
the abandoned search term instead of reverting to the bound job;
the blur timer was untracked and could fire a state update after
unmount. Also added keyboard selection (arrow keys + Enter,
role=listbox/option, aria-activedescendant) -- the dropdown was
mouse-only.
- PoListPage: the row was reachable only by mouse click; added a
real Link on the PO number cell, matching CompaniesListPage's
existing pattern.
- decimal.ts: parseDecimalInput accepted anything Number() parses,
including hex/octal/binary literals and bare exponents, and sent
them to the wire verbatim as garbage Decimal strings. Validates
fixed-point syntax explicitly now.
- Stale features/company/* paths in rewrite-status.md's build-order
table, left behind by the features/shared/company move.
- Comments narrating port history or test intent rewritten to state
the rejected alternative and the constraint (ADR 0043) in
ItemSelect, SaveFailedBadge, CreateCompanyModal, CompanyLookup;
two flagged sites (the E2E-repair-loop comment, the sonner test
comment) were already compliant and left as-is.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix uv.lock/pyproject.toml desync on the django constraint
PR #54 (main) widened pyproject.toml's django specifier to
>=6.0,<6.2 but never regenerated the matching uv.lock, which still
recorded <6.1 in its requires-dist metadata. Every `uv run` this
session silently self-corrected the lock locally; committing that
fix rather than leaving a lockfile permanently out of sync with the
manifest it's supposed to pin.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Gitignore the root-level test-results/ Playwright writes too
frontend/test-results/ was covered; run_e2e.sh's Playwright
invocation also writes artifacts relative to the repo root in some
invocations, leaving an untracked test-results/ dangling after every
E2E run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix the actual bug behind the stray root test-results/: cwd-relative config
playwright.config.ts resolved .env.test, testDir, outputDir and the
html reporter's folder relative to process.cwd(). Any invocation
whose cwd wasn't frontend/ (npm --prefix from the repo root, a bare
npx playwright test) silently missed .env.test -- dropping
E2E_TEST_USERNAME/PASSWORD with no error -- or wrote artifacts to
the wrong location instead of erroring loudly, which is how a
root-level test-results/ kept reappearing. Anchored every one of
these to import.meta.dirname instead: verified from an unrelated
cwd, .env/.env.test still resolve correctly and no stray directory
gets created anywhere. The root-level gitignore entry from the
previous commit stays as a defensive backstop, but this is the real
fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Phase 3a (1/3): CRM services + API, schema-parity instrument - #3

Merged
corrin merged 2 commits into
mainfrom
phase3a-1-crm
Aug 2, 2026
Merged

Phase 3a (1/3): CRM services + API, schema-parity instrument#3
corrin merged 2 commits into
mainfrom
phase3a-1-crm

Conversation

@corrin

Copy link
Copy Markdown
Owner

CRM: 18/18 v1 operations with exact-URL pins for the external phone-provider portal; celery tasks + beat-in-code; 48 tests. Infra: schema_parity_diff.py CI gate (wire-level, ledger-aware). Part 1 of the 3a stack.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222

CRM: 18/18 v1 operations (phone calls, recordings with ETag/304, endpoints
CRUD, provider settings), exact-URL pins for the external provider portal,
requests-based client, celery tasks + beat-in-code, 48 tests.
Infrastructure: scripts/schema_parity_diff.py (live OpenAPI vs v1's frozen
contract, ledger-aware, wire-level) wired into CI.
Stacked 1/3; company impl and tests follow.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@coderabbitai

coderabbitaiBot commented Aug 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 95664726-a6bb-43bf-b5a1-b729ea799a5b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f129cb and 42b51fe.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • .env.example
  • .github/workflows/ci.yml
  • .gitignore
  • apps/crm/api.py
  • apps/crm/schemas.py
  • apps/crm/services/__init__.py
  • apps/crm/services/phone_call_service.py
  • apps/crm/tasks.py
  • apps/crm/tests/__init__.py
  • apps/crm/tests/helpers.py
  • apps/crm/tests/test_phone_call_api.py
  • apps/crm/tests/test_phone_call_service.py
  • apps/crm/tests/test_phone_endpoint_guard.py
  • apps/crm/tests/urls.py
  • config/api.py
  • config/celery.py
  • config/settings.py
  • config/settings_test.py
  • docs/accepted-api-differences.yml
  • pyproject.toml
  • scripts/schema-parity-baseline.txt
  • scripts/schema_parity_diff.py

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.

- Recording-download params: null origin/destination stays empty (v1
omit-on-None semantics), not the literal string None (provider-boundary
parity).
- Phone-endpoint number stripped as v1's serializer did.
- schema_parity_diff.py: ratcheting baseline of ever-matched operationIds
(scripts/schema-parity-baseline.txt) - a ported operation vanishing is
now DRIFT, not 'not yet ported'; malformed ledger entries hard-fail;
--update-baseline grows the file deliberately. (Reviewer caught the
instrument's regression blindness.)
- Ledger: loose v1 query values (page=last, yes-booleans) now 422.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@corrin
corrin merged commit b5b6c4d into mainAug 2, 2026
3 checks passed
@corrin
corrin deleted the phase3a-1-crm branch August 2, 2026 03:51
corrin added a commit that referenced this pull request Aug 11, 2026
* Purchasing PO create/list/detail: po-created-by green
PoCreatePage (CompanyLookup + reference + save -> 201 redirect),
PoListPage, PoDetailPage with PoSummaryCard (created-by input,
reference autosave, status select), usePoLines over the single PATCH
endpoint, and the missing 'po' concurrency invalidator so 412/428
recovery actually refetches. Ports createTestPurchaseOrder +
waitForPoAutosave and the po-created-by spec.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* PO lines grid: create-purchase-order green
PoLinesTable as the third grid on the useReactTable + useDraftRows
pattern (phantom row, no add-line button, row-exit draft commit —
unit-cost stays the row's last focusable cell so the spec's Tab exits
the row). ItemSelect generalised for stock-only consumers (optional
jobId/line, label + wrapper overrides; labour-rates query gated on
jobId presence only, since textOnly labels need rate names). Inline
JobSelect over purchasing_all_jobs_retrieve — the unfiltered endpoint
v1's PO page uses, because fresh jobs are draft and the filtered
sibling excludes them. Spec ported with the autosave waiter armed
before the pick/status clicks (v2 saves immediately; v1's debounce is
what made arm-after work).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Stock page: stock-search green
Spec-lean StockPage: full active-stock list on load, 300ms-debounced
server FTS from 3 characters, enabled-gated so clearing the box
renders the cached list with no /search/ request.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Rewrite-status: purchasing slice recorded (29 of 40)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unit tests for PO line draft rules and JobSelect filtering
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unify the table layer: shared DataTable/ListTable, no cross-domain imports
The purchasing slice's PoLinesTable was about to become a third
hand-rolled grid emitting the DataTable-row-N/data-grid-* contract
inline. features/shared/DataTable.tsx is now the one owner of that
contract; SmartTimesheetTable, CostLineGrid and PoLinesTable all
render through it.
Auditing further found the same pathology in the plain list pages:
PoListPage and StockPage (both new this slice) had copied
CompaniesListPage's table shell and loading/error/retry block
verbatim, taking an existing duplicate (also in WipReportPage) from
2 instances to 4. features/shared/ListTable.tsx is the one owner of
that block instead -- deliberately separate from DataTable, since it
has no react-table dependency and forcing static lists through
column-def machinery would be indirection, not rigor. A hand-rolled
debounce-into-query-state pattern in CompaniesListPage and StockPage
is now features/shared/useDebouncedValue.ts (KanbanSearchInput keeps
its own URL-driven debounce -- a different concept).
ItemSelect, the decimal helpers and the Save-failed badge move to
features/shared/ because purchasing consumed them cross-domain -- a
domain feature is not a library.
ADR 0039 strengthened: unification is never deferred, shared concepts
get shared homes, the bar is reference quality.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Review fixes: QueryState primitive, item-label bug, company relocated
Adversarial review on the branch found the ListTable audit had stopped
short: JobMovementReportPage and CompanyDetailPage hand-rolled the same
loading/error/retry block ListTable was built to own (6 real instances,
only 4 fixed). Split the block itself out as
features/shared/QueryState.tsx -- the pending/error gate alone, no
table -- so it fits pages that show something other than a table too.
ListTable now composes QueryState instead of duplicating it. PoDetailPage,
CostLineGrid, JobMovementReportPage and CompanyDetailPage all render
through it.
Fixed: PoLinesTable's item-picker label read `item_code ?? 'Select Item'`
with no description fallback, so a bound stock item with a null code
(nullable, v1 parity) misread as unbound. Now poLineItemLabel() in
lines.ts, unit-tested.
Moved features/company to features/shared/company: it was already
cross-imported by features/job (JobCreatePage, JobSettingsTab) before
this slice added a third importer (purchasing/PoSummaryCard) -- it never
had a route of its own, a shared widget library in a domain-shaped box.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* ADR 0039: name the deadline-vs-duplicate resolution explicitly
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Docs: Slice 3 (SSE + serving model) is MUST before cutover, not deferred
Reversed 2026-08-11. The 2026-08-10 record said SSE ships with the
production-serving decision, filed under "Post-cutover -- decided,
deliberately NOT before 15 August." That was overturned: racing bad
architecture into production defeats the point of the rewrite
(non-negotiable #3 in the Cutover section), and the interim polling
shape plus the un-runnable apps/xero/sync_stream.py view are exactly
that. Slice 3 -- live updates done properly (serving model fix + SSE
ticker + discard the interim shortcuts) moves to its own MUST-tier
section with a milestone checkbox, and every stale "deferred
post-cutover" cross-reference in the file is corrected. The
purchasing-slice PR is unaffected -- it is unrelated and merges as
planned; this correction only fixes the durable record for the next
session.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Third review pass: 4 more QueryState conversions; rewrite-status cleanup
QueryState grew optional loadingNode/errorNode override props so a
spinner-based caller keeps its visual shell instead of losing it to
the plain-text default. JobFinishTab, DailyOverviewPage, and both
gates in TimesheetEntryPage's EntryWorkspace now render through it.
XeroQuoteCard, JobInvoiceCard and JobSettingsTab's pay-item field stay
excluded -- richer branching than binary success/fail, not the
page-level gate QueryState owns. TimesheetEntryPage's own outer gate
stays as guard-clause `if` returns, matching CLAUDE.md's stated
preference rather than converting to a shape that would abandon it.
Also: rewrite-status.md's narration of "review round found X, we
fixed Y" replaced with what the outcomes actually are -- which shared
component owns which contract, which sites are deliberately excluded
and why, which constraints a future change must respect. And the
Cutover section now states the actual two-question go/no-go criteria
(functional parity, proxied by MUST-tier E2E; materially better
architecture, judged directly) rather than only the E2E proxy, with
the honest fallback (abort and stay on v1) stated explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Address CodeRabbit review: real bugs and comment cleanup
- CompaniesListPage/JobFinishTab: QueryState was passed the raw
isError, breaking the first-load-only rule the other converted
sites already follow -- a background refetch failure was
unmounting already-rendered content instead of leaving it on
screen.
- JobSelect: closing the picker without a selection (blur or
Escape) left `editing` true forever, so the input kept showing
the abandoned search term instead of reverting to the bound job;
the blur timer was untracked and could fire a state update after
unmount. Also added keyboard selection (arrow keys + Enter,
role=listbox/option, aria-activedescendant) -- the dropdown was
mouse-only.
- PoListPage: the row was reachable only by mouse click; added a
real Link on the PO number cell, matching CompaniesListPage's
existing pattern.
- decimal.ts: parseDecimalInput accepted anything Number() parses,
including hex/octal/binary literals and bare exponents, and sent
them to the wire verbatim as garbage Decimal strings. Validates
fixed-point syntax explicitly now.
- Stale features/company/* paths in rewrite-status.md's build-order
table, left behind by the features/shared/company move.
- Comments narrating port history or test intent rewritten to state
the rejected alternative and the constraint (ADR 0043) in
ItemSelect, SaveFailedBadge, CreateCompanyModal, CompanyLookup;
two flagged sites (the E2E-repair-loop comment, the sonner test
comment) were already compliant and left as-is.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix uv.lock/pyproject.toml desync on the django constraint
PR #54 (main) widened pyproject.toml's django specifier to
>=6.0,<6.2 but never regenerated the matching uv.lock, which still
recorded <6.1 in its requires-dist metadata. Every `uv run` this
session silently self-corrected the lock locally; committing that
fix rather than leaving a lockfile permanently out of sync with the
manifest it's supposed to pin.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Gitignore the root-level test-results/ Playwright writes too
frontend/test-results/ was covered; run_e2e.sh's Playwright
invocation also writes artifacts relative to the repo root in some
invocations, leaving an untracked test-results/ dangling after every
E2E run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix the actual bug behind the stray root test-results/: cwd-relative config
playwright.config.ts resolved .env.test, testDir, outputDir and the
html reporter's folder relative to process.cwd(). Any invocation
whose cwd wasn't frontend/ (npm --prefix from the repo root, a bare
npx playwright test) silently missed .env.test -- dropping
E2E_TEST_USERNAME/PASSWORD with no error -- or wrote artifacts to
the wrong location instead of erroring loudly, which is how a
root-level test-results/ kept reappearing. Anchored every one of
these to import.meta.dirname instead: verified from an unrelated
cwd, .env/.env.test still resolve correctly and no stray directory
gets created anywhere. The root-level gitignore entry from the
previous commit stays as a defensive backstop, but this is the real
fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Phase 3a (1/3): CRM services + API, schema-parity instrument - #3

Merged
corrin merged 2 commits into
mainfrom
phase3a-1-crm
Aug 2, 2026
Merged

Phase 3a (1/3): CRM services + API, schema-parity instrument#3
corrin merged 2 commits into
mainfrom
phase3a-1-crm

Conversation

@corrin

Copy link
Copy Markdown
Owner

CRM: 18/18 v1 operations with exact-URL pins for the external phone-provider portal; celery tasks + beat-in-code; 48 tests. Infra: schema_parity_diff.py CI gate (wire-level, ledger-aware). Part 1 of the 3a stack.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222

CRM: 18/18 v1 operations (phone calls, recordings with ETag/304, endpoints
CRUD, provider settings), exact-URL pins for the external provider portal,
requests-based client, celery tasks + beat-in-code, 48 tests.
Infrastructure: scripts/schema_parity_diff.py (live OpenAPI vs v1's frozen
contract, ledger-aware, wire-level) wired into CI.
Stacked 1/3; company impl and tests follow.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@coderabbitai

coderabbitaiBot commented Aug 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 95664726-a6bb-43bf-b5a1-b729ea799a5b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f129cb and 42b51fe.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • .env.example
  • .github/workflows/ci.yml
  • .gitignore
  • apps/crm/api.py
  • apps/crm/schemas.py
  • apps/crm/services/__init__.py
  • apps/crm/services/phone_call_service.py
  • apps/crm/tasks.py
  • apps/crm/tests/__init__.py
  • apps/crm/tests/helpers.py
  • apps/crm/tests/test_phone_call_api.py
  • apps/crm/tests/test_phone_call_service.py
  • apps/crm/tests/test_phone_endpoint_guard.py
  • apps/crm/tests/urls.py
  • config/api.py
  • config/celery.py
  • config/settings.py
  • config/settings_test.py
  • docs/accepted-api-differences.yml
  • pyproject.toml
  • scripts/schema-parity-baseline.txt
  • scripts/schema_parity_diff.py

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.

- Recording-download params: null origin/destination stays empty (v1
omit-on-None semantics), not the literal string None (provider-boundary
parity).
- Phone-endpoint number stripped as v1's serializer did.
- schema_parity_diff.py: ratcheting baseline of ever-matched operationIds
(scripts/schema-parity-baseline.txt) - a ported operation vanishing is
now DRIFT, not 'not yet ported'; malformed ledger entries hard-fail;
--update-baseline grows the file deliberately. (Reviewer caught the
instrument's regression blindness.)
- Ledger: loose v1 query values (page=last, yes-booleans) now 422.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@corrin
corrin merged commit b5b6c4d into mainAug 2, 2026
3 checks passed
@corrin
corrin deleted the phase3a-1-crm branch August 2, 2026 03:51
corrin added a commit that referenced this pull request Aug 11, 2026
* Purchasing PO create/list/detail: po-created-by green
PoCreatePage (CompanyLookup + reference + save -> 201 redirect),
PoListPage, PoDetailPage with PoSummaryCard (created-by input,
reference autosave, status select), usePoLines over the single PATCH
endpoint, and the missing 'po' concurrency invalidator so 412/428
recovery actually refetches. Ports createTestPurchaseOrder +
waitForPoAutosave and the po-created-by spec.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* PO lines grid: create-purchase-order green
PoLinesTable as the third grid on the useReactTable + useDraftRows
pattern (phantom row, no add-line button, row-exit draft commit —
unit-cost stays the row's last focusable cell so the spec's Tab exits
the row). ItemSelect generalised for stock-only consumers (optional
jobId/line, label + wrapper overrides; labour-rates query gated on
jobId presence only, since textOnly labels need rate names). Inline
JobSelect over purchasing_all_jobs_retrieve — the unfiltered endpoint
v1's PO page uses, because fresh jobs are draft and the filtered
sibling excludes them. Spec ported with the autosave waiter armed
before the pick/status clicks (v2 saves immediately; v1's debounce is
what made arm-after work).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Stock page: stock-search green
Spec-lean StockPage: full active-stock list on load, 300ms-debounced
server FTS from 3 characters, enabled-gated so clearing the box
renders the cached list with no /search/ request.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Rewrite-status: purchasing slice recorded (29 of 40)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unit tests for PO line draft rules and JobSelect filtering
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unify the table layer: shared DataTable/ListTable, no cross-domain imports
The purchasing slice's PoLinesTable was about to become a third
hand-rolled grid emitting the DataTable-row-N/data-grid-* contract
inline. features/shared/DataTable.tsx is now the one owner of that
contract; SmartTimesheetTable, CostLineGrid and PoLinesTable all
render through it.
Auditing further found the same pathology in the plain list pages:
PoListPage and StockPage (both new this slice) had copied
CompaniesListPage's table shell and loading/error/retry block
verbatim, taking an existing duplicate (also in WipReportPage) from
2 instances to 4. features/shared/ListTable.tsx is the one owner of
that block instead -- deliberately separate from DataTable, since it
has no react-table dependency and forcing static lists through
column-def machinery would be indirection, not rigor. A hand-rolled
debounce-into-query-state pattern in CompaniesListPage and StockPage
is now features/shared/useDebouncedValue.ts (KanbanSearchInput keeps
its own URL-driven debounce -- a different concept).
ItemSelect, the decimal helpers and the Save-failed badge move to
features/shared/ because purchasing consumed them cross-domain -- a
domain feature is not a library.
ADR 0039 strengthened: unification is never deferred, shared concepts
get shared homes, the bar is reference quality.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Review fixes: QueryState primitive, item-label bug, company relocated
Adversarial review on the branch found the ListTable audit had stopped
short: JobMovementReportPage and CompanyDetailPage hand-rolled the same
loading/error/retry block ListTable was built to own (6 real instances,
only 4 fixed). Split the block itself out as
features/shared/QueryState.tsx -- the pending/error gate alone, no
table -- so it fits pages that show something other than a table too.
ListTable now composes QueryState instead of duplicating it. PoDetailPage,
CostLineGrid, JobMovementReportPage and CompanyDetailPage all render
through it.
Fixed: PoLinesTable's item-picker label read `item_code ?? 'Select Item'`
with no description fallback, so a bound stock item with a null code
(nullable, v1 parity) misread as unbound. Now poLineItemLabel() in
lines.ts, unit-tested.
Moved features/company to features/shared/company: it was already
cross-imported by features/job (JobCreatePage, JobSettingsTab) before
this slice added a third importer (purchasing/PoSummaryCard) -- it never
had a route of its own, a shared widget library in a domain-shaped box.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* ADR 0039: name the deadline-vs-duplicate resolution explicitly
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Docs: Slice 3 (SSE + serving model) is MUST before cutover, not deferred
Reversed 2026-08-11. The 2026-08-10 record said SSE ships with the
production-serving decision, filed under "Post-cutover -- decided,
deliberately NOT before 15 August." That was overturned: racing bad
architecture into production defeats the point of the rewrite
(non-negotiable #3 in the Cutover section), and the interim polling
shape plus the un-runnable apps/xero/sync_stream.py view are exactly
that. Slice 3 -- live updates done properly (serving model fix + SSE
ticker + discard the interim shortcuts) moves to its own MUST-tier
section with a milestone checkbox, and every stale "deferred
post-cutover" cross-reference in the file is corrected. The
purchasing-slice PR is unaffected -- it is unrelated and merges as
planned; this correction only fixes the durable record for the next
session.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Third review pass: 4 more QueryState conversions; rewrite-status cleanup
QueryState grew optional loadingNode/errorNode override props so a
spinner-based caller keeps its visual shell instead of losing it to
the plain-text default. JobFinishTab, DailyOverviewPage, and both
gates in TimesheetEntryPage's EntryWorkspace now render through it.
XeroQuoteCard, JobInvoiceCard and JobSettingsTab's pay-item field stay
excluded -- richer branching than binary success/fail, not the
page-level gate QueryState owns. TimesheetEntryPage's own outer gate
stays as guard-clause `if` returns, matching CLAUDE.md's stated
preference rather than converting to a shape that would abandon it.
Also: rewrite-status.md's narration of "review round found X, we
fixed Y" replaced with what the outcomes actually are -- which shared
component owns which contract, which sites are deliberately excluded
and why, which constraints a future change must respect. And the
Cutover section now states the actual two-question go/no-go criteria
(functional parity, proxied by MUST-tier E2E; materially better
architecture, judged directly) rather than only the E2E proxy, with
the honest fallback (abort and stay on v1) stated explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Address CodeRabbit review: real bugs and comment cleanup
- CompaniesListPage/JobFinishTab: QueryState was passed the raw
isError, breaking the first-load-only rule the other converted
sites already follow -- a background refetch failure was
unmounting already-rendered content instead of leaving it on
screen.
- JobSelect: closing the picker without a selection (blur or
Escape) left `editing` true forever, so the input kept showing
the abandoned search term instead of reverting to the bound job;
the blur timer was untracked and could fire a state update after
unmount. Also added keyboard selection (arrow keys + Enter,
role=listbox/option, aria-activedescendant) -- the dropdown was
mouse-only.
- PoListPage: the row was reachable only by mouse click; added a
real Link on the PO number cell, matching CompaniesListPage's
existing pattern.
- decimal.ts: parseDecimalInput accepted anything Number() parses,
including hex/octal/binary literals and bare exponents, and sent
them to the wire verbatim as garbage Decimal strings. Validates
fixed-point syntax explicitly now.
- Stale features/company/* paths in rewrite-status.md's build-order
table, left behind by the features/shared/company move.
- Comments narrating port history or test intent rewritten to state
the rejected alternative and the constraint (ADR 0043) in
ItemSelect, SaveFailedBadge, CreateCompanyModal, CompanyLookup;
two flagged sites (the E2E-repair-loop comment, the sonner test
comment) were already compliant and left as-is.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix uv.lock/pyproject.toml desync on the django constraint
PR #54 (main) widened pyproject.toml's django specifier to
>=6.0,<6.2 but never regenerated the matching uv.lock, which still
recorded <6.1 in its requires-dist metadata. Every `uv run` this
session silently self-corrected the lock locally; committing that
fix rather than leaving a lockfile permanently out of sync with the
manifest it's supposed to pin.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Gitignore the root-level test-results/ Playwright writes too
frontend/test-results/ was covered; run_e2e.sh's Playwright
invocation also writes artifacts relative to the repo root in some
invocations, leaving an untracked test-results/ dangling after every
E2E run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix the actual bug behind the stray root test-results/: cwd-relative config
playwright.config.ts resolved .env.test, testDir, outputDir and the
html reporter's folder relative to process.cwd(). Any invocation
whose cwd wasn't frontend/ (npm --prefix from the repo root, a bare
npx playwright test) silently missed .env.test -- dropping
E2E_TEST_USERNAME/PASSWORD with no error -- or wrote artifacts to
the wrong location instead of erroring loudly, which is how a
root-level test-results/ kept reappearing. Anchored every one of
these to import.meta.dirname instead: verified from an unrelated
cwd, .env/.env.test still resolve correctly and no stray directory
gets created anywhere. The root-level gitignore entry from the
previous commit stays as a defensive backstop, but this is the real
fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Phase 3a (1/3): CRM services + API, schema-parity instrument - #3

Merged
corrin merged 2 commits into
mainfrom
phase3a-1-crm
Aug 2, 2026
Merged

Phase 3a (1/3): CRM services + API, schema-parity instrument#3
corrin merged 2 commits into
mainfrom
phase3a-1-crm

Conversation

@corrin

Copy link
Copy Markdown
Owner

CRM: 18/18 v1 operations with exact-URL pins for the external phone-provider portal; celery tasks + beat-in-code; 48 tests. Infra: schema_parity_diff.py CI gate (wire-level, ledger-aware). Part 1 of the 3a stack.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222

CRM: 18/18 v1 operations (phone calls, recordings with ETag/304, endpoints
CRUD, provider settings), exact-URL pins for the external provider portal,
requests-based client, celery tasks + beat-in-code, 48 tests.
Infrastructure: scripts/schema_parity_diff.py (live OpenAPI vs v1's frozen
contract, ledger-aware, wire-level) wired into CI.
Stacked 1/3; company impl and tests follow.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@coderabbitai

coderabbitaiBot commented Aug 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 95664726-a6bb-43bf-b5a1-b729ea799a5b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f129cb and 42b51fe.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • .env.example
  • .github/workflows/ci.yml
  • .gitignore
  • apps/crm/api.py
  • apps/crm/schemas.py
  • apps/crm/services/__init__.py
  • apps/crm/services/phone_call_service.py
  • apps/crm/tasks.py
  • apps/crm/tests/__init__.py
  • apps/crm/tests/helpers.py
  • apps/crm/tests/test_phone_call_api.py
  • apps/crm/tests/test_phone_call_service.py
  • apps/crm/tests/test_phone_endpoint_guard.py
  • apps/crm/tests/urls.py
  • config/api.py
  • config/celery.py
  • config/settings.py
  • config/settings_test.py
  • docs/accepted-api-differences.yml
  • pyproject.toml
  • scripts/schema-parity-baseline.txt
  • scripts/schema_parity_diff.py

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.

- Recording-download params: null origin/destination stays empty (v1
omit-on-None semantics), not the literal string None (provider-boundary
parity).
- Phone-endpoint number stripped as v1's serializer did.
- schema_parity_diff.py: ratcheting baseline of ever-matched operationIds
(scripts/schema-parity-baseline.txt) - a ported operation vanishing is
now DRIFT, not 'not yet ported'; malformed ledger entries hard-fail;
--update-baseline grows the file deliberately. (Reviewer caught the
instrument's regression blindness.)
- Ledger: loose v1 query values (page=last, yes-booleans) now 422.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@corrin
corrin merged commit b5b6c4d into mainAug 2, 2026
3 checks passed
@corrin
corrin deleted the phase3a-1-crm branch August 2, 2026 03:51
corrin added a commit that referenced this pull request Aug 11, 2026
* Purchasing PO create/list/detail: po-created-by green
PoCreatePage (CompanyLookup + reference + save -> 201 redirect),
PoListPage, PoDetailPage with PoSummaryCard (created-by input,
reference autosave, status select), usePoLines over the single PATCH
endpoint, and the missing 'po' concurrency invalidator so 412/428
recovery actually refetches. Ports createTestPurchaseOrder +
waitForPoAutosave and the po-created-by spec.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* PO lines grid: create-purchase-order green
PoLinesTable as the third grid on the useReactTable + useDraftRows
pattern (phantom row, no add-line button, row-exit draft commit —
unit-cost stays the row's last focusable cell so the spec's Tab exits
the row). ItemSelect generalised for stock-only consumers (optional
jobId/line, label + wrapper overrides; labour-rates query gated on
jobId presence only, since textOnly labels need rate names). Inline
JobSelect over purchasing_all_jobs_retrieve — the unfiltered endpoint
v1's PO page uses, because fresh jobs are draft and the filtered
sibling excludes them. Spec ported with the autosave waiter armed
before the pick/status clicks (v2 saves immediately; v1's debounce is
what made arm-after work).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Stock page: stock-search green
Spec-lean StockPage: full active-stock list on load, 300ms-debounced
server FTS from 3 characters, enabled-gated so clearing the box
renders the cached list with no /search/ request.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Rewrite-status: purchasing slice recorded (29 of 40)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unit tests for PO line draft rules and JobSelect filtering
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unify the table layer: shared DataTable/ListTable, no cross-domain imports
The purchasing slice's PoLinesTable was about to become a third
hand-rolled grid emitting the DataTable-row-N/data-grid-* contract
inline. features/shared/DataTable.tsx is now the one owner of that
contract; SmartTimesheetTable, CostLineGrid and PoLinesTable all
render through it.
Auditing further found the same pathology in the plain list pages:
PoListPage and StockPage (both new this slice) had copied
CompaniesListPage's table shell and loading/error/retry block
verbatim, taking an existing duplicate (also in WipReportPage) from
2 instances to 4. features/shared/ListTable.tsx is the one owner of
that block instead -- deliberately separate from DataTable, since it
has no react-table dependency and forcing static lists through
column-def machinery would be indirection, not rigor. A hand-rolled
debounce-into-query-state pattern in CompaniesListPage and StockPage
is now features/shared/useDebouncedValue.ts (KanbanSearchInput keeps
its own URL-driven debounce -- a different concept).
ItemSelect, the decimal helpers and the Save-failed badge move to
features/shared/ because purchasing consumed them cross-domain -- a
domain feature is not a library.
ADR 0039 strengthened: unification is never deferred, shared concepts
get shared homes, the bar is reference quality.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Review fixes: QueryState primitive, item-label bug, company relocated
Adversarial review on the branch found the ListTable audit had stopped
short: JobMovementReportPage and CompanyDetailPage hand-rolled the same
loading/error/retry block ListTable was built to own (6 real instances,
only 4 fixed). Split the block itself out as
features/shared/QueryState.tsx -- the pending/error gate alone, no
table -- so it fits pages that show something other than a table too.
ListTable now composes QueryState instead of duplicating it. PoDetailPage,
CostLineGrid, JobMovementReportPage and CompanyDetailPage all render
through it.
Fixed: PoLinesTable's item-picker label read `item_code ?? 'Select Item'`
with no description fallback, so a bound stock item with a null code
(nullable, v1 parity) misread as unbound. Now poLineItemLabel() in
lines.ts, unit-tested.
Moved features/company to features/shared/company: it was already
cross-imported by features/job (JobCreatePage, JobSettingsTab) before
this slice added a third importer (purchasing/PoSummaryCard) -- it never
had a route of its own, a shared widget library in a domain-shaped box.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* ADR 0039: name the deadline-vs-duplicate resolution explicitly
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Docs: Slice 3 (SSE + serving model) is MUST before cutover, not deferred
Reversed 2026-08-11. The 2026-08-10 record said SSE ships with the
production-serving decision, filed under "Post-cutover -- decided,
deliberately NOT before 15 August." That was overturned: racing bad
architecture into production defeats the point of the rewrite
(non-negotiable #3 in the Cutover section), and the interim polling
shape plus the un-runnable apps/xero/sync_stream.py view are exactly
that. Slice 3 -- live updates done properly (serving model fix + SSE
ticker + discard the interim shortcuts) moves to its own MUST-tier
section with a milestone checkbox, and every stale "deferred
post-cutover" cross-reference in the file is corrected. The
purchasing-slice PR is unaffected -- it is unrelated and merges as
planned; this correction only fixes the durable record for the next
session.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Third review pass: 4 more QueryState conversions; rewrite-status cleanup
QueryState grew optional loadingNode/errorNode override props so a
spinner-based caller keeps its visual shell instead of losing it to
the plain-text default. JobFinishTab, DailyOverviewPage, and both
gates in TimesheetEntryPage's EntryWorkspace now render through it.
XeroQuoteCard, JobInvoiceCard and JobSettingsTab's pay-item field stay
excluded -- richer branching than binary success/fail, not the
page-level gate QueryState owns. TimesheetEntryPage's own outer gate
stays as guard-clause `if` returns, matching CLAUDE.md's stated
preference rather than converting to a shape that would abandon it.
Also: rewrite-status.md's narration of "review round found X, we
fixed Y" replaced with what the outcomes actually are -- which shared
component owns which contract, which sites are deliberately excluded
and why, which constraints a future change must respect. And the
Cutover section now states the actual two-question go/no-go criteria
(functional parity, proxied by MUST-tier E2E; materially better
architecture, judged directly) rather than only the E2E proxy, with
the honest fallback (abort and stay on v1) stated explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Address CodeRabbit review: real bugs and comment cleanup
- CompaniesListPage/JobFinishTab: QueryState was passed the raw
isError, breaking the first-load-only rule the other converted
sites already follow -- a background refetch failure was
unmounting already-rendered content instead of leaving it on
screen.
- JobSelect: closing the picker without a selection (blur or
Escape) left `editing` true forever, so the input kept showing
the abandoned search term instead of reverting to the bound job;
the blur timer was untracked and could fire a state update after
unmount. Also added keyboard selection (arrow keys + Enter,
role=listbox/option, aria-activedescendant) -- the dropdown was
mouse-only.
- PoListPage: the row was reachable only by mouse click; added a
real Link on the PO number cell, matching CompaniesListPage's
existing pattern.
- decimal.ts: parseDecimalInput accepted anything Number() parses,
including hex/octal/binary literals and bare exponents, and sent
them to the wire verbatim as garbage Decimal strings. Validates
fixed-point syntax explicitly now.
- Stale features/company/* paths in rewrite-status.md's build-order
table, left behind by the features/shared/company move.
- Comments narrating port history or test intent rewritten to state
the rejected alternative and the constraint (ADR 0043) in
ItemSelect, SaveFailedBadge, CreateCompanyModal, CompanyLookup;
two flagged sites (the E2E-repair-loop comment, the sonner test
comment) were already compliant and left as-is.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix uv.lock/pyproject.toml desync on the django constraint
PR #54 (main) widened pyproject.toml's django specifier to
>=6.0,<6.2 but never regenerated the matching uv.lock, which still
recorded <6.1 in its requires-dist metadata. Every `uv run` this
session silently self-corrected the lock locally; committing that
fix rather than leaving a lockfile permanently out of sync with the
manifest it's supposed to pin.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Gitignore the root-level test-results/ Playwright writes too
frontend/test-results/ was covered; run_e2e.sh's Playwright
invocation also writes artifacts relative to the repo root in some
invocations, leaving an untracked test-results/ dangling after every
E2E run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix the actual bug behind the stray root test-results/: cwd-relative config
playwright.config.ts resolved .env.test, testDir, outputDir and the
html reporter's folder relative to process.cwd(). Any invocation
whose cwd wasn't frontend/ (npm --prefix from the repo root, a bare
npx playwright test) silently missed .env.test -- dropping
E2E_TEST_USERNAME/PASSWORD with no error -- or wrote artifacts to
the wrong location instead of erroring loudly, which is how a
root-level test-results/ kept reappearing. Anchored every one of
these to import.meta.dirname instead: verified from an unrelated
cwd, .env/.env.test still resolve correctly and no stray directory
gets created anywhere. The root-level gitignore entry from the
previous commit stays as a defensive backstop, but this is the real
fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Phase 3a (1/3): CRM services + API, schema-parity instrument - #3

Merged
corrin merged 2 commits into
mainfrom
phase3a-1-crm
Aug 2, 2026
Merged

Phase 3a (1/3): CRM services + API, schema-parity instrument#3
corrin merged 2 commits into
mainfrom
phase3a-1-crm

Conversation

@corrin

Copy link
Copy Markdown
Owner

CRM: 18/18 v1 operations with exact-URL pins for the external phone-provider portal; celery tasks + beat-in-code; 48 tests. Infra: schema_parity_diff.py CI gate (wire-level, ledger-aware). Part 1 of the 3a stack.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222

CRM: 18/18 v1 operations (phone calls, recordings with ETag/304, endpoints
CRUD, provider settings), exact-URL pins for the external provider portal,
requests-based client, celery tasks + beat-in-code, 48 tests.
Infrastructure: scripts/schema_parity_diff.py (live OpenAPI vs v1's frozen
contract, ledger-aware, wire-level) wired into CI.
Stacked 1/3; company impl and tests follow.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@coderabbitai

coderabbitaiBot commented Aug 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 95664726-a6bb-43bf-b5a1-b729ea799a5b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f129cb and 42b51fe.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • .env.example
  • .github/workflows/ci.yml
  • .gitignore
  • apps/crm/api.py
  • apps/crm/schemas.py
  • apps/crm/services/__init__.py
  • apps/crm/services/phone_call_service.py
  • apps/crm/tasks.py
  • apps/crm/tests/__init__.py
  • apps/crm/tests/helpers.py
  • apps/crm/tests/test_phone_call_api.py
  • apps/crm/tests/test_phone_call_service.py
  • apps/crm/tests/test_phone_endpoint_guard.py
  • apps/crm/tests/urls.py
  • config/api.py
  • config/celery.py
  • config/settings.py
  • config/settings_test.py
  • docs/accepted-api-differences.yml
  • pyproject.toml
  • scripts/schema-parity-baseline.txt
  • scripts/schema_parity_diff.py

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.

- Recording-download params: null origin/destination stays empty (v1
omit-on-None semantics), not the literal string None (provider-boundary
parity).
- Phone-endpoint number stripped as v1's serializer did.
- schema_parity_diff.py: ratcheting baseline of ever-matched operationIds
(scripts/schema-parity-baseline.txt) - a ported operation vanishing is
now DRIFT, not 'not yet ported'; malformed ledger entries hard-fail;
--update-baseline grows the file deliberately. (Reviewer caught the
instrument's regression blindness.)
- Ledger: loose v1 query values (page=last, yes-booleans) now 422.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@corrin
corrin merged commit b5b6c4d into mainAug 2, 2026
3 checks passed
@corrin
corrin deleted the phase3a-1-crm branch August 2, 2026 03:51
corrin added a commit that referenced this pull request Aug 11, 2026
* Purchasing PO create/list/detail: po-created-by green
PoCreatePage (CompanyLookup + reference + save -> 201 redirect),
PoListPage, PoDetailPage with PoSummaryCard (created-by input,
reference autosave, status select), usePoLines over the single PATCH
endpoint, and the missing 'po' concurrency invalidator so 412/428
recovery actually refetches. Ports createTestPurchaseOrder +
waitForPoAutosave and the po-created-by spec.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* PO lines grid: create-purchase-order green
PoLinesTable as the third grid on the useReactTable + useDraftRows
pattern (phantom row, no add-line button, row-exit draft commit —
unit-cost stays the row's last focusable cell so the spec's Tab exits
the row). ItemSelect generalised for stock-only consumers (optional
jobId/line, label + wrapper overrides; labour-rates query gated on
jobId presence only, since textOnly labels need rate names). Inline
JobSelect over purchasing_all_jobs_retrieve — the unfiltered endpoint
v1's PO page uses, because fresh jobs are draft and the filtered
sibling excludes them. Spec ported with the autosave waiter armed
before the pick/status clicks (v2 saves immediately; v1's debounce is
what made arm-after work).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Stock page: stock-search green
Spec-lean StockPage: full active-stock list on load, 300ms-debounced
server FTS from 3 characters, enabled-gated so clearing the box
renders the cached list with no /search/ request.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Rewrite-status: purchasing slice recorded (29 of 40)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unit tests for PO line draft rules and JobSelect filtering
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unify the table layer: shared DataTable/ListTable, no cross-domain imports
The purchasing slice's PoLinesTable was about to become a third
hand-rolled grid emitting the DataTable-row-N/data-grid-* contract
inline. features/shared/DataTable.tsx is now the one owner of that
contract; SmartTimesheetTable, CostLineGrid and PoLinesTable all
render through it.
Auditing further found the same pathology in the plain list pages:
PoListPage and StockPage (both new this slice) had copied
CompaniesListPage's table shell and loading/error/retry block
verbatim, taking an existing duplicate (also in WipReportPage) from
2 instances to 4. features/shared/ListTable.tsx is the one owner of
that block instead -- deliberately separate from DataTable, since it
has no react-table dependency and forcing static lists through
column-def machinery would be indirection, not rigor. A hand-rolled
debounce-into-query-state pattern in CompaniesListPage and StockPage
is now features/shared/useDebouncedValue.ts (KanbanSearchInput keeps
its own URL-driven debounce -- a different concept).
ItemSelect, the decimal helpers and the Save-failed badge move to
features/shared/ because purchasing consumed them cross-domain -- a
domain feature is not a library.
ADR 0039 strengthened: unification is never deferred, shared concepts
get shared homes, the bar is reference quality.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Review fixes: QueryState primitive, item-label bug, company relocated
Adversarial review on the branch found the ListTable audit had stopped
short: JobMovementReportPage and CompanyDetailPage hand-rolled the same
loading/error/retry block ListTable was built to own (6 real instances,
only 4 fixed). Split the block itself out as
features/shared/QueryState.tsx -- the pending/error gate alone, no
table -- so it fits pages that show something other than a table too.
ListTable now composes QueryState instead of duplicating it. PoDetailPage,
CostLineGrid, JobMovementReportPage and CompanyDetailPage all render
through it.
Fixed: PoLinesTable's item-picker label read `item_code ?? 'Select Item'`
with no description fallback, so a bound stock item with a null code
(nullable, v1 parity) misread as unbound. Now poLineItemLabel() in
lines.ts, unit-tested.
Moved features/company to features/shared/company: it was already
cross-imported by features/job (JobCreatePage, JobSettingsTab) before
this slice added a third importer (purchasing/PoSummaryCard) -- it never
had a route of its own, a shared widget library in a domain-shaped box.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* ADR 0039: name the deadline-vs-duplicate resolution explicitly
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Docs: Slice 3 (SSE + serving model) is MUST before cutover, not deferred
Reversed 2026-08-11. The 2026-08-10 record said SSE ships with the
production-serving decision, filed under "Post-cutover -- decided,
deliberately NOT before 15 August." That was overturned: racing bad
architecture into production defeats the point of the rewrite
(non-negotiable #3 in the Cutover section), and the interim polling
shape plus the un-runnable apps/xero/sync_stream.py view are exactly
that. Slice 3 -- live updates done properly (serving model fix + SSE
ticker + discard the interim shortcuts) moves to its own MUST-tier
section with a milestone checkbox, and every stale "deferred
post-cutover" cross-reference in the file is corrected. The
purchasing-slice PR is unaffected -- it is unrelated and merges as
planned; this correction only fixes the durable record for the next
session.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Third review pass: 4 more QueryState conversions; rewrite-status cleanup
QueryState grew optional loadingNode/errorNode override props so a
spinner-based caller keeps its visual shell instead of losing it to
the plain-text default. JobFinishTab, DailyOverviewPage, and both
gates in TimesheetEntryPage's EntryWorkspace now render through it.
XeroQuoteCard, JobInvoiceCard and JobSettingsTab's pay-item field stay
excluded -- richer branching than binary success/fail, not the
page-level gate QueryState owns. TimesheetEntryPage's own outer gate
stays as guard-clause `if` returns, matching CLAUDE.md's stated
preference rather than converting to a shape that would abandon it.
Also: rewrite-status.md's narration of "review round found X, we
fixed Y" replaced with what the outcomes actually are -- which shared
component owns which contract, which sites are deliberately excluded
and why, which constraints a future change must respect. And the
Cutover section now states the actual two-question go/no-go criteria
(functional parity, proxied by MUST-tier E2E; materially better
architecture, judged directly) rather than only the E2E proxy, with
the honest fallback (abort and stay on v1) stated explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Address CodeRabbit review: real bugs and comment cleanup
- CompaniesListPage/JobFinishTab: QueryState was passed the raw
isError, breaking the first-load-only rule the other converted
sites already follow -- a background refetch failure was
unmounting already-rendered content instead of leaving it on
screen.
- JobSelect: closing the picker without a selection (blur or
Escape) left `editing` true forever, so the input kept showing
the abandoned search term instead of reverting to the bound job;
the blur timer was untracked and could fire a state update after
unmount. Also added keyboard selection (arrow keys + Enter,
role=listbox/option, aria-activedescendant) -- the dropdown was
mouse-only.
- PoListPage: the row was reachable only by mouse click; added a
real Link on the PO number cell, matching CompaniesListPage's
existing pattern.
- decimal.ts: parseDecimalInput accepted anything Number() parses,
including hex/octal/binary literals and bare exponents, and sent
them to the wire verbatim as garbage Decimal strings. Validates
fixed-point syntax explicitly now.
- Stale features/company/* paths in rewrite-status.md's build-order
table, left behind by the features/shared/company move.
- Comments narrating port history or test intent rewritten to state
the rejected alternative and the constraint (ADR 0043) in
ItemSelect, SaveFailedBadge, CreateCompanyModal, CompanyLookup;
two flagged sites (the E2E-repair-loop comment, the sonner test
comment) were already compliant and left as-is.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix uv.lock/pyproject.toml desync on the django constraint
PR #54 (main) widened pyproject.toml's django specifier to
>=6.0,<6.2 but never regenerated the matching uv.lock, which still
recorded <6.1 in its requires-dist metadata. Every `uv run` this
session silently self-corrected the lock locally; committing that
fix rather than leaving a lockfile permanently out of sync with the
manifest it's supposed to pin.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Gitignore the root-level test-results/ Playwright writes too
frontend/test-results/ was covered; run_e2e.sh's Playwright
invocation also writes artifacts relative to the repo root in some
invocations, leaving an untracked test-results/ dangling after every
E2E run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix the actual bug behind the stray root test-results/: cwd-relative config
playwright.config.ts resolved .env.test, testDir, outputDir and the
html reporter's folder relative to process.cwd(). Any invocation
whose cwd wasn't frontend/ (npm --prefix from the repo root, a bare
npx playwright test) silently missed .env.test -- dropping
E2E_TEST_USERNAME/PASSWORD with no error -- or wrote artifacts to
the wrong location instead of erroring loudly, which is how a
root-level test-results/ kept reappearing. Anchored every one of
these to import.meta.dirname instead: verified from an unrelated
cwd, .env/.env.test still resolve correctly and no stray directory
gets created anywhere. The root-level gitignore entry from the
previous commit stays as a defensive backstop, but this is the real
fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Phase 3a (1/3): CRM services + API, schema-parity instrument - #3

Merged
corrin merged 2 commits into
mainfrom
phase3a-1-crm
Aug 2, 2026
Merged

Phase 3a (1/3): CRM services + API, schema-parity instrument#3
corrin merged 2 commits into
mainfrom
phase3a-1-crm

Conversation

@corrin

Copy link
Copy Markdown
Owner

CRM: 18/18 v1 operations with exact-URL pins for the external phone-provider portal; celery tasks + beat-in-code; 48 tests. Infra: schema_parity_diff.py CI gate (wire-level, ledger-aware). Part 1 of the 3a stack.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222

CRM: 18/18 v1 operations (phone calls, recordings with ETag/304, endpoints
CRUD, provider settings), exact-URL pins for the external provider portal,
requests-based client, celery tasks + beat-in-code, 48 tests.
Infrastructure: scripts/schema_parity_diff.py (live OpenAPI vs v1's frozen
contract, ledger-aware, wire-level) wired into CI.
Stacked 1/3; company impl and tests follow.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@coderabbitai

coderabbitaiBot commented Aug 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 95664726-a6bb-43bf-b5a1-b729ea799a5b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f129cb and 42b51fe.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • .env.example
  • .github/workflows/ci.yml
  • .gitignore
  • apps/crm/api.py
  • apps/crm/schemas.py
  • apps/crm/services/__init__.py
  • apps/crm/services/phone_call_service.py
  • apps/crm/tasks.py
  • apps/crm/tests/__init__.py
  • apps/crm/tests/helpers.py
  • apps/crm/tests/test_phone_call_api.py
  • apps/crm/tests/test_phone_call_service.py
  • apps/crm/tests/test_phone_endpoint_guard.py
  • apps/crm/tests/urls.py
  • config/api.py
  • config/celery.py
  • config/settings.py
  • config/settings_test.py
  • docs/accepted-api-differences.yml
  • pyproject.toml
  • scripts/schema-parity-baseline.txt
  • scripts/schema_parity_diff.py

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.

- Recording-download params: null origin/destination stays empty (v1
omit-on-None semantics), not the literal string None (provider-boundary
parity).
- Phone-endpoint number stripped as v1's serializer did.
- schema_parity_diff.py: ratcheting baseline of ever-matched operationIds
(scripts/schema-parity-baseline.txt) - a ported operation vanishing is
now DRIFT, not 'not yet ported'; malformed ledger entries hard-fail;
--update-baseline grows the file deliberately. (Reviewer caught the
instrument's regression blindness.)
- Ledger: loose v1 query values (page=last, yes-booleans) now 422.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@corrin
corrin merged commit b5b6c4d into mainAug 2, 2026
3 checks passed
@corrin
corrin deleted the phase3a-1-crm branch August 2, 2026 03:51
corrin added a commit that referenced this pull request Aug 11, 2026
* Purchasing PO create/list/detail: po-created-by green
PoCreatePage (CompanyLookup + reference + save -> 201 redirect),
PoListPage, PoDetailPage with PoSummaryCard (created-by input,
reference autosave, status select), usePoLines over the single PATCH
endpoint, and the missing 'po' concurrency invalidator so 412/428
recovery actually refetches. Ports createTestPurchaseOrder +
waitForPoAutosave and the po-created-by spec.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* PO lines grid: create-purchase-order green
PoLinesTable as the third grid on the useReactTable + useDraftRows
pattern (phantom row, no add-line button, row-exit draft commit —
unit-cost stays the row's last focusable cell so the spec's Tab exits
the row). ItemSelect generalised for stock-only consumers (optional
jobId/line, label + wrapper overrides; labour-rates query gated on
jobId presence only, since textOnly labels need rate names). Inline
JobSelect over purchasing_all_jobs_retrieve — the unfiltered endpoint
v1's PO page uses, because fresh jobs are draft and the filtered
sibling excludes them. Spec ported with the autosave waiter armed
before the pick/status clicks (v2 saves immediately; v1's debounce is
what made arm-after work).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Stock page: stock-search green
Spec-lean StockPage: full active-stock list on load, 300ms-debounced
server FTS from 3 characters, enabled-gated so clearing the box
renders the cached list with no /search/ request.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Rewrite-status: purchasing slice recorded (29 of 40)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unit tests for PO line draft rules and JobSelect filtering
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unify the table layer: shared DataTable/ListTable, no cross-domain imports
The purchasing slice's PoLinesTable was about to become a third
hand-rolled grid emitting the DataTable-row-N/data-grid-* contract
inline. features/shared/DataTable.tsx is now the one owner of that
contract; SmartTimesheetTable, CostLineGrid and PoLinesTable all
render through it.
Auditing further found the same pathology in the plain list pages:
PoListPage and StockPage (both new this slice) had copied
CompaniesListPage's table shell and loading/error/retry block
verbatim, taking an existing duplicate (also in WipReportPage) from
2 instances to 4. features/shared/ListTable.tsx is the one owner of
that block instead -- deliberately separate from DataTable, since it
has no react-table dependency and forcing static lists through
column-def machinery would be indirection, not rigor. A hand-rolled
debounce-into-query-state pattern in CompaniesListPage and StockPage
is now features/shared/useDebouncedValue.ts (KanbanSearchInput keeps
its own URL-driven debounce -- a different concept).
ItemSelect, the decimal helpers and the Save-failed badge move to
features/shared/ because purchasing consumed them cross-domain -- a
domain feature is not a library.
ADR 0039 strengthened: unification is never deferred, shared concepts
get shared homes, the bar is reference quality.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Review fixes: QueryState primitive, item-label bug, company relocated
Adversarial review on the branch found the ListTable audit had stopped
short: JobMovementReportPage and CompanyDetailPage hand-rolled the same
loading/error/retry block ListTable was built to own (6 real instances,
only 4 fixed). Split the block itself out as
features/shared/QueryState.tsx -- the pending/error gate alone, no
table -- so it fits pages that show something other than a table too.
ListTable now composes QueryState instead of duplicating it. PoDetailPage,
CostLineGrid, JobMovementReportPage and CompanyDetailPage all render
through it.
Fixed: PoLinesTable's item-picker label read `item_code ?? 'Select Item'`
with no description fallback, so a bound stock item with a null code
(nullable, v1 parity) misread as unbound. Now poLineItemLabel() in
lines.ts, unit-tested.
Moved features/company to features/shared/company: it was already
cross-imported by features/job (JobCreatePage, JobSettingsTab) before
this slice added a third importer (purchasing/PoSummaryCard) -- it never
had a route of its own, a shared widget library in a domain-shaped box.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* ADR 0039: name the deadline-vs-duplicate resolution explicitly
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Docs: Slice 3 (SSE + serving model) is MUST before cutover, not deferred
Reversed 2026-08-11. The 2026-08-10 record said SSE ships with the
production-serving decision, filed under "Post-cutover -- decided,
deliberately NOT before 15 August." That was overturned: racing bad
architecture into production defeats the point of the rewrite
(non-negotiable #3 in the Cutover section), and the interim polling
shape plus the un-runnable apps/xero/sync_stream.py view are exactly
that. Slice 3 -- live updates done properly (serving model fix + SSE
ticker + discard the interim shortcuts) moves to its own MUST-tier
section with a milestone checkbox, and every stale "deferred
post-cutover" cross-reference in the file is corrected. The
purchasing-slice PR is unaffected -- it is unrelated and merges as
planned; this correction only fixes the durable record for the next
session.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Third review pass: 4 more QueryState conversions; rewrite-status cleanup
QueryState grew optional loadingNode/errorNode override props so a
spinner-based caller keeps its visual shell instead of losing it to
the plain-text default. JobFinishTab, DailyOverviewPage, and both
gates in TimesheetEntryPage's EntryWorkspace now render through it.
XeroQuoteCard, JobInvoiceCard and JobSettingsTab's pay-item field stay
excluded -- richer branching than binary success/fail, not the
page-level gate QueryState owns. TimesheetEntryPage's own outer gate
stays as guard-clause `if` returns, matching CLAUDE.md's stated
preference rather than converting to a shape that would abandon it.
Also: rewrite-status.md's narration of "review round found X, we
fixed Y" replaced with what the outcomes actually are -- which shared
component owns which contract, which sites are deliberately excluded
and why, which constraints a future change must respect. And the
Cutover section now states the actual two-question go/no-go criteria
(functional parity, proxied by MUST-tier E2E; materially better
architecture, judged directly) rather than only the E2E proxy, with
the honest fallback (abort and stay on v1) stated explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Address CodeRabbit review: real bugs and comment cleanup
- CompaniesListPage/JobFinishTab: QueryState was passed the raw
isError, breaking the first-load-only rule the other converted
sites already follow -- a background refetch failure was
unmounting already-rendered content instead of leaving it on
screen.
- JobSelect: closing the picker without a selection (blur or
Escape) left `editing` true forever, so the input kept showing
the abandoned search term instead of reverting to the bound job;
the blur timer was untracked and could fire a state update after
unmount. Also added keyboard selection (arrow keys + Enter,
role=listbox/option, aria-activedescendant) -- the dropdown was
mouse-only.
- PoListPage: the row was reachable only by mouse click; added a
real Link on the PO number cell, matching CompaniesListPage's
existing pattern.
- decimal.ts: parseDecimalInput accepted anything Number() parses,
including hex/octal/binary literals and bare exponents, and sent
them to the wire verbatim as garbage Decimal strings. Validates
fixed-point syntax explicitly now.
- Stale features/company/* paths in rewrite-status.md's build-order
table, left behind by the features/shared/company move.
- Comments narrating port history or test intent rewritten to state
the rejected alternative and the constraint (ADR 0043) in
ItemSelect, SaveFailedBadge, CreateCompanyModal, CompanyLookup;
two flagged sites (the E2E-repair-loop comment, the sonner test
comment) were already compliant and left as-is.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix uv.lock/pyproject.toml desync on the django constraint
PR #54 (main) widened pyproject.toml's django specifier to
>=6.0,<6.2 but never regenerated the matching uv.lock, which still
recorded <6.1 in its requires-dist metadata. Every `uv run` this
session silently self-corrected the lock locally; committing that
fix rather than leaving a lockfile permanently out of sync with the
manifest it's supposed to pin.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Gitignore the root-level test-results/ Playwright writes too
frontend/test-results/ was covered; run_e2e.sh's Playwright
invocation also writes artifacts relative to the repo root in some
invocations, leaving an untracked test-results/ dangling after every
E2E run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix the actual bug behind the stray root test-results/: cwd-relative config
playwright.config.ts resolved .env.test, testDir, outputDir and the
html reporter's folder relative to process.cwd(). Any invocation
whose cwd wasn't frontend/ (npm --prefix from the repo root, a bare
npx playwright test) silently missed .env.test -- dropping
E2E_TEST_USERNAME/PASSWORD with no error -- or wrote artifacts to
the wrong location instead of erroring loudly, which is how a
root-level test-results/ kept reappearing. Anchored every one of
these to import.meta.dirname instead: verified from an unrelated
cwd, .env/.env.test still resolve correctly and no stray directory
gets created anywhere. The root-level gitignore entry from the
previous commit stays as a defensive backstop, but this is the real
fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@corrin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Phase 3a (1/3): CRM services + API, schema-parity instrument - #3

Merged
corrin merged 2 commits into
mainfrom
phase3a-1-crm
Aug 2, 2026
Merged

Phase 3a (1/3): CRM services + API, schema-parity instrument#3
corrin merged 2 commits into
mainfrom
phase3a-1-crm

Conversation

@corrin

Copy link
Copy Markdown
Owner

CRM: 18/18 v1 operations with exact-URL pins for the external phone-provider portal; celery tasks + beat-in-code; 48 tests. Infra: schema_parity_diff.py CI gate (wire-level, ledger-aware). Part 1 of the 3a stack.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222

CRM: 18/18 v1 operations (phone calls, recordings with ETag/304, endpoints
CRUD, provider settings), exact-URL pins for the external provider portal,
requests-based client, celery tasks + beat-in-code, 48 tests.
Infrastructure: scripts/schema_parity_diff.py (live OpenAPI vs v1's frozen
contract, ledger-aware, wire-level) wired into CI.
Stacked 1/3; company impl and tests follow.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@coderabbitai

coderabbitaiBot commented Aug 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@corrin, you've reached your PR review limit, so we couldn't start this review.

Next review available in:8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 95664726-a6bb-43bf-b5a1-b729ea799a5b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f129cb and 42b51fe.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • .env.example
  • .github/workflows/ci.yml
  • .gitignore
  • apps/crm/api.py
  • apps/crm/schemas.py
  • apps/crm/services/__init__.py
  • apps/crm/services/phone_call_service.py
  • apps/crm/tasks.py
  • apps/crm/tests/__init__.py
  • apps/crm/tests/helpers.py
  • apps/crm/tests/test_phone_call_api.py
  • apps/crm/tests/test_phone_call_service.py
  • apps/crm/tests/test_phone_endpoint_guard.py
  • apps/crm/tests/urls.py
  • config/api.py
  • config/celery.py
  • config/settings.py
  • config/settings_test.py
  • docs/accepted-api-differences.yml
  • pyproject.toml
  • scripts/schema-parity-baseline.txt
  • scripts/schema_parity_diff.py

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.

- Recording-download params: null origin/destination stays empty (v1
omit-on-None semantics), not the literal string None (provider-boundary
parity).
- Phone-endpoint number stripped as v1's serializer did.
- schema_parity_diff.py: ratcheting baseline of ever-matched operationIds
(scripts/schema-parity-baseline.txt) - a ported operation vanishing is
now DRIFT, not 'not yet ported'; malformed ledger entries hard-fail;
--update-baseline grows the file deliberately. (Reviewer caught the
instrument's regression blindness.)
- Ledger: loose v1 query values (page=last, yes-booleans) now 422.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4BKR43FfAFod7TFXMe222
@corrin
corrin merged commit b5b6c4d into mainAug 2, 2026
3 checks passed
@corrin
corrin deleted the phase3a-1-crm branch August 2, 2026 03:51
corrin added a commit that referenced this pull request Aug 11, 2026
* Purchasing PO create/list/detail: po-created-by green
PoCreatePage (CompanyLookup + reference + save -> 201 redirect),
PoListPage, PoDetailPage with PoSummaryCard (created-by input,
reference autosave, status select), usePoLines over the single PATCH
endpoint, and the missing 'po' concurrency invalidator so 412/428
recovery actually refetches. Ports createTestPurchaseOrder +
waitForPoAutosave and the po-created-by spec.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* PO lines grid: create-purchase-order green
PoLinesTable as the third grid on the useReactTable + useDraftRows
pattern (phantom row, no add-line button, row-exit draft commit —
unit-cost stays the row's last focusable cell so the spec's Tab exits
the row). ItemSelect generalised for stock-only consumers (optional
jobId/line, label + wrapper overrides; labour-rates query gated on
jobId presence only, since textOnly labels need rate names). Inline
JobSelect over purchasing_all_jobs_retrieve — the unfiltered endpoint
v1's PO page uses, because fresh jobs are draft and the filtered
sibling excludes them. Spec ported with the autosave waiter armed
before the pick/status clicks (v2 saves immediately; v1's debounce is
what made arm-after work).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Stock page: stock-search green
Spec-lean StockPage: full active-stock list on load, 300ms-debounced
server FTS from 3 characters, enabled-gated so clearing the box
renders the cached list with no /search/ request.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Rewrite-status: purchasing slice recorded (29 of 40)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unit tests for PO line draft rules and JobSelect filtering
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Unify the table layer: shared DataTable/ListTable, no cross-domain imports
The purchasing slice's PoLinesTable was about to become a third
hand-rolled grid emitting the DataTable-row-N/data-grid-* contract
inline. features/shared/DataTable.tsx is now the one owner of that
contract; SmartTimesheetTable, CostLineGrid and PoLinesTable all
render through it.
Auditing further found the same pathology in the plain list pages:
PoListPage and StockPage (both new this slice) had copied
CompaniesListPage's table shell and loading/error/retry block
verbatim, taking an existing duplicate (also in WipReportPage) from
2 instances to 4. features/shared/ListTable.tsx is the one owner of
that block instead -- deliberately separate from DataTable, since it
has no react-table dependency and forcing static lists through
column-def machinery would be indirection, not rigor. A hand-rolled
debounce-into-query-state pattern in CompaniesListPage and StockPage
is now features/shared/useDebouncedValue.ts (KanbanSearchInput keeps
its own URL-driven debounce -- a different concept).
ItemSelect, the decimal helpers and the Save-failed badge move to
features/shared/ because purchasing consumed them cross-domain -- a
domain feature is not a library.
ADR 0039 strengthened: unification is never deferred, shared concepts
get shared homes, the bar is reference quality.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Review fixes: QueryState primitive, item-label bug, company relocated
Adversarial review on the branch found the ListTable audit had stopped
short: JobMovementReportPage and CompanyDetailPage hand-rolled the same
loading/error/retry block ListTable was built to own (6 real instances,
only 4 fixed). Split the block itself out as
features/shared/QueryState.tsx -- the pending/error gate alone, no
table -- so it fits pages that show something other than a table too.
ListTable now composes QueryState instead of duplicating it. PoDetailPage,
CostLineGrid, JobMovementReportPage and CompanyDetailPage all render
through it.
Fixed: PoLinesTable's item-picker label read `item_code ?? 'Select Item'`
with no description fallback, so a bound stock item with a null code
(nullable, v1 parity) misread as unbound. Now poLineItemLabel() in
lines.ts, unit-tested.
Moved features/company to features/shared/company: it was already
cross-imported by features/job (JobCreatePage, JobSettingsTab) before
this slice added a third importer (purchasing/PoSummaryCard) -- it never
had a route of its own, a shared widget library in a domain-shaped box.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* ADR 0039: name the deadline-vs-duplicate resolution explicitly
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Docs: Slice 3 (SSE + serving model) is MUST before cutover, not deferred
Reversed 2026-08-11. The 2026-08-10 record said SSE ships with the
production-serving decision, filed under "Post-cutover -- decided,
deliberately NOT before 15 August." That was overturned: racing bad
architecture into production defeats the point of the rewrite
(non-negotiable #3 in the Cutover section), and the interim polling
shape plus the un-runnable apps/xero/sync_stream.py view are exactly
that. Slice 3 -- live updates done properly (serving model fix + SSE
ticker + discard the interim shortcuts) moves to its own MUST-tier
section with a milestone checkbox, and every stale "deferred
post-cutover" cross-reference in the file is corrected. The
purchasing-slice PR is unaffected -- it is unrelated and merges as
planned; this correction only fixes the durable record for the next
session.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Third review pass: 4 more QueryState conversions; rewrite-status cleanup
QueryState grew optional loadingNode/errorNode override props so a
spinner-based caller keeps its visual shell instead of losing it to
the plain-text default. JobFinishTab, DailyOverviewPage, and both
gates in TimesheetEntryPage's EntryWorkspace now render through it.
XeroQuoteCard, JobInvoiceCard and JobSettingsTab's pay-item field stay
excluded -- richer branching than binary success/fail, not the
page-level gate QueryState owns. TimesheetEntryPage's own outer gate
stays as guard-clause `if` returns, matching CLAUDE.md's stated
preference rather than converting to a shape that would abandon it.
Also: rewrite-status.md's narration of "review round found X, we
fixed Y" replaced with what the outcomes actually are -- which shared
component owns which contract, which sites are deliberately excluded
and why, which constraints a future change must respect. And the
Cutover section now states the actual two-question go/no-go criteria
(functional parity, proxied by MUST-tier E2E; materially better
architecture, judged directly) rather than only the E2E proxy, with
the honest fallback (abort and stay on v1) stated explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Address CodeRabbit review: real bugs and comment cleanup
- CompaniesListPage/JobFinishTab: QueryState was passed the raw
isError, breaking the first-load-only rule the other converted
sites already follow -- a background refetch failure was
unmounting already-rendered content instead of leaving it on
screen.
- JobSelect: closing the picker without a selection (blur or
Escape) left `editing` true forever, so the input kept showing
the abandoned search term instead of reverting to the bound job;
the blur timer was untracked and could fire a state update after
unmount. Also added keyboard selection (arrow keys + Enter,
role=listbox/option, aria-activedescendant) -- the dropdown was
mouse-only.
- PoListPage: the row was reachable only by mouse click; added a
real Link on the PO number cell, matching CompaniesListPage's
existing pattern.
- decimal.ts: parseDecimalInput accepted anything Number() parses,
including hex/octal/binary literals and bare exponents, and sent
them to the wire verbatim as garbage Decimal strings. Validates
fixed-point syntax explicitly now.
- Stale features/company/* paths in rewrite-status.md's build-order
table, left behind by the features/shared/company move.
- Comments narrating port history or test intent rewritten to state
the rejected alternative and the constraint (ADR 0043) in
ItemSelect, SaveFailedBadge, CreateCompanyModal, CompanyLookup;
two flagged sites (the E2E-repair-loop comment, the sonner test
comment) were already compliant and left as-is.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix uv.lock/pyproject.toml desync on the django constraint
PR #54 (main) widened pyproject.toml's django specifier to
>=6.0,<6.2 but never regenerated the matching uv.lock, which still
recorded <6.1 in its requires-dist metadata. Every `uv run` this
session silently self-corrected the lock locally; committing that
fix rather than leaving a lockfile permanently out of sync with the
manifest it's supposed to pin.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Gitignore the root-level test-results/ Playwright writes too
frontend/test-results/ was covered; run_e2e.sh's Playwright
invocation also writes artifacts relative to the repo root in some
invocations, leaving an untracked test-results/ dangling after every
E2E run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
* Fix the actual bug behind the stray root test-results/: cwd-relative config
playwright.config.ts resolved .env.test, testDir, outputDir and the
html reporter's folder relative to process.cwd(). Any invocation
whose cwd wasn't frontend/ (npm --prefix from the repo root, a bare
npx playwright test) silently missed .env.test -- dropping
E2E_TEST_USERNAME/PASSWORD with no error -- or wrote artifacts to
the wrong location instead of erroring loudly, which is how a
root-level test-results/ kept reappearing. Anchored every one of
these to import.meta.dirname instead: verified from an unrelated
cwd, .env/.env.test still resolve correctly and no stray directory
gets created anywhere. The root-level gitignore entry from the
previous commit stays as a defensive backstop, but this is the real
fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WENcwU8rC6o7bxuHL6md2L
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@corrin