feat(run): summarize multi-iteration stability

[googs1025]

Summary

• Print a simple terminal-only stability summary after multi-iteration runs, e.g. case_a: 3 trials, 2 PASS, 1 FAIL -> flaky.
• Clarify --iteration help/docs as stability/flakiness sampling.
• Keep per-iteration artifacts unchanged and avoid creating stability report files.

Test Plan

• make fmt
• make verify
• GIT_CONFIG_GLOBAL=/dev/null make test

Note: plain make test can inherit local Git GPG signing config in temporary test repositories; the isolated global Git config avoids that unrelated local setting.

[roark47]

Thanks for adding a per-case stability summary. The underlying problem is real: the existing final totals cannot show which case changed status across iterations, and the proposed aggregation is small, deterministic, and correctly excludes without_skill baseline results.

I am requesting changes before merge for the following reasons:

1. Restore the Markdown report documentation. This PR removes skill-up report --format markdown from docs/guide/cli-reference.md, including its examples and format table entry. Markdown reporting is still supported by the code and was already present in the base commit, so this is an unrelated documentation regression. Please restore those lines.

2. Test the actual terminal behavior. TestRunner_Evaluate_MultipleIterationsDoNotWriteStabilityFiles creates the useful PASS/FAIL/PASS plus without_skill ERROR scenario, but only asserts that no stability files are written. Please capture ui.Output and assert that the summary contains case_a: 3 trials, 2 PASS, 1 FAIL -> flaky and does not include the baseline errors. It would also be valuable to cover an all-stable case and confirm that a single iteration prints no stability summary.

3. Add a changelog entry. This is a user-visible CLI behavior change, and the repository contribution guidance asks for CHANGELOG.md updates for such changes.

Non-blocking suggestion: use map[judge.Status]int and the existing judge.Status* constants instead of string keys and string literals for stronger type safety.

Local verification: make verify passed with zero lint issues, and go test -race ./internal/runner ./internal/cli passed. Once the unrelated documentation deletion and coverage gaps are fixed, the implementation should be low risk to merge.

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds a terminal-only cross-iteration stability/flakiness summary to skill-up run when multiple iterations are executed, and updates CLI/docs to frame --iteration as sampling rather than “rounds”.

Changes:

• Print a per-case stability summary after multi-iteration runs (PASS/FAIL/ERROR/SKIP, plus a “flaky” marker on mixed outcomes).
• Update --iteration help text and documentation (EN + ZH) to clarify “sampling” semantics and default 0 auto-append behavior.
• Add Go tests for summary formatting and to ensure stability report files are not written.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
skills/skill-upper/references/cli.md	Clarifies --iteration meaning and adds example terminal summary line.
skills/skill-upper/SKILL.md	Rewords --iteration as stability/flakiness sampling and documents terminal summary output.
internal/runner/workspace_options_test.go	Adds tests for summary formatting and “no stability files written”.
internal/runner/runner.go	Implements summary aggregation/formatting and prints it after multi-iteration runs.
internal/cli/run.go	Updates --iteration flag help text to match new behavior.
docs/zh/guide/cli-reference.md	Updates ZH CLI docs for new --iteration semantics and example.
docs/guide/cli-reference.md	Updates EN CLI docs for --iteration semantics; also updates skill-up report format docs.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[googs1025]

Addressed the review feedback in ad6eaa7:

• Recursively checks the whole workspace for stability.json / stability.md, not just the workspace root.
• Restores markdown in the English and Chinese skill-up report --format docs.
• Uses an ASCII INFO prefix for the stability summary heading.
• Prints unknown/non-standard statuses after known statuses in sorted order so all trials are represented.

Verification:

• go test -race ./internal/runner -run 'TestFormatIterationStabilitySummary|TestFormatIterationStabilitySummaryIncludesUnknownStatuses|TestRunner_Evaluate_MultipleIterationsDoNotWriteStabilityFiles'\n- make verify\n- GIT_CONFIG_GLOBAL=/dev/null make test

[zpzjzj]

The requested terminal-behavior coverage is still missing in ad6eaa7. TestRunner_Evaluate_MultipleIterationsDoNotWriteStabilityFiles builds the right PASS/FAIL/PASS plus without_skill ERROR scenario, but it only checks for absent files. Please capture ui.Output around Runner.Evaluate and assert that the rendered output contains case_a: 3 trials, 2 PASS, 1 FAIL -> flaky and does not include the baseline ERROR result. An all-stable case and a single-iteration no-summary case would be valuable additional coverage, but the direct output assertion and baseline exclusion are the required part of this review item.

[zpzjzj]

The requested changelog update is still missing in ad6eaa7: CHANGELOG.md is unchanged by this PR. This adds user-visible terminal behavior for multi-iteration runs, so please add a concise entry under Unreleased describing the per-case stability/flakiness summary.

[zpzjzj]

Scope clarification: current-invocation sampling vs accumulated iterations

The existing --iteration flag has two distinct behaviors:

• A positive --iteration N runs N samples in the current invocation.
• The default --iteration 0 appends one run after the latest existing iteration-N/ directory.

The current implementation only summarizes the in-memory allResults from the current invocation. In auto mode it explicitly sets runCount = 1, and the summary is only printed when runCount > 1 (auto-mode setup, summary gate). As a result, the default auto-append workflow never prints a stability summary, and existing iteration-*/result.json files are never included.

This is internally consistent if the feature is intentionally limited to an explicit positive --iteration N in one command. However, the CLI help and several documentation surfaces now describe --iteration more broadly as stability/flakiness sampling, which makes the contract ambiguous.

Please clarify and implement one of these contracts:

1. Current invocation only: keep the implementation, but explicitly document that the summary is produced only for a positive --iteration N, covers only samples executed by that command, and does not summarize auto-appended history. Add coverage for the auto-mode behavior.
2. Accumulated workspace iterations: load compatible prior iteration-*/result.json artifacts and include them in the summary, with a clear rule for excluding iterations produced from different case sets or Skill versions.

The second option is not automatically preferable because historical iterations may represent different Skill revisions. The important part is to choose and document the intended product contract before merging; otherwise the default workflow and the newly advertised stability behavior do not line up.

[googs1025]

Updated the PR branch with a clean diff based on current upstream main (29f97e1).

Addressed the latest review items:

• Added terminal-output coverage for the multi-iteration summary and verified baseline without_skill ERROR results are excluded.
• Added single-iteration no-summary coverage.
• Added an Unreleased changelog entry for the user-visible summary behavior.
• Chose and documented the current-invocation-only contract: explicit --iteration N > 1 prints a summary for samples from that command only; --iteration 0 auto-appends one run and does not summarize historical iteration-* results.

[zpzjzj]

The terminal-output test is closer, but it still does not exercise the actual UI output path requested in the review. The production code now adds mutable uiBlank / uiStatus function seams and the test replaces those functions, even though internal/ui.Output is explicitly designed to be overridden in tests. This verifies the arguments passed to the seams rather than the real ui.Blank / ui.Status rendering path, and it adds test-only globals to production code. Please capture ui.Output into a buffer, restore it with t.Cleanup, call Runner.Evaluate, and keep the same summary/baseline assertions; then the new production seams can be removed.

[zpzjzj]

The current-invocation-only contract is now documented clearly, but the requested auto-mode coverage is not present yet. TestRunner_Evaluate_SingleIterationDoesNotPrintStabilitySummary uses Iteration: 1; it does not exercise the default Iteration: 0 path or an existing iteration-* workspace. Please add or adjust a test that creates an existing iteration directory, runs with Iteration: 0, verifies the next iteration is appended, and asserts that no historical stability summary is printed. This directly locks in the newly documented auto-append contract.

[googs1025]

Updated the PR branch to 984ea63 for the latest review comments.

Changes in this update:

• Removed the production uiBlank / uiStatus test seams and restored direct ui.Blank / ui.Status calls.
• Updated the runner test to capture the real internal/ui.Output path while keeping the summary and baseline-exclusion assertions.
• Reworked the no-summary test to exercise Iteration: 0 auto mode with an existing iteration-1/, verifying it appends iteration-2/result.json and does not print a historical stability summary.

[zpzjzj]

Re-reviewed 984ea63. The two previously outstanding behavior items are now addressed: the tests capture the real UI output path, and auto mode covers an existing iteration-1/, appends iteration-2/result.json, and prints no historical summary. The stability implementation and documented current-command-only contract otherwise look consistent.

I left one focused inline request: replace the new //go:linkname + unsafe access with a normal import of the already-exported internal/ui.Output. The direct-import version passes go test -race ./internal/runner.

Local verification on this HEAD: make verify, make test, and the focused runner race test all pass. Remote CI/Docs/CodeQL remain action_required with no jobs started, so they still need maintainer approval before their results can be evaluated.

Overall: ⚠️ conditionally mergeable after the inline test-cleanup request and real remote checks.

[zpzjzj]

LGTM