(MOT-4096) feat(eval): compare live session metrics - #734
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 55 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthroughThe evaluation system now compares 2–5 existing root sessions using live lifecycle and Harness metrics. A new asynchronous function exposes the comparison API. The console defaults to session comparison while retaining prompt experiments. ChangesSession comparison
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant SessionComparison
participant EvalApi
participant eval_compare_sessions
participant HarnessMetrics
participant ComparisonMatrix
SessionComparison->>EvalApi: Load visible root sessions
SessionComparison->>EvalApi: Submit selected sessions and baseline
EvalApi->>eval_compare_sessions: Request live comparison
eval_compare_sessions->>HarnessMetrics: Collect session metrics and lifecycle data
HarnessMetrics-->>eval_compare_sessions: Return metrics and partial-read states
eval_compare_sessions-->>EvalApi: Return summaries, deltas, and errors
EvalApi-->>ComparisonMatrix: Provide comparison response
ComparisonMatrix-->>SessionComparison: Render metrics, context, and lifecycle details
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
ytallo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
eval/README.md (1)
49-59: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winComplete the public function list.
eval/src/functions.rsalso registersRERUN_IDandNORMALIZED_TEXT_IDas non-internal functions. The new list omits both. Add them so the documented surface matches the registered surface.📝 Proposed documentation addition
- `eval::start` — validate, persist, and enqueue an evaluation. +- `eval::rerun` — repeat a terminal evaluation from its persisted request. - `eval::list` — list recent evaluations as lightweight summaries. @@ - `eval::assert::exact` — built-in deep JSON/string equality evaluator. +- `eval::assert::normalized-text` — built-in normalized text equality evaluator.Confirm the exact function IDs from the
RERUN_IDandNORMALIZED_TEXT_IDconstants before applying.🤖 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 `@eval/README.md` around lines 49 - 59, Update the “Public functions” list in README.md to include the exact IDs defined by the RERUN_ID and NORMALIZED_TEXT_ID constants in eval/src/functions.rs, preserving the existing list format and ensuring the documentation matches all registered non-internal functions.
🧹 Nitpick comments (3)
eval/src/comparison.rs (3)
437-501: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the metric key set with a test.
valuesduplicates the field list ofObjectiveSummaryV1as string keys. If a field is added to the struct and not added here, the metric disappears fromdeltaswith no compile error.The existing assertion at line 656 compares
null_metric_deltas().len()tocurrent.values().len(). Both derive from this same method, so it cannot detect the omission.Add a test that asserts the exact key set, so a new field forces a deliberate update.
♻️ Proposed test
+ #[test] + fn objective_metric_keys_are_pinned() { + let keys: Vec<String> = ObjectiveSummaryV1::default().values().into_keys().collect(); + assert_eq!( + keys, + [ + "cache_read_tokens", + "cache_write_tokens", + "compacted_sessions", + "context_free_tokens", + "context_occupancy", + "context_total_tokens", + "context_usable_tokens", + "cost_per_generation_usd", + "descendants", + "error_span_count", + "function_call_errors", + "function_calls", + "function_error_rate", + "generations", + "input_tokens", + "max_depth", + "output_tokens", + "reasoning_tokens", + "sessions", + "span_count", + "subject_cost_usd", + "tokens_per_generation", + "total_tokens", + "trace_count", + "trace_duration_ms", + ] + ); + }🤖 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 `@eval/src/comparison.rs` around lines 437 - 501, Add a test for ObjectiveSummaryV1::values that asserts its keys exactly match the expected metric-key set, rather than only comparing lengths derived from values(). Include every current field name and make the test fail when a struct field is added without updating values().
247-249: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the no-op
shrink_to_fitor move the ordering comment.The comment describes ordering behavior, but the statement below it only releases spare capacity.
sessionsis already built in request order by the index-based fill. The pairing suggests thatshrink_to_fitpreserves the order, which it does not do.Move the ordering note to the
observationsfill at line 217, and drop theshrink_to_fitcall.♻️ Proposed cleanup
- let mut sessions = observations + // Preserve the user's explicit order, with the reference first only when + // they sent it first; selection order remains meaningful in the matrix. + let sessions = observations .into_iter() .map(|observation| item_from_observation(observation, baseline_summary.as_ref())) .collect::<Vec<_>>(); - // Preserve the user's explicit order, with the reference first only when - // they sent it first; selection order remains meaningful in the matrix. - sessions.shrink_to_fit();🤖 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 `@eval/src/comparison.rs` around lines 247 - 249, Move the ordering comment to the index-based observations fill where request order is established, and remove the no-op sessions.shrink_to_fit() call. Keep the existing session construction and ordering behavior unchanged.
556-658: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
lifecycle_from.The tests cover validation, metadata, summaries, and deltas. They do not cover
lifecycle_from.Two rules in that function carry real risk.
partialmust be true whenevercompleteis notSome(true).terminalmust be false for a terminal turn that expects a wake. The second rule duplicatesterminal_snapshotinharness/src/functions/metrics.rs, so the two definitions can drift apart without any test failing.Add a test that pins both rules.
🤖 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 `@eval/src/comparison.rs` around lines 556 - 658, Add a test in the existing tests module covering lifecycle_from, with cases asserting partial is true whenever complete is not Some(true), and terminal is false when a terminal turn expects a wake. Reuse the relevant lifecycle/session metrics symbols and keep the assertions aligned with terminal_snapshot behavior.
🤖 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 `@eval/src/comparison.rs`:
- Line 25: Adjust the timeout constants used by the comparison worker and client
so the client timeout exceeds the worker’s COLLECTION_TIMEOUT_MS, avoiding
identical 30-second limits. Preserve the existing worker timeout behavior while
updating the corresponding UI timeout configuration.
In `@eval/src/error.rs`:
- Around line 58-59: Update the EvalError-to-SdkError conversion implementation
in eval/src/error.rs to add explicit arms for the NotFound, SessionNotFound, and
Conflict variants. Map each variant to the corresponding SDK status/error code,
ensuring SessionNotFound produces a not-found response rather than the generic
SdkError::Handler mapping, while preserving existing mappings for other
variants.
In `@eval/tests/golden/schemas/eval.compare-sessions.json`:
- Around line 349-394: The schema definition for SessionComparisonItemV1 must
declare a default for the errors array, matching the field’s serde default
behavior. Update the schema generation or golden-schema source associated with
SessionComparisonItemV1::errors to emit the schemars default for an empty vector
while preserving its optional serialization behavior.
In `@eval/ui/src/page/SessionComparison.tsx`:
- Around line 204-215: Prevent stale comparison state in SessionComparison by
clearing the comparison matrix when compare starts and whenever the picker
reference changes. While comparing is true, disable refresh and all
session/reference picker controls so inputs cannot change during the pending
request; preserve the existing response and error handling.
In `@harness/desktopcommander-scan`:
- Line 1: Update the desktopcommander-scan submodule configuration in
.gitmodules so its URL and branch point to a remote containing commit
1eccc8b09cc09805202a1737fd20d605356c3671, or recreate and repin the submodule to
a reachable commit. Verify the resulting submodule pointer is fetchable through
the configured remote before release.
In `@harness/repo`:
- Line 1: Configure harness/repo as a proper submodule by adding its .gitmodules
entry and updating CI workflows to fetch and initialize submodules recursively,
ensuring the pinned commit is populated; if the dependency is managed elsewhere,
remove the harness/repo gitlink instead.
In `@harness/src/functions/metrics.rs`:
- Around line 154-159: Update the terminal-check logic around terminal_snapshot
so session_expects_wake is called only when terminal_status(Some(turn.status))
is true. Preserve terminal_snapshot’s existing result while short-circuiting the
binding call for non-terminal sessions, including the corresponding logic at the
other reported occurrence.
---
Outside diff comments:
In `@eval/README.md`:
- Around line 49-59: Update the “Public functions” list in README.md to include
the exact IDs defined by the RERUN_ID and NORMALIZED_TEXT_ID constants in
eval/src/functions.rs, preserving the existing list format and ensuring the
documentation matches all registered non-internal functions.
---
Nitpick comments:
In `@eval/src/comparison.rs`:
- Around line 437-501: Add a test for ObjectiveSummaryV1::values that asserts
its keys exactly match the expected metric-key set, rather than only comparing
lengths derived from values(). Include every current field name and make the
test fail when a struct field is added without updating values().
- Around line 247-249: Move the ordering comment to the index-based observations
fill where request order is established, and remove the no-op
sessions.shrink_to_fit() call. Keep the existing session construction and
ordering behavior unchanged.
- Around line 556-658: Add a test in the existing tests module covering
lifecycle_from, with cases asserting partial is true whenever complete is not
Some(true), and terminal is false when a terminal turn expects a wake. Reuse the
relevant lifecycle/session metrics symbols and keep the assertions aligned with
terminal_snapshot behavior.
🪄 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: b7878905-b59c-46d2-96de-5655c53030b7
⛔ Files ignored due to path filters (2)
eval/Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
eval/README.mdeval/iii.worker.yamleval/src/comparison.rseval/src/error.rseval/src/functions.rseval/src/lib.rseval/src/main.rseval/src/manifest.rseval/src/surface.rseval/tests/golden/schemas/eval.compare-sessions.jsoneval/tests/schemas.rseval/ui/package.jsoneval/ui/src/api.tseval/ui/src/page/SessionComparison.tsxeval/ui/src/page/index.tsxeval/ui/src/sessionComparison.test.tseval/ui/src/sessionComparison.tseval/ui/src/types.tseval/ui/styles.cssharness/desktopcommander-scanharness/repoharness/src/functions/metrics.rs
| use crate::runtime::Deps; | ||
|
|
||
| pub const SCHEMA_VERSION: &str = "1"; | ||
| const COLLECTION_TIMEOUT_MS: u64 = 30_000; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Compare the UI trigger timeout with the worker collection timeout.
set -euo pipefail
fd -t f 'api.ts' eval/ui --exec rg -nP -C 3 'TIMEOUT_MS'
rg -nP -C 2 'COLLECTION_TIMEOUT_MS' --type=rust eval/srcRepository: iii-hq/workers
Length of output: 1134
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '180,640p' eval/src/comparison.rs
sed -n '360,430p' eval/ui/src/api.tsRepository: iii-hq/workers
Length of output: 16954
Avoid using the same timeout for the client and worker calls.
eval/ui/src/api.ts uses TIMEOUT_MS = 30_000, while each triggers in eval/src/comparison.rs also uses COLLECTION_TIMEOUT_MS = 30_000. When a dependency is on the 30 s boundary, the client can hit timeoutMs before the worker returns the partial comparison. Client timeout must exceed worker timeout, or the error will come only as a generic UI response.
🤖 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 `@eval/src/comparison.rs` at line 25, Adjust the timeout constants used by the
comparison worker and client so the client timeout exceeds the worker’s
COLLECTION_TIMEOUT_MS, avoiding identical 30-second limits. Preserve the
existing worker timeout behavior while updating the corresponding UI timeout
configuration.
| #[error("session not found: {0}")] | ||
| SessionNotFound(String), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the EvalError conversion to the SDK error type.
set -euo pipefail
rg -nP -C 12 'impl\s+From<EvalError>' --type=rust eval/src
rg -nP -C 4 'EvalError::(NotFound|SessionNotFound)' --type=rust eval/srcRepository: iii-hq/workers
Length of output: 2781
🏁 Script executed:
#!/bin/bash
# Description: Map the SDK error type and search for status-code handling in EvalError conversion.
set -euo pipefail
echo "== EvalError enum =="
sed -n '1,90p' eval/src/error.rs
echo
echo "== SDKError definitions/usages =="
rg -n -C 8 'enum\s+SdkError|SdkError::|impl\s+.*SdkError|404|status_code|StatusCode' eval/src --type=rust
echo
echo "== All EvalError errors =="
rg -n 'Err\(EvalError|\.err\(\)|\.map_err\(|unwrap_err\(.*EvalError|EvalError::' eval/src --type=rustRepository: iii-hq/workers
Length of output: 7999
Map EvalError variants to the SDK error status.
eval/src/error.rs only implements From<EvalError> as SdkError::Handler(...) for all variants, including NotFound, SessionNotFound, and Conflict. Add the missing variant arms so a missing session returns a not-found status/error code instead of a generic handler error.
🤖 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 `@eval/src/error.rs` around lines 58 - 59, Update the EvalError-to-SdkError
conversion implementation in eval/src/error.rs to add explicit arms for the
NotFound, SessionNotFound, and Conflict variants. Map each variant to the
corresponding SDK status/error code, ensuring SessionNotFound produces a
not-found response rather than the generic SdkError::Handler mapping, while
preserving existing mappings for other variants.
| "deltas": { | ||
| "additionalProperties": { | ||
| "$ref": "#/definitions/MetricDeltaV1" | ||
| }, | ||
| "default": {}, | ||
| "description": "One entry per objective numeric metric. A missing value is represented by null in either delta field, never by zero.", | ||
| "type": "object" | ||
| }, | ||
| "errors": { | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "type": "array" | ||
| }, | ||
| "lifecycle": { | ||
| "$ref": "#/definitions/SessionLifecycleV1" | ||
| }, | ||
| "metrics": { | ||
| "anyOf": [ | ||
| { | ||
| "$ref": "#/definitions/SessionMetricsResponseV1" | ||
| }, | ||
| { | ||
| "type": "null" | ||
| } | ||
| ] | ||
| }, | ||
| "session": { | ||
| "$ref": "#/definitions/SessionMetaProjectionV1" | ||
| }, | ||
| "summary": { | ||
| "anyOf": [ | ||
| { | ||
| "$ref": "#/definitions/ObjectiveSummaryV1" | ||
| }, | ||
| { | ||
| "type": "null" | ||
| } | ||
| ] | ||
| } | ||
| }, | ||
| "required": [ | ||
| "lifecycle", | ||
| "session" | ||
| ], | ||
| "type": "object" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the field attributes on SessionComparisonItemV1.
ast-grep run --pattern 'pub struct SessionComparisonItemV1 { $$$ }' --lang rust eval/src/comparison.rs
rg -nP -B6 -A25 'struct SessionComparisonItemV1' eval/src/comparison.rsRepository: iii-hq/workers
Length of output: 2552
🏁 Script executed:
#!/bin/bash
# Inspect schemars/cargo dependencies and schema generation for comparison module.
set -euo pipefail
printf '--- Cargo schemars references ---\n'
rg -n 'schemars|JsonSchema|serde_json::schema' Cargo.toml eval/Cargo.toml eval/src --glob '!eval/tests/golden/**' || true
printf '\n--- comparison.rs relevant sections ---\n'
sed -n '1,120p' eval/src/comparison.rs
printf '\n--- schema file excerpt around Comparison ---\n'
sed -n '310,410p' eval/tests/golden/schemas/eval.compare-sessions.jsonRepository: iii-hq/workers
Length of output: 243
🏁 Script executed:
#!/bin/bash
# Inspect schemars/cargo dependencies and schema generation for comparison module.
set -euo pipefail
printf '%s\n' '--- Cargo schemars references ---'
rg -n 'schemars|JsonSchema|serde_json::schema' Cargo.toml eval/Cargo.toml eval/src --glob '!eval/tests/golden/**' || true
printf '%s\n' ''
printf '%s\n' '--- comparison.rs relevant sections ---'
sed -n '1,120p' eval/src/comparison.rs
printf '%s\n' ''
printf '%s\n' '--- schema file excerpt around Comparison ---'
sed -n '310,410p' eval/tests/golden/schemas/eval.compare-sessions.jsonRepository: iii-hq/workers
Length of output: 12990
Add the schemars default for errors.
SessionComparisonItemV1::errors uses #[serde(default, skip_serializing_if = "Vec::is_empty")], but the golden schema only marks it optional without a default. Use a schemars default so the schema documents the serialized empty-array case consistently with the Rust field.
🤖 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 `@eval/tests/golden/schemas/eval.compare-sessions.json` around lines 349 - 394,
The schema definition for SessionComparisonItemV1 must declare a default for the
errors array, matching the field’s serde default behavior. Update the schema
generation or golden-schema source associated with
SessionComparisonItemV1::errors to emit the schemars default for an empty vector
while preserving its optional serialization behavior.
| const compare = async () => { | ||
| if (selected.length < 2 || !baseline) return | ||
| setComparing(true) | ||
| try { | ||
| setComparison(await api.compareSessions(selected, baseline)) | ||
| setError(null) | ||
| } catch (compareError) { | ||
| setError(errorMessage(compareError)) | ||
| } finally { | ||
| setComparing(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent stale comparison results.
The picker remains interactive while compareSessions() is pending. A user can change the selected sessions or reference before the request resolves. The completed response then renders data for the previous request.
Line 255 also changes the picker reference without clearing the existing matrix. Clear the matrix when the reference changes. Disable refresh and picker controls while comparing is true. Clear the matrix when a new comparison starts.
Proposed fix
const compare = async () => {
if (selected.length < 2 || !baseline) return
setComparing(true)
+ setComparison(null)
try {
setComparison(await api.compareSessions(selected, baseline))
@@
- <button className="eval-ui-button" type="button" onClick={() => void loadSessions(true)} disabled={refreshing}>
+ <button className="eval-ui-button" type="button" onClick={() => void loadSessions(true)} disabled={refreshing || comparing}>
@@
- <input type="checkbox" checked={checked} onChange={() => choose(session.session_id)} />
+ <input type="checkbox" checked={checked} disabled={comparing} onChange={() => choose(session.session_id)} />
@@
- <input type="radio" name="eval-baseline" checked={baseline === session.session_id} disabled={!checked} onChange={() => setBaseline(session.session_id)} />
+ <input
+ type="radio"
+ name="eval-baseline"
+ checked={baseline === session.session_id}
+ disabled={!checked || comparing}
+ onChange={() => {
+ setBaseline(session.session_id)
+ setComparison(null)
+ }}
+ />Also applies to: 224-225, 248-255
🤖 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 `@eval/ui/src/page/SessionComparison.tsx` around lines 204 - 215, Prevent stale
comparison state in SessionComparison by clearing the comparison matrix when
compare starts and whenever the picker reference changes. While comparing is
true, disable refresh and all session/reference picker controls so inputs cannot
change during the pending request; preserve the existing response and error
handling.
| @@ -0,0 +1 @@ | |||
| Subproject commit 1eccc8b09cc09805202a1737fd20d605356c3671 | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
path="harness/desktopcommander-scan"
expected="1eccc8b09cc09805202a1737fd20d605356c3671"
gitlink="$(git ls-tree HEAD -- "$path" | awk '{print $3}')"
test "$gitlink" = "$expected"
test -e "$path/.git"
actual="$(git -C "$path" rev-parse HEAD)"
test "$actual" = "$expected"
git -C "$path" cat-file -e "${expected}^{commit}"
matches="$(
git -C "$path" grep -n -E \
'harness::metrics|SessionMetricsResponseV1|root_session_id|metric' \
"$expected" -- || true
)"
test -n "$matches"
printf '%s\n' "$matches"Repository: iii-hq/workers
Length of output: 152
🏁 Script executed:
#!/usr/bin/env bash
set -u
path="harness/desktopcommander-scan"
expected="1eccc8b09cc09805202a1737fd20d605356c3671"
printf 'repository path entry:\n'
git ls-tree HEAD -- "$path" || true
printf '\nlocal submodule status:\n'
if [ -e="$path/.git" ]; then
printf '.git exists\n'
git -C "$path" rev-parse HEAD 2>&1 || true
git -C "$path" status --short 2>&1 || true
else
printf '.git does not exist\n'
fi
printf '\nmodule commit existence:\n'
git -C "$path" cat-file -e "${expected}^{commit}" 2>&1 && printf 'exists\n' || printf 'missing\n'
printf '\nroot_session_id usages in current branch:\n'
rg -n "root_session_id|SessionMetricsResponseV1|harness::metrics" . || trueRepository: iii-hq/workers
Length of output: 15861
🌐 Web query:
github iii-hq mobile-commander-external-session 1eccc8b09cc09805202a1737fd20d605356c3671 harness::metrics SessionMetricsResponseV1 root_session_id
💡 Result:
The elements in your query refer to distinct, unrelated projects and technical concepts found in the GitHub ecosystem as of August 2026. 1. iii-hq / mobile-commander-external-session This appears to be a reference to components within the iii-hq/workers project [1]. The iii-hq organization manages an architecture for durable, reactive, and branching session storage [1][2]. The session-manager worker is a core component that handles append-only message logs for conversations [1][2]. 2. 1eccc8b09cc09805202a1737fd20d605356c3671 This is a specific Git commit SHA from the wonderwhy-er/DesktopCommanderMCP repository [3][4]. This repository provides MCP (Model Context Protocol) skills for agents to interact with desktop environments, including terminal access, file system operations, and Claude Desktop/MCP server configuration [5][6][7]. The commit (dated July 21, 2026) updated documentation links to satisfy a repository linting check [3]. 3. harness::metrics / SessionMetricsResponseV1 / root_session_id These terms relate to session tracking and delegation tree management in software systems [8][9][10]. - root_session_id: This is a denormalized identifier used to track the root of a delegation tree (e.g., a hierarchy of subagents) [9][10]. It allows systems to group all tasks or sessions belonging to a specific conversation or process tree efficiently [8][10]. - Context: This field is common in platforms managing session graphs, such as the Everruns platform [9] or various agentic session handlers [11][12], where it is used to filter work across a tree of tasks or sessions without needing to traverse a parent-child chain [9][10]. While harness::metrics appears as a namespace in your query, it likely refers to a monitoring or telemetry system capturing these session metrics. The provided terms do not constitute a single unified codebase or schema but rather aggregate references from different technical domains involving agentic frameworks, session management, and desktop automation tools.
Citations:
- 1: https://github.com/iii-hq/workers/tree/main/session-manager
- 2: https://github.com/iii-hq/workers/blob/main/session-manager/architecture/README.md
- 3: wonderwhy-er/DesktopCommanderMCP@1eccc8b
- 4: [External Plugin]: Desktop Commander github/awesome-copilot#2378
- 5: https://github.com/wonderwhy-er/DesktopCommanderMCP/blob/1eccc8b09cc09805202a1737fd20d605356c3671/plugins/claude/skills/desktop-commander-overview/SKILL.md
- 6: https://github.com/wonderwhy-er/DesktopCommanderMCP/blob/1eccc8b09cc09805202a1737fd20d605356c3671/plugins/claude/skills/terminal/SKILL.md
- 7: https://github.com/wonderwhy-er/DesktopCommanderMCP/blob/1eccc8b09cc09805202a1737fd20d605356c3671/plugins/cursor/skills/ai-tools-setup/SKILL.md
- 8: https://docs.everruns.com/api/operations/list_tasks/
- 9: everruns/everruns@06bc33b
- 10: feat(session-tasks): surface root_session_id on task reads (EVE-681) everruns/everruns#2690
- 11: https://github.com/SuperagenticAI/superqode/blob/main/src/superqode/session/switchboard.py
- 12: https://github.com/ctxrs/ctx/blob/main/crates/ctx-history-store/src/sessions.rs
Resolve the broken Harness module pointer before merge.
harness/desktopcommander-scan is pinned to 1eccc8b09cc09805202a1737fd20d605356c3671, but that commit is unreachable through the configured submodule remote. Fix the .gitmodules URL/branch or re-create the submodule commit before release.
🤖 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 `@harness/desktopcommander-scan` at line 1, Update the desktopcommander-scan
submodule configuration in .gitmodules so its URL and branch point to a remote
containing commit 1eccc8b09cc09805202a1737fd20d605356c3671, or recreate and
repin the submodule to a reachable commit. Verify the resulting submodule
pointer is fetchable through the configured remote before release.
| if !terminal_snapshot( | ||
| Some(turn.status), | ||
| crate::bindings::session_expects_wake(deps, &node.session_id).await, | ||
| ) { | ||
| complete = false; | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
The new session_expects_wake call adds a per-session round trip to every poll.
The comment at lines 180-182 states the design rule for this loop. The context backfill was deliberately moved out of the walk so that "a large tree does not pay a round trip per session on every poll".
Line 156 now calls crate::bindings::session_expects_wake inside the walk, once per session, sequentially, and unconditionally. This restores the cost the surrounding code avoids. harness::metrics is polled as a progress signal by the watchdog, and eval/src/comparison.rs now calls it for up to five session trees per comparison.
Two options reduce the cost. Call session_expects_wake only when terminal_status(Some(turn.status)) is true, because terminal_snapshot returns false anyway for a non-terminal status. That short-circuit removes the round trip for every actively running session, which is the common polling case.
⚡ Proposed short-circuit
- if !terminal_snapshot(
- Some(turn.status),
- crate::bindings::session_expects_wake(deps, &node.session_id).await,
- ) {
+ let terminal = terminal_status(Some(turn.status))
+ && !crate::bindings::session_expects_wake(deps, &node.session_id).await;
+ if !terminal {
complete = false;
}This preserves the result of terminal_snapshot and skips the binding call whenever the status is not terminal.
Also applies to: 180-183
🤖 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 `@harness/src/functions/metrics.rs` around lines 154 - 159, Update the
terminal-check logic around terminal_snapshot so session_expects_wake is called
only when terminal_status(Some(turn.status)) is true. Preserve
terminal_snapshot’s existing result while short-circuiting the binding call for
non-terminal sessions, including the corresponding logic at the other reported
occurrence.
1 participant
Summary
eval::compare-sessionsAPI with isolated per-session collection failures and null-safe deltasThe comparison is read-only and does not persist snapshots, create jobs, rank sessions, or apply scores/judges.
Validation
cargo test --manifest-path eval/Cargo.tomlcargo test --manifest-path harness/Cargo.toml-D warningsfor eval and harnessFixes MOT-4096