refactor(workflow): S12 dashboard API CLI workflow projection#1585
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s11-branch-group-subgraphs
branch
from
c807552 to
d117765
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s12-workflow-projections
branch
from
cc3566e to
5de2ea1
Compare
Greptile SummaryAdds a new
Confidence Score: 4/5Safe to merge as a draft handoff; the dispatch bug affects a specific combination that is unlikely to be exercised until dashboard/API/CLI consumers are wired up in later slices. The dispatch function does not branch on packages/core/src/workflow-work-projection.ts — the sort comparator and dispatch logic both need an explicit branch for Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[projectWorkflowWorkStatus] --> B{sort workItems by priority}
B --> C{find first non-succeeded non-cancelled}
C -->|no active item| D{any succeeded items?}
D -->|yes| E[status: complete]
D -->|no| F[status: legacy]
C -->|active item found| G{kind === recovery?}
G -->|yes| H[status: recovery]
G -->|no| I{kind === manual-hold or state === manual-required/held?}
I -->|yes| J[status: manual-hold]
I -->|no| K{state === retrying?}
K -->|yes| L[status: retrying]
K -->|no| M{state === running?}
M -->|yes| N[status: merge-running]
M -->|no| O{state === failed/exhausted?}
O -->|yes| P[status: failed]
O -->|no| Q[status: merge-queued - catch-all also hits retry-kind runnable items]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[projectWorkflowWorkStatus] --> B{sort workItems by priority}
B --> C{find first non-succeeded non-cancelled}
C -->|no active item| D{any succeeded items?}
D -->|yes| E[status: complete]
D -->|no| F[status: legacy]
C -->|active item found| G{kind === recovery?}
G -->|yes| H[status: recovery]
G -->|no| I{kind === manual-hold or state === manual-required/held?}
I -->|yes| J[status: manual-hold]
I -->|no| K{state === retrying?}
K -->|yes| L[status: retrying]
K -->|no| M{state === running?}
M -->|yes| N[status: merge-running]
M -->|no| O{state === failed/exhausted?}
O -->|yes| P[status: failed]
O -->|no| Q[status: merge-queued - catch-all also hits retry-kind runnable items]
Reviews (8): Last reviewed commit: "fix(FN-000): address PR review feedback ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s11-branch-group-subgraphs
branch
from
d117765 to
57dca58
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s12-workflow-projections
branch
from
5de2ea1 to
84b5065
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s11-branch-group-subgraphs
branch
from
57dca58 to
9f59559
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s12-workflow-projections
branch
from
84b5065 to
b98d27d
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s11-branch-group-subgraphs
branch
from
9f59559 to
8df8f05
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s12-workflow-projections
branch
from
b98d27d to
b82bea7
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s11-branch-group-subgraphs
branch
from
8df8f05 to
3fcc5af
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s12-workflow-projections
branch
2 times, most recently
from
3e77551 to
05ae169
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s11-branch-group-subgraphs
branch
from
3fcc5af to
2734b50
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s12-workflow-projections
branch
from
05ae169 to
84619cb
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s11-branch-group-subgraphs
branch
2 times, most recently
from
18b620e to
e42a519
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s12-workflow-projections
branch
from
84619cb to
f284530
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s11-branch-group-subgraphs
branch
from
e42a519 to
d0ae3da
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s12-workflow-projections
branch
2 times, most recently
from
80c1d22 to
9b7c73a
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s11-branch-group-subgraphs
branch
from
d0ae3da to
2a66eff
Compare
Fusion-Task-Id: FN-000
Address PR #1585 feedback by keeping manual-hold projection dispatch in the same priority order as work-item selection.
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s11-branch-group-subgraphs
branch
from
2a66eff to
dd36403
Compare
gsxdsm
force-pushed
the
feature/workflow-owned-merge-s12-workflow-projections
branch
from
9b7c73a to
dd47771
Compare
Addressed: projection dispatch now honors manual-hold priority before retrying state, matching the sort order and preventing stale retry display from shadowing manual holds. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/workflow-work-projection.ts (1)
1-85:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd required FNXC_LOG documentation comments for this new projection logic.
This file introduces user-facing workflow-status projection behavior, but no FNXC_LOG requirement/change comments were added.
As per coding guidelines, "Add FNXC_LOG comments describing the date of the change (format yyyy-MM-dd-hh:mm) and describing the requirements or the change in requirements. Write FNXC:Area-of-product in front of all comments... Ensure ALL important user-facing requirements must be written as comments somewhere in the codebase."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/workflow-work-projection.ts` around lines 1 - 85, Add FNXC_LOG documentation comments to the top of the workflow-work-projection.ts file to document the user-facing workflow-status projection behavior. Include comments that describe the date of the change in yyyy-MM-dd-hh:mm format and explain the key requirements and functionality of the module, specifically noting what the projectWorkflowWorkStatus function does, the WorkflowWorkProjectionStatus type values it produces, and how WorkflowWorkItem states map to projection statuses. Prefix all comments with FNXC:Area-of-product (using an appropriate area designation) to identify the product area affected by this projection logic.Source: Coding guidelines
🧹 Nitpick comments (1)
packages/core/src/__tests__/workflow-work-projection.test.ts (1)
24-80: ⚡ Quick winAdd regression coverage for the
completeprojection surface.Please add a case where all workflow items are terminal and at least one is
succeeded, assertingstatus: "complete"andsource: "workflow"so this projection path is locked.As per coding guidelines, "When fixing a bug, the regression test must assert the general invariant across ALL known surfaces — not only the single reported reproduction."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/__tests__/workflow-work-projection.test.ts` around lines 24 - 80, Add a new test case in the workflow work projection test suite that covers the complete status projection. Create a test that calls projectWorkflowWorkStatus with the task and an array of workflow items where all items are in terminal states and at least one has succeeded. Assert that the returned projection contains both status: "complete" and source: "workflow" to ensure this projection path is properly tested and locked in per the regression test guidelines.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/__tests__/workflow-work-projection.test.ts`:
- Around line 1-80: The test file
`packages/core/src/__tests__/workflow-work-projection.test.ts` is missing
FNXC_LOG requirement-change comments that are required per coding guidelines for
files with user-facing requirement validations. Add FNXC_LOG comments at the top
of the file (before or within the describe block) that include: the current date
in yyyy-MM-dd-hh:mm format, the FNXC:Area-of-product prefix, and a clear
description of what requirements or changes the tests in this file are
validating (such as the workflow work projection status requirements and
fallback behavior to legacy task fields). Follow the format:
FNXC:Area-of-product yyyy-MM-dd-hh:mm description of requirements/changes.
In `@packages/core/src/workflow-work-projection.ts`:
- Around line 31-33: The current implementation uses find() to select the first
succeeded work item, which returns results in the order items appear in the
array, leading to non-deterministic workItemId values for identical logical
states. To fix this, sort the workItems array by a stable, unique identifier
(such as ID or timestamp) before calling find() to locate the succeeded item,
ensuring that the same logical state always returns the same workItemId
regardless of the input order.
---
Outside diff comments:
In `@packages/core/src/workflow-work-projection.ts`:
- Around line 1-85: Add FNXC_LOG documentation comments to the top of the
workflow-work-projection.ts file to document the user-facing workflow-status
projection behavior. Include comments that describe the date of the change in
yyyy-MM-dd-hh:mm format and explain the key requirements and functionality of
the module, specifically noting what the projectWorkflowWorkStatus function
does, the WorkflowWorkProjectionStatus type values it produces, and how
WorkflowWorkItem states map to projection statuses. Prefix all comments with
FNXC:Area-of-product (using an appropriate area designation) to identify the
product area affected by this projection logic.
---
Nitpick comments:
In `@packages/core/src/__tests__/workflow-work-projection.test.ts`:
- Around line 24-80: Add a new test case in the workflow work projection test
suite that covers the complete status projection. Create a test that calls
projectWorkflowWorkStatus with the task and an array of workflow items where all
items are in terminal states and at least one has succeeded. Assert that the
returned projection contains both status: "complete" and source: "workflow" to
ensure this projection path is properly tested and locked in per the regression
test guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4246133a-6ef4-4a4c-9525-8225080a1877
📒 Files selected for processing (4)
docs/plans/workflow-owned-merge-stack/s12-workflow-projections.mdpackages/core/src/__tests__/workflow-work-projection.test.tspackages/core/src/index.tspackages/core/src/workflow-work-projection.ts
- Make complete workflow projection selection deterministic - Add FNXC:WorkflowProjections requirement comments to projection + tests Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Ready to review this PR? Stage has broken it down into 3 individual chapters for you:
Chapters generated by Stage for commit 03dabb0 on Jun 17, 2026 2:47pm UTC. |
1 participant
Stack Slice
feature/workflow-owned-merge-s11-branch-group-subgraphsdocs/plans/2026-06-09-003-refactor-workflow-owned-merge-full-migration-slices-plan.mdGoal
Surface workflow-native queued, retrying, merging, manual-hold, failed, stalled, and recovered reasons across inspection surfaces.
Dependency
S1/S2 state foundations, S7 merge work handoff, S9 retry state, and S10 recovery events.
Expected Scope
dashboard TaskCard/detail/API/CLI output files, retry summary, task merge projection files, dashboard/API tests.
Expected Tests
Workflow-first merge queued, retry due time, manual hold, recovery reason, stale badge hiding, branch-group target identity.
Exit Gate
UI/API/CLI tests prove workflow state is the first projection source.
Status
Draft stack placeholder. This PR reserves ordering and review context; implementation should replace or extend the handoff artifact before this slice is marked ready.
Implementation Added
Summary by CodeRabbit
New Features
Documentation