Skip to content

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

Open
ARE404 wants to merge 5 commits into
apache:mainfrom
ARE404:codex/workhub-session-router
Open

feat(desktop): add conservative WorkHub session router#3497
ARE404 wants to merge 5 commits into
apache:mainfrom
ARE404:codex/workhub-session-router

Conversation

@ARE404

@ARE404 ARE404 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:

Policy Main choice Runtime / authority shape Same-snapshot result
R1 Aggressive routing with broad contextual carry-over Experimental runtime 10/11
R2 Conservative routing with more clarification Experimental WorkHub routing memory 9/11 routing; 8/11 strict E2E
R2.1 Evidence scoring tuned to the idealized cases Experimental WorkHub routing memory 11/11 idealized
R2.2 Unified-conversation/projection-first policy Experimental runtime 5/11
R2.3 Session-authoritative hybrid adopted here Production Session facts plus transient focus/correction memory 8/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-Han Astro-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.

);
}

function looksLikeTargetUncertainty(value: string): boolean {

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.

[P1] Several routing signals have no English path, and the ones that do are tuned on Chinese.

The description says the policies were evaluated against the same 10-Session snapshot and Chinese acceptance guide. That shows through in the implementation in ways that leave English input on a different code path rather than a slightly worse one:

  • looksLikeTargetUncertainty is Chinese-only. There is no English alternative in the pattern at all. An English user typing "I'm not sure which one this belongs to" never reaches the clarification branch — it falls through to correction/focus/entity matching and gets routed somewhere with no prompt. This is the one signal whose entire job is to stop the router from guessing, and English input cannot trigger it.
  • The boilerplate stripping in routingTerms is Chinese-only (只分析风险和测试点, 不要修改任何文件, 先不动代码). The English equivalents — "just analyse the risk", "don't change any files", "don't touch the code yet" — stay in the term set and score against session identities.
  • ROUTING_STOP_TERMS is 18 Chinese terms and 4 English ones (please, continue, work, task). Common English routing noise — the, this, file, issue, check, update, fix, code, test — is absent, so those words all contribute to score and to longestMatch.
  • workHubNewSessionName only extracts a title from Chinese markers (标题|名称|名字), and only strips Chinese creation prefixes. create a new session called Parser Cleanup produces a session named from the first 48 characters of the whole sentence rather than Parser Cleanup — even though looksLikeExplicitNewSession does have an English branch, so English users reach this function and then get the degraded name.
  • looksExecutable's design-question veto includes \bwhat\b, so "fix what is broken in the parser" is classified as a design question and routed to discussion instead of being executable.

This is ordinary-user-path reachable, and the failure is silent: the message goes to the wrong Session, or to discussion, with no signal that routing was uncertain.

We are not asking for full parity in this PR — an MVP is allowed to be narrow. What we would ask for is that the narrowness be declared rather than latent. Either scope the feature explicitly (the preference could state that routing is currently tuned for Chinese input), or add the English counterpart to looksLikeTargetUncertainty specifically, since that one is the safety valve: when the router cannot tell, asking is always the correct fallback, and right now English users cannot reach it.

right.score - left.score || right.session.updatedAt - left.session.updatedAt);
}

function routingTerms(value: string): string[] {

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.

[P1, same finding] The scoring layer treats Chinese and English asymmetrically, and then applies one threshold to both.

routingTerms builds terms two different ways:

  • Chinese: every 2-to-6 character n-gram of each run, fully expanded. A 6-character phrase yields 15 terms, so longestMatch for a genuine phrase match is naturally 4–6.
  • English: [a-z0-9_./-]{2,} — whole tokens only, no n-grams. longestMatch is just the length of the longest matching word.

Then rankRelatedSessions matches with identity.includes(term) against a normalizeIdentityText string that has had all whitespace and punctuation removed. For Chinese n-grams that is fine, since the terms are already substrings by construction. For English it means every term is matched as an unanchored substring of a concatenated blob, so word boundaries are gone.

Concrete failure: a Session named profile-settings normalizes to profilesettings. A user types "check the file parser". file survives (not a stop term), identity.includes('file') is true via profilesettings, giving score = 4 and longestMatch = 4. If no other session matches at 4, this hits the related[0].longestMatch >= 4 && (related[1]?.longestMatch ?? 0) < 4 branch and routes directly with evidence core_entity — no clarification. The word file had nothing to do with that session.

The same >= 4 threshold means "one meaningful Chinese phrase" and "any four-letter English word" — test, data, main, code, file, user. That asymmetry is why English input is more likely to route confidently and more likely to route wrong.

Two changes would address the mechanism rather than the symptom: match Latin terms on word boundaries instead of raw includes (keeping the concatenated form only for the Chinese n-gram path), and give the Latin path its own threshold rather than sharing the Chinese one. Both are local to this function and rankRelatedSessions.

A regression case worth pinning either way: the profile-settings / "file" shape above, asserting it yields clarification rather than a confident target.

ARE404 added 4 commits August 23, 2026 02:36
Preserve WorkHub availability semantics across the extracted Workbar controller.

Generated-by: Codex
Sign up for free to 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