Skip to content

Map missing agentic replay and video quality eval fields reported by Shield drift check - #159

Merged
dcvijeticTT merged 8 commits into
tenstorrent:mainfrom
dcvijeticTT:fix/shield-missing-metrics-2
Oct 8, 2026
Merged

dcvijeticTT merged 8 commits into
tenstorrent:mainfrom
dcvijeticTT:fix/shield-missing-metrics-2

Conversation

@dcvijeticTT

@dcvijeticTT dcvijeticTT commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Maps keys that Shield metric drift alerts reported as unmapped:

  • Agentic replay (Kimi, GLM, Llama): 12 new benchmark metrics (*_e2e_norm_intvty, p95_isl/p95_osl, p99_adj_*, prefix_cache_*_tokens_measured) and 2 new config fields (repetition, trace_idle_gap_cap_seconds)
  • MiniMax-H3 video quality eval (51 keys): summary.* counts and elapsed_seconds as eval metrics. Settings, thresholds and per-sample detailed_results as config.
  • Speculative decoding sweep (Kimi-K2.7-Code, Qwen3-VL-32B, gemma-4-31B): route kind: aiperf_spec_decode sections as benchmarks. Add acceptance_rate, mean_accepted_length and output_throughput as metrics and output_len as config. Use public_dataset as the dataset name.
  • Target checks (gemma-4-12B/31B, Qwen3.8-27B, MiniMax-M3): tput_input, tput_input_ratio, tput_input_check

run_type change: agentic_traces and aiperf_spec_decode rows now get their own run_type (same as the section kind) instead of benchmark. They reuse LLM metric names such as mean_ttft_ms for a different workload, so they no longer mix with token sweeps in consumers that filter on run_type = 'benchmark'. Token sweeps (vllm, benchmarks) stay benchmark, evals stay eval, measurements keep step_name = 'benchmark', and rows keep config_params.report_section_kind.

Agentic rows already stored (80 rows in 51 runs since 2026-08-31, all stamped report_section_kind = 'agentic_traces') stay under benchmark, because sw_test is insert-only. After this merges, a tt-shield backfill re-collects those runs (plus past spec-decode runs) under the new run_type values. Old copies can be excluded from benchmark queries with report_section_kind IS DISTINCT FROM 'agentic_traces'.

Adds regression tests.

Copilot AI lite review requested due to automatic review settings September 24, 2026 11:00
@dcvijeticTT
dcvijeticTT requested a review from a team as a code owner September 24, 2026 11:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

All reviewed changes have regression coverage and no unresolved issues were identified.

Review effort: Lite
Findings: None

What changed in this PR

Maps 12 newly reported agentic replay metrics and 2 configuration fields, with regression coverage.

Changes:

  • Adds 12 benchmark metric mappings.
  • Adds repetition and trace_idle_gap_cap_seconds.
  • Adds regression assertions for all 14 fields.
File Description
.github/​actions/​collect_data/​test/​test_benchmark_mapper.py Verifies all 14 fields are mapped correctly.
.github/​actions/​collect_data/​src/​benchmark.py Registers the new metrics and configuration fields.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI review requested due to automatic review settings September 25, 2026 09:45
@dcvijeticTT dcvijeticTT changed the title Map missing agentic replay metrics reported by Shield drift check Map missing agentic replay and video quality eval fields reported by Shield drift check Sep 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

All reviewed changes have regression coverage and no unresolved blocking issues.

Review effort: Lite
Findings: None

Copilot AI lite review requested due to automatic review settings October 6, 2026 11:00
@dcvijeticTT dcvijeticTT closed this Oct 6, 2026
@dcvijeticTT dcvijeticTT reopened this Oct 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Two moderate mapping gaps remain for clip_enabled and section-level targets settings.

Review effort: Lite
Findings: 1 Medium severity

Open (1)

Comment thread .github/actions/collect_data/src/benchmark.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 6, 2026 11:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

evaluation_type remains unmapped, and regression coverage should assert clip_enabled.

Review effort: Lite
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Add evaluation_type to eval config mappings and regression coverage

.github/​actions/​collect_data/​src/​benchmark.py:431

evaluation_type is a run setting in the new video-quality payload (video_quality_eval.json:14), but it is absent from both the eval metric list and _CONFIG_FIELDS_BY_STEP["eval"]. Because these declarations are the drift-checker whitelist, this field remains unmapped and the reported drift is not fully resolved; add it as eval configuration and cover it in the regression assertion.

Copilot AI lite review requested due to automatic review settings October 8, 2026 08:14
@dcvijeticTT
dcvijeticTT requested a review from a team as a code owner October 8, 2026 08:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Additional video evaluation settings are not preserved by the configuration whitelist.

0 open findings

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

… drift check

- Route kind:aiperf_spec_decode sections (Kimi-K2.7-Code, Qwen3-VL-32B,
  gemma-4-31B) as benchmarks; add acceptance_rate, mean_accepted_length
  and output_throughput metrics, output_len as config, and use
  public_dataset as the dataset name.
- Add tput_input, tput_input_ratio and tput_input_check target checks
  (gemma-4-12B/31B, Qwen3.8-27B, MiniMax-M3).
Copilot AI lite review requested due to automatic review settings October 8, 2026 08:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Video-evaluation settings and speculative-decoding dataset precedence still need correction.

0 open findings

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Both sections reuse LLM metric names (mean_ttft_ms, ...) for a different
workload, so under run_type 'benchmark' they mix with token sweeps in
consumers that filter on it. Token sweeps (vllm, benchmarks) stay
'benchmark' and evals stay 'eval'; measurements keep step_name
'benchmark' and rows keep config_params.report_section_kind.
Copilot AI lite review requested due to automatic review settings October 8, 2026 11:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved blocking issues were identified.

0 open findings

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

_SECTION_RUN_TYPES now lists vllm, benchmarks and evals too, and both
_process_benchmarks and _process_evals read it, instead of a silent
'benchmark' default plus a hardcoded 'eval'. A test requires its keys to
match _SECTION_METRIC_STEPS, so a newly routed kind cannot fall into
'benchmark' unnoticed. Legacy top-level lists have no section kind and
keep 'benchmark' / 'eval'. Stored rows are unchanged.
Copilot AI lite review requested due to automatic review settings October 8, 2026 11:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Two moderate mapping issues remain unresolved.

0 open findings

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@dcvijeticTT
dcvijeticTT requested a review from mjeremicTT October 8, 2026 11:58
@dcvijeticTT
dcvijeticTT merged commit d48ae54 into tenstorrent:main Oct 8, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants