fix: restore runnerRoundTrips agent-cost field dropped in #968#970
Merged
Conversation
PR #968 (apple-platform-consolidation) accidentally dropped the runnerRoundTrips field from the agent-cost block during an over-broad conflict resolution. This restores the shipped Phase-4 feature: - src/kernel/contracts.ts: re-add ResponseCost.runnerRoundTrips: number - src/utils/diagnostics.ts: restore the countDiagnosticEventsByPhase() accessor over the flush-surviving phaseCounts tally (the tally itself survived #968; only the accessor was removed) - src/daemon/request-router.ts: repopulate runnerRoundTrips in the cost graft by counting the two real round-trip diagnostic phases (ios_runner_command_send + ios_runner_readiness_preflight). The RUNNER_ROUND_TRIP_PHASES constant is now defined locally (its former home, the dev-only runner-request-count.ts, was removed in #968 and is out of scope to restore) - request-router-cost.test.ts: restore the round-trip counting test and the runnerRoundTrips:0 assertion Byte-identical-default invariant preserved: with --cost OFF or on an error response the serialized payload is unchanged.
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
Member
Author
|
Review finding for head 5ef1e70: plans/phase3-platform-plugin-progress.md still says the cost.runnerRoundTrips runtime surface was removed in #968, but this PR intentionally restores that public agent-cost field. If this lands as-is, the plan becomes stale/misleading for the next Apple/agent-cost worker. Please update the Step (c) request-count bullet to distinguish the dev-only request-count CI gate, which remains removed, from the restored runtime cost.runnerRoundTrips surface. |
|
thymikee
added a commit
that referenced
this pull request
Jul 1, 2026
…n Phase 3 plan (#971) The Step (c) request-count bullet claimed both the dev-only CI gate AND the runtime `cost.runnerRoundTrips` surface were removed in #968. #970 restored the public agent-cost field, so the bullet is stale/misleading for the next Apple/agent-cost worker. Split the bullet: the dev-only request-count CI gate (the #966 --debug ndjson counter + smoke-ios assertion) stays removed (zero runner events on main runs); the runtime `cost.runnerRoundTrips` agent-cost field (ResponseCost / buildResponseCost over RUNNER_ROUND_TRIP_PHASES) is a separate pre-existing surface, restored in #970, and remains part of the agent-cost contract.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Regression
PR #968 (apple-platform-consolidation, merge
26ac865c6) accidentally removed therunnerRoundTripsfield from the agent-cost block — a shipped Phase-4 feature. The removal was an over-broad conflict resolution: the merge intended to keepmain's round-trip wiring, but it got dropped. The commit message is just "refactor: consolidate Apple platform internals" with no mention of dropping the feature, and the cost graft's call site still carries the comment "Runs inside the diagnostics scope so cost can read this request's runner-round-trip tally" — i.e. the intent was preserved while the implementation was lost.On the regressed
main,runnerRoundTripsappeared nowhere insrc/:ResponseCosthad onlywallClockMs+ optionalnodeCount, and the request-router cost graft only setwallClockMs.What was restored
src/kernel/contracts.ts— re-addResponseCost.runnerRoundTrips: number(always present when cost is included; 0 when no runner was hit).src/utils/diagnostics.ts— restore thecountDiagnosticEventsByPhase()accessor over the flush-survivingphaseCountstally. The tally mechanism itself (phaseCountsmap on the diagnostics scope, populated inemitDiagnostic) survived refactor: consolidate Apple platform internals #968; only the public accessor was removed, so this re-adds just the accessor.src/daemon/request-router.ts— repopulaterunnerRoundTripsinbuildResponseCostby counting the two real round-trip diagnostic phases (ios_runner_command_send+ios_runner_readiness_preflight). The graft already runs inside the request's diagnostics scope, so it reads the per-request tally. TheRUNNER_ROUND_TRIP_PHASESconstant is now defined locally in this file: its former home, the dev-onlyrunner-request-count.tsCI gate, was removed in refactor: consolidate Apple platform internals #968 and is intentionally out of scope to restore (a separate maintainer decision).src/daemon/__tests__/request-router-cost.test.ts— restore therunnerRoundTrips: 0assertion in the additive-only test and re-add the round-trip counting test (1 preflight + 2 command_send = 3, excluding_skipped/unrelated phases).Out of scope (intentionally not restored)
The dev-only ndjson gate deleted by #968 (
scripts/runner-request-count/,src/daemon/runner-request-count.ts) — a separate maintainer decision. Only therunnerRoundTripsagent-cost field is restored here.Invariant preserved
Byte-identical-default: with
--costOFF (or on an error response) the serialized response is unchanged. The cost block is still purely additive on the success path.Verification
tsc -p tsconfig.json --noEmit— 0 errorsvitest run request-router-cost.test.ts— 6/6 passoxfmt --check+oxlint --deny-warningson all 4 changed files — cleanfallow audit --base origin/main— no issuesrslib build— success