Make unreported node load explicit - #308
Conversation
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughNode load telemetry now distinguishes measured values from unavailable values. The engine persists explicit reporting state, treats ChangesNode load telemetry
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SDK
participant FleetWire
participant Engine
participant Database
participant Placement
SDK->>FleetWire: Send heartbeat with optional load
FleetWire->>Engine: Validate load and load_reported
Engine->>Database: Persist load reporting and capacity state
Engine->>Placement: Provide liveness and capacity data
Placement->>Engine: Select node using capacity ordering
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
packages/sdk-python/tests/test_node.py (1)
336-339: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the provenance flag in both SDK heartbeat tests.
Both tests verify that
loadis absent but do not preventload_reported: truefrom appearing without a numeric load.
packages/sdk-python/tests/test_node.py#L336-L339: asserthb.get("load_reported") is not True.packages/sdk-swift/Tests/RelaycastTests/NodeProviderTests.swift#L414-L421: assertheartbeat["load_reported"]is not.bool(true).🤖 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/sdk-python/tests/test_node.py` around lines 336 - 339, Update the heartbeat assertions in packages/sdk-python/tests/test_node.py lines 336-339 to verify hb.get("load_reported") is not True, and update packages/sdk-swift/Tests/RelaycastTests/NodeProviderTests.swift lines 414-421 to verify heartbeat["load_reported"] is not .bool(true); preserve the existing assertions that load is absent.packages/types/src/__tests__/fleet-wire-fixtures.test.ts (1)
123-144: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a regression case for numeric legacy loads.
The schema promises that numeric legacy
loadvalues remain parseable whenload_reportedis absent. This test covers omitted and null values, but not that compatibility path.Proposed regression case
+ expect(parseFleetBrokerToRelaycastMessage({ ...heartbeat, load: 0 })).toEqual({ + ...heartbeat, + load: 0, + });🤖 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/types/src/__tests__/fleet-wire-fixtures.test.ts` around lines 123 - 144, Extend the test “accepts unreported heartbeat load and rejects values outside [0,1]” to assert that a numeric legacy load parses successfully when load_reported is omitted. Use a valid numeric value and verify the parsed result preserves the original heartbeat fields and load value.packages/sdk-typescript/src/__tests__/node-provider.test.ts (1)
218-219: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert that the provider heartbeat is not marked as measured.
The test verifies that
loadis absent, but it does not verify the new provenance field. Add an assertion thatload_reportedis nottrue.Proposed test assertion
expect(hb).not.toHaveProperty('load'); +expect(hb).not.toHaveProperty('load_reported', true);🤖 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/sdk-typescript/src/__tests__/node-provider.test.ts` around lines 218 - 219, Update the heartbeat assertions in the node-provider test to verify that the provider heartbeat’s load provenance field, load_reported, is not true, while retaining the existing assertions for provider, active_agents, handlers_live, and the absence of load.packages/engine/src/engine/placement.ts (1)
93-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd deterministic placement-order coverage.
claimSpawnNodeandchooseNodeForActionboth sort candidates withcompareNodeCapacityLoad. Add tests that assert the selection priority for reported versus unreported load and theactiveAgentstie-break for unreported nodes.🤖 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/engine/src/engine/placement.ts` around lines 93 - 100, Add deterministic tests covering candidate selection in claimSpawnNode and chooseNodeForAction, both of which use compareNodeCapacityLoad. Assert reported-load candidates take priority over unreported candidates, and unreported candidates with equal load are ordered by activeAgents, preserving deterministic selection.
🤖 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 `@CHANGELOG.md`:
- Around line 21-24: Update the Fleet node roster changelog entry to state that
broker-node load is reported only when every constituent provider reports a
genuine normalized capacity-utilization measurement; otherwise it remains null.
Match the direct-node and broker terminology already used in README.md and
openapi.yaml, including the max_agents: 0 unlimited behavior.
- Around line 21-24: Update the changelog’s unreleased section to use the
appropriate SemVer release-level heading format ([Unreleased - Patch],
[Unreleased - Minor], or [Unreleased - Major]). Split the existing fleet roster
entry into two short, impact-first bullets: one describing unavailable load as
null until explicitly reported, and another describing max_agents: 0 as
unlimited.
In `@openapi.yaml`:
- Around line 739-743: Update the max_agents property in the heartbeat response
schema to include a minimum of 0, while retaining its integer type and existing
description so negative values are rejected and zero remains the unlimited
sentinel.
In `@packages/engine/CHANGELOG.md`:
- Around line 14-15: Split the changelog entry into separate short, impact-first
bullets covering load provenance, migration 0034’s historical placeholder
behavior, GET /v1/nodes nullability, future-dated heartbeat freshness, and
unbounded aggregate capacity. Keep migration 0034 only with the historical-data
bullet, and omit internal or test-only details.
- Around line 12-15: Update the preceding changelog heading in
packages/engine/CHANGELOG.md at lines 12-15 and packages/sdk-swift/CHANGELOG.md
at lines 9-10 from [Unreleased] to the appropriate explicit SemVer release
level: [Unreleased - Patch], [Unreleased - Minor], or [Unreleased - Major].
In `@packages/sdk-rust/CHANGELOG.md`:
- Around line 9-14: Update the unreleased headings to use the explicit major
SemVer level for these breaking changes: change packages/sdk-rust/CHANGELOG.md
lines 9-14, packages/sdk-typescript/CHANGELOG.md lines 10-15, and
packages/types/CHANGELOG.md lines 10-15 to [Unreleased - Major].
---
Nitpick comments:
In `@packages/engine/src/engine/placement.ts`:
- Around line 93-100: Add deterministic tests covering candidate selection in
claimSpawnNode and chooseNodeForAction, both of which use
compareNodeCapacityLoad. Assert reported-load candidates take priority over
unreported candidates, and unreported candidates with equal load are ordered by
activeAgents, preserving deterministic selection.
In `@packages/sdk-python/tests/test_node.py`:
- Around line 336-339: Update the heartbeat assertions in
packages/sdk-python/tests/test_node.py lines 336-339 to verify
hb.get("load_reported") is not True, and update
packages/sdk-swift/Tests/RelaycastTests/NodeProviderTests.swift lines 414-421 to
verify heartbeat["load_reported"] is not .bool(true); preserve the existing
assertions that load is absent.
In `@packages/sdk-typescript/src/__tests__/node-provider.test.ts`:
- Around line 218-219: Update the heartbeat assertions in the node-provider test
to verify that the provider heartbeat’s load provenance field, load_reported, is
not true, while retaining the existing assertions for provider, active_agents,
handlers_live, and the absence of load.
In `@packages/types/src/__tests__/fleet-wire-fixtures.test.ts`:
- Around line 123-144: Extend the test “accepts unreported heartbeat load and
rejects values outside [0,1]” to assert that a numeric legacy load parses
successfully when load_reported is omitted. Use a valid numeric value and verify
the parsed result preserves the original heartbeat fields and load value.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b1ee25f1-b88e-43af-833d-650f0b03ffb2
📒 Files selected for processing (34)
.agentworkforce/trajectories/completed/2026-08/traj_ae0vmeog5qmo/summary.md.agentworkforce/trajectories/completed/2026-08/traj_ae0vmeog5qmo/trajectory.jsonCHANGELOG.mdREADME.mdopenapi.yamlpackages/engine/CHANGELOG.mdpackages/engine/src/__tests__/conformance/node.test.tspackages/engine/src/__tests__/conformance/nodeLiveness.test.tspackages/engine/src/__tests__/conformance/nodeProviders.test.tspackages/engine/src/db/migrations/0034_node_load_reporting.sqlpackages/engine/src/db/schema.tspackages/engine/src/engine/node.tspackages/engine/src/engine/nodeProvider.tspackages/engine/src/engine/placement.tspackages/sdk-python/src/relay_sdk/models.pypackages/sdk-python/src/relay_sdk/node.pypackages/sdk-python/src/relay_sdk/ws.pypackages/sdk-python/tests/test_node.pypackages/sdk-rust/CHANGELOG.mdpackages/sdk-rust/src/types.rspackages/sdk-rust/src/ws.rspackages/sdk-swift/CHANGELOG.mdpackages/sdk-swift/Sources/Relaycast/Models.swiftpackages/sdk-swift/Sources/Relaycast/NodeProvider.swiftpackages/sdk-swift/Sources/Relaycast/WsClient.swiftpackages/sdk-swift/Tests/RelaycastTests/NodeProviderTests.swiftpackages/sdk-typescript/CHANGELOG.mdpackages/sdk-typescript/src/__tests__/node-provider.test.tspackages/sdk-typescript/src/node-provider.tspackages/sdk-typescript/src/types.tspackages/sdk-typescript/src/ws.tspackages/types/CHANGELOG.mdpackages/types/src/__tests__/fleet-wire-fixtures.test.tspackages/types/src/fleet-wire.ts
💤 Files with no reviewable changes (2)
- packages/sdk-swift/Sources/Relaycast/NodeProvider.swift
- packages/sdk-typescript/src/node-provider.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 34 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/sdk-rust/src/ws.rs`:
- Around line 759-760: Update the heartbeat assertion in the websocket test to
verify that the load field exists and equals JSON null, using
heartbeat.get("load") rather than indexing heartbeat["load"]; retain the
existing load_reported absence assertion.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 691b2c38-97af-4fa4-b293-aa2f1ba37237
📒 Files selected for processing (15)
CHANGELOG.mdopenapi.yamlpackages/engine/CHANGELOG.mdpackages/engine/src/__tests__/conformance/node.test.tspackages/engine/src/db/migrations/0034_node_load_reporting.sqlpackages/sdk-python/src/relay_sdk/ws.pypackages/sdk-python/tests/test_ws.pypackages/sdk-rust/CHANGELOG.mdpackages/sdk-rust/src/ws.rspackages/sdk-swift/CHANGELOG.mdpackages/sdk-swift/Sources/Relaycast/WsClient.swiftpackages/sdk-typescript/CHANGELOG.mdpackages/sdk-typescript/src/__tests__/agent-ws.test.tspackages/sdk-typescript/src/ws.tspackages/types/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/sdk-typescript/CHANGELOG.md
- openapi.yaml
- packages/engine/CHANGELOG.md
- packages/types/CHANGELOG.md
- packages/sdk-rust/CHANGELOG.md
- packages/sdk-swift/CHANGELOG.md
- CHANGELOG.md
- packages/engine/src/db/migrations/0034_node_load_reporting.sql
- packages/engine/src/tests/conformance/node.test.ts
3 participants
Summary
load: nulluntil a node/provider explicitly marks a bounded utilization value as measuredmax_agents: 0consistently as unlimited, including mixed-provider aggregationRoot cause
All shipped node providers sent a literal
load: 0; Relaycast persisted it, provider aggregation took the maximum (also starting at zero),GET /v1/nodesreturned the number, andagent-relay fleet nodesprinted it without further calculation. The value was a placeholder, not a measurement.This change adds explicit
load_reportedprovenance. Existing rows remain unreported during migration; no historical zero is backfilled as measured.Semantics
load: normalized managed-agent capacity utilization in[0,1]max_agents: 0: unlimited capacityVerification
XCTestNo Cloud dashboard changes. Do not merge or deploy without the Relaycast merge/deploy gates.