test(e2e): preserve first-turn latency subdurations

[HOYALIM]

Summary

Preserve command wall-clock latency and OpenClaw internal agent duration in cold-onboard evidence. Treat only a sole successful-turn root-end-to-first-turn overage as a non-blocking first observation. Enforce recurrence when the current anomaly is corroborated in a full 12-sample exact-cohort window from trusted scheduled main runs.

Related Issue

Related to #6660.

Changes

• write firstTurnCommandMs and tagged firstTurnAgentDuration evidence to the v3 cold-performance artifact
• record missing or malformed OpenClaw duration metadata instead of fabricating a measurement
• preserve agent, model, provider, inference mode, and prompt-contract cohort identity
• keep exit, timeout, sentinel, root-start, phase, silence, and BuildKit or fallback failures blocking
• store eligible first-turn evidence in the trusted e2e-runtime-summary artifact
• compare the 12 most recent eligible samples for an exact cohort and fail the scheduled scorecard when the current anomaly has an earlier corroborating anomaly
• scan the bounded retained history before filtering missing or different-cohort samples
• preserve legacy runtime-summary trend data while v2 first-turn history accumulates
• open scorecard artifacts without following symlinks, then validate and read through the same file descriptor
• sync current main, including the credential-safe fork E2E controller remediation

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior - justification:
• Tests not applicable - justification:
• Docs updated for user-facing behavior changes
• Docs not applicable - justification:
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded - reviewer/approval link/justification: exact-head maintainer approval at test(e2e): preserve first-turn latency subdurations #7597 (review)
• Non-success, skipped, or missing CI check accepted by maintainer - check name, approval link, and follow-up issue:

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: docs-updated
• Evidence: test/e2e/README.md documents the trusted scheduled history source, exact cohort, 12-sample window, corroboration rule, eligibility boundary, and 14-day artifact retention. No docs/ page is needed because this changes internal E2E evidence and scorecard policy.
• Agent: Codex Desktop

Review Resolution

Current head 156e8e86b7b101bf6773976f46a43edbea031cb1 resolves PR Review Advisor finding PRA-1, the earlier requested changes, final automated-review cleanup, and the CodeQL file-system race finding.

• The first isolated successful-turn latency tail remains non-blocking.
• The trusted scheduled scorecard reads only GitHub Actions artifacts from scheduled main runs.
• A full window contains the 12 most recent eligible samples with the same agent, provider, model, inference mode, and prompt contract.
• The current anomaly fails when at least one earlier anomaly corroborates it.
• Missing, malformed, functionally unsuccessful, and different-cohort samples do not enter the window.
• The loader scans the retained scheduled history before cohort filtering, so recent ineligible summaries cannot hide older eligible evidence.
• Missing current artifacts are rejected explicitly, and displayed history limits derive from the configured limit.
• Artifact metadata validation and JSON parsing use one no-follow file descriptor, removing the path-based time-of-check/time-of-use race.

Verification

• PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set - 89 E2E-support tests passed across the onboarding evaluator, first-turn history, runtime history, and workflow boundary suites
• Applicable broad gate passed - exact-head CI, CodeQL, automated review, and trusted fork E2E passed
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed

Signed-off-by: Ho Lim subhoya@gmail.com

Summary by CodeRabbit

• Bug Fixes

  • Updated onboarding performance evaluation to treat a single “hosted first-turn” tail overage as a structured non-blocking hosted-latency anomaly when other deterministic budgets pass.
  • Ensured the anomaly remains blocking when it coincides with root-start or phase-budget failures.

• Improvements

  • Enhanced E2E evidence and assertions with model/provider context and richer first-turn performance metadata.
  • Added first-turn latency recurrence scoring to nightly runtime trend reporting, including backward-compatible handling of existing runtime summaries.
  • Improved scheduled workflow behavior so runtime summaries are uploaded more consistently.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

📝 Walkthrough

Walkthrough

Cold onboard performance evaluation now classifies an isolated first-turn latency overage as a structured non-blocking anomaly, while combined budget failures remain blocking. E2E evidence includes hosted model metadata and anomaly details, with nightly scorecard recurrence analysis and runtime-history integration.

Changes

Cold onboard latency classification

Layer / File(s)	Summary
Structured findings and anomaly classification test/e2e/fixtures/onboard-performance.ts	Budget checks now produce typed findings and classify an isolated root-end-to-first-turn overage as a first-turn-latency-tail anomaly; combined failures remain violations.
Performance evidence and hosted metadata test/e2e/live/full-e2e.test.ts, test/e2e/README.md	The full E2E flow records hosted model metadata and anomalies in the progress-budget artifact, emits warnings, and documents the classification rule.
Anomaly and blocking-path coverage test/e2e/support/onboard-performance.test.ts	Tests cover empty anomaly results, isolated non-blocking anomalies, and blocking behavior when another budget fails.

First-turn recurrence

Layer / File(s)	Summary
Sample validation and recurrence analysis scripts/scorecard/analyze-first-turn-latency.mts, test/e2e/support/e2e-first-turn-latency-history.test.ts	First-turn artifacts are validated and normalized, same-cohort history is evaluated for recurring anomalies, recurrence results are formatted, and the behavior is covered by filesystem-backed and recurrence tests.
Runtime history and workflow integration scripts/scorecard/analyze-runtime-history.mts, .github/workflows/e2e.yaml	Runtime summaries carry first-turn latency across legacy and current schemas, nightly history evaluates recurrence, and scheduled workflows provide samples and upload summaries.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant fullE2ETest
  participant evaluateColdOnboardPerformance
  participant onboardProgressBudget
  participant scorecardWorkflow
  participant runtimeHistory
  fullE2ETest->>evaluateColdOnboardPerformance: evaluate cold onboard trace
  evaluateColdOnboardPerformance-->>fullE2ETest: return performance and anomalies
  fullE2ETest->>onboardProgressBudget: write cohort and anomaly evidence
  scorecardWorkflow->>onboardProgressBudget: read current latency sample
  scorecardWorkflow->>runtimeHistory: provide current first-turn latency
  runtimeHistory-->>scorecardWorkflow: append recurrence result and runtime history

Loading

Suggested labels: feature, area: observability

Suggested reviewers: cv, prekshivyas, brandonpelfrey

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the PR’s primary E2E change: preserving first-turn latency timing evidence.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 1 warning · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 1 fewer warning, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-inference, cloud-onboard, security-posture, full-e2e

1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — Cover filesystem rejection paths for first-turn artifacts

• Location: test/e2e/support/e2e-first-turn-latency-history.test.ts:76
• Category: tests
• Problem: The new first-turn artifact reader defends against duplicate files, symlinks, oversized files, and traversal limits, but the focused tests cover only an absent artifact and failed sentinel evidence.
• Impact: A later change can weaken a filesystem boundary and admit ambiguous or unsafe artifact input without a focused regression test.
• Recommendation: Add focused temporary-directory cases that verify multiple matching files, a symbolic-link artifact, and an oversized artifact produce no eligible sample.
• Verification: Inspect `findArtifactFiles` and `readCurrentArtifact`, then confirm each constructed filesystem case makes `readCurrentFirstTurnLatencySample` return `null`.
• Test coverage: Extend `hosted first-turn latency history` with cases for duplicate matching files, a symlinked matching file, and a file larger than the reader limit.
• Evidence: `scripts/scorecard/analyze-first-turn-latency.mts:103-151` implements depth, entry-count, symlink, exact-one-match, size, and `O_NOFOLLOW` guards. `test/e2e/support/e2e-first-turn-latency-history.test.ts:76-97` tests no artifact, a valid artifact, and a failed sentinel, but not the filesystem guards.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[prekshivyas]

The accepted #6660 contract is not complete on PR SHA 935debd28.

• test/e2e/fixtures/onboard-performance.ts makes every sole root-end-to-first-turn overage non-blocking. No checked-in consumer reads performance.anomalies, so repeated same-model and same-mode anomalies can never become blocking. A maintainer must define the minimum sample size, bounded sample retention, and corroboration rule before implementation. I will not choose those policy values by assumption.
• test/e2e/live/full-e2e.test.ts records only firstTurnCommandMs. Run 30207389748 proves the OpenClaw result contains result.meta.durationMs: 8916, but onboard-progress-budget.json omits it. Record the internal-agent duration with an explicit unavailable state for malformed or absent metadata, and test the artifact shape.

The focused E2E-support suite passes 28 tests, npm run build:cli passes, and npm run check:diff passes. Those results do not satisfy the missing recurrence contract.

[HOYALIM]

@prekshivyas Your CHANGES_REQUESTED review on 935debd28 is addressed on current head bc3cf8eb0:

• 500e2592c removes the unapproved non-blocking anomaly classification; focused coverage keeps both sole and corroborated first-turn overages fail-closed and blocking.
• The v3 artifact now records firstTurnCommandMs separately from tagged firstTurnAgentDuration evidence, with available, missing, and malformed metadata cases covered.
• The later head is a maintainer-authored merge of main and does not alter either contract. The PR body contains the commit mapping and current verification details.

I do not have permission to use GitHub’s formal re-request-review action on this repository. Could you re-review the current head when available?

[cjagwani]

Maintainer follow-up on exact head bee4b7bac748fe7be4f4e24d6b6a5aa44b3e7fa4 (plain comment; not Changes Requested): the earlier policy blockers are resolved. The diff removes the unapproved anomaly downgrade, keeps every configured overage blocking, records command wall time separately from a tagged available/missing/malformed OpenClaw duration, and binds the artifact to model/provider/inference-mode/prompt cohort identity. I reproduced the focused E2E-support suite: 31/31 passed.

One PR-template gate is still incomplete: the Documentation Writer Review section says Result: blocked and carries docs-review-head-sha: 500e2592c, not this exact head. Please run the required documentation-writer review for bee4b7bac748fe7be4f4e24d6b6a5aa44b3e7fa4 and update the exact-head receipt (it may report that the existing test/e2e/README.md update is sufficient). A PR-body-only receipt update is enough if no content change is needed; do not merge main or create an empty code commit for this.

I will re-check the receipt and the currently running exact-head CI/E2E before approval.

[cjagwani]

Correction to my earlier review comment at exact head bee4b7bac748fe7be4f4e24d6b6a5aa44b3e7fa4: I re-checked the accepted maintainer decision on #6660. The 2026-07-26 collaborator comment explicitly refines the contract to treat a sole successful-turn rootEndToFirstTurnCompletion overage as a structured non-blocking first-observation anomaly, while keeping exit, timeout, malformed/missing sentinel, root-start/phase, silence, and BuildKit/fallback failures blocking. This head still ends with an unconditional expect(performanceEvaluation.passed).toBe(true), so the exact-head PR Review Advisor blocker is valid and my prior statement that the policy blocker was resolved was incorrect. Please implement and test that narrow anomaly-only classification, preserving failures for every listed deterministic/functional condition. The exact-head documentation-writer receipt is also still required. This is a plain review comment, not a Changes Requested review.

[cjagwani]

Exact-head follow-up on 3d0463c683847563b851bd25e3ba3753b401b964: PR Review Advisor finding PRA-1 is valid. The change records firstTurnCohort and performance.anomalies, but a repository-wide exact-head search finds no producer or consumer that aggregates prior onboard-progress-budget.json artifacts or turns recurring same-cohort anomalies back into a blocking verdict. As written, every sole successful-turn root-end overage remains non-blocking indefinitely.

The accepted collaborator scope on #6660 also requires statistical enforcement for a recurring same-model/same-mode cohort with a documented minimum sample size and corroboration rule. Completing that requires a maintainer decision about the canonical history source, cohort dimensions, minimum sample size, threshold, and retention/availability behavior; I will not invent those product/validation semantics in a salvage commit. Please record that decision or an explicit landing-order waiver/follow-up before approval.

The nine newest fork workflow runs were approved after the head stayed unchanged for more than one hour, and CI is now running. This is a plain review comment, not a Changes Requested review; I will continue babysitting this exact head.

[cjagwani]

Exact-head E2E follow-up on 3d0463c683847563b851bd25e3ba3753b401b964: substantive CI / Pull Request attempt 2 passed, but the trusted coordination check now requires the legacy manual approve-e2e operation for this fork. Current main at 1ce71fa0fe93650b191c396355c817085e515a41 implements that operation by dispatching the fork checkout through .github/workflows/e2e.yaml; the selected cloud-inference, cloud-onboard, security-posture, and full-e2e jobs include repository credential inputs such as NVIDIA_INFERENCE_API_KEY. I will not authorize untrusted fork code to run with repository credentials.

This exact head therefore also needs the credential-safe fork-skip controller path to land and a fresh PR revision/CI cycle, or another maintainer-approved safe gate resolution. The current coordination check is 90951620394, sourced from controller run 30571740481. This is a plain review comment, not a Changes Requested review; I will keep monitoring the PR.

[prekshivyas]

I need one behavior decision before pushing. #6660 defines neither the recurrence threshold nor a trusted history source, and existing v2 artifacts lack cohort identity.

Should I narrow #7597 to preserve v3 evidence while keeping the 14-second overage blocking, or implement a broader trusted history gate where the second anomaly among 12 eligible same-cohort samples blocks?

[coderabbitai]

🧹 Nitpick comments (2)

scripts/scorecard/analyze-first-turn-latency.mts (1)

163-214: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Null-safety here depends on condition ordering, not an explicit guard.

artifact can be null (from asRecord(readCurrentArtifact(root))), and only the first || condition uses artifact?.schemaVersion; every subsequent condition (artifact.installExitCode, etc.) accesses artifact without optional chaining. This is safe today only because the first condition is guaranteed true when artifact is null, short-circuiting the || chain. A future reorder of these conditions would reintroduce a crash, and this function is called unguarded at the top level of the scorecard script, so an uncaught throw here would fail the entire nightly scorecard step before buildRuntimeHistory's own try/catch even runs.

🛡️ Add an explicit early return

 export function readCurrentFirstTurnLatencySample(root: string): FirstTurnLatencySample | null {
   const artifact = asRecord(readCurrentArtifact(root));
+  if (!artifact) return null;
   const performance = asRecord(artifact?.performance);
   const phaseMeasurements = asRecord(artifact?.phaseMeasurements);
   const budget = asRecord(artifact?.budget);
   const cohort = normalizeCohort(artifact?.firstTurnCohort);
   if (
-    artifact?.schemaVersion !== FIRST_TURN_ARTIFACT_SCHEMA ||
+    artifact.schemaVersion !== FIRST_TURN_ARTIFACT_SCHEMA ||
     artifact.installExitCode !== 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 `@scripts/scorecard/analyze-first-turn-latency.mts` around lines 163 - 214,
Update readCurrentFirstTurnLatencySample to explicitly return null immediately
when artifact is not a record, before evaluating artifact.installExitCode and
the remaining validation conditions. Keep the existing validation behavior
unchanged for valid artifacts and avoid relying on condition ordering for null
safety.

scripts/scorecard/analyze-runtime-history.mts (1)

451-455: 📐 Maintainability & Code Quality | 🔵 Trivial

Hardcoded "10" duplicates the new RUNTIME_TREND_LIMIT constant.

The trend header text still says "up to 10 prior completed scheduled runs" as a literal, while the actual slice now uses RUNTIME_TREND_LIMIT. They agree today, but will silently drift if the constant changes.

♻️ Interpolate the constant

-    "Current timing compared with up to 10 prior completed scheduled runs; manual runs are excluded from history.",
+    `Current timing compared with up to ${RUNTIME_TREND_LIMIT} prior completed scheduled runs; manual runs are excluded from history.`,

🤖 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 `@scripts/scorecard/analyze-runtime-history.mts` around lines 451 - 455, Update
the trend header construction in the lines array to interpolate the existing
RUNTIME_TREND_LIMIT constant instead of hardcoding “10,” keeping the displayed
wording aligned with the limit used by the history slice.

🤖 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.

Nitpick comments:
In `@scripts/scorecard/analyze-first-turn-latency.mts`:
- Around line 163-214: Update readCurrentFirstTurnLatencySample to explicitly
return null immediately when artifact is not a record, before evaluating
artifact.installExitCode and the remaining validation conditions. Keep the
existing validation behavior unchanged for valid artifacts and avoid relying on
condition ordering for null safety.

In `@scripts/scorecard/analyze-runtime-history.mts`:
- Around line 451-455: Update the trend header construction in the lines array
to interpolate the existing RUNTIME_TREND_LIMIT constant instead of hardcoding
“10,” keeping the displayed wording aligned with the limit used by the history
slice.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0b04cf7a-51ff-4d11-a891-52818f653d26

📥 Commits

Reviewing files that changed from the base of the PR and between 935debd and f1811a1.

📒 Files selected for processing (3)

• .github/workflows/e2e.yaml
• scripts/scorecard/analyze-first-turn-latency.mts
• scripts/scorecard/analyze-runtime-history.mts

[prekshivyas]

Approved at exact PR SHA 156e8e86b7b101bf6773976f46a43edbea031cb1 against recorded base 73786425f6d5d6b3c3189e01dea9c1057fccac7f. The recurrence contract, retained cohort history, internal-agent duration evidence, and no-follow artifact reads address the prior requested changes. Exact-head CI, JavaScript/TypeScript CodeQL, both automated advisor lanes, documentation writer receipt, and the first-attempt trusted fork E2E plan (cloud-inference, cloud-onboard, security-posture, and full-e2e) all passed.