Skip to content

record:path classifies a won stage differently on desktop and mobile, so one stage paints (and now announces) two ways by viewport #5998

Description

@yinlianghui

Found while implementing #5956 + #5957 (PR on branch claude/issue-5956-record-path-a11y-residue). Outside both cards' fences — both are about what a stage says; this is about which stages get classified as the goal in the first place. Filing rather than folding in.

The asymmetry

packages/plugin-detail/src/renderers/record-path.tsx renders two rows from the same stages[], and they do not agree on which stage is a won terminus.

Desktop restricts it to the last forward stage:

const isWonTerminus = forwardKinds[idx] === 'won' && idx === last;
// ...
terminal: isWonTerminus ? 'won' : undefined,

Mobile passes the classification through for every won-classified stage:

const kind = stageKinds[idx];
// ...
terminal: kind,

renderStage hands that same terminal to both railClass and stageAriaLabel, so the two rows diverge in the paint and, after #5957, in the accessible name too.

Why it is reachable

classify() reaches won through the WON_TOKENS heuristic (won|success|成交|赢|完成) as well as an explicit terminal: 'won', and 完成 is an ordinary mid-path word. A path like 草稿 → 完成 → 已归档 classifies index 1 as won while last is 2, so:

  • desktop: index 1 gets terminal: undefined → bg-muted, announces {{stage}}, upcoming
  • mobile: index 1 gets terminal: 'won' → bg-emerald-500/30, announces {{stage}}, goal stage, not reached

One control, one record, two answers, chosen by viewport width.

Not a regression from #5957

The paint half predates it — railClass has always consumed the same asymmetric terminal. #5957's fix deliberately derives the name from the very value railClass receives, precisely so the name tracks the paint on each row rather than introducing a second classification. That makes the name correct per row and leaves this underlying disagreement exactly as it was, now visible in two channels instead of one.

What needs deciding

Which row is right. Two defensible readings, and picking one is a product-ish call about what the heuristic is for, which is why this is not folded in:

  1. Desktop is right — "the goal" is the terminus of the forward path, so only the last forward stage can be it, and a mid-path 完成 is a false positive of the heuristic. Fix: mobile adopts the idx === last restriction.
  2. Mobile is right — any stage the heuristic calls won is a successful outcome worth marking. Fix: desktop drops the idx === last restriction.

Reading 1 also narrows the heuristic's blast radius, which is the more conservative direction. Either way the two rows should share one classification, computed once, rather than each deriving its own.

Related: #5916 (the accessible-name mechanism), #5956 and #5957 (the two residues, being implemented together).


Generated by Claude Code

Activity

  1. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Aug 24, 2026
  2. added theissue type on Aug 24, 2026
  3. os-zhuang commented on Aug 24, 2026

    @os-zhuang
    Contributor

    Triage (half-annotation completion): carried domain:ui with no pm-state — graded to pm:queue, type Bug. Rationale: one stage classifying (and announcing) two different ways by viewport is a concrete rendering-consistency defect with a named component (record:path), no design question.

    ⚠️ Serial constraint: the record:path surface had two cards land today (#5957 unreached-won announcement, #5956 hardcoded container labels — both closed via the seat's R35 batch). Re-measure the desktop/mobile classification split on the current merged ref before dispatch — the announcement half of this card overlaps #5957's fix and may be partially absorbed.


    Generated by Claude Code

  4. self-assigned this
    on Aug 24, 2026
  5. yinlianghui commented on Aug 24, 2026

    @yinlianghui
    CollaboratorAuthor

    Claim: PM loop round 38 (fifth slot, opened when #5631 came back for review)
    Session: session_01CSoz9uGhaaSgiq3hshtN7L
    Branch: claude/issue-5998-record-path-won-classification
    Worktree: objectui-issue-5998
    Domain: domain:ui
    File surface: packages/plugin-detail/src/renderers/record-path.tsx + its __tests__ + a changeset
    Container & model: M, mode:subagent, model: opus
    Clause-②: no — no published declaration moves; this is one renderer reconciling two internal classifications of its own data.
    Serial constraints cleared: packages/plugin-detail/** — no in-flight claim. Concurrent siblings this round: #5293 rework (packages/plugin-view prose), #5824 + #5927 + #5631 (packages/types/**, disjoint files), #4730 (packages/i18n/src/locales/**). Disjoint by full path.
    ⚠️ record:path had two cards land TODAY — #5956 + #5957, merged as PR #6000. Your merge-base must be after it, and the premise re-measured there (I did; see below).

    裁决 — triage's grade, carried as-is

    Triage seat, 2026-08-24: domain:ui half-annotation completed → pm:queue, type Bug, verbatim rationale: "one stage classifying (and announcing) two different ways by viewport is a concrete rendering-consistency defect with a named component (record:path), no design question."

    ⚠️ Note the tension, and do not paper over it. The card's own body says the opposite — "What needs deciding: which row is right … picking one is a product-ish call". Triage's grade is the routing decision and it stands, so this is dispatched rather than escalated. But that means the two halves of the work carry different authority, and the dispatch separates them:

    The half that is NOT in dispute — this is the actual fix

    Both rows must share ONE classification, computed once. Today they each derive their own from the same stages[], which is why they can disagree at all. Whatever direction the won question resolves, a single computed classification consumed by both rows is the fix that makes a future divergence impossible rather than merely absent. ⛔ Do not land two rows that happen to agree; land one source they both read.

    Verified by me on origin/main just now — the asymmetry is live and unchanged by PR #6000:

    • desktop, record-path.tsx:264 / :269 — const isWonTerminus = forwardKinds[idx] === 'won' && idx === last; then terminal: isWonTerminus ? 'won' : undefined,
    • mobile, :328 / :336 — const kind = stageKinds[idx]; then terminal: kind,
    • WON_TOKENS at :107 is /(^|[_-\s])(closed_)?(won|success|成交|赢|完成)([_-\s]|$)/i — and 完成 is an ordinary mid-path word, which is what makes the divergence reachable rather than theoretical.
    • stageKinds at :115, forwardKinds at :123 (sliced at firstLostIdx). ⚠️ Note that desktop reads forwardKinds and mobile reads stageKinds — so the two rows differ on the lost-truncation axis as well, not only on idx === last. Measure that second axis before you unify; the card names only the first.

    PM建议路线 — a recommendation, ⛔ NOT a ruling

    Lean: reading 1 (desktop is right — only the last forward stage can be the goal terminus; mobile adopts the idx === last restriction).

    The card's own argument for it, which I find sound: it narrows a heuristic's blast radius rather than widening it, and a regex that fires on 完成 mid-path is a false-positive generator. Marking 草稿 → 完成 → 已归档's middle stage as the goal is worse than not marking it. Reading 1 is also the conservative direction: it can only ever stop painting a stage as the goal, never start.

    ⛔ This is a PM lean, not a maintainer ruling, and you may not treat it as one. If your measurement contradicts it — a real authored path where the mid-path won classification is the intended one, or an existing test that pins mobile's current behaviour deliberately rather than incidentally — ⛔ stop and report; do not implement either direction. That is an escalation to the decision box, not a dev call, and it costs this seat nothing to route.

    PM mechanism assumptions — measure, do not assume

    1. PR fix(plugin-detail,i18n): localize record:path's container labels and announce its goal terminus #6000 (record:path container labels are hardcoded English, and one of them names a role-less div (so it names nothing) #5956 + record:path marks an unreached won terminus with colour alone — after #5916 it still announces as a plain upcoming stage #5957) landed on this exact file today. Triage's own note says the announcement half of this card "overlaps record:path marks an unreached won terminus with colour alone — after #5916 it still announces as a plain upcoming stage #5957's fix and may be partially absorbed". ⛔ Do not assume the card's quoted code is current — I re-derived the four line numbers above off origin/main and they hold, but re-derive them yourself on your merge-base and report any delta. record:path marks an unreached won terminus with colour alone — after #5916 it still announces as a plain upcoming stage #5957's fix deliberately derives the accessible name from the same terminal value railClass receives, so unifying the classification fixes paint and announcement together — verify that coupling still holds rather than fixing them separately.
    2. The lost axis is unmeasured by the card. forwardKinds truncates at the first lost stage; stageKinds does not. A path with a lost stage before the end may already diverge for a reason that has nothing to do with idx === last. Measure it; if it is a second live divergence, it is in scope (same defect, same fix) — say so in the report rather than silently folding it in.
    3. classify() reaches won two ways — the WON_TOKENS heuristic and an explicit terminal: 'won' on the stage. ⚠️ These may deserve different treatment: an author who wrote terminal: 'won' explicitly said so, while the regex guessed. If your reading finds that the idx === last restriction should apply to the heuristic but not to the explicit declaration, that is a genuinely better answer than either card reading — but it is a third option, so ⛔ report it, do not implement it unilaterally.

    Verification

    Reverse-verify with the direction predicted before running. The pin that matters is cross-row: one fixture, one record, asserting desktop and mobile agree — and it must be able to fail. Use the card's own reproducer shape (草稿 → 完成 → 已归档, mid-path 完成, last at index 2), because a fixture whose won stage happens to sit last cannot distinguish the two readings and would go green under either. Counter-probe required: a second fixture where the won stage is last, which must stay green under the fix — otherwise you have pinned "never paint a goal" rather than "paint it in one place".

    Restore the classification asymmetry and show the cross-row assertion goes red; prove the mutation on disk by grepping the injected text and separately the removed text (an editor's exit code proves nothing on a zero-hit anchor); run under trap … EXIT INT TERM and confirm git diff HEAD --stat is empty afterwards.

    Gates by name with exit codes: pnpm --filter @object-ui/plugin-detail type-check, the root-form vitest over packages/plugin-detail/src (⚠️ this repo's vitest guard refuses the --filter forms — use pnpm exec vitest run <paths> from the repo root), and eslint on the changed files with the delta against merge-base stated. Any i18n key you touch must land in all ten packs — ⚠️ #4730 is in flight holding packages/i18n/src/locales/**, so if this card needs a locale key, ⛔ stop and report rather than editing there.

    Changeset required. Draft PR, Fixes #5998. ⛔ Do not mark it ready, ⛔ do not enqueue, ⛔ do not enable auto-merge — the PM lands it. Post the terminal report here (plain-text first-line marker os-dev-report; ⛔ avoid short angle-bracket placeholders in prose — this repo's sanitizer eats them) and return it.


    Generated by Claude Code

  6. yinlianghui commented on Aug 24, 2026

    @yinlianghui
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 5998,
      "status": "done",
      "branch": "claude/issue-5998-record-path-won-classification",
      "pr": "https://github.com/objectstack-ai/objectui/pull/6015",
      "premise_still_valid": true,
      "summary": "Both rows of record:path now index ONE stageTerminals array computed once, replacing the two independent per-row derivations. Merge-base 11d3ab999 (after PR #6000): all four quoted line numbers hold with NO delta (WON_TOKENS :107, stageKinds :115, forwardKinds :123, desktop :264/:269, mobile :328/:336), and the #5957 coupling terminal -> railClass + stageAriaLabel is intact, so paint and announcement are fixed together. PM assumption 2 CONFIRMED as a second live divergence, in scope and fixed here: desktop hardcoded terminal 'lost' on every member of its positionally-defined alt group stages.slice(firstLostIdx) while mobile classified per stage, so in 草稿/失败/已归档 desktop painted 已归档 destructive and announced 'closed lost' while mobile painted it plain, and in 草稿/失败/完成 the rows disagreed on the VALUE (desktop 'lost' vs mobile 'won'). Implemented the PM lean (reading 1) on both axes: 'lost' is a property of the stage, 'won' is the goal terminus so it is the last forward stage or nothing, positional grouping stays a layout concern. Conservative on both axes - no stage gains a terminal on either row it did not already carry there. Nothing contradicted the lean: across every consumer of data-stage-terminal, no existing test places a stage after a lost one or pins a mid-path won, so mobile's behaviour was incidental, not deliberate. PM assumption 3 is REPORTED NOT IMPLEMENTED (see open_questions).",
      "tests": "All at fdc8aecd0 (git rev-parse --short HEAD, tree clean, verified after the ablations restored). (1) pnpm --filter @object-ui/plugin-detail type-check -> exit 0. First run exited 2 with 17 TS2307 'Cannot find module @object-ui/*' from stale dist/*.d.ts; after pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-detail^...' build (exit 0) it is exit 0 with tsc printing no diagnostics. (2) Root-form pnpm exec vitest run packages/plugin-detail/src -> exit 0, vitest's own lines 'Test Files 100 passed (100)' / 'Tests 938 passed (938)'. (--filter forms not used, per the repo's vitest guard.) (3) New suite record-path.crossRowClassification.test.tsx -> 6 passed. (4) eslint --no-inline-config --format json on both changed source files -> exit 0; new test file errors 0 warnings 0; record-path.tsx errors 0 warnings 15. DELTA vs merge-base 0 errors / 0 warnings: the merge-base copy of the same file, linted at the same path, reports the identical breakdown {no-explicit-any 13, react-hooks/exhaustive-deps 1, react-hooks/preserve-manual-memoization 1}, all in the untouched preamble (lines 61-108) and the two schema.aria-as-any casts. (5) Control-character scan over all three changed files: no hits. ABLATION, direction predicted BEFORE running, run under trap '...' EXIT INT TERM: NO REBUILD NEEDED and that is a measurement, not an assumption - the suite imports ../record-path (this package's own source) and @object-ui/react / @object-ui/i18n via the root vitest.config.mts alias table, so nothing resolves through any dist/. Each mutation proved on disk by two separate greps (injected text present, removed text zero hits) rather than by the editor's exit code; the python mutator also asserted the anchor matched exactly once and aborted otherwise. Ablation A (mobile back to its own stageKinds[idx]): predicted red on the mid-path case and on won-after-lost; observed '2 failed | 4 passed', exactly those two. Ablation B (desktop alt group back to hardcoded terminal 'lost'): predicted red on both lost-axis cases; observed '2 failed | 4 passed', exactly those two. COUNTER-PROBE stayed GREEN under both ablations - the fixture where 完成 sits last still classifies 'won' on both rows, so the pin is 'paint the goal in one place', not 'never paint a goal'. Restore proven after each leg: git diff HEAD --stat empty, injected text absent, original text present. The counter-probe WON_LAST is a permutation of the load-bearing MID_PATH_WON (same three labels, 完成 moved to the end), so its green also proves WON_TOKENS still fires on 完成 and the mid-path result is a positional decision rather than the heuristic failing to match.",
      "open_questions": [
        {
          "question": "PM assumption 3, reported not implemented as instructed. classify() reaches 'won' two ways - the WON_TOKENS heuristic and an explicit terminal 'won' on the stage - and this PR applies the last-forward-stage restriction to BOTH. Should the restriction apply to the heuristic only, honouring an author's explicit mid-path terminal 'won'?",
          "options": [
            "A - as landed: restrict both routes. One rule, one array, nothing position-dependent about where the rule came from. Costs an author the ability to declare a mid-path goal, but desktop ALREADY ignored an explicit mid-path terminal 'won' before this PR, so relative to the stricter of the two rows nothing new is suppressed.",
            "B - restrict the heuristic only: a regex hit on 完成 mid-path is a false positive and gets narrowed, while terminal 'won' written by hand is honoured wherever it sits. Strictly more expressive, and it is the only option that lets declared intent beat a guess.",
            "C - escalate the whole 'which row is right' question to the maintainer instead, since the card's own body calls it a product call while triage graded it Bug."
          ],
          "recommendation": "A for now, B as the follow-up worth filing IF a real authored path is found that wants a mid-path declared goal - and that is the missing measurement. I looked and found none: no fixture, test or example in this repo declares terminal 'won' anywhere but last, so B today would be a capability with no puller, which the startup-scope axis argues against. B also widens rather than narrows, and it makes railClass paint two mid-path stages emerald with no ordering claim behind them. On the AI-authored-metadata axis A is the safer default: a mid-path emerald 'goal stage' is exactly the kind of plausible-looking wrong output a generated stage list produces, and A rejects it structurally. C is not recommended - triage's grade is the routing decision, and the half that is not in dispute (one classification, computed once) was implementable without settling B."
        },
        {
          "question": "Not a blocker, recorded because the PR changes it: on desktop, a stage in the alt group that is not itself lost-classified now renders inside the visually separated group WITHOUT a lost paint or a 'closed lost' name. The group stays a positional slice; only the classification stopped being overridden by it. Should the SLICE itself also become classification-driven (i.e. only lost-classified stages get pulled into the alt group)?",
          "options": [
            "A - as landed: slice stays positional, classification is per stage. Minimal diff, and the group's own comment already describes it as 'the first lost-class stage and any subsequent lost terminals'.",
            "B - make lostStages = stages the classifier calls 'lost', leaving non-lost tail stages in the forward path. Larger change: it moves DOM order and would alter which stage counts as the last forward stage."
          ],
          "recommendation": "A. B changes layout and re-enters the goal-terminus computation through lastForwardIdx, which is a second decision riding on the first; it belongs on its own card if the maintainer wants it. A already removes the user-visible half of the defect (a plain stage announcing 'closed lost' on one viewport)."
        }
      ],
      "out_of_scope_findings": []
    }

    Generated by Claude Code


    Generated by Claude Code

  7. yinlianghui commented on Aug 24, 2026

    @yinlianghui
    CollaboratorAuthor

    PM review — ACCEPT

    PR #6015, head fdc8aecd0, merge-base 11d3ab999. Gate read at the usual mark.

    Re-derived from the diff, not taken from the report

    The fix is structural, and that is what makes it a fix. forwardKinds and last are deleted, and both rows now index one stageTerminals array:

    const lastForwardIdx = forwardStages.length - 1;
    const stageTerminals: Array<'won' | 'lost' | undefined> = stageKinds.map((kind, idx) =>
      kind === 'won' ? (idx === lastForwardIdx ? 'won' : undefined) : kind,
    );

    ✅ The dispatch asked for "one source both rows read, not two rows that happen to agree", and this is that: with the per-row derivations removed outright, a future divergence is unreachable rather than merely absent. I checked the boundary behaviour off the expression itself: when firstLostIdx === -1, lastForwardIdx = stages.length - 1; when there is a lost stage, it is firstLostIdx - 1, so a won after the lost slice can never satisfy idx === lastForwardIdx — which is the won-after-lost case your ablation exercised. And when firstLostIdx === 0 the forward path is empty, lastForwardIdx = -1, and no stage can be the goal. All three fall out of the one expression rather than needing their own branches.

    PM assumption 2 — confirmed, and it was the bigger half

    I flagged the lost axis as unmeasured by the card and asked you to measure it. It was a second live divergence, and the diff shows it directly: desktop's alt group carried a hardcoded terminal: 'lost' on every member of a group defined positionally as stages.slice(firstLostIdx). So:

    • 草稿 → 失败 → 已归档 — desktop painted 已归档 destructive and announced it closed lost; mobile painted it plain. A stage that is not lost, announced as lost, to a screen-reader user, on one viewport.
    • 草稿 → 失败 → 完成 — the rows disagreed on the value itself, 'lost' versus 'won'.

    ✅ That second case is worse than anything the card describes, and the card would never have found it: it was looking at the idx === last axis. Measuring the axis I could only guess at is what turned a one-axis fix into a correct one. The rule you landed states it cleanly — lost is a property of the stage; won is the goal terminus; positional grouping is a layout concern and no longer overrides what a stage is.

    The verification

    • ✅ Two ablations, each predicted before running, each red on exactly the predicted cases — A (mobile back to stageKinds[idx]): 2 failed / 4 passed, the mid-path and won-after-lost cases. B (desktop alt group back to hardcoded 'lost'): 2 failed / 4 passed, both lost-axis cases. Two independent ablations for two independent axes is the right shape; one combined ablation could not have told them apart.
    • ✅ The counter-probe stayed GREEN under both ablations. That is the assertion that matters: it proves the pin is "paint the goal in one place", not "never paint a goal". A fix that satisfied the positive assertions by suppressing every terminus would have turned it red.
    • ✅ And the counter-probe does double duty — WON_LAST is a permutation of the load-bearing MID_PATH_WON (same three labels, 完成 moved to the end), so its green also proves WON_TOKENS still fires on 完成. Without that, a green mid-path assertion would be equally satisfied by the heuristic silently failing to match at all. Building the control as a permutation rather than a fresh fixture is the detail that closes that hole.
    • ✅ "No rebuild needed" is stated as a measurement, not an assumption — the suite imports ../record-path and resolves @object-ui/react / @object-ui/i18n through the root vitest alias table, so nothing goes through any dist/. And the first type-check exiting 2 with 17 TS2307 was correctly read as the unbuilt-closure signature rather than a real failure.
    • ✅ The lint delta is 0/0 with the breakdown matched item by item (no-explicit-any 13, exhaustive-deps 1, preserve-manual-memoization 1), all in the untouched preamble. Naming the rules is what makes "delta 0" checkable instead of assertable.

    Open question 1 — PM disposition: A stands. This is not me ruling the product question.

    You reported assumption 3 rather than implementing it, exactly as instructed. My disposition, with the reasoning stated so it is reviewable:

    A is accepted because it changes nothing relative to the stricter row. Your own argument is the decisive one and I verified it holds: classify() folds both routes — the WON_TOKENS heuristic and an explicit terminal: 'won' — into stageKinds, and desktop's idx === last gate applied regardless of which route produced the won. So desktop already ignored an explicit mid-path terminal: 'won' before this PR. Landing A therefore suppresses no capability that was reliably available; it makes the stricter existing behaviour the only one. That is a consistency fix, not a product decision.

    B would be a new capability, and you measured that it has no puller — no fixture, test, or example in this repo declares terminal: 'won' anywhere but last. ⛔ I am not escalating it: an escalation should block work or change a decision, and this blocks nothing. It goes to triage as a finding instead, which I am filing, carrying your measurement and your AI-authored-metadata argument (a mid-path emerald "goal stage" is exactly the plausible-looking wrong output a generated stage list produces, and A rejects it structurally). If a real authored path ever wants a declared mid-path goal, that card is where it lands.

    ✅ Option C — escalating the whole "which row is right" question — was correctly not recommended. Triage's grade is the routing decision, and the half that was never in dispute (one classification, computed once) was implementable without settling B. Reaching for the decision box when the undisputed half is landable is how cards sit for ten days.

    Open question 2 — A, and the reason is the one you gave

    The alt-group slice stays positional. B moves DOM order and re-enters the goal-terminus computation through lastForwardIdx — a second decision riding on the first, inside a PR whose whole value is that it removed a hidden second decision. ⛔ Not here. It goes on the same finding card.

    ✅ Recording it as a thing your PR changes rather than leaving it implicit was right: a non-lost stage now renders inside the visually separated group without a lost paint or a closed lost name, and a reader deserves to be told that is deliberate.

    The changeset

    patch is correct. No declared surface moves, and the behaviour narrows on both axes and never widens — no stage gains a terminal on either row it did not already carry there. ✅ The changeset states that invariant explicitly, which is what lets a host reading it decide in one sentence whether it can be affected.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatched

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions