Skip to content

refactor(case): retire customer_rating, customer_feedback and the CSAT flow that fed them - #1699

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-1428-retire-csat-fields-and-flow
Sep 6, 2026
Merged

refactor(case): retire customer_rating, customer_feedback and the CSAT flow that fed them#1699
os-steve merged 1 commit into
mainfrom
claude/issue-1428-retire-csat-fields-and-flow

Conversation

@claude

@claude claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #1428

Maintainer ruling, decision batch #21 (2026-09-03): Option A — retire
crm_case.customer_rating and customer_feedback under ADR-0049
enforce-or-remove, and retire or rewrite case_csat_followup in the same
change. This is that change.

What this closes is a declared-≠-enforced hole, not two spare columns

Both fields were writable, translated in four locales, and reachable from no
screen in the product — no form, no detail section, no list column, no filter.
Nothing in src/ wrote them outside seed data. And neither field was named in
any profile's fields map, which on this platform is not a neutral fact:
field permissions are built only from the fields a permission set names, so an
unnamed field is default-open. internal_notes sits beside them and is
named per profile — service_agent editable, sales_manager read-only,
sales_rep not readable — which is what the two survey fields looked like they
had and did not.

A satisfaction score typed by the person being scored is not the same fact as
one the customer gave, and a CRM that cannot tell the two apart reports a number
it cannot defend. There was no column that could record which kind a value was,
and no surface through which a customer could give one. Retiring is the only
move that makes the declared shape equal the enforced shape; the
customer-answered survey remains the sound long-term design and is not
precluded — it is a feature, and it would arrive as its own card.

The flow: retired, not rewritten

case_csat_followup fired a day after a case closed and notified the case owner
to "Reach out to the contact and log their satisfaction rating", with
actionUrl: '/crm_case/{record.id}' — a record page with no input for a rating.
A shipped feedback loop with no landing point.

Rewriting it would have meant inventing a purpose. Its only data destination was
the two retired fields; nothing else in the app was measured to want a
post-close timer, and the maintainer ruled the survey out for now, so there was
no second job waiting to be given to it. A flow kept alive for its name is worse
than one removed. Measurement is on record and so is its cost: this was the
app's only flow with a wait node, so retiring it leaves the platform's
timer capability with no worked example in-tree. That is not a reason to keep a
flow that feeds nothing, and it is filed separately as #1697 rather than left to
be rediscovered.

Two claims conflicted; the measurement decides

The ruling says case_metrics reporting reads these fields. The analytics prose
that PR #1683 just rewrote says case_metrics declares no measure over
customer_rating. The prose is right. src/datasets/case.dataset.ts
declares six dimensions (status, priority, origin, type, created_date,
resolved_article) and eight measures, and not one of them names either field.
src/datasets/ was therefore outside this change — the ruling's parenthetical
was carrying a claim from an earlier comment, not a reading of the tree.

Those six analytics pages were re-read from the freshest origin/main, not from
any description. The sentence that concerned this card —

CSAT by Agent fails on both halves: Customer Satisfaction
(customer_rating) is a real 1–5 field on the case, yet case_metrics
declares no measure over it, and there is no agent to group by.

— would have become false the moment the field went. It now says the stronger
true thing: there is no satisfaction data at all, and still no agent dimension.
Same treatment for the matching bullet on the cubes page, in all three locales.

Reconciled with #1505, and a sharper mechanism than expected

#1505 closed the anonymous-intake hole by having the guest branch null both
fields. With the fields retired, that path closes by construction — but
"closed by construction" and "nulled" are different facts, and toBeNull()
reads green against both. test/guest-submission-sanitisation.test.ts now
asserts the columns are gone, in both halves: absent from crm_case's
declared field map (read off the compiled stack) and absent from the stored row,
with an anti-vacuum guard so an empty field map cannot make it pass by
describing nothing.

Measured while making that change, and it is why the two keys left the planted
payload rather than staying as a stronger plant: this engine refuses an
undeclared write outright.
Re-running the case with them still in the document
failed the whole insert with

Error: Unknown field 'customer_rating' on object 'crm_case'
  ❯ undeclaredWriteFieldErrors @objectstack/objectql

before the hooks, before sharing, before security. A submitter naming a retired
column no longer gets a case with the value dropped — they get no case. Keeping
the plant would have made that case assert the refusal instead of the
sanitisation it exists for, and taken its four other assertions down with it.
No change to #1505's own card scope was needed; it is closed and its pin lives
here.

The two epic-#1579 files — exactly what changed, and why each is entailed

The PM ruled that the epic fence does not block a ruled retirement: an assertion
disappearing because the thing it asserted was deleted is a different act
from the epic's (deleting an assertion because a platform rule covers it). Both
edits are one line, and both are the deleted flow's own name:

file epic item change why it is a direct consequence
test/smoke.test.ts F2 (#1583) dropped 'case_csat_followup' from the record-change flow list on line 46 the list asserts every named flow is registered; the flow is not registered any more, so the entry asserts a flow that does not exist
test/flow-condition-totality.test.ts F1 (#1582) dropped 'case_csat_followup' from the boot([...]) list boot() loads the named flows to drive their conditions; a name with no flow behind it loads nothing

Nothing else in either file was touched — no assertion weakened, no case
removed, no count changed. git diff on the two files is two lines out, two
lines in.

In-place boundary, declared

content/docs/service/cases.{mdx,zh-Hans,zh-Hant} name the two fields in three
places each. This PR subtracts only: the two names come out, and the
arithmetic that moved because of the subtraction moves with it (the object's
field total 27 to 25, the "eleven not on the Details tab" to nine, the form
table's own row sum). It does not re-derive the other numbers on that page,
because they are stale for reasons this card did not cause — PR #1427 narrowed
the form to one section of nine fields and PR #1506 added internal_notes to
the Details tab, and neither updated the page. Correcting that is a different
defect class and a whole table replaced in three locales, so it is filed as
#1696 with the measurement rather than ridden in here. The page is left
exactly as consistent, and exactly as stale, as it was found.

Untouched on purpose

The surface, re-derived on origin/main @ d6c930e9

git grep -ln for the two fields found 21 files (17 source and docs, plus 4
changesets); for the flow, 17 (15 plus CHANGELOG.md and 2 changesets) —
two more than the dispatch's reading, which had not named
docs/feature-inventory.md or docs/architecture/module-split-inventory.json.
src/data/service.seed.ts is in neither list, confirmed. Beyond the identifier
greps, a label-level grep (Customer Satisfaction / Customer Feedback and
their four locale spellings) found three more readers the identifier grep could
not see — the three content/docs/service/cases faces.

docs/feature-inventory.md keeps its SVC-011 row and marks it removed, which
is that file's own documented rule for a retired capability rather than a choice
made here.

Verification

pnpm verify fully green on d66dd468, the commit this PR carries, every heavy
run through the container's shared verify lock with the exit code captured
before any pipe:

✓ Validation passed              Data: 18 Objects  331 Fields
✓ i18n lint gate: 0 `i18n/missing-*` issues        Logic: 27 Flows
✓ source hygiene clean
✓ interaction layer ~37,963 tokens (ceiling ~40,000)
✓ authored total ~136,794 tokens (ceiling ~140,000)
✓ Build complete                 dist/objectstack.json (1908.2 KB)
  Test Files  161 passed (161)
  Tests  3407 passed | 1 skipped (3408)
os-verify-lock: VERDICT command-exit 0 · held the lock 231s

pnpm changeset status --since=origin/main exit 0 (hotcrm patch). No control
characters in any changed file (grep -naP over the staged set, clean).

Reverse verification of the reconciled pin, run from the committed state,
direction predicted before it was run: re-declare customer_rating on
crm_case and the pin should go red naming the re-declaration, and the
docs/STATUS.md field-count rule should go red beside it. Both moved —
Test Files 2 failed (2) — the pin with

AssertionError: crm_case re-declares customer_rating. #1428 retired it under
ADR-0049 — if it is coming back, it needs a writer, a profile entry and a
surface decided first. ⛔ Do not relax this into a null check.:
expected { owner_id: { …(15) }, …(25) } to not have property "customer_rating"

The mutation was proved on disk before the run (injected-text count 0 then 1,
git diff --stat naming the file) and restored under an EXIT INT TERM trap
with absolute paths via git checkout HEAD -- path: restored blob
8b47c7aa0f9b89ec76203f2db7fa9dfe88fb86fb identical to the HEAD blob, and
git diff HEAD empty. No build or dist step is involved — these suites import
../src and ../objectstack.config directly, so no dist preflight applies.

Upgrade note

Stored customer_rating / customer_feedback values stop being read or written
by the app, and the engine now refuses writes naming either column. Nothing here
migrates them; the changeset says so for the release-notes reader.


Generated by Claude Code

…T flow

Both fields were writable, translated in four locales, and reachable from no
screen: no form, no detail section, no list column, no filter. Nothing wrote
them outside seed data, and neither was named in any profile's `fields` map —
on this platform field permissions are built only from named fields, so an
unnamed field is default-open. A satisfaction score typed by the person being
scored is not the same fact as one the customer gave, and a CRM that cannot
tell them apart reports a number it cannot defend. Retired under ADR-0049
enforce-or-remove per the maintainer ruling on #1428 (decision batch #21).

`case_csat_followup` is retired rather than rewritten. Its only purpose was to
notify the case owner, a day after close, to log a rating against a record page
that had nowhere to take one. With the fields gone it feeds nothing, and no
other purpose was measured to want it.

Also removed: the four-locale label/help entries, the flow's row and the flow
numeral in the built-in automation table (three doc locales), the CSAT knob and
the satisfaction section of the packaged admin/service guides, and the flow's
frontmatter source refs. The analytics pages now explain "CSAT by Agent" as
having no satisfaction data at all rather than a field no measure aggregates.

`crm_case` re-measured: `case_metrics` never declared a dimension or measure
over either field, so `src/datasets/` was outside this change.

The #1505 pin is reconciled: it asserts the columns are GONE rather than
nulled. Measured while doing so — this engine refuses an undeclared write
(`Unknown field 'customer_rating' on object 'crm_case'`, before hooks, sharing
and security), so the plant path closes by construction.

Co-authored-by: Claude
@vercel

vercel Bot commented Sep 6, 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 6, 2026 11:25am UTC

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation 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 6, 2026
@os-steve
os-steve marked this pull request as ready for review September 6, 2026 11:29
@os-steve
os-steve added this pull request to the merge queue Sep 6, 2026
Merged via the queue into main with commit 01c659e Sep 6, 2026
11 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 documentation Improvements or additions to documentation metadata Declarative metadata — schema, security posture, UI surfaces

Projects

None yet

2 participants