Skip to content

docs(architecture): map WorkHub decisions and authority - #4819

Merged
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:docs/workhub-decision-pipeline-architecture
Sep 6, 2026
Merged

docs(architecture): map WorkHub decisions and authority#4819
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:docs/workhub-decision-pipeline-architecture

Conversation

@ARE404

@ARE404 ARE404 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add the current WorkHub stage-to-module map beside the authoritative domain terms
  • map Action Intent and Session Resolver to their shared Core modules
  • map Action Policy, Action Proposal, Action Gate, and Projection to their current implementation modules

Ablation result

  • reduced the PR from 4 files and 106 added lines to 1 file and 11 added lines
  • deleted the parallel architecture document because its invariants and stage descriptions duplicated the accepted Coordination Session ADR and WorkHub domain language
  • removed the documentation-index and ADR cross-links that existed only for that deleted document
  • retained only the unique implementation mapping

Corrections incorporated

  • Action Intent and Session Resolver are mapped to packages/core, not the renderer
  • the inaccurate blanket statement about proposal target identities was removed; the protocol remains authoritative for action-specific preconditions

Validation

  • verified all mapped source files exist at the PR head
  • verified no references to the deleted architecture document remain
  • git diff --check

This PR is documentation-only.

@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 doing the ablation before opening this; 249 → 101 is the right instinct, and I checked every row of the module map against main: all six files exist, the resolver really has no create_new (workhub-session-resolver.ts:83-92), creation really is decided in workhub-route-policy.ts:48-61, and the Gate really re-reads Host state through listSessions / claimAction. But the ablation needs one more pass, because what is left is still a third copy of two existing authorities.

Line by line: invariants 1–5 are the ADR at workhub-coordination-session-adr.md:42, :67-69, :89-90 and :194-202; the gate-semantics paragraph is :58-61 and :89-96; the closing "not designed here" sentence is :53-56. The five stage names and their one-line responsibilities are the definitions at workhub-domain-language.md:70-105, including "never returns create_new" (:76) and "creation is a policy decision" (:84); the canonical flow is those five entries read in order. docs/README.md:110 says it directly: prefer updating an existing authority over adding a parallel document. A file that has to stay in sync with the ADR, the domain language and the source at once raises the maintenance surface without adding a fact.

The one thing here that is not written anywhere else is the term → file column, and that is worth keeping. Proposal: drop the new file and add that table as a short section after the terms in workhub-domain-language.md, one line per stage with the path. That keeps the mapping next to the definitions it maps, and the README / ADR link lines become unnecessary.

Two corrections to carry into the table:

  • The diagram puts Action Intent and Session Resolver inside subgraph Renderer. Both live in packages/core, the domain language calls the resolver "shared" (:74), and the Gate itself imports the intent module on the Host side (workhub-coordination-action-gate.ts:42-45) to re-read creation and correction intent. For a document whose purpose is a source map, that is a wrong mapping.
  • "Existing targets use opaque candidateRef values" is true for delegate_existing and replace, but stop_work carries a real targetSessionId in its preconditions (protocol/workhub-coordination.ts:151-158). Say "delegate and replace use candidateRef; stop names the resolved Session".

Also, if the file were to stay, it self-describes as an implementation map but is listed under ## Current contracts in the README.

Evidence boundary: static read against main; I did not render the mermaid or read the full 1254-line Gate, only its effects interface and imports.

AI-assisted review: drafted with Maka; I verified the README rule, the ADR overlap, the Host-side intent import and the stop preconditions myself.

@ARE404

ARE404 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Agreed. I ran the deletion test again and folded the only unique content into the existing authority.

The parallel architecture document and its README / ADR / domain-language cross-links are gone. The PR now adds only a six-row Stage → Current module table after the terms in workhub-domain-language.md: 4 files / +106 lines became 1 file / +11 lines.

Both corrections are incorporated: Action Intent and Session Resolver map to packages/core, and the inaccurate blanket statement about proposal target identities was removed rather than copied into the table. The action-specific protocol remains authoritative for candidateRef versus targetSessionId preconditions.

All six mapped files exist at head 69c24a1f1, no references to the deleted document remain, and git diff --check passes. Thanks for identifying the remaining duplicate authority.

@hqhq1025 hqhq1025 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 69c24a1f1fafd51c6a1b437ef99cac256b7eac23 against current main 12ebc45e6fe43b2eebd7dfe61f5c1fa92c0c873a. The ablation is a substantial improvement: the parallel architecture document and its temporary cross-links cancel out of the final diff, leaving only an 11-line implementation map in the existing domain-language authority.

I found one P2 in that remaining map. The Projection row points only to the Coordination Session projection adapter, while the document's own definition and the accepted ADR cover projections derived from both Coordination and ordinary Session facts. The ordinary Session summaries, recent turns, and delegated execution feedback are implemented in a separate module, so the current row is incomplete. Details are inline.

Validation: all six linked files exist; the removed architecture document has no remaining repository references; git diff --check, the ASF header audit, the exact-head hosted test check, and a clean synthetic merge with current main passed. The synthetic merge preserves the PR patch exactly. Markdown is excluded by the repository's Biome configuration, so no formatter check applies to this file.

Unable to determine: this documentation-only review did not execute the complete WorkHub behavioral test matrix; implementation claims were checked by reading the mapped modules and their composition paths.

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

Comment thread docs/workhub-domain-language.md 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.

Re-reviewed at a95b384 against main (70d1516a0b). Eleven lines, one file, and every one of them is a fact that exists nowhere else: the six stage → module rows sit directly under the term definitions they map, all seven linked paths exist on main, Action Intent and Session Resolver now point at packages/core, and the Projection row names both owners, workhub-coordination-port.ts for coordination turns and active delegations and workhub-session-port.ts for ordinary Session facts and delegation feedback, which is what the term above it says a projection is built from. Leaving the controller out is right; it composes those two and owns no projection of its own. The parallel architecture document, the README index line and the ADR cross-links are gone, so nothing has to be kept in sync with the ADR any more.

Nothing to fix. One thing to know: there is no check in the repo that relative links under docs/ resolve, so a rename will silently break a row until someone reads the page. Not worth a script for six rows.

Evidence boundary: static read; verified the seven paths and the projection functions named in the inline thread against main.

AI-assisted review: drafted with Maka; I verified the paths and the projection split myself.

@Astro-Han
Astro-Han merged commit 2bfea3c into apache:main Sep 6, 2026
1 check passed
@ARE404
ARE404 deleted the docs/workhub-decision-pipeline-architecture branch September 6, 2026 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants