Repository navigation
fix(views): window "Closing This Quarter" to deals that close this quarter (#743) - #746
Merged
Merged
Conversation
…arter (#743) `crm_opportunity.closing_this_quarter` was labelled "Closing This Quarter" in metadata and in all four locale bundles while its filter carried no condition on `close_date` at all. It returned every open commit/best-case deal regardless of when it was due to close, so a deal slated for next March sat under the heading and a rep who summed the Amount column got a number that was not this quarter's commit. The tab that opens it is labelled "Closing Soon", which was honest about an unbounded filter — the two names already disagreed with each other. Found by the generic label/filter-scope guard #745 introduced, which carried it as a self-expiring KNOWN_DEBT exemption pointing here. Unlike `crm_forecast.period_start`, which one equality pins because it stores a period's first day, `close_date` is an arbitrary day inside the quarter and needs a RANGE. The macro vocabulary expresses both ends: `DATE_MACRO_PERIOD_TOKENS` carries `current_quarter_end` beside `current_quarter_start`, and both resolve on the read path. Measured on the pinned 17.0.0-rc.2 rather than inferred — rows on the quarter's first and last day survive, next quarter's first day, next year and last year do not. - The filter gains `close_date >= {current_quarter_start}` and `close_date <= {current_quarter_end}`, in the canonical `VIEW_FILTER_OPERATORS` spelling (`greater_than_or_equal` / `less_than_or_equal`, not the `*_or_equal` aliases the issue body proposed, which the schema only normalizes on parse). - The upper bound is inclusive rather than half-open against `{next_quarter_start}`. A `*_end` token is the period's last calendar DAY, which on a datetime column would stop at midnight — but `close_date` is `Field.date()`, `YYYY-MM-DD` TEXT on both sides, the same field-type property the dashboards already rely on (#460). Both forms measured identical here. - An empty state is authored and translated in all four locales. The seeds do qualify for most of a quarter (six deals at daysFromNow offsets 11…45), but those offsets run from the install day, so a demo seeded with under 11 days left opens this tab on nothing — and a real org meets the same state whenever the quarter's commit slips out. - The four labels are unchanged. They were right all along; the filter lied. Tests: - `test/forecast-current-quarter-view.test.ts` — the KNOWN_DEBT exemption is deleted (the map stays, empty, for the next instance), and a second runtime block pins the RANGE case the same three ways the #730 block pins the equality case: 0 rows = macro unresolved, all rows = unscoped, one quarter = contract. Plus a bound-by-bound subtraction, since a range can rot by half while still looking scoped, and boundary rows on the quarter's first and last day so the inclusive bound cannot quietly become exclusive. The filter translator now implements the comparison operators instead of throwing on them. Fixes #743 Claude-Session: https://claude.ai/code/session_01VHrPAGEgFDoHjphqYG4BMa Co-authored-by: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
yinlianghui
marked this pull request as ready for review
August 5, 2026 17:22
This was referenced Aug 5, 2026
Closed
Merged
yinlianghui
added a commit
to yinlianghui/hotcrm
that referenced
this pull request
Aug 10, 2026
…ipped views (objectstack-ai#757) The "Standard list views" section on `content/docs/sales/opportunities.mdx` (and its zh-Hans / zh-Hant siblings) listed six views, five of which do not exist (My Opportunities, Closing This Month, At Risk, Top Deals by Amount, Pipeline Kanban), and gave the `closing_this_quarter` view a THIRD name — "Pipeline This Quarter" — where the metadata label and all four locale bundles say "Closing This Quarter". The section is now the real roster of nine saved views declared in `src/views/opportunity.view.ts`, each with what it filters, how it sorts and where it is reached from. `closing_this_quarter` is documented as it behaves after objectstack-ai#743/objectstack-ai#746: open commit/best-case deals windowed to the current quarter, with the empty state explaining the legitimately-empty case. Also corrected in the same sweep: the rep tip pointed at an "At Risk" view that does not exist (now: time in stage on Stale Opportunities, plus the daily Stalled Deal Alert), and the kanban board's sidebar entry is labelled "Pipeline", not "Sales Pipeline" (that is the view's own name). Fixes objectstack-ai#752. Claude-Session: https://claude.ai/code/session_01VHrPAGEgFDoHjphqYG4BMa Co-authored-by: Claude <noreply@anthropic.com>
yinlianghui
added a commit
to yinlianghui/hotcrm
that referenced
this pull request
Aug 10, 2026
…lter never applied (objectstack-ai#769) (objectstack-ai#776) * fix(i18n): stop the Review Queue tab claiming a 180-day window its filter never applied (objectstack-ai#769) All four locale packs named `crm_knowledge_article.stale_articles` "Stale (>180d)" / 过期 (>180 天) / "Obsoletos (>180d)" / 古い (>180日) over a filter reading only `status = published`. The tab therefore returned EVERY published article — one reviewed minutes ago included, merely sorted last — under a heading promising a six-month cut. Same defect class as objectstack-ai#730, and as there the lie lived only in the translated half: the metadata label ("Review Queue · Oldest First") promised an ordering throughout, and the `last_reviewed_at asc` sort delivers it. The window was measured before being ruled out, not assumed impossible. On the pinned 17.0.0-rc.2 `{180_days_ago}` resolves on the read path and lands on the START of the calendar day 180 days back, so `less_than` excludes that whole day — the #3777 day-boundary convention, and the right sense for "> 180d". But `$lt` matches neither null nor absent values, so the window deletes never-reviewed articles, which are a review queue's most overdue population. The honest condition is a DISJUNCTION ("older than the window OR never reviewed"), and a view `filter` is a flat, strict array of {field, operator, value} rules combined with AND: no `or`, no nesting, no logic key. Spelled the only way the grammar allows, the two rules AND together and the tab returns zero rows. The engine itself answers correctly with `$or` — the limit is the view-authoring grammar, not the data layer. So the fix is the name, not the filter. Which rows the tab returns is unchanged, and no emptyState is authored because no new empty state exists (the objectstack-ai#746 reason does not apply here). The house-rule guard grew a second claim family. It read only current-calendar-period phrases and so was blind to "> 180d": run against these four labels verbatim it passes 17/17. It now also reads parameterised day windows in all four locales, and carries a positive control — the deleted labels, fed through the shipped filter, must be reported as a breach — so the family cannot pass vacuously now that its only instances are gone. A new runtime block pins every measurement above against a real engine. Fixes objectstack-ai#769 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VHrPAGEgFDoHjphqYG4BMa * docs(comments): name the write path that actually leaves a published article unreviewed (objectstack-ai#769) The note added in the previous commit said `last_reviewed_at` is nullable without saying how a published article reaches that state, and the measurement behind it was taken on a bare `ObjectQL.create()` — which does NOT register the engine's `sys_fetch_previous_update` builtin. Re-measured with a faithful replica of that builtin bound (priority 5, beforeUpdate, object '*'), the first reading was wrong in one direction and right in another: single-row update prev-fetch sets previous, hook sees previous.status "published" and re-stamps — a null write is overwritten multi:true update input.id is undefined, so the builtin fetches nothing, the hook sees no status and stamps nothing — a bulk write puts null straight onto published rows, and a bulk edit never marks them reviewed So the never-reviewed population is not produced by ordinary editing; it is produced by bulk loads and mass edits — which is how an org imports an existing knowledge base. That makes the window's loss worse than "rare and pathological", not better: the rows it would hide are the freshly imported ones, which no human has ever reviewed. Comment only. The runtime block's assertions are about `$lt`, null and the filter grammar, none of which depend on hooks, so they are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VHrPAGEgFDoHjphqYG4BMa --------- Co-authored-by: Claude <noreply@anthropic.com>
yinlianghui
added a commit
to yinlianghui/hotcrm
that referenced
this pull request
Aug 10, 2026
…tstack-ai#767) (objectstack-ai#787) * fix(i18n): complete the English view labels, and guard view-label parity across locales (objectstack-ai#783, objectstack-ai#767) Nine saved list views had no `_views` label entry in the `en` bundle while zh-CN, ja-JP and es-ES all carried the full set: `crm_task.todays_tasks` / `overdue_tasks` (objectstack-ai#783), `crm_lead.hot_leads` and `crm_account.renewals_due` / `at_risk_accounts` (objectstack-ai#767), plus `crm_case.my_open_cases` / `sla_at_risk` and `crm_opportunity.stale_opportunities` / `closing_this_quarter`, which the guard's derivation turned up beside them. Each entry added here is the view's own metadata `label` verbatim — no name a user reads changes, and no locale invents a reading of its own. The four `crm_case` / `crm_opportunity` rows are closed in the same PR rather than filed: they are the identical defect in the identical file, and the guard this issue asks for cannot ship green without a disposition for them. The only alternative was an exemption ledger, which the select-field guard's own comment in this file calls a regression. The `closing_this_quarter` comment claiming "no label here on purpose" is removed with it. It was written by objectstack-ai#746 after objectstack-ai#679 had already ruled the other way for every other locale surface, and its premise — that `en` needs no entry because the resolver falls back to the metadata label — is exactly the invisibility that let these nine sit unnoticed. ## The guard Nothing mechanical could have found any of this. `pnpm lint` hard-codes `--skip-i18n`, and dropping the flag does not report them either: the run emits one line about 2298 hidden platform built-ins, because app-authored `_views` completeness is not in the set the linter checks. So this is not the objectstack-ai#494 family (real warnings suppressed by a flag) — it is a surface nobody checked, and both gaps were found by a human comparing four bundles column by column. Three assertions in `test/metadata-references.test.ts`, derived from the compiled stack rather than a hand-kept list, so a view added tomorrow is held to the bar on the PR that adds it: - every canonical view has a `label` in every locale (70 views x 4 locales); - no locale carries a `_views` entry for a view the stack no longer ships — an orphan reads as coverage while translating nothing; - the `en` label is byte-identical to the metadata label it stands in for. The third is what parity alone cannot do, and it is objectstack-ai#767's actual concern: with the key present in all four bundles, editing the metadata label used to let `en` track the rename for free while the other three kept translating the old name, with nothing red. It proves the ENGLISH pair agrees and nothing more — it makes a rename loud, it cannot make the other three correct. All 61 `en` view labels that existed before this change were already byte-identical, because objectstack-ai#679 extracted them programmatically rather than transcribing them. An anti-vacuum assertion guards the derivation itself: >= 50 canonical views over >= 10 objects, every one resolving an object and a name, and at least one `_views` table parsed out of every locale pack. ## Verification Reverse-verified in four directions, each predicted before it was run: - delete `en.crm_task.overdue_tasks` -> parity RED naming `en: crm_task._views.overdue_tasks.label`; byte-identity stays green (it has nothing to compare), which is the correct split of duties; - rewrite that label to a plausible `⏰ Overdue Tasks` -> byte-identity RED naming both strings, parity GREEN — the case parity cannot see; - add a `_views` entry for a view that does not exist -> orphan check RED; - run the guard against `origin/main`'s `en.ts` -> exactly the 9 rows above, all in `en`, none in ja-JP. That last run also disproves half of objectstack-ai#783's premise: it claims ja-JP is missing `todays_tasks` / `overdue_tasks`, but ja-JP has carried both since objectstack-ai#679, at `ja-JP.ts:943-944` — already true at the `7667c9b4` baseline the issue cites. Only the `en` half of objectstack-ai#783 was real, so no ja-JP edit was needed. Full suite green: validate, typecheck, lint (13 warnings, unchanged), hygiene, build, and 1517 tests across 63 files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VHrPAGEgFDoHjphqYG4BMa * docs(test): the anti-vacuum comment miscounted the assertions it guards It said "both assertions below" while three now sit under it (parity, orphan, and the en byte-identity pin). A guard's comment that undercounts what it covers is how the next reader concludes one of them is unprotected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VHrPAGEgFDoHjphqYG4BMa --------- Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #743
前提复核:成立,且区间可表达
派单要求先证伪再实现。两半都复核了,都成立:
缺陷本身在
origin/main上确实存在(src/views/opportunity.view.ts:236):closing_this_quarter的 filter 只有forecast_category in [commit, best_case]和stage not_in [closed_won, closed_lost],对close_date没有任何条件。四个语言包(metadata "Closing This Quarter" / zh-CN 本季度待成交商机 / es-ES Cierres de Este Trimestre / ja-JP 今四半期にクローズ予定)都在承诺这个时间范围。test/forecast-current-quarter-view.test.ts:109的 KNOWN_DEBT 例外和它的自过期断言也都在原处。区间在 17.0.0-rc.2 上可以表达 —— 这是不能靠推断的那一半,所以是跑出来的。宏词表在
@objectstack/spec/src/data/date-macros.zod.ts的DATE_MACRO_PERIOD_TOKENS里,current_quarter_end和current_quarter_start并列存在;解析实现在@objectstack/core的resolvePeriodToken,*_end走next period start 减一天。在 pinned 17.0.0-rc.2 上实测:所以不需要 needs_decision,也没有把算出来的日期冻进 metadata。
改了什么
src/views/opportunity.view.tsclose_date边界。算子用规范拼写greater_than_or_equal/less_than_or_equal——不是 issue 正文里建议的greater_or_equal/less_or_equal。后者在VIEW_FILTER_OPERATOR_ALIASES里,parse 时会被折叠成规范形式,所以运行起来一样,但源码就偏离了VIEW_FILTER_OPERATORS这唯一的 authoring 词表。测试里对拼写做了结构钉。<= {current_quarter_end},而不是半开的< {next_quarter_start}。这是刻意选的,理由写进了注释:*_end是周期的最后一个日历日,spec 明确警告在datetime列上它会停在午夜——但close_date是Field.date(),两侧都是YYYY-MM-DDTEXT,闭区间是精确的。这正是metadata-references.test.ts里「dashboard 只能 windowdate字段」那条规则(Service dashboard renders empty by default — dateRange defaults to last_30_days but seed cases are older #460)依据的同一个字段类型性质。两种写法实测同一结果集,闭区间读起来就是它的字面意思。emptyState。这是本 PR 唯一的用户可见副作用,判断依据说清楚:和 fix(views): scope the "This Quarter" forecast view to the current quarter (#730) #745 那个不一样。forecast 视图是全新安装必然为空(窗口归扫描任务);这里种子会产出行——查了src/data/sales.seed.ts,六条商机在 forecast_category + stage 上合格,close_date 分别是daysFromNow的 11/14/24/30/38/45。但这些偏移都是相对安装日的,所以当季度剩余不足 11 天时六条全部落到下个季度,刚种子化的演示打开这个 tab 就是空的;真实组织在本季度承诺滑出时同样会遇到。空是合法状态,不解释就像坏了。四个语言包 —— label 一个字没改(它们本来就是对的,说谎的是 filter),只加
emptyState的 title/message。en.ts里closing_this_quarter原本就没有条目(和stale_opportunities一样,英文名走 metadata label),所以只补 emptyState、不补 label,并把这个取舍写在注释里。测试
test/forecast-current-quarter-view.test.tswhereFromViewFiltertranslator 原本对非equals直接抛异常(刻意如此:静默丢条件会把红测试变绿)。现在实现了比较算子,并且同字段两条规则合并成一个比较对象;标量等值和区间撞在同一字段上仍然抛异常,因为那必然要丢掉一条。反向验证(方向事先预判,两个都跑了,都命中)
预判红 —— 删掉两条 range 条件(即恢复 main 的 filter)→ 5 条转红,且各自点名真实原因:
预判绿 —— 同一次运行里
every KNOWN_DEBT exemption is still needed保持绿,因为 map 已清空,没有例外可以过期。这条不是疏漏,是这个断言的语义。第二个反向验证(自过期断言本身):在修好的 filter 之上把 KNOWN_DEBT 条目加回去 → 正好这一条转红,措辞就是 issue 承诺的那句:
docblock 里「它在修好那天转红、直到删掉这行」这句话因此是实测的,不是转述的。
验证
平台依赖未动,仍锁在
@objectstack/* 17.0.0-rc.2;没有任何绕行实现或冻结日期,用的就是平台自己的宏词表。越界的部分:没改
src/views/opportunity.view.ts:219-221(stale_opportunities的注释)仍然写着「the list data path resolves no date macros」 —— 这是 Two source comments still assert the list-view path resolves no date macro — one of them tells the next author not to generalise #744 那一类过期注释的第三处,而且和本 PR 在同一个文件里、在我改的 filter 上方十几行,读起来是直接自相矛盾的。没有在这里顺手改,也没有另开 issue(那会和 Two source comments still assert the list-view path resolves no date macro — one of them tells the next author not to generalise #744 撞成孪生),而是把这处补充到了 Two source comments still assert the list-view path resolves no date macro — one of them tells the next author not to generalise #744 的评论里,让三处在同一次修正中保持一致口径。它不影响stale_opportunities的形态:那条注释还给了第二个独立且仍然成立的理由(days_in_stage是公式字段,引擎查询后才算)。content/docs/sales/opportunities.mdx:117用第三个名字("Pipeline This Quarter")称呼这个视图、并列出仓库里并不存在的视图名 —— issue 正文已记录,属于 forecasting.zh-Hans/zh-Hant 相对英文页存在早于 #627 的整页漂移 —— 桶语义、承诺定义、缺失的汇总提示框需要整页重译 #736/content/docs/sales/activities.mdx 与事件模型对账:把「活动 = Task 记录」讲窄了,并承诺了 crm_opportunity 上并不存在的 last activity date 字段 #739 同轮在飞的文档面,本 PR 一行未动。Generated by Claude Code