Skip to content

Client API design rev 2.2: heartbeat phases, per-launch tokens, F3 dependencies; add TASK_PAYLOAD_READY#4866

Closed
YuanTingHsieh wants to merge 1 commit into
NVIDIA:mainfrom
YuanTingHsieh:yuantingh/client-api-design-rev22
Closed

Client API design rev 2.2: heartbeat phases, per-launch tokens, F3 dependencies; add TASK_PAYLOAD_READY#4866
YuanTingHsieh wants to merge 1 commit into
NVIDIA:mainfrom
YuanTingHsieh:yuantingh/client-api-design-rev22

Conversation

@YuanTingHsieh

Copy link
Copy Markdown
Collaborator

Design/plan revision 2.2 for Client API Execution Modes (follow-up to #4853), addressing design-review findings, plus the matching protocol-vocabulary follow-up to #4856.

Design (client_api_execution_modes.md):

  • Forward-path heartbeat rule narrowed: the no-expiry exemption now covers only the payload-materialization phase — TASK_ACCEPTED → new TASK_PAYLOAD_READY message — bounded by a materialization deadline; heartbeats stay authoritative while user code trains, so a wedged process is detected at heartbeat timeout, not task timeout. Sequence diagrams updated (all four re-validated).
  • launch_once=False moves to launch-scoped tokens: the CJ regenerates the token in each per-launch bootstrap config and stopping a process invalidates its token — a surviving stale process cannot authenticate against a later launch.
  • Explicit mapping between lifecycle transfer states and the F3 TransferOutcome vocabulary (TRANSFER_COMPLETE ⇔ COMPLETED; TRANSFER_FAILED ⇔ FAILED or ABORTED; receiver truth applies before the mapping).
  • ClientAPIBackendSpec classified internal (frozen for parallel development, not public API in 2.9); Open Questions annotated with their deciding PRs; status set to Approved.

Plan (client_api_execution_modes_plan.md):

  • F3-4 now hard-depends on F3-2 + F3-3 (the facade's returns-means-delivered guarantee holds only over receiver-confirmed, retry-aware outcomes with budget-bounded resolution); critical path updated — F3-2/F3-3 run in parallel behind F3-1, still inside the 10–14-week floor.
  • Bookkeeping: PR-0 recorded as landing inside Add Client API execution modes design docs and F3 aggregate transfer outcome #4853; EX-5 moved to Wave 4 and assigned P1; CT-5 assigned P1; tier accounting made exact (36 PRs; CT-4 halves noted).

Vocabulary (client/cell/defs.py): adds Topic.TASK_PAYLOAD_READY#4856 merged before the phase-boundary message was introduced; the freeze tests are updated accordingly.

🤖 Generated with Claude Code

…s, status

Address design-review findings:

- Forward-path heartbeat rule narrowed: the no-expiry exemption now covers only
  the payload-materialization phase (TASK_ACCEPTED -> new TASK_PAYLOAD_READY
  message), bounded by a materialization deadline; heartbeats stay authoritative
  while user code trains, so a wedged process is detected at heartbeat timeout,
  not task timeout. Diagrams updated (mermaid re-validated).
- launch_once=False moves to launch-scoped tokens: the CJ regenerates the token
  in each per-launch bootstrap config and stopping a process invalidates its
  token, so a surviving stale process cannot authenticate against a later launch.
- Plan dependencies now enforce the design's payload-safety prerequisites:
  F3-4 hard-depends on F3-2 + F3-3 (receiver-confirmed, retry-aware outcomes;
  budget-bounded resolution); critical path updated (F3-2/F3-3 parallel behind
  F3-1, one M added, still inside the 10-14 week floor).
- Explicit mapping between lifecycle transfer states and the F3 TransferOutcome
  vocabulary (TRANSFER_COMPLETE <=> COMPLETED; TRANSFER_FAILED <=> FAILED or
  ABORTED; receiver truth applies before the mapping).
- Bookkeeping: PR-0 recorded as landing inside the F3-1 PR (NVIDIA#4853); EX-5 moved
  to Wave 4 (its EP-4 dependency) and assigned P1; CT-5 assigned P1; tier
  accounting line added (36 PRs, CT-4 halves in P1/P2); P0+P1 ~36 engineer-weeks.
- ClientAPIBackendSpec classified as internal (frozen for parallel development,
  not public API in 2.9). Open Questions annotated with their deciding PRs.
- Status set to Approved; Revision 2.2 entry added.
- Add Topic.TASK_PAYLOAD_READY to the frozen protocol vocabulary (follow-up to
  NVIDIA#4856, which merged before the phase-boundary message was introduced).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 7, 2026 23:57
@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates the Client API execution-mode design and protocol vocabulary.

  • Adds the TASK_PAYLOAD_READY client-cell topic.
  • Updates the topic freeze tests for the new wire value.
  • Narrows the heartbeat exemption to the payload-materialization phase.
  • Revises the implementation plan for F3 dependencies, EX-5 timing, and tier accounting.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • The protocol constant is additive and covered by the updated freeze tests.
  • The documentation updates are consistent with the new materialization phase boundary.

Important Files Changed

Filename Overview
nvflare/client/cell/defs.py Adds the TASK_PAYLOAD_READY topic constant to the client-cell protocol vocabulary.
tests/unit_test/client/cell/defs_test.py Updates expected topic names and wire values to include TASK_PAYLOAD_READY.
docs/design/client_api_execution_modes.md Documents the revised heartbeat phase boundary, per-launch token behavior, transfer-outcome mapping, and internal backend-spec status.
docs/design/client_api_execution_modes_plan.md Updates plan sequencing, dependency notes, tier counts, and critical-path estimates.

Reviews (1): Last reviewed commit: "Design/plan updates: heartbeat phases, p..." | Re-trigger Greptile

Copilot AI 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.

Pull request overview

Updates the Client API Execution Modes design/plan (rev 2.2) and aligns the frozen Cell protocol vocabulary with the newly introduced TASK_PAYLOAD_READY phase boundary used to narrow the forward-path heartbeat exemption.

Changes:

  • Add Topic.TASK_PAYLOAD_READY to the Client API Cell protocol vocabulary and update freeze tests.
  • Revise the design doc to introduce the payload-materialization phase boundary and per-launch token semantics.
  • Revise the implementation plan to reflect updated dependencies/critical path and tier accounting.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/unit_test/client/cell/defs_test.py Extends protocol freeze tests to include TASK_PAYLOAD_READY.
nvflare/client/cell/defs.py Adds the TASK_PAYLOAD_READY topic constant and documents its intended semantics.
docs/design/client_api_execution_modes.md Design rev 2.2 updates: new phase boundary message, heartbeat rule refinement, token scoping, and outcome mapping details.
docs/design/client_api_execution_modes_plan.md Plan rev 2.2 updates: dependency adjustments (notably F3-4), critical path, and bookkeeping/tier accounting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 264 to 268
@@ -260,7 +267,7 @@ sequenceDiagram
Note over E,T: WAIT_TRANSFER_COMPLETE (hold process until terminal)
E->>C: result
C->>E: task
E->>T: TASK_READY (task_id, FLModel ref)
T->>E: TASK_ACCEPTED
T->>E: TASK_PAYLOAD_READY (payload pulled)
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.27%. Comparing base (fb6ef21) to head (5a81394).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4866      +/-   ##
==========================================
- Coverage   60.28%   60.27%   -0.02%     
==========================================
  Files         971      971              
  Lines       92463    92464       +1     
==========================================
- Hits        55745    55732      -13     
- Misses      36718    36732      +14     
Flag Coverage Δ
unit-tests 60.27% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@YuanTingHsieh

Copy link
Copy Markdown
Collaborator Author

Folded into #4865 per review consolidation — the design revision and protocol vocabulary land with the F3 code they document.

@YuanTingHsieh YuanTingHsieh deleted the yuantingh/client-api-design-rev22 branch July 8, 2026 01:34
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.

3 participants