Skip to content

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

Open
ARE404 wants to merge 14 commits into
apache:mainfrom
ARE404:feat/workhub-slice6-8
Open

feat(workhub): add routing strategy experiments and filtered anchor rail#4924
ARE404 wants to merge 14 commits into
apache:mainfrom
ARE404:feat/workhub-slice6-8

Conversation

@ARE404

@ARE404 ARE404 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

WorkHub needs a repeatable way to compare routing strategies and navigate relevant Sessions. This adds a frozen-fixture experiment runner for R2.4, R3-A and R3-B behind the same controller and Action Gate, plus a filterable Anchor Rail capped at eight Sessions, prioritized by focus, active assignments and recency.

Rebased onto 93224038a; Rail linkage is derived from the current durable assignment projection. Slice 7 metrics and winner selection remain outside this PR.

Refs #3492

Verification

After rebase and compatibility fixes:

  • Desktop main compilation and renderer TypeScript check passed.
  • WorkHub tests: 140 passed, 0 failed.
  • Repository lint, format check and git diff --check passed.
  • Renderer architecture fixture tests: 101 passed; incremental check against upstream/main passed.
  • Workspace dependency build failed in packages/ui on Astryx component prop types. Full workspace typecheck/build and Electron E2E are not claimed as passing.

The earlier simulated validation is recorded in the validation report; its commit, baseline and PR-status observations are historical, not post-rebase results.

Before requesting review

  • Attach before/after screenshots or a recording of the Rail and filters.
  • Verify Session entry/return and cross-Workspace navigation in an isolated Electron profile.
  • Complete full typecheck/build validation after resolving the UI dependency build failure, and confirm CI on this head.

AI use

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

Tool(s) and scope: Codex implemented routing strategies, Rail UI, tests and documentation, resolved rebase compatibility, and prepared this PR. Affected commits include Generated-by: Codex; retain it in the squash commit.

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

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.

};
}

if (mayChooseCandidate) {

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.

R3-A maps the model response straight onto a WorkHubRouteDecision here (answer_here / clarify / create_new just above, then delegate_existing via candidateRef). On main, ordinary routing still goes through Policy.resolve() for ambiguous-command, linked correction, exact-name / related / focus ranking, and implicit create.

A model-assisted experiment can replace Intent classification and Session ranking. It should not replace that policy combination step.

Smallest fix: have the model emit an Intent and/or a ranked candidate, then call the existing policy (with create_new still gated on explicit+imperative trusted text).

// baseline `new_session` result is deliberately not executable here.
const baselineDecision = baseline?.resolve({
text: routeInput.text,
sessions: [...routeInput.sessions],

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.

R3-B asks the model against boundedInput (max 12 candidates) but then runs R2.4 against routeInput.sessions (the full routable list). Slice 6 asked every arm to see the same bounded snapshot.

A name that falls outside the 12-candidate window can succeed on R3-B / R2.4 and fail-closed on R3-A, so the comparison is not on one fixture. Pass the same bounded session list into baseline.resolve().

}): WorkHubController {
const { coordination } = deps;
let routePolicy = createWorkHubRoutePolicy();
let routePolicy = deps.routingStrategy ?? createWorkHubR24RoutingStrategy();

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.

This is the production seam that turns WorkHubRoutingStrategy into the controller’s routing owner. Default is still the R2.4 wrapper, so today’s writes are unchanged — but every later injection now skips createWorkHubRoutePolicy() entirely.

If the experiment is reshaped to replaceable Intent / Resolver ports, this dependency should stay on WorkHubRoutePolicy (or those two ports), not on a parallel resolve() owner.

assert.equal((await controller.read()).focusSessionId, 'payment');
});

test('an injected R3 strategy still delegates through the shared controller and Action Gate', async () => {

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.

This test (and the experiment shell below) is named as going through the Action Gate, but createWorkHubController in this file still admits via the in-memory act stub, not workhub-coordination-action-gate.ts.

That proves the controller calls coordination.act(). It does not prove Host admission of an R3 proposal. Please either send one R3 proposal through the real Gate, or drop “Action Gate” from the test name.

? props.copy.archived
: props.copy.states[anchor.session.state];
return (
<Button

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.

deriveWorkHubAnchors is a rebuildable projection and onOpenSession only navigates — no new execution authority.

The surface itself is a second navigation idiom: Session entry is navigation, so this should be a Link (or Astryx List / Item with StatusDot), not a Button plus a custom state span. Slice 8 is also a separate review loop from the routing experiment; it should not land in the same PR as the strategy interface.

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

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