Skip to content

复核队列要不要真的变成 180 天切分 —— 卡在 view filter 写不出析取,产品决策待定(#769 的余额) #781

Description

@yinlianghui

#769 的余额,单独记录。#769 已由 PR #776 关闭 label 与 filter 的不一致(四个语言包回归 canonical 的「Review Queue · Oldest First」),但没有回答产品问题:这个页签本来就该是排序,还是该变成真正的 180 天切分?

四位译者各自独立写出了窗口承诺,这本身是产品意图的一个信号,不该当成纯粹的翻译失误一笔勾销。

为什么 PR #776 没有直接加窗(实测,非推断)

pinned 17.0.0-rc.2 + InMemoryDriver + 真 ObjectQL.find:

  1. 窗口本身可表达。 {180_days_ago} 在读路径上解析,落到「180 天前那个日历日的 00:00:00.000Z」;配 less_than(排他上界)整天被排除,正是 > 180d 的语义。
  2. 但 $lt 不命中空值。 显式 null 与键缺失两种写法都掉出窗口。
  3. 空值行由批量写入路径真实产生(见 批量写(multi: true)路径上 ctx.previous 恒为空 —— 15 个读 previous 的 hook 在该路径全部空转,知识文章可被批量写成「已发布但从未复核」 #779):multi: true 更新上 ctx.previous 恒为空,补戳 hook 什么也不做,于是「已发布但从未复核」的行由批量导入/批量编辑造出来 —— 而批量导入正是把既有知识库搬进 CRM 的常规方式。这批行恰恰最需要复核。
  4. 诚实的条件是析取,而 view filter 语法写不出析取。 ViewFilterRuleSchema 是 {field, operator, value} 的扁平严格数组,规则之间 AND,没有 or、没有嵌套、没有 logic key。用语法允许的唯一写法拼出来(同一字段上两条规则)结果是零行:
where {"last_reviewed_at":{"$lt":"{180_days_ago}","$null":true}}   -> []  (n=0)

一个空白页签和「没有文章需要复核」在界面上完全一样。

  1. 引擎本身能答对 —— 限制在 view 授权语法,不在数据层:
where {"status":"published","$or":[{"last_reviewed_at":{"$lt":"{180_days_ago}"}},{"last_reviewed_at":{"$null":true}}]}
  -> [reviewed-181d, reviewed-400d, never-reviewed-NULL, never-reviewed-ABSENT]  (n=4)

三条可能的出路,都需要先定夺

C 的成立与否取决于 #779 怎么修,所以这条不是能立刻开工的活。

Blocked-by: #779(出路 C);出路 B 另需平台 spec 支持 view filter 析取。

守卫已就位:test/forecast-current-quarter-view.test.ts 的参数化天数窗口词表会在任何 label 重新声明 N 天窗口而 filter 不带 {N_days_ago} 时报红,所以本条无论怎么定,label 都不会再悄悄跑偏。

Activity

  1. yinlianghui commented on Aug 5, 2026

    @yinlianghui
    CollaboratorAuthor

    PM 分诊(修复线):产品决策,入决策收件箱,不派

    三条出路(A 维持排序 / B 等平台给 view filter 析取能力再加窗 / C 先修 #779 立「已发布 ⇒ 必有复核时间戳」不变量 + 历史 null 回填,再单条 less_than 加窗)各带成本,且 C 的可行性取决于 #779 怎么修(现 Blocked-by objectstack#5574)、B 需要平台 spec 变更 —— 这不是修复线可决的口径,等维护者定夺。

    修复线的两点补充意见,供决策参考:


    Generated by Claude Code

  2. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    Collaborator

    Ruling (maintainer, 2026-08-11, PM chat, verbatim: 「接受你的全部建议」): Option A for now — the review queue keeps its sorting semantics, and this card goes pm:on-hold rather than staying in the decision inbox.

    Hold record: date 2026-08-11; reason: the honest 180-day slice needs a disjunction the view-filter syntax cannot express, and the only viable windowing route (Option C) depends on how #779 (ctx.previous empty on multi: true writes — currently pm:blocked upstream) gets fixed, plus a stock backfill; restart condition: when #779 lands, re-read Option C against the actual fix shape and re-decide.


    Generated by Claude Code

  3. os-warren commented on Aug 24, 2026

    @os-warren
    Collaborator

    Premise refresh — route C's stated blocker is closed, and one of this card's own arguments is now dead. Recording only; ⛔ nothing decided here.

    Posted by the repo:hotcrm PM seat while landing #1246 (PR #1267). This card stays needs-user-decision and untouched in substance — it is the maintainer's call. What follows is a re-measurement of the facts the card was filed under, because two of them have changed since.

    1. The comment this card's argument rests on was false, and has just been corrected

    The note at src/views/knowledge_article.view.ts is the recorded reason stale_articles ships as a ranking rather than a 180-day cut. Its argument ran: a window would hide the never-reviewed rows → those rows are produced by the bulk path, which stamps nothing → therefore the honest condition is a disjunction → therefore the window is unsafe.

    The second premise is dead. Measured this round on the repo's pinned @objectstack/* 17.1.0 with a real engine — not inferred from the source, and not quoted forward from #779's report:

    rows matched: 3 | hook dispatch count: 3
    C1 dispatched PER ROW (not once per batch): true
    C2 input.id present on every dispatch     : true
    C3 previous bound on every dispatch       : true
    C4 hook saw status and STAMPED every row  : true
    

    Bulk load (batch insert) measured separately, since that is the scenario the comment actually names ("how an org imports an existing knowledge base"): 4 rows, 4 dispatches, every published row stamped, imported historical published_at preserved per row.

    So new bulk writes no longer manufacture never-reviewed rows. PR #1267 corrected both comments and marked the rc.2 account as history.

    2. What still holds, and it is the part that actually blocks route B

    ⚠️ Do not read the above as "the window is now safe". Two things are unchanged:

    • The grammar limit stands. ViewFilterRuleSchema (objectstack main, packages/spec/src/ui/view.zod.ts) is still a flat strictObject({field, operator, value}) combined with AND — no or, no nesting, no logic key. "earlier than {180_days_ago} OR empty" is still unsayable in a view filter. That, not the bulk-path premise, is what holds this view to a ranking today.
    • $lt still matches neither null nor an absent key, so a bare window still silently drops any row that has none.

    3. Route C's precondition changed, and its remaining cost did not

    This card records route C ("fix #779 so published implies a review timestamp, then add the window with a single less_than rule") as unavailable, because it depended on how #779 would be fixed.

    #779 closed as completed 2026-08-14 and is measured fixed above. So route C's stated blocker is gone. Its other stated cost is not: rows already stored with a null last_reviewed_at do not self-heal, so route C still needs a backfill for the historical population. That half of the card is unchanged and is the real remaining question.

    ⚠️ Why this is being posted at all

    This card sat parked on a blocker that had been closed for nine days before anyone noticed — nothing in the label machine re-opens a premise when its blocker dies. That failure is exactly what #1246 existed to prevent, so leaving it to happen a second time on the same card would be perverse.

    Recording that a blocker died is not deciding the question. The decision — ranking vs. 180-day cut, and whether the backfill is worth it — is untouched and remains yours.


    Generated by Claude Code

  4. huangyiirene commented on Aug 31, 2026

    @huangyiirene
    Collaborator

    裁决:复核队列页签删除(维护者 2026-08-31)

    项目总监席 · session session_01KGtaLpkW1mycWgkbSb3H6t · hotcrm 决裁批 #16 ②

    维护者原话(逐字):「781 什么叫 知识复核队列,建议删除。」—— 决裁流转中的一行回批即裁决,录为删除。⚠️ 透明记录:原话作「建议」,若维护者本意仅为倾向而非裁决,一行改裁即撤。

    裁决内容

    1. 知识库「复核队列」页签整体删除:页签/视图本体(Review Queue · Oldest First)+ 四个语言包的对应 label 条目 + 针对该页签的守卫断言(test/forecast-current-quarter-view.test.ts 中 stale_articles 的四个语言包 label 都写着 “>180d”,但 filter 只有 status = published —— 列表返回每一篇已发布文章 #769 运行时块的相关断言,按仓内测试纪律随功能一并退场,⛔ 不是为绿而删测试 —— 是被测物退役);
    2. 范围钉死:last_reviewed_at 字段与 批量写(multi: true)路径上 ctx.previous 恒为空 —— 15 个读 previous 的 hook 在该路径全部空转,知识文章可被批量写成「已发布但从未复核」 #779 的批量补戳修复不在删除范围(数据层资产);该字段本身要不要退休是另一张卡的问题,⛔ 不搭车;
    3. 卡面呈报的 A(维持排序)/ B(等平台析取)/ C(加窗 + 回填)三路全部不采 —— 维护者裁的是第四路:功能本体不保留。四位译者的「180 天」窗口承诺随页签一并退场,stale_articles 的四个语言包 label 都写着 “>180d”,但 filter 只有 status = published —— 列表返回每一篇已发布文章 #769 的 label-filter 一致性问题就地消解,emptyState(fix(views): window "Closing This Quarter" to deals that close this quarter (#743) #746)义务不再产生。

    状态转移(同笔)

    needs-user-decision → pm:queue。


    Generated by Claude Code

  5. added
    pm:queueReady for the PM dispatch loop
    and removed
    needs-user-decisionNeeds the maintainer's call before work proceeds
    on Aug 31, 2026
  6. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Sep 3, 2026
  7. self-assigned this
    on Sep 3, 2026
  8. os-sales commented on Sep 3, 2026

    @os-sales
    Collaborator

    Claim: PM loop round R28
    Session: session_019hUuCQStzXGMFSX4dzww5t
    Branch: claude/issue-781-delete-review-queue-tab
    Worktree: hotcrm-issue-781
    Domain: repo:hotcrm execution seat (single-lane repo — ⛔ no domain:*)
    File surface: src/views/knowledge_article.view.ts, src/translations/{en,es-ES,ja-JP,zh-CN}.ts, test/forecast-current-quarter-view.test.ts (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus — ⛔ fable measured exhausted for this account 2026-09-02 (HTTP 429); standing downgrade exemption (maintainer 2026-08-13 「fable 如果用完了,可以用 opus」), floor opus
    Clause-②: no
    Serial constraints cleared: src/translations/** is shared with #837 (crm_case → 「工单」, also ruled) and #1511 (sla_overview) — both pm:queue, neither dispatched, so the file is uncontended this round. ⛔ Neither may be dispatched until this card lands. src/views/knowledge_article.view.ts and test/forecast-current-quarter-view.test.ts sole occupant. pm:dispatched lane-wide was 0 at round open; no open PR touches these paths.

    Premise re-verified on origin/main @ d3fb7ca (⛔ not carried from the card): the stale_articles view is live at src/views/knowledge_article.view.ts:70 with label: 'Review Queue · Oldest First' at :145; all four locale entries present (en.ts:285, es-ES.ts:300, ja-JP.ts:308, zh-CN.ts:274); the parameterised day-window vocabulary is live in test/forecast-current-quarter-view.test.ts:148-156. The ruling's named action is still undone.


    Generated by Claude Code

  9. os-sales commented on Sep 3, 2026

    @os-sales
    Collaborator

    Claim (dev seat — separate from the PM claim in the comment above; ⛔ assignee untouched)

    Session: session_019hUuCQStzXGMFSX4dzww5t
    Branch: claude/issue-781-delete-review-queue-tab (pushed empty at claim time as a write-route probe — HTTP OK)
    Worktree: hotcrm-issue-781
    Base: origin/main @ d3fb7ca

    Acting on the 2026-08-31 ruling recorded in comment 5479980713 (deletion of the knowledge-base "Review Queue" tab). Scope as pinned there: view/tab body + four locale label entries + the guard assertions whose subject is that tab. last_reviewed_at and #779's bulk-stamping fix are ⛔ out of scope.

    Read before starting: issue body plus all 7 prior comments. No sanitizer truncation found in the body.


    Generated by Claude Code

  10. os-sales commented on Sep 3, 2026

    @os-sales
    Collaborator

    os-dev-report

    {
      "issue": 781,
      "status": "done",
      "branch": "claude/issue-781-delete-review-queue-tab",
      "pr": "https://github.com/objectstack-ai/hotcrm/pull/1519",
      "premise_still_valid": true,
      "summary": "Deleted the knowledge-base Review Queue tab per the 2026-08-31 ruling: the stale_articles view, its four locale labels, and the tab-bound guard block. Re-derived the premise on origin/main @ d3fb7ca — all five of the PM's cited locations were live and the ruling's action was still undone. GUARD TRIAGE (the question the PM left open): the parameterised day-window vocabulary is NOT tab-specific and SURVIVES — stale_articles was its origin, but the derivation runs over every list view the app ships. Only the runtime describe block bound to the deleted view retires. One assertion in the family was genuinely tab-bound (it fed breachesOf the filter stale_articles shipped); it was RECONSTRUCTED inline rather than deleted, because it is the only assertion driving a day-window breach end to end — without it the stack-wide derivations pass over an empty set and a broken reporter is indistinguishable from a clean tree. DEVIATION — file surface exceeded: the PM's declared surface named 5 files; 6 more were required and changed, all found by the tree-wide grep the PM's own ZONE 2 mandated: content/docs/service/knowledge-base.{mdx,zh-Hans.mdx,zh-Hant.mdx}, docs/feature-inventory.md (KB-006), src/views/opportunity.view.ts (a cross-reference that would dangle), plus the changeset. Contention checked before reporting: 4 open PRs, only #1520 is non-dependabot, and it has ZERO overlap with any file I touched. The Traditional Chinese face spells the tab 複核佇列, which none of the PM's five grep patterns reach — it was found only by reading the file. Assignee untouched; my own claim comment posted separately. ISSUE READ: body plus ALL comments — the API reports 8 comments, not 7; the 8th is the PM's own claim comment. No sanitizer truncation in the issue body (1796 chars, ends mid-sentence nowhere). Note that comment 5383819213 carries a literal HTML comment marker that survived intact in the API payload. BOUNDED IN-PLACE FIX (declared): the view file's header docblock listed 3 views for a file shipping 4; removing the stale line would have left it listing 2 of 3, so the missing my_drafts line was added in the same edit.",
      "tests": "pnpm verify (validate && typecheck && lint && lint:i18n-gate && hygiene && hygiene:tokens && build && test) run under the shared verify lock AFTER the final commit, on sha 85c7d01, working tree clean: os-verify-lock VERDICT command-exit 0. Gate verdict lines as printed by the gates themselves: 'Validation passed'; 'i18n lint gate: 0 i18n/missing-* issues'; 'source hygiene clean' (includes 'no raw control bytes in first-party files'); 'source token ratchet clean'; 'Build complete'; 'Test Files 156 passed (156) / Tests 3285 passed | 1 skipped (3286)'. Exit codes captured after redirect to a file, never through a pipe. TOKEN RATCHET: interaction layer 37,534 -> 37,426. No re-anchor owed, checked by calling the script's own exported anchor(): anchor(37426) = 40,000 which EQUALS the committed ceiling, and the script re-anchors only when anchor(reading) is strictly less than ceiling. Ceilings untouched. ABLATION 1 (the retained guard is still live over the shipped stack, not decoration): gave a different, still-shipping view a window claim — published_articles label 'Published' -> 'Published (>90d)' in src/translations/en.ts. Mutation confirmed ON DISK before running, anchored on both texts: injected count 1, original-text count 0, and git hash-object 6d45e884 differing from the HEAD blob c20d1368. Result RED as predicted: 2 failed | 19 passed, failing assertions 'every current-period label is backed by a filter that pins that period' and 'no name this stack actually ships claims a day window', reporting 'crm_knowledge_article.published_articles is labelled en:\"Published (>90d)\" but its filter carries no {90_days_ago}'. It caught a view that is NOT the deleted one, which is the claim. Restored and PROVEN restored: git diff HEAD empty and blob hash back to c20d1368. ABLATION 2 (the reconstructed positive control itself fires): blinded the '>Nd' pattern in DAY_WINDOW_PATTERNS. Mutation confirmed on disk (anchor comment count 0, hash differs from HEAD blob). Result RED on exactly the two control assertions: 'the day-window vocabulary reads a window out of every label #769 deleted' and 'a filter without the token is reported as a breach'. Restored and proven by hash equality dcc1c72c == HEAD blob. Both ablation scripts carried a trap with an absolute REPO_ROOT path and restored via 'git checkout HEAD -- path', never a bare 'git checkout -- path'. No rebuild step was required: this repo's tests import from src, and the full union above re-ran green on the restored tree.",
      "mcp_calls": "3 — issue comment (claim), create_pull_request, issue comment (this report). All issue/PR/contention READS went through unauthenticated repo-scoped REST, which probed HTTP 200; gh is absent from this container.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  11. os-sales commented on Sep 3, 2026

    @os-sales
    Collaborator

    ACCEPT — PR #1519, R28

    Reviewer of record: repo:hotcrm seat, session session_019hUuCQStzXGMFSX4dzww5t. Verified against GitHub, ⛔ not against the report's self-description.

    Checklist

    item reading
    PR form draft, base main, Fixes #781 first line ✅
    Path fork (get_files, ⛔ not the report) 12 files: 1 changeset · 3 content/docs/service/knowledge-base.* · docs/feature-inventory.md · 4 src/translations/* · 2 src/views/* · 1 test/*. No governed path ⇒ merge-queue eligible
    CI all 10 checks completed: success, read directly on head 85c7d01 — the head that will merge. ⛔ Not the aggregate, ⛔ no branch update in the same breath
    Changeset present, minor, real (⛔ not a skip-changeset exemption) — correct for a user-visible removal

    The load-bearing judgement, and why it holds

    The dispatch order left one question open: which guard assertions are tab-specific. The answer delivered is the right one and is argued from the derivation, not from convenience: stale_articles was the day-window family's origin, not its subject — the derivation runs over every list view the app ships, so the vocabulary survives in full and only the tab-bound runtime block retires. It retires because its subject retired, ⛔ not to make anything green.

    What makes this ACCEPT rather than a request for evidence: ablation 1. A different, still-shipping view (published_articles) was given a bogus Published (>90d) claim, and the retained guard went red naming it — 2 failed / 19 passed. That proves the guard is live over the shipped stack, which is precisely the claim in doubt when a guard's origin is deleted. Mutation was confirmed on disk before running (anchored counts on injected and removed text, plus git hash-object divergence from the HEAD blob), and restoration proven by hash equality — ⛔ not by an editor's exit code. Ablation 2 did the same for the reconstructed control.

    ⚠️ The residual risk is correctly identified in the diff itself, which is why it is acceptable: with the origin view gone, the two stack-wide derivations can only return [] until some future view claims a window, so the reconstructed inline assertion is now the only thing proving the vocabulary matches anything at all. That is stated in the test's own comment rather than left for the next reader to discover. The filter still goes through the real filterTokens rather than a hand-written [], so token extraction stays inside the path being proven.

    Deviation — file surface exceeded, and it was right to

    Declared surface named 5 files; 11 + changeset were changed. Accepted: every extra file is a reference to the deleted view, and all were found by the tree-wide grep the dispatch order's ZONE 2 explicitly mandated — a dangling reference is the likeliest way this lands red. src/views/opportunity.view.ts is the good catch: a cross-reference that would otherwise point at a view that no longer exists.

    ⭐ The Traditional Chinese face spells the tab 複核佇列, which none of the five grep patterns in my dispatch order reach — it was found by reading the file. That is this lane's own §6 rule (a line-oriented grep is not proof of absence) run forward rather than recited, and it caught a real miss in the PM's own dispatch order.

    Bounded in-place fix declared and accepted: the view file's header docblock listed 3 views for a file shipping 4; removing the stale line alone would have left it listing 2 of 3, so the missing my drafts line was added in the same edit — same defect class, same file, mechanical.

    One observation, ⛔ not blocking

    The PR title carries a conventional-commit breaking marker (feat(knowledge)!:) while the changeset is minor. For a private: true app that versions itself this only moves the version number, and the minor choice cites the slim-nav-one-entry-per-destination precedent for a navigation-surface change. Recorded so the release compiler's input is not silently at odds with the title; ⛔ no rework asked.

    Scope kept

    last_reviewed_at and #779's bulk-stamping fix are untouched, as the ruling pinned — the publish hook still stamps, the field stays on the form under Engagement, its four locale labels are unchanged. Whether the field itself should retire remains a separate, unanswered question.

    ⇒ Flipping to ready and queueing (SQUASH). ⛔ Auto-merge is enabled only now that every check was verified green by direct read, and ⛔ not in the same breath as any branch update.


    Generated by Claude Code

  12. removed their assignment
    on Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions