Skip to content

feat(revenue): line_number gets its writer on both line items, plus a one-off back-fill (#1828) - #1960

Merged
os-warren merged 4 commits into
mainfrom
claude/issue-1828-line-number-writer
Sep 25, 2026
Merged

os-warren merged 4 commits into
mainfrom
claude/issue-1828-line-number-writer

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #1828

Ruling: batch #163 item 2, letter 1 (comment 5731202044) — line_number gets a writer on both line-item objects; readonly: true stays; existing null rows are back-filled once by this PR.

What changed

  • createLineItemNumbering (new factory in src/revenue/objects/_line-item-price-fill.ts, the shared helper this pair already uses), registered on both objects as opportunity_line_item_line_number / quote_line_item_line_number: beforeInsert + beforeUpdate, runAs: 'system', priority 110.
    • Insert: line_number = (max line_number under the parent) + 1. A batch insert adds ctx.dispatch.index (every row's beforeInsert runs before any row is stored, so without it a batch would share one number).
    • Update: acts only when the stored row has no ordinal (the rows that predate this hook). An existing ordinal is never renumbered. Predicate (per-row) updates stand down, per the D3 rule the repo's other hooks follow.
    • A no-user system write that supplies a number keeps it — the seed and its boot-time replay stay exactly as authored (the 1..n fixture pin in test/seed-consistency.test.ts is untouched and green).
    • Body-only safe: the parent key is chosen from ctx.object inside the handler, never from a factory parameter.
  • scripts/backfill-line-number.ts — the one-off back-fill, in the shape scripts/backfill-owner-id.ts already established (REST-only, report-only unless --apply, rerunnable).
  • test/line-item-line-number.test.ts — the runtime pin on a real ObjectQL; added to RUNTIME_TEST_FILES in test/runtime-coverage.test.ts (that gate requires every registered hook be named in a runtime file).
  • One changeset (minor), which carries the run instruction for existing deployments.

Mechanism (PM assumption A1, measured)

The engine keeps a payload key that a before-hook wrote through its readonly strip (hookWrittenKeys in objectql 17.4.0 stripReadonlyFields), and drops the same key when the caller supplied it. The engine's own strip warning says it: "a beforeInsert hook does NOT need this because hook-written keys are not caller-supplied". So the stamp lands on a rep's insert with no elevated write at all; runAs: 'system' elevates only the sibling read that computes the max — the smallest elevation available, below the #1807 system sub-flow, because nothing else has to be elevated. The test file keeps the control that makes this reading mean something: with the hook unbound, a rep's supplied line_number: 99 is dropped and the row stores none.

Back-fill site (A4)

A REST script cannot write a readonly column (an admin is not a system caller; the strip bites). So the script never sends a number: it sends { line_number: null } to each null row, parent by parent, oldest first (created_at, then id), and the hook's update branch stamps it. It is the repo's existing one-off shape (backfill-owner-id.ts), not a migration framework, and not a scheduled flow — get_record has no sort, so a flow sweep could not keep creation order, and a new flow would also have to join the automation docs table in three locales. Rows nobody back-fills heal on their next edit anyway.

Concurrency (A3)

Max + 1 is read-then-write. There is no unique index on (parent, line_number) and the hook opens no transaction, so two inserts under the same parent at the same instant can get the same number. From the UI the only line-item creator is the related-list Add picker, and objectui RelatedList.tsx awaits each create in sequence, so one user action cannot collide with itself; a collision needs two people adding lines to the same deal or quote concurrently. No locking was added.

Measurements

Live, two dev servers on one SQLite file: base (aa61e1d5) to create the null rows, then this branch on a copy of that DB.

reading base this branch
admin REST insert with line_number: 99 stored null (both objects) opportunity line: stored 6 on a deal numbered 1..5, 4 on a deal numbered 1..3
admin REST insert, no ordinal stored null (both objects) the next one: 7 and 5
back-fill report — 2 null rows per object, 1 parent each
back-fill --apply — opp parent 1,2,3,null,null becomes 1,2,3,4,5; quote parent 1..4,null,null becomes 1..6, in creation order; exit 0
back-fill rerun — 0 rows, exit 0
billing hand-off on close-won (captured at a local receiver via HOTCRM_BILLING_ENDPOINT) — line_items[].line_number = 1, 2, 3 (seed) and 4, 5 (rep-created) — non-null on every line

The quote line item's insert path was measured on the real engine only (test/line-item-line-number.test.ts runs every case on both objects); live, the quote side was measured through the back-fill.

Gates (at head 946e409)

pnpm verify chain, each step's exit captured: validate 0 · typecheck 0 · lint 0 · lint:i18n-gate 0 · hygiene 0 · hygiene:tokens 0 · build 0 · test 0 (173 files, 3713 passed, 1 skipped).

  • field-no-consumers: 5 findings before, 4 after; crm_quote_line_item.line_number is no longer named.
  • Token ratchet, src/revenue: business semantics 15,193 → 15,626 (ceiling 16,000, headroom 374); authored total 18,303 → 18,736 (ceiling 19,000, headroom 264). No ceiling moved.
  • Ablation (one-off): the stamp line replaced on disk (anchor count 1 → 0, marker 0 → 1), test/line-item-line-number.test.ts went 14 failed / 10 passed of 24; restored with git checkout HEAD --, git diff HEAD empty, blob hash equal to HEAD's, rerun 24/24.

Acceptance notes

  • Re-parenting a line to another parent keeps its old ordinal (the update branch only fills a missing one). Not in the ruling; noted, not changed.
  • Batch inserts spanning several parents get distinct but gapped ordinals (the batch index is global to the batch).
  • The live billing payload carries line-item columns outside LINE_ITEM_FIELDS (organization_id, owner_id, created_by, updated_by, created_at, updated_at, owning_business_unit_id, crm_opportunity): the get_record fields projection is not what reaches the receiver. Out of scope here; reported to the seat.

Generated by Claude Code

@vercel

vercel Bot commented Sep 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
hotcrm Ignored Ignored Sep 25, 2026 1:54pm UTC

Request Review

@github-actions github-actions Bot added ci/cd CI plumbing and the verification pipeline metadata Declarative metadata — schema, security posture, UI surfaces backend Server-side behaviour — hooks, flows, actions labels Sep 25, 2026
@os-warren
os-warren marked this pull request as ready for review September 25, 2026 14:01
@os-warren
os-warren added this pull request to the merge queue Sep 25, 2026
Merged via the queue into main with commit 1551914 Sep 25, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Server-side behaviour — hooks, flows, actions ci/cd CI plumbing and the verification pipeline metadata Declarative metadata — schema, security posture, UI surfaces

Projects

None yet

Development

Successfully merging this pull request may close these issues.

crm_opportunity_line_item.line_number has no writer, yet the billing hand-off payload ships it on every won deal

2 participants