Skip to content

feat(workhub): add composable routing experiments and filtered anchor rail - #4924

Merged
ARE404 merged 19 commits into
apache:mainfrom
ARE404:feat/workhub-slice6-8
Sep 7, 2026
Merged

feat(workhub): add composable routing experiments and filtered anchor rail#4924
ARE404 merged 19 commits into
apache:mainfrom
ARE404:feat/workhub-slice6-8

Conversation

@ARE404

@ARE404 ARE404 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

WorkHub routing experiments combine one independently replaceable Intent classifier and one Resolver behind the controller's fixed Action Policy and unchanged Host Action Gate. R2.4 uses deterministic components, R3-A uses model-assisted intent and recall, and R3-B uses model-assisted intent with deterministic recall. Production remains on R2.4.

Deterministic components preserve full request text. Model adapters cap text at 2,000 characters at the call boundary; a lossy prefix can no longer veto the default baseline's full-text executable intent. Every arm shares the bounded recall context and the full trusted Policy snapshot, including exact-name targets outside the 12-candidate model window.

The Session navigation list remains alongside separate conversation prompt anchors. Session rows open Sessions; prompt ticks navigate messages in the current WorkHub transcript. Slice 6 and Slice 8 remain together.

Refs #3492

Review fixes

  • Added a controller regression for an execution instruction after 2,000 characters of background/logs; it failed as discussion before the fix and now creates the intended task.
  • Removed the parallel Policy assembly from combination tests. The shared fixture runs the real controller; component tests retain bounds and malformed-schema coverage.
  • Replaced the unused production experiment shell with a reusable comparison fixture that explicitly hydrates Sessions, Coordination transcript and candidate-set state for fresh controllers. Tests inspect the model transcript and actual proposals across all three arms and repetitions. This is repeatable comparison, not Slice 7 metrics or winner selection.
  • Removed the obsolete simulated-validation report, duplicate Rail types, configurable anchor limit, reason wrappers and unused CSS. The Session list retains the fixed eight-item cap, ordering, filtering, deduplication and focus display.
  • Retained exact-name-outside-window, frozen-focus, malformed-output and keyboard coverage; added streaming growth while reading an earlier prompt.

Verification

  • Full build and workspace typecheck passed.
  • Desktop suite: 2,366 passed, 0 failed.
  • Eight browser runs: Session Rail and conversation anchors, wide/narrow × light/dark. Click/Enter navigation, filtering, scroll selection and streamed transcript growth passed.
  • Lint, format, Desktop/UI Knip, ASF headers, locale hygiene, Astryx inventory, Electron test budget and incremental renderer architecture checks passed.
  • CI for this review-fix update is pending. Prior head 39e1bb116 was green.

Controller tests use a stubbed coordination.act boundary; they do not prove real Host admission. Browser checks are author verification, not independent reviewer acceptance.

Conversation prompt anchor evidence

The production WorkHubSurface fixture shows fourteen ticks and the seventh prompt selected with its preview open.

Wide, light Narrow, dark
Conversation anchors, wide Conversation anchors, narrow

Session navigation visual evidence

Screenshots are from the production WorkHubSurface Storybook fixture, after its interaction checks. The wide viewport is 1280 × 900; narrow is 720 × 900.

View Before After
Wide, light Before After
Narrow, dark Before After

The navigation component feedback is implemented. The suggested PR split is not adopted; Slice 6 and Slice 8 remain in this PR. Reviewer re-review is pending.

AI use

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

Codex implemented and revised the routing components, navigation, tests and documentation. Affected commits carry Generated-by: Codex; retain it in the squash commit.

Checklist

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

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Main integration validation (24bb315)

Merged main at 7779d69 and resolved the WorkHub surface conflict, preserving Session navigation, conversation prompt anchors, and main's removal of compact chat density. Regenerated the surface inventory and refreshed the after screenshots above.

  • Build, workspace typecheck, lint, formatting, renderer architecture, locale hygiene, ASF headers, surface inventory and E2E budget checks passed.
  • Clean desktop suite: 2390/2390 passed. An initial Rive child-start timeout passed both an isolated retry and the final full-suite rerun.
  • Eight browser runs passed across wide/narrow and light/dark: Session navigation, prompt selection, and streaming scroll stability. Measured bubble and composer radii both 28px in all four prompt variants.
  • Hosted checks for this integration commit must be assessed separately from the approval on 61a8b30.

Move new routing and navigation ownership behind the WorkHub feature API, preserving the legacy compatibility seam without growing AppShell debt.

Generated-by: Codex
Adapt Slice 8 to the rebased conversation projection and update the Action Policy module link.

Generated-by: Codex
@github-actions github-actions Bot added the effort/XL Under 2500 readable lines label Sep 6, 2026
@ARE404
ARE404 marked this pull request as ready for review September 6, 2026 15:59
Keep the shared resume/stop admission flow and delegate named resume resolution through each routing strategy. Cover all three strategy IDs through resume admission.

Generated-by: Codex
The new Electron budget correctly rejected the renderer-only Rail test retained during the main merge. Exercise filtering and wide/narrow geometry in Storybook play functions, leaving Host startup recovery in Electron.

Generated-by: Codex
Regenerate the renderer surface inventory after adding WorkHubNavigationRail so the CI coverage gate includes the new surface.

Generated-by: Codex

@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 for rebasing this onto the current assignment projection and for keeping writes on coordination.act.

Reviewed head f99648e3ad335f4c6de61928b4cdba98a74917b6. The default R2.4 path still fails closed through the Action Gate: models never see Session IDs, R3-B does not execute an R2.4 create_new, resume/stop still go through the baseline policy plus Gate, and the rail is a rebuildable projection. Those write-side invariants hold. app-shell does not inject a model strategy, so production routing today is still R2.4.

The issue that should be settled before the rest of the implementation is reviewed is the proposal owner.

After the 2026-09-02 update on Discussion #3286 and the merged map in #4819, routing experiments are replaceable Action Intent and Session Resolver implementations behind the existing Action Policy and Action Gate. This PR implements the older Slice 6 wording on #3492 — a standing WorkHubRoutingStrategy whose R3-A resolve() chooses disposition and target in one step and skips Policy.resolve() (ambiguous-command, linked correction, exact-name / related / focus ranking). The ADR and domain-language additions then record that parallel owner as if it were the accepted architecture.

That is the same class of design problem as a second write path: merging it freezes the stale experiment shape in the accepted ADR. The smallest shape that still meets Slice 6 is model-assisted intent and/or model-ranked recall, deterministic policy, unchanged gate. Please either reshape to that, or take the old “swap the whole resolver” experiment back to #3492 / Discussion #3286 before amending the ADR.

Slice 8 (filters + rail) is a separate loop and should not ride along with the routing experiment. The projection helper is fine; the custom Button rail is called out inline.

I am not reviewing the rest of the implementation until the proposal pipeline matches the agreed map, or the tracker is explicitly updated.

Drafted with AI assistance. I verified the head, the default production path, Discussion #3286 / #4819 / #3492, and the cited lines against the PR files.

Comment thread apps/desktop/src/renderer/features/workhub/model/routing-strategy.ts Outdated
Comment thread apps/desktop/src/renderer/features/workhub/model/routing-strategy.ts Outdated
Comment thread apps/desktop/src/renderer/workhub-controller.ts Outdated
Comment thread apps/desktop/src/main/__tests__/workhub-controller.test.ts Outdated
Comment thread apps/desktop/src/renderer/features/workhub/ui/workhub-navigation-rail.tsx Outdated

@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.

I independently reviewed this PR at head f99648e3 (21 files, +2135/−59) against base 8bb23a3b, and verified the design concern raised in the earlier review against the agreed architecture records.

What the change does: adds a frozen-fixture routing experiment runner (R2.4 / R3-A / R3-B) behind the WorkHub controller, plus a filterable Anchor Rail (max 8 Sessions, focus → delegated → recent priority) and an injectable routing-strategy seam, with ADR and domain-language amendments.

Design-layer finding (gates the implementation review):

  • The agreed architecture (Discussion #3286, 2026-09-02 update; mirrored by the merged stage map in #4819) expresses routing experiments as replaceable components: model-assisted intent classification, a model-ranked Session Resolver that returns ranked candidates only (never create_new, no execution authority), behind the deterministic per-action policy and the unchanged Action Gate.
  • This PR implements a standing WorkHubRoutingStrategy whose R3-A resolve() picks disposition and target in one model step and returns the target directly, skipping Policy.resolve() entirely — no ambiguous-command handling, no linked correction, no exact-name/related/focus ranking for the delegation decision (routing-strategy.ts, R3-A branch calls no baseline.resolve). R3-A also decides create_new/clarify itself: it replaces the policy rather than resolving behind it.
  • The PR amends the accepted Coordination Session ADR (docs/architecture/workhub-coordination-session-adr.md, +8 lines) to record this versioned-strategy shape as accepted architecture. Merging would freeze the stale experiment shape into the architecture documents, conflicting with the merged #4819 stage map.

Production impact: app-shell does not inject a model strategy, so production routing remains R2.4 today; this is an architecture-direction issue, not a runtime regression. The implementation details under it (input bounding, strict response validation, fail-closed paths, R3-B determinism) are well-built, but per the review order the design layer gates them.

Slice 8 (filters + rail) is orthogonal and fine: deriveWorkHubAnchors is a pure, rebuildable projection (no routing authority, does not mutate the candidate set) — it can be split into its own PR.

Suggested direction: reshape to the agreed minimal form — model-assisted intent and/or model-ranked recall as a resolver component, deterministic policy, unchanged gate — or explicitly update the tracker (#3492 / Discussion #3286) to accept the whole-resolver experiment shape before amending the ADR. The rail portion can proceed separately.

Not verified: I did not re-read #3492's full Slice-6 wording line by line (Discussion #3286's 09-02 update and #4819 sufficiently establish the agreed shape); the real model adapter (prompt construction / structured-output parsing) is not in this PR — only the port and test mocks.

PR body note: the verification section claims a packages/ui build failure and that typecheck/E2E are unclaimed, but CI on this exact head shows Build, Typecheck, affected workspace tests and Desktop e2e all passing — the body predates the final rebase and should be updated.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

Replace the proposal-owning strategy with independent Intent and Resolver ports. Keep trusted routing constraints, focus and proposal construction in the shared Action Policy and retain Host Gate admission. Validate bounded model evidence and clarify controller fixture test claims.

Generated-by: Codex
@ARE404 ARE404 changed the title feat(workhub): add routing strategy experiments and filtered anchor rail feat(workhub): add composable routing experiments and filtered anchor rail Sep 7, 2026

@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 for reshaping the experiment around independent Intent and Resolver components and the fixed Policy.

Re-reviewed head 39e1bb116e9a51ace58afb0f28140d19fdbe6162 (+1946/-62, 29 files). CI is green on this head. The previous proposal-owner P1 is resolved: the controller owns Policy/focus, strategies only provide evidence, and the ADR reflects that boundary. The shared candidate context, fixture-test naming, and Astryx ListItem/StatusDot feedback are also addressed. I am no longer treating a separate Slice 8 PR as a requirement.

There is one remaining P2 regression and a few P3 simplifications, detailed inline. This is not a new complexity P1 based on line count alone.

Smallest cleanup before landing

  • Remove the 116-line historical simulated-validation report. It explicitly describes the superseded strategy and makes obsolete verification claims; the PR body and Git history can retain the historical evidence.
  • Share the Rail copy/filter types instead of defining the same structures in the locale and navigation modules. Remove the unused .workhub-anchor-content rule.
  • Remove the configurable anchor limit (only a test passes it) and retain the fixed eight-item cap. The projection can return ordered Sessions directly: its consumer only uses reason to recognize focus, which can be derived from the existing focus Session ID. A scratch ablation over 192 cases preserved ordering, filtering, deduplication, the cap, and displayed focus. This is projection evidence, not browser acceptance.
  • Keep component tests for bounds/schema validation, but exercise routing combinations through the real controller rather than rebuilding its assembly in a test helper. Retain outside-window exact-name, frozen-focus, malformed-model-output, and keyboard-navigation coverage. Fold duplicate projection assertions into the ordering/cap tests, with actual duplicate IDs in the input.

The experiment runner deserves a separate simplification pass within this PR: its only caller is a test, and that caller does not hydrate the supplied transcript/runtime state into its controller. Prefer an explicit reusable experiment/controller fixture over a generic shell whose contract leaves that wiring implicit. Do not remove repeatable comparison itself; that remains part of Slice 6.

The production/test-and-Storybook/docs split is +1053/-41, +722/-6, and +171/-15. There is clear removable scaffolding, but I have not established that half the PR can be removed while preserving its acceptance.

Please fix the long-input regression and make the concrete cleanup above. I have not independently completed wide/narrow, light/dark, keyboard and streaming-scroll acceptance for the new rails; the author's reported runs and green CI do not substitute for that remaining check.

Automated review notice: this review was prepared and posted by a Codex agent at Astro-Han's request. The source comparison and scratch reproductions were performed by the agent; this is not an independent human review.

Comment thread apps/desktop/src/renderer/features/workhub/model/route-policy.ts
Comment thread apps/desktop/src/main/__tests__/workhub-routing-strategy.test.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Contributor

I think we need to revisit the WorkHub boundary before extending this approach further. Handwritten natural-language enumeration should not become the contract for orchestration or action admission. Adding more synonyms, negation patterns, and correction phrases turns conversation into an implicit command grammar.

The Coordination Session should interpret context and propose structured actions. The Host should validate permissions, target identity, lifecycle, delegation ownership, idempotency, and any required user confirmation. A model’s assertion is not authorization; confirmation should bind to the concrete action and target.

Please stop expanding the phrase-matching machinery and propose a migration toward this boundary, with behavior-level evaluation and removal of the superseded rules. This concern applies to the existing WorkHub architecture, not only the changes in #4924.

AI assistance disclosure: drafted and posted with Codex assistance; Astro-Han explicitly reviewed and approved the architectural position above.

@Astro-Han

Copy link
Copy Markdown
Contributor

Following our discussion, I want to consolidate the direction here and clarify my earlier review. My concern is broader than the individual matching bugs or the size of this PR.

WorkHub should be a Coordination Session that uses an LLM to orchestrate other Sessions. Handwritten semantic enumeration should not become its long-term decision mechanism.

Trying to enumerate synonyms, negations, corrections, references, and sentence patterns cannot match the language capability we are using an LLM for. Each missed expression becomes another rule, while the system remains brittle to context and phrasing. The observed cases—“请停止 Payments” taking the stop path while “请把 Payments 停掉” takes the delegation path, or a model's discussion classification being overridden by a name match—are symptoms of that boundary, not requests to add more patterns. These reproductions establish controller proposals, not real Host execution.

The direction I propose is:

  • The LLM owns semantic interpretation and orchestration decisions. Give the Coordination Session clear instructions, bounded access to relevant Session context, and structured tools. Let it decide whether to answer, clarify, delegate, create, stop, or resume, and identify the appropriate targets and ordering.
  • Runtime owns execution constraints. Validate structured arguments, target identity, permissions, lifecycle, delegation ownership, idempotency, concurrency, and any required confirmation. Confirmation must bind to the concrete action and target. Runtime should not then reinterpret the user's original sentence through a phrase matcher to decide whether the LLM understood it correctly. A model's claim of authorization remains insufficient.
  • Eval measures semantic and orchestration quality. Build scenarios with 1–5 Sessions, different states, histories and delegation relationships, and realistic single-turn and multi-turn requests. Compare models on actual tool calls, targets, ordering and resulting state, including unwanted actions and unnecessary clarification. Failures should drive improvements to prompts, tools, context and model selection, rather than expansion of a Runtime language grammar.

The practical next step is a programmatic entry into the same WorkHub/Host path, usable by the existing maka eval infrastructure. A stronger model can help design cases; inexpensive models can then be evaluated on the same fixtures and constraints. The experiment should improve the LLM-driven approach and establish its behavior, rather than turn the hand-built language parser into a permanent competing orchestrator.

My earlier review concentrated on keeping the proposal path behind a fixed Policy and Gate. That was necessary for the boundary under review, but too narrow as a statement of the product direction: a fixed execution boundary does not require freezing the heuristic language rules into Policy. The old deterministic route can remain temporarily as a comparison or migration baseline, with an explicit retirement path.

I am comfortable shipping a small first slice and improving model quality from evidence. I do not want the temporary semantic-enumeration approach recorded as the intended long-term WorkHub architecture. Please use this as the direction for the next revision, with the transition and removable rules made explicit. The existing architecture is part of this discussion; this is not all new debt introduced by #4924.

AI assistance disclosure: consolidated and posted with Codex assistance at Astro-Han's explicit request, reflecting the direction discussed and approved by Astro-Han.

Bound text only at model adapters so deterministic intent retains instructions after long context. Reuse the real controller fixture for combinations and repeatable comparisons, simplify Session anchors, and remove obsolete validation scaffolding.

Generated-by: Codex
@ARE404

ARE404 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for clarifying this direction. I agree: a fixed execution boundary does not mean freezing handwritten language rules into Policy. The Coordination Session should use the LLM for semantic interpretation and orchestration, while Runtime validates structured actions, target identity, permissions, lifecycle, delegation ownership, idempotency, concurrency, and any required confirmation. A model's assertion alone is not authorization.

In the follow-up work, we will evaluate multiple intent-recognition and routing strategies, including different models and component combinations, against the same WorkHub/Host execution path and fixtures. We will work toward a programmatic entry usable by maka eval, covering realistic single-turn and multi-turn scenarios with different Session states and delegation relationships. The comparison will measure actual tool calls, targets, ordering, resulting state, unwanted actions, and unnecessary clarification.

The deterministic route will remain a temporary comparison and migration baseline, with an explicit retirement path—not the intended long-term orchestration mechanism. Evaluation failures should guide improvements to prompts, tools, context, and model selection rather than continued expansion of phrase-matching rules.

AI assistance disclosure: drafted and posted with Codex assistance at the PR author's request.

Astro-Han

This comment was marked as duplicate.

@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 for addressing the review feedback. Approving the reviewed head 61a8b306887a28f273f41ed3473fd8a899937137 for this incremental scope.

Astro-Han has explicitly accepted deferring the broader LLM-driven orchestration, removal of heuristic semantic rules, and production-path maka eval work. Those remain acknowledged follow-ups; this approval does not claim they are implemented. The seven original inline findings are resolved, the affected tests pass (113/113), and the independent browser checks covered the rails across wide/narrow and light/dark layouts.

Before merging, resolve the workhub-surface.tsx conflict while preserving main’s removal of compact chat density, verify the intended 28 px bubble radius and rail/scroll behavior on the resulting head, and obtain its checks. This head currently has no check rollup; approval of this head does not validate future conflict-resolution changes.

AI assistance disclosure: Codex agents performed the source, test and browser verification. Astro-Han explicitly accepted the deferred design scope and authorized this approval.

中文

谢谢处理反馈。本次批准已审查的 head 和当前增量范围。Astro-Han 已明确接受将 LLM 编排、手写语义规则退出及生产路径 maka eval 留到后续;批准不代表这些目标已经实现。7 条旧意见已解决,113 个相关测试通过,浏览器检查覆盖宽窄屏和 light/dark 的导航界面。

合并前仍需解决 workhub-surface.tsx 冲突,保留 main 去掉 compact density 的修复,并在最终 head 确认 28px 气泡、导航及滚动行为,取得最新检查结果。当前批准不替代解冲后的验证。本次由 Codex 代理执行审查,Astro-Han 明确接受后续范围并授权批准。

Keep the Session navigation and conversation prompt anchors while retaining main’s removal of compact chat density. Refresh browser evidence with matching 28px bubble and composer radii.

Generated-by: Codex
@ARE404
ARE404 merged commit 8c8ad27 into apache:main Sep 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants