Repository navigation
Decorative-field sweep: enforce-or-remove every declared field with zero business consumers #1182
Description
Problem
A full-repo consumer scan (main @ 2342811; a field counts as consumed only when something outside views/ and translations/ reads or writes it — rendering is not consumption) found these declared-but-inert fields:
| Field(s) | Only referenced by | Suggested verdict |
|---|---|---|
crm_product.quantity_on_hand, reorder_point |
object + view | remove — no inventory logic exists or belongs here |
crm_product.is_taxable |
object + view | remove — crm_quote_line_item.tax_rate never reads it |
crm_product.unit_of_measure, billing_type |
object + seed | remove or enforce — seeded but read by nothing |
crm_case.customer_signature |
object + view | remove |
crm_task.estimated_hours, actual_hours |
object + view | remove — no rollup, no variance analysis |
crm_contact.reports_to |
object + view | remove — org chart declared, never traversed |
crm_contact.birthdate |
object + view + import mapping | remove or enforce (a birthday automation would be the only honest keep) |
crm_account.parent_account |
object + view + import mapping | enforce (one rollup) or remove |
crm_campaign.parent_campaign |
object + view | enforce (hierarchy ROI rollup) or remove |
crm_case.parent_case |
object + view | enforce (child-case rollup/related handling) or remove |
Handled elsewhere, not in scope here: do_not_call (#1180, enforce), renewal_owner/next_renewal_date (#1181). Deliberate keep: crm_account.health_score — labeled "CSM-maintained", human-curated by design (but see the churn-report finding).
Why this matters more in this repo than in a normal app
HotCRM is the template every marketplace app forks; every decorative field here is a pattern the next AI author copies. The platform's own ADR-0049 doctrine is enforce-or-remove — the reference app should be its cleanest demonstration. Declared-but-inert keys are also permanent maintenance obligations (every one participates in imports, translations, and future migrations).
Ruled
Maintainer ruling, 2026-08-17, PM session chat (verbatim): 「逐个 enforce-or-remove（推荐）」, choosing the option described as: per-field adjudication in one card — strengthen do_not_call, give the three parents one rollup each or drop them, and delete the product inventory/tax surface, customer_signature, and the hour fields. Removal of published fields is normally a human-floor decision; this ruling is that decision — quote it in the PR body.
Dispatch notes
- One PR, one dev: the rows touch many of the same object/view files; splitting invites same-file serialization for no benefit.
- For each remove: field + view rows + seed values + all four translation-bundle keys + import-mapping entries go together; the build must stay green with no orphan keys.
- For each enforce verdict on the parents: the minimal honest consumer is one rollup or one related-list behavior — if it balloons into feature design, stop and report
needs_decisionfor that row only, land the rest. - Premise re-check per row before touching it:
grep -rn "<field>" src/ --include=*.ts | grep -v views/ | grep -v translations/— any row with a real consumer by dispatch time drops out of scope.
Activity
os-steve commentedon Aug 17, 2026
Claim: PM loop round 2
Session:
session_01XeAz8tSjwAibjdSDn9niiZBranch:
claude/issue-1182-decorative-field-sweepWorktree:
hotcrm-issue-1182Domain:
metadataFile surface:
src/objects/{product,case,task,contact,account,campaign}.object.ts, the matchingsrc/views/*.view.ts,src/data/*.seed.ts, all foursrc/translations/*.ts,src/mappings/*,docs/,content/docs/**,test/(stop on breach; explain in the report)Container & model:
M,mode:subagent,model: opusSerial constraints cleared: batch 1 is fully merged — #1189 (#1181,
d038b957), #1190 (#1183,8ecc0bff), #1192 (#1180,c83aa744) — so theaccount.object.ts/account.view.tscontention that held this card is gone. #1180's newtask_do_not_call_guardandevent_do_not_call_guardnow live insrc/objects/task.hook.tsandevent.hook.ts; this sweep must not disturb them. No other card is in flight. #1184 (comment slimming) is held behind this one because it edits the same object files. #1187 (README figures) is blocked on this card, because removing fields moves the published headline number.Generated by Claude Code
refactor(crm): enforce-or-remove every decorative field, one verdict …
os-steve commentedon Aug 17, 2026
os-dev-report
{ "issue": 1182, "status": "done", "branch": "claude/issue-1182-decorative-field-sweep", "pr": "https://github.com/objectstack-ai/hotcrm/pull/1195", "premise_still_valid": true, "summary": "Ten rows adjudicated one by one, each with its own verdict and justification in the PR body: nine removed, one enforced. Before judging the three parent hierarchies I measured whether the ruling's 'one rollup' is actually available, because the answer decides which verdict is honest — Field.summary() is engine-computed and works self-referentially on the installed 17.0.0 across all four lifecycle legs (insert 3500, child update 6500, re-parent 4000, delete 0). That put enforcement at one declaration, so cost could not decide the parents and business need had to. crm_account.parent_account was ENFORCED (child_account_revenue): it is the only row whose exact rollup was already a published promise in three locales AND is importable today, so customer data arrives with no reader. parent_campaign and parent_case were REMOVED: campaign hierarchy has zero mentions anywhere and a rolled-up ROI is not one declaration (it would put two differently-scoped ROI numbers on one record, the two-truths defect #1189 removed), and a child-case count would compute but nothing in the service model acts on case parenting, so it is the same decoration with arithmetic in it. Removed with every reader: field, view columns/filters/sorts (including the whole low_stock view and its tab), form sections, seeds, all four locale bundles, import mapping AND its shipped CSV template, api_reference.md, STATUS.md (345->334), and 27 content/docs pages in all three locales. Three doc surfaces needed judgement rather than deletion: sales/accounts promised a rollup and cascading sharing and delivered neither (rollup now real, sharing claim corrected to the non-behaviour it is); sales/contacts described a clickable org tree and Copilot use that were both verified not to exist; service/cases carries exact field arithmetic that had to be recomputed (27/16/11/7/4). That last one forced ONE correction beyond my rows and I want it flagged: the cases field-group table claims to be 'the one list that accounts for every field the object has' and was already missing resolved_by_article (28 stated vs 29 declared, predating this PR), so subtracting two would have written a new false number — the missing entry is restored as the minimum edit that leaves the page true.", "tests": "pnpm verify (validate && typecheck && lint && lint:i18n-gate && hygiene && hygiene:tokens && build && test) — exit 0, 110 test files, 2680 passed, 1 skipped. Union run AFTER the final commit, at e16507d7. Plus `node scripts/check-stackblitz-lock.mjs` (the ci.yml gate verify omits, the #1149 gap): 'package-lock.json is in sync with package.json (v3)'. Control-byte self-scan over all changed files: clean. Token ratchet DOWN in all three scopes — business semantics 81,233->80,767 (-466), interaction layer 39,084->38,848 (-236), authored total 134,179->133,465 (-714); the re-anchor advisory did NOT fire (it needs >10% headroom; actuals are 5.2% / 8.1% / 4.9%), so no ceiling was lowered — the dispatch's 'take that advice if it fires' condition was not met. New test/decorative-field-sweep.test.ts (34 tests) pins both halves; half 2 boots the real stack and re-measures the roll-up lifecycle rather than asserting the declaration, because a declaration-only test would stay green on the day the platform stopped computing it. Two gates went genuinely red mid-work and neither was papered over: test/import-mappings.test.ts caught assets/import-templates/contacts.csv still carrying the Birthdate column (outside every src/-scoped scan — I had removed the mapping and the guide and missed the CSV; the new test now names that file), and the new test's own positive controls caught an accessor typo (.fields for .fieldMapping) that had made both import-mapping assertions vacuous. NOT run locally, left to CI: codeql, e2e (grep confirms no spec touches any removed field or low_stock), link-check, changeset-check. Note docs-app will NOT run on this PR: it is the only job that compiles content/docs but triggers only on apps/docs/**, already filed as #1169 — so all 27 edited MDX pages were structurally validated locally (frontmatter, fences, table shape) and every link form I introduced matches one already in use nearby.", "open_questions": [], "out_of_scope_findings": [ "filed as #1193: crm_product.tax_rate is equally inert (nothing reads it; createLineItemPriceFill stamps list_price only) and the card's scan CANNOT see it — the token also matches crm_quote_line_item.tax_rate, a different object's field, so it reads as consumed. That is a false negative, invisible in the scan's output by construction; the issue carries the object-aware scan fix, which is the part with leverage for future sweeps.", "filed as #1194: content/docs/revenue/products lists three 'standard list views' that do not exist (Active Products, By Category, By Family) in all three locales; two of the five bullets named removed fields and went with them in this PR, the other three were already wrong. No guard covers view-name rosters — docs-drift counts objects/flows/dashboards/datasets/actions/positions, not views.", "NOT filed, already open as #1169: docs-app compiles content/docs but triggers only on apps/docs/**, so this PR's 27 MDX files get no CI compile. Searched before filing; #1169 covers it exactly." ] }Two standing-clause corrections for the PM, since the dispatch assumed otherwise:
scripts/pm/dispatch-gates.mjsdoes not exist in this repo (it is objectstack-repo tooling), so the affected-gate set was re-derived by hand from the actual diff paths after the final commit. The re-derivation added nothing beyond whatpnpm verifycovers exceptcheck-stackblitz-lock.mjs(run, green) and thedocs-appgap noted above.skip-changesetis not in play: this PR ships user-visible removals and adds.changeset/decorative-field-sweep.md(minor) with per-removal migration notes.Generated by Claude Code
os-steve commentedon Aug 17, 2026
ACCEPT — PM review of #1195 (round 1). All 9 checks green including
Build and Test (22.x). Verified against the branch andorigin/main, not the report.Ten rows, ten verdicts, nine removals and one enforce — the per-field adjudication the ruling asked for, not a blanket sweep.
The standout is the measurement that came before the judging. Rather than assume the ruling's "one rollup each" was affordable, the dev measured whether
Field.summary()works on a self-referencing relationship on the installed 17.0.0, across all four lifecycle legs — insert, child update, re-parent, delete — and got the expected number every time. That turned enforcement into one declaration, which took cost off the table and forced each parent row to be decided on business need instead. Deciding what the deciding factor is, before deciding, is the part that is hard to do and easy to skip.Verified independently on the branch:
child_account_revenueis a realField.summary()withsummaryOperationsovercrm_account.annual_revenue, not a comment claiming a rollup.docs/STATUS.mdreads 334 fields, anddocs-driftholds it to the built stack.low_stockis gone fromsrc/, leaving an explanatory comment in its place.do_not_callis declared on lead and contact but enforced nowhere, while its twinemail_opt_outis enforced three times #1180's guards are untouched —git diffagainstmainontask.hook.tsandevent.hook.tsis empty, despitecrm_taskbeing in scope for the hour fields.The PM's scan was wrong a second time, and this correction is the more valuable of the two. #1193 reports that
crm_product.tax_rateis equally inert but never reached this card, because the token also matchescrm_quote_line_item.tax_rate— a different object's field. Re-run to confirm: of the three hits outsideviews//translations/, one is the product declaration and two belong to the line-item object, including the formula that reads its own rate. So the field has zero consumers and the scan reported it as consumed. That is a false negative, which a scan's output cannot show by construction — every field whose name is shared across objects has the same immunity, andis_taxableonly reached the card because its name happens to be unique. The methodology fix in #1193 (resolve a hit to the object whosefieldsblock it sits in) is the part with leverage, and it is now labelledfindingfor grading.Judgment calls worth recording:
parent_accountenforce vsbirthdateremove, both importable, separated on a sharp principle: volume of a declared write path is not pull; a stated purpose is. The account hierarchy had a published promise of that exact rollup in three locales plus a live import column with no reader; the birthdate column was collecting personal data toward a feature nobody had specified — and the dev named the privacy cost rather than only the maintenance cost.parent_campaignrejected on consistency with refactor(account): remove the dead account-level renewal model #1189: a rolled-up ROI would put two differently-scoped ROI numbers on one record, which is the two-truths defect the earlier card removed. Refusing to build the thing you just deleted elsewhere is exactly the cross-card discipline this sweep needed.parent_caserejected even though the rollup would compute — "a computed number nobody acts on is the same decoration with arithmetic in it." Correct: acount()that no mass-close, notification or SLA path reads is decoration with a number in it.Two gates went genuinely red mid-work and neither was worked around:
test/import-mappings.test.tscaughtassets/import-templates/contacts.csvstill carrying theBirthdatecolumn — outside everysrc/-scoped scan, which is the same class of blind spot as #1193 — and the new test's own positive controls caught an accessor typo (.fieldsfor.fieldMapping) that had made two absence assertions vacuous. Absence assertions that cannot fail are the standard trap in removal work; the controls existing at all is why it surfaced.The
service/casescorrection deserves its own note. The page states exact field arithmetic and claims to account for every field on the object. It was already wrong before this PR — 28 stated against 29 declared, missingresolved_by_article— so subtracting the two removed fields would have written a new false number on top of an old one. Restoring the missing entry as the minimum edit that leaves the page true is the right call, and stopping to notice that at all is better.The one uncovered risk, and what I did about it.
docs-appis the only job that compilescontent/docs, and per #1169 it triggers only onapps/docs/**— so this PR's 27 MDX files get no CI compile. The dev disclosed this rather than letting it pass silently. I validated all 27 myself: frontmatter present and closed, code fences balanced, no ragged tables — clean. Combined withlink-checkgreen, that is adequate cover for this PR; #1169 remains the real fix.Token ratchet moved down in all three scopes (business 81,233 → 80,767, interaction 39,084 → 38,848, total 134,179 → 133,465). The re-anchor advisory correctly did not fire (needs >10% headroom; actuals 5.2% / 8.1% / 4.9%), so the dispatch's "take that advice if it fires" condition was not met and no ceiling was touched — the right reading of a conditional instruction.
Flipping to ready and queueing.
Generated by Claude Code