Skip to content

feat(desktop): add conservative WorkHub session router - #3497

Merged
Astro-Han merged 6 commits into
apache:mainfrom
ARE404:codex/workhub-session-router
Aug 23, 2026
Merged

feat(desktop): add conservative WorkHub session router#3497
Astro-Han merged 6 commits into
apache:mainfrom
ARE404:codex/workhub-session-router

Conversation

@ARE404

@ARE404ARE404 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds the first usable WorkHub MVP as a unified entry point over existing ordinary Sessions.

  • Keeps Session as the sole source of truth. A Work is a user-facing view of one ordinary Session; WorkHub owns no second database, event log, or lifecycle.
  • Projects existing Sessions into a unified WorkHub conversation and routes executable input with strategy R2.3-session-authoritative-hybrid.
  • Supports direct routing, clarification between plausible Sessions, explicit creation, conservative creation for clear unmatched executable goals, and correction through the UI.
  • Keeps discussion and creation-deliberation in WorkHub instead of mutating Session state.
  • Adds a rollout preference, focused controller/policy tests, and the WorkHub domain-language note.

Refs #3492

Routing choices and experiments

We evaluated all policies against the same 10-Session snapshot and Chinese acceptance guide. The scores describe different test layers and runtime shapes, so they are not directly interchangeable:

PolicyMain choiceRuntime / authority shapeSame-snapshot result
R1Aggressive routing with broad contextual carry-overExperimental runtime10/11
R2Conservative routing with more clarificationExperimental WorkHub routing memory9/11 routing; 8/11 strict E2E
R2.1Evidence scoring tuned to the idealized casesExperimental WorkHub routing memory11/11 idealized
R2.2Unified-conversation/projection-first policyExperimental runtime5/11
R2.3Session-authoritative hybrid adopted hereProduction Session facts plus transient focus/correction memory8/11 routing/interaction; 7/11 strict E2E

R2.3 is not selected because it has the highest raw score. R2 and R2.1 were measured in the earlier experiment, where WorkHubSnapshot.routingMemory gave WorkHub its own durable routing state and the surrounding runtime had more mature interaction surfaces. Porting that result directly would violate the chosen architecture boundary. R2.3 deliberately trades some measured accuracy for a smaller production shape: Session remains the sole persistent authority, while WorkHub keeps only discardable in-memory focus and correction hints. The current 7/11 strict result therefore supports shipping this as an opt-in MVP experiment, not declaring the routing problem solved.

Its precedence is: exact full Session name; explicit creation/correction; core entity evidence; then bounded focus confirmed inside the current WorkHub lifetime. Weak multi-match input asks for clarification. Clear unmatched executable input creates an ordinary Session, while discussion, negated creation, and creation questions remain in WorkHub. At equal evidence, the latest correction wins.

Adversarial review also hardened the implementation so archived Sessions remain inspectable but cannot be routed to, creation negation/questions cannot write, same-frame double submissions are gated synchronously, failed/waiting submissions preserve the draft, English routing uses whole-word evidence and explicit uncertainty, and routing only consumes evidence populated by the production Session adapter.

Verification

  • npm run lint — passed (2,591 files)
  • npm run format:check — passed
  • npm run typecheck — passed
  • npm run build — passed
  • Desktop test suite — 1,228/1,228 passed
  • Focused WorkHub tests — 41/41 passed
  • Astryx surface inventory — 203/203 aligned
  • npx knip --workspace apps/desktop — passed
  • npx knip --workspace packages/ui — passed
  • git diff --check — passed

UI evidence

Captured from the real development app on this branch with the current 10-Session projection.

WorkHub overview — the unified entry point models the existing ordinary Sessions without creating a second Work store.

WorkHub overview

Routed to an existing Session — the request stays visible in WorkHub together with the selected target and its current status.

WorkHub routed to an existing Session

Correction picker — the user can inspect and change the selected Session without introducing another authority.

WorkHub correction target picker

Follow-up scope

  • Initialize bounded recent/previous focus when WorkHub first opens.
  • Recognize first-time natural-language corrections without relying on the correction menu.
  • Reconstruct the aggregate WorkHub timeline after navigating into a Session and back.
  • Delegate ordinary stop/cancel actions; current stop delegation is correction-only.
  • Add WorkHub overview, routed-session, and correction-picker screenshots.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the WorkHub MVP, produced routing experiments and tests, and performed parallel Standards/Spec adversarial review. The affected commit includes a Generated-by: Codex trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@ARE404
ARE404 marked this pull request as ready for review August 22, 2026 13:27

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — the architectural discipline here is real and worth saying first. "A Work is a user-facing view of one ordinary Session; WorkHub owns no second database, event log, or lifecycle" is exactly the right constraint for a feature like this, and the code holds to it: workhub-session-port.ts reads through to the Session, the policy module owns only transient inference state, and there is no shadow store. The rollout preference and the archived-Sessions-inspectable-but-not-routable rule are both good conservative choices. Publishing the policy comparison table instead of just asserting a choice is also the right instinct.

Reviewed at exact head 8177693fd2deee796246290b236a9a75090dff00 against base 17f9351a849821cd4f2401ecea8f062966614348. One P1 and two P2, plus two P3. CI is currently red on this head.

P1 — the routing policy is substantially a Chinese-language heuristic, in an English-first product. Details inline; the short version is that several routing signals have no English path at all, and the ones that do have English use the same numeric thresholds across two languages where those thresholds do not mean the same thing.

P2 — the description's own table argues against the policy it selects. R2 is listed at 9/11 routing; R2.3 at 8/11 routing and 7/11 strict E2E. The text says R2.3 is the best current compromise under the product constraints: no new authority, conservative behavior when evidence is ambiguous, and a usable path for creating new work — but R2 is described as conservative routing with more clarification, which sounds like it satisfies those same three constraints while scoring higher on the shared axis. What is the dimension on which R2.3 beats R2? It may well exist — R2's extra clarification prompts could be judged too costly in practice — but the table as written does not show it, and a reader has to take the conclusion on faith. Since the whole point of publishing the comparison is to let a reviewer check the reasoning, this gap matters more than it would in an ordinary PR. One or two sentences would close it.

Worth stating plainly: at 7/11 strict E2E, roughly three in ten submissions land somewhere other than where the user meant. The rollout preference makes that an opt-in cost rather than a regression, so it does not block. But it does mean the honest framing of this MVP is "an experiment that needs real usage data", and the description's confidence should match that.

P2 — CI is red for a generated artifact, not a logic failure.astryx:surface-inventory fails because .paths is missing apps/desktop/src/renderer/styles/workhub.css and workhub-surface.tsx, and docs/astryx-surface-file-inventory.md no longer matches. Running npm run astryx:surface-inventory:write and committing the result should clear it. Flagging because the description lists a full green verification run, so this was presumably generated before the last two files landed.

P3 — the routing thresholds are unnamed magic numbers.longestMatch >= 4 appears twice with different meaning, alongside >= 3 in correctedTarget, >= 2 in the exact-name filter, and slice(0, 5) / slice(0, 4) for clarification options. For a policy that will be tuned against real usage, these are the knobs — naming them as constants with a one-line rationale each would make the next tuning pass reviewable.

P3 — recentTargets[1] as "the previous focus" is positional.looksLikePreviousFocus resolves to index 1 of an array whose ordering is maintained by rememberTarget. It works, but the meaning lives in an array position rather than in a name, and a future change to how targets are remembered would silently change what "previous" means.


This review was AI-assisted. Findings were verified against the exact head listed above; any mistakes are ours to correct — please push back where we got it wrong.

Comment threadapps/desktop/src/renderer/workhub-route-policy.ts
Comment threadapps/desktop/src/renderer/workhub-route-policy.ts

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at exact head 13507e37abb743a4b9d24b5dd724bcd38ded8fe1. The earlier review was bound to 8177693f, and four commits have landed since, so its conclusions were re-derived rather than carried forward. Required test is completed / success on this exact SHA (job), and the branch merges cleanly against current main.

First, the good news, because it is most of the story. The four new commits fix nearly everything the earlier line raised: uncertainty handling, stop words and boilerplate, whole-word ranking, separate Han and Latin thresholds, English titles, and the what misclassification. The recentTargets[1] issue, the magic thresholds, the Astryx inventory and the PR-body R2/R2.3 framing are all resolved too. The four new commits introduce no new findings of their own — both problems below predate them.

Two [P1]s, both inline. One is a partial closure of the earlier English-routing finding; the other the earlier line missed entirely.

Why the first one matters more than its size suggests. The exact-name path is an older authority than everything this PR added. All the new whole-word and threshold work sits behind it, so a fix that is correct in isolation never gets consulted on the path that reaches users first. That is the shape worth naming: not a wrong rule, but a right rule installed downstream of the rule it was meant to replace.

Verification: 42/42 focused tests; desktop main/preload/renderer/Storybook typecheck; UI and main builds; Biome on the seven files that matter; Astryx inventory 203 files / 1 exclusion. The reproductions below were walked through the actual controller rather than reasoned about.

Candidates deliberately withdrawn rather than left in as filler: the Workbar merge resolution, routing-state rebuild on project change, conservative clarification for English which one, and the 41→42 test-count line in the PR body.

This review is AI-assisted. Under CONTRIBUTING.md §Review it does not count as the required independent human review — it means the code has been checked, not that the gate is open.

Comment threadapps/desktop/src/renderer/workhub-route-policy.ts
Comment threadapps/desktop/src/renderer/workhub-controller.ts Outdated

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed exact head dee1cf26cc5ec424d0a32f8e1e5256b33f868aa4, scoped to whether the two [P1]s from the previous round are actually closed.

Both are closed, and closed at the right layer — which is the part I checked rather than assumed, because the previous round's failure mode was a fix landing downstream of the authority that decides first.

① The two-character Latin name substring hit is gone. The fix is inside exactIdentityMatchLength itself (workhub-route-policy.ts:238-250), not in the downstream ranking: a Latin identity is now matched as a complete token sequence against the input's tokens, so AI no longer hits inside repair. The punctuation-compacting path is retained only for Han names, and the comment says exactly why. This is the shape the last round asked for.

② The correction no longer stops the target Session's root Turn unconditionally.steered now survives the whole chain: the IPC main returns it, the preload bridge contract keeps the field, workhub-session-port.ts:126 forwards it instead of discarding the disposition, and workhub-controller.ts:214 gates the stop on !input.correction.steered. The disposition also reaches the submitted result. The specific path we reported — disposition lost in the adapter, so stop() runs regardless — is unreachable at this head.


[P2] Single-character tokens are dropped, so names that differ only in a trailing digit or letter collapse into the same identity.

latinTokens (workhub-route-policy.ts:382-384) matches /[a-z0-9]{2,}/giu — the {2,} discards every one-character token. The new token-sequence comparison then runs on a token list that has silently lost the distinguishing part of the name:

"GPT-4" -> ["gpt"]
"GPT-3" -> ["gpt"]
"Project A" -> ["project"]
"Project B" -> ["project"]
"Agent X" -> ["agent"]

So with a Session named GPT-4 and no GPT-3 Session, the input 请处理 GPT-3 的问题 tokenizes to ["gpt"], matches, and exactIdentityMatchLength returns a positive length. The single exact-name branch at :112-127 then submits the request to GPT-4, reporting evidence: 'exact_session_name' — the most confident evidence value the policy has, for a name the user did not write.

This is the same defect as the original AI/repair P1, one layer down. That one was "the compaction erased the boundary"; this one is "the tokenizer erased the discriminator". Both end in the exact-name branch returning a confident match on a name the user did not name, and both are user-visible mis-routing on an ordinary path — Session names are free-form (the shared normalizer only does Unicode cleanup and an 80-code-point cap), so GPT-4 and Project A are perfectly normal names, not malformed input.

The fix wants to preserve single-character tokens in the exact-name comparison (or compare the raw alphanumeric token sequence), plus a GPT-4 vs GPT-3 regression — and, worth adding, a Project A vs Project B case, since the letter form is if anything more common than the digit form.


Not approving at this head for a separate reason:repos/apache/maka/commits/dee1cf26…/check-runs returns total_count = 0 — no hosted checks have run, so there is nothing to call green. The pending CI workflow run was in action_required; I have approved it, so checks should now execute against this head.


AI-assisted review. The token-collapse behaviour above was reproduced against this exact head, and both prior findings were re-derived from the source rather than taken as fixed. Under CONTRIBUTING.md §Review this does not count as the required independent human review.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head ba355aaeb7f3b056f0915fd84b29ff761561cc39 — required test is completed / success bound to that SHA. No P0–P3. All three findings from the previous two rounds are closed.

The tokenization [P2] is fixed, and the fix is better than what was asked for. The obvious repair was to drop {2,} from latinTokens — that would have worked here and broken something else, because latinTokens also feeds the identity-terms path, which relies on the length >= 2 filter to discard noise tokens. Instead this adds a separate exactLatinTokens (/[a-z0-9]+/giu) used only by the exact-name comparison, leaving the original function and its other caller untouched. Verified by running both against the current source:

exactLatinTokens("GPT-4") -> ["gpt","4"]
exactLatinTokens("GPT-3") -> ["gpt","3"] ← no longer collide
exactLatinTokens("Project A") -> ["project","a"]
exactLatinTokens("Project B") -> ["project","b"]
exactLatinTokens("AI") -> ["ai"]
exactLatinTokens("repair the parser") -> ["repair","the","parser"] ← round-one P1 stays closed

I also checked that admitting single-character tokens does not open a new hole: a Session literally named A normalizes to length 1 and is rejected by MIN_EXACT_SESSION_NAME_LENGTH before the token comparison is ever reached.

The two [P1]s from the round before remain closed at this head, re-derived rather than assumed:

  • The short-Latin substring match is still fixed insideexactIdentityMatchLength — Latin identities match as a complete token sequence, with the punctuation-compacting path reserved for Han names. That was the important part: the earlier fix attempt had put the repair downstream of the authority that returns first.
  • steered still survives the whole chain: IPC main returns it, the preload bridge contract keeps the field, workhub-session-port.ts:126 forwards it instead of dropping the disposition, and workhub-controller.ts:214 gates the stop on !input.correction.steered.

A note on how this PR went, because it is the useful part of the record. Round one found that a two-character Session name matched inside an unrelated word — the punctuation-compacting normalizer had erased the word boundary. The fix moved matching into the exact-name branch and compared token sequences, which was correct. But the tokenizer it introduced discarded single-character tokens, so the discriminator was erased instead of the boundary, and GPT-3 routed to GPT-4 with the policy's highest-confidence evidence value. Same defect, one layer down.

The generalizable point: when a finding is fixed by moving the decision into an earlier authority, the normalization that authority introduces becomes load-bearing and needs its own review. Relocating logic to the right layer does not mean the relocated logic is lossless. Both rounds ended in the same place — the exact-name branch returning a confident match on a name the user did not write — for two different reasons.


AI-assisted review. The tokenizer behaviour above was executed against this exact head rather than read; both prior findings were re-derived from the source. Under CONTRIBUTING.md §Review this does not count as the required independent human review — merge still needs a human committer other than the author to give LGTM and to decide.

@ARE404
ARE404force-pushed the codex/workhub-session-router branch from ba355aa to 5e8ac55CompareAugust 23, 2026 09:58
@Astro-Han
Astro-Han merged commit 8adb74b into apache:mainAug 23, 2026
1 check passed
@ARE404
ARE404 deleted the codex/workhub-session-router branch August 23, 2026 16:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ARE404@Astro-Han