refactor: decompose runSession in token-benchmark.ts#2040
Open
carlos-alm wants to merge 1 commit into
Conversation
runSession exceeded codegraph's cognitive/cyclomatic/maxNesting thresholds (27/25/4). Extracts the pure, non-orchestration pieces into scripts/lib/session-metrics.ts, matching the existing scripts/lib/ convention: buildSessionOptions (mode-dependent options), the usage/turn field-fallback logic (extractUsageMetrics + firstTruthy), and the tool_use-block scan (collectToolUseBlocks + tallyToolCalls). Pure behavior-preserving decomposition, same pattern as #1775's runBuildBenchmarks/runQueryBenchmarks extraction. runSession is now 2/3/0. Adds tests/unit/session-metrics.test.ts covering the extracted helpers, including firstTruthy's `||`-chain (not `??`) falsy-0 semantics.
Contributor
Contributor
Codegraph Impact Analysis21 functions changed → 6 callers affected across 2 files
|
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.
Summary
Pure decomposition (no behavior change) of
runSession, which exceeded cognitive/cyclomatic/maxNesting thresholds:runSession(before)runSession(after)Following the issue's suggested seams and the existing
scripts/lib/convention (bench-timing.ts,hub-selection.ts), the extracted pieces went into a newscripts/lib/session-metrics.ts:buildSessionOptions— mode-dependentsystemPrompt/mcpServersoption building (kept local totoken-benchmark.ts, since it closes over script-level CLI-flag constants)extractUsageMetrics+firstTruthy— the SDK-result usage/turn field-fallback logic (snake_case vs. camelCase), which was the single biggest contributor via fivea || b || 0chainscollectToolUseBlocks+tallyToolCalls— the nestedfor (msg) { for (block) { if (tool_use) ... } }scan, split into a flat collection pass + a flat tally pass (both using earlycontinues to drop nesting)Moving the pure helpers into
scripts/lib/(rather than keeping them as unexported functions in the script) makes them unit-testable in isolation —token-benchmark.tsitself can't be imported for testing since it runsmain()unconditionally at module load.Closes #1906
Test plan
tests/unit/session-metrics.test.ts(13 tests) coveringfirstTruthy,extractUsageMetrics,collectToolUseBlocks,tallyToolCalls— includingfirstTruthy's||-chain (not??) falsy-0semantics, which the refactor must preserve exactlynpx vitest run tests/unit/session-metrics.test.ts tests/unit/hub-selection.test.ts— 18/18 passnpm test(excluding the unrelated 34-languageresolution-benchmark.test.ts, which takes 10+ minutes and has zero relation to this script) — 3133 passed, 305 skipped, 6 pre-existing failures unrelated to this change (missing@optave/codegraph-darwin-arm64native binary in this worktree'snode_modules— a known local-dev-environment gap, not caused by this diff)npm run lint— clean on all touched files (scripts/confirmed outside Biome's scope;tests/unit/session-metrics.test.tsis in scope and clean)node --experimental-strip-types --checkon both changedscripts/files — syntax OKcodegraph diff-impact --staged -Tconfirms the 21 changed/added functions have zero transitive callers outside these two filesgit diffconfirms the moved logic is behavior-identical (same field-fallback order, same iteration order, same return shapes)Stacked on #1904 (base branch
fix/issue-1904-benchmark-hub-selection-queries-pick-non) — only the token-benchmark.ts/session-metrics.ts diff is this issue's change.