feat(eval)!: add eval measurements and EM-001 trace_fitness scorer by ascerra · Pull Request #6036 · fullsend-ai/fullsend

ascerra · 2026-08-10T11:44:37Z

Summary

Introduce eval measurements: fail-open same-job scoring of wild agent OTEL traces (fullsend eval-measure), writing portable eval-measurements.jsonl beside telemetry when at least one new score is produced. First scorer is trace_fitness (EM-001). Decision recorded in ADR 0087.

Companion default-policy PR: fullsend-ai/agents#722 (manifests under eval/measurements/).

Activation is two-step: merge agents#722 and cut a v0.x.y release that re-points floating agents@v0. Until that release lands, managed GHA/GitLab measure steps stay provisional (clean skip when the remote manifest is missing). Tracking: #6384. Local FULLSEND_DIR overrides work today.

Breaking change

GitLab fullsend-agent scaffold default --output-dir now writes under ${CI_PROJECT_DIR}/output (was an ephemeral tmp path) so eval-measurements.jsonl can be retained as a CI artifact. Re-sync adds top-level output/, excludes it from the sandbox upload/git when nested under --target-repo, and keeps telemetry as a 1-week artifact. Migrate by re-syncing the scaffold (or pin an older scaffold if you must keep the tmp layout).

Ownership (please read)

Concern	Repo
Parser, scorer implementations, CLI, GHA/GitLab post-step	this PR (`internal/evalmeasure/`)
Default manifests for stock agents	agents#722
Org overrides / BYOA manifests	Consumer `FULLSEND_DIR`

Stock-agent defaults are SHA-pinned from fullsend-ai/agents@v0 when no local file exists — installs do not copy manifests to score stock agents.
Local ${FULLSEND_DIR}/eval/measurements/${AGENT}.yaml is override / opt-out / custom-agent only.
Executable logic stays in fullsend because eval-measure is the released binary that reads run-telemetry.jsonl (which fullsend writes). Agents is content/policy, not that binary.
EM-001 is a platform fitness check on the Level 1/2 telemetry metadata contract; stock agents enable it via agents manifests.
Planned: content-aware scorers on Level 3 prompt/completion capture once Level 3 is implemented — that is where quality judgments live. Metadata fitness is the foundation first.
Change guide: new Go scorer or (future) declarative assert: → fullsend PR; new id / enable / thresholds on an existing scorer for a stock agent → agents-only; org-specific policy → local override.
Planned (not in this PR): declarative logic-as-config in manifests so most agent-specific policy is YAML-only.

Tool-agnostic export

Core does not pick an observability product. Scores land in local eval-measurements.jsonl. Remote score export (when implemented) reuses the same OTEL_EXPORTER_OTLP_* path as ADR 0050. No vendor Assessments adapters or MLFLOW_* (or similar) wiring in managed workflows.

Changes

ADR 0087 + guides (ownership, two-step v0 activation, declarative sketch, planned L3 content scorers), glossary / tracing cross-links
internal/evalmeasure parser + trace_fitness + local JSONL/ledger
fullsend eval-measure CLI; fail-open post-step in action.yml and GitLab fullsend-agent.yml
Manifest resolution: local FULLSEND_DIR then SHA-pinned agents@v0 fetch (--offline supported)
Platform telemetry discovery prefers host agent-<name>-… runDirs; EM-001 skips incomplete/runner-health traces (no agent span, missing root run span, pre-script skip)
GitLab: always export empty-or-real GITLAB_ISSUE_URL; keep host output/ out of sandbox tarball + git exclude

Testing

go test ./internal/evalmeasure/ (+ focused CLI / sandbox / scaffold tests)
Local FULLSEND_DIR manifest path produces eval-measurements.jsonl
pre-commit hooks on commit
Stock manifests on agents@v0 after merge + release cut (agents#722, #6384)

Optional dogfood (outside core): post-process the portable JSONL into an org-chosen backend. Example MLflow UI showing trace_fitness assessments — not core MLFLOW_* wiring:

Checklist

PR title follows Conventional Commits
Commits are signed off (DCO)
I wrote this contribution myself and can explain all changes in it

Notes for reviewers

Portable OTLP score export is the ADR remote contract but not wired yet (local JSONL today).
CLI flag remains --registry (path to the YAML); rename to “manifest” is follow-up.
Explainer HTML kept local / out of this PR.
Managed measure steps are provisional until manifests exist on floating v0 (Track agents@v0 release cut for eval measurement manifests #6384).

fullsend-ai-review · 2026-08-10T11:45:52Z

🤖 Finished Review · ✅ Success · Started 11:45 AM UTC · Completed 12:02 PM UTC

Commit: fdf5632 · View workflow run →

qodo-code-review · 2026-08-10T11:46:19Z

PR Summary by Qodo

Add eval measurements CLI and EM-001 trace_fitness scoring

✨ Enhancement 📝 Documentation 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add fail-open, same-job trace scoring that writes eval-measurements.jsonl beside telemetry.
• Introduce EM-001 trace_fitness scorer driven by per-agent measurement manifests.
• Document architecture/ownership via ADR 0087 and new operator guide.

Diagram

graph TD
  A["GitHub Action job"] --> B["fullsend run"] --> C[("run-telemetry.jsonl")] --> G{{"Resolve manifest"}} --> H["registry.yaml"] --> D["fullsend eval-measure"] --> E[("eval-measurements.jsonl")]
  E --> F["upload-artifact"]
  G --> I["skip scoring"]
  subgraph Legend
    direction LR
    _proc["Process"] ~~~ _file[("Artifact")] ~~~ _dec{{"Decision"}}
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Embed stock manifests into the fullsend binary

➕ No network fetch from agents@v0; works offline in CI
➕ Eliminates runtime dependency on a pinned agents ref
➖ Couples policy/content updates to fullsend releases
➖ Harder for agents repo to evolve defaults independently
➖ Increases binary churn and complicates ownership boundaries

2. Export measurements to OTLP immediately (instead of JSONL-first)

➕ Immediate backend dashboards without additional ingestion work
➕ Single ingestion path for traces and scores
➖ Forces OTLP exporter configuration correctness for scoring to be useful
➖ Increases surface area/risk for first ship (auth, batching, schema)
➖ Conflicts with the stated tool-agnostic requirement if not done carefully

3. Make manifests fully declarative now (YAML asserts, no Go scorers)

➕ Faster iteration for per-agent policy without code changes
➕ Reduces need for many tiny scorer implementations
➖ Requires designing a stable DSL/runtime early
➖ Harder to express complex multi-span logic safely at first
➖ Higher risk for correctness and backwards compatibility

Recommendation: Proceed with the PR’s approach: JSONL-first portable outputs + fail-open same-job scoring + agent-owned enablement manifests. This cleanly separates engine (fullsend) from policy (agents/local overrides), avoids prematurely committing to an OTLP score export schema, and provides an always-available artifact (eval-measurements.jsonl) for any downstream system.

Files changed (31) +1831 / -4

Enhancement (8) +818 / -0

evalmeasure.goAdd 'fullsend eval-measure' Cobra command +71/-0

Add 'fullsend eval-measure' Cobra command

• Implements the eval-measure CLI command, validates required flags, calls the evalmeasure engine, and prints pass/warn results without gating (exit 0 on failed scores).

internal/cli/evalmeasure.go

root.goRegister eval-measure subcommand on root CLI +1/-0

Register eval-measure subcommand on root CLI
• Wires the new eval-measure command into the CLI root so it is available in the released binary.
internal/cli/root.go

export_local.goImplement local JSONL export and idempotency ledger +83/-0

Implement local JSONL export and idempotency ledger
• Adds portable 'eval-measurements.jsonl' append logic and a simple ledger file used to ensure per-trace per-measurement idempotency.
internal/evalmeasure/export_local.go

fitness.goAdd EM-001 trace_fitness scorer implementation +190/-0

Add EM-001 trace_fitness scorer implementation

• Implements a trace fitness score over expected span tree and key attributes (identity, work item, model, usage, cost/tools/turns, exit presence). Produces a pass only when all checks pass; otherwise emits a detailed explanation.

internal/evalmeasure/fitness.go

parse.goParse run-telemetry.jsonl OTLP JSON lines into trace/span model +187/-0

Parse run-telemetry.jsonl OTLP JSON lines into trace/span model
• Adds a minimal OTLP TracesData JSON parser that merges spans by trace ID and extracts scalar attributes into a portable in-memory representation, with increased scanner buffer limits.
internal/evalmeasure/parse.go

registry.goLoad measurement manifest YAML and dispatch scorers +81/-0

Load measurement manifest YAML and dispatch scorers

• Introduces Registry/MeasurementSpec types, validation rules, and scorer dispatch. Only runs measurements when the trace agent matches the manifest agent; unknown scorers are skipped for forward compatibility.

internal/evalmeasure/registry.go

run.goOrchestrate parse → score → append JSONL with ledger idempotency +58/-0

Orchestrate parse → score → append JSONL with ledger idempotency
• Implements the main engine function to parse telemetry, load the registry, score matching traces, append results to JSONL, and then record ledger entries (persist-first ordering).
internal/evalmeasure/run.go

types.goDefine portable Span/Trace/EvaluationResult types and helpers +147/-0

Define portable Span/Trace/EvaluationResult types and helpers
• Introduces core data model for parsed spans and emitted measurement results, plus helpers for typed attribute access, durations, span lookup, and agent identity extraction.
internal/evalmeasure/types.go

Tests (13) +627 / -0

evalmeasure_test.goAdd CLI tests for eval-measure scoring and flag requirements +56/-0

Add CLI tests for eval-measure scoring and flag requirements
• Adds tests that run the command against fixtures and assert eval-measurements.jsonl output, verifies subcommand registration, and checks missing-required-flag behavior.
internal/cli/evalmeasure_test.go

export_local_test.goTest local export and ledger behavior +67/-0

Test local export and ledger behavior
• Covers empty writes, parent directory creation, ledger miss/hit semantics, and ledger file creation.
internal/evalmeasure/export_local_test.go

parse_test.goTest telemetry parsing, merging, and error handling +54/-0

Test telemetry parsing, merging, and error handling
• Adds tests for successful parsing, merging split traces across lines, invalid JSON line errors, and missing file behavior.
internal/evalmeasure/parse_test.go

registry_test.goTest manifest loading validation and error cases +86/-0

Test manifest loading validation and error cases
• Covers valid manifests and multiple failure cases (missing agent/id/scorer, invalid version, illegal characters, invalid YAML, missing file).
internal/evalmeasure/registry_test.go

run_test.goTest end-to-end scoring, idempotency, and cancellation +96/-0

Test end-to-end scoring, idempotency, and cancellation
• Adds integration-style tests for scoring via registry, idempotency across runs, persistence ordering (append before ledger), and error/cancelled-context behavior.
internal/evalmeasure/run_test.go

score_test.goTest trace_fitness pass/fail scenarios and scorer dispatch behavior +72/-0

Test trace_fitness pass/fail scenarios and scorer dispatch behavior
• Validates complete pass output, failure when required attributes are missing, work-item sentinel handling, agent mismatch behavior, and skipping unknown scorers.
internal/evalmeasure/score_test.go

README.mdDocument evalmeasure test fixtures purpose and ownership +6/-0

Document evalmeasure test fixtures purpose and ownership
• Explains that testdata JSONL and sample registry are synthetic fixtures and that production manifests live in the agents repository.
internal/evalmeasure/testdata/README.md

complete.jsonlAdd complete OTLP trace fixture for passing fitness +1/-0

Add complete OTLP trace fixture for passing fitness
• Provides a single-line OTLP JSON trace fixture with expected spans/attributes used by parser and scoring tests.
internal/evalmeasure/testdata/complete.jsonl

missing-cost.jsonlAdd OTLP fixture missing cost attribute for negative coverage +1/-0

Add OTLP fixture missing cost attribute for negative coverage
• Adds a variant fixture omitting cost fields to ensure trace_fitness fails appropriately.
internal/evalmeasure/testdata/missing-cost.jsonl

review-unknown-workitem.jsonlAdd OTLP fixture with unknown work item sentinel +1/-0

Add OTLP fixture with unknown work item sentinel
• Adds a fixture representing a review run where work_item_id is "unknown" to exercise work item fitness logic.
internal/evalmeasure/testdata/review-unknown-workitem.jsonl

sample-registry.yamlAdd sample measurement manifest fixture +5/-0

Add sample measurement manifest fixture
• Adds a minimal registry YAML enabling em-001/trace_fitness for triage, used by unit and CLI tests.
internal/evalmeasure/testdata/sample-registry.yaml

split.jsonlAdd split-line OTLP trace fixture for merge behavior +2/-0

Add split-line OTLP trace fixture for merge behavior
• Adds a multi-line fixture with the same trace split across lines to validate trace merging logic.
internal/evalmeasure/testdata/split.jsonl

types_test.goTest attribute coercion, duration math, and trace helpers +180/-0

Test attribute coercion, duration math, and trace helpers
• Adds unit tests for AttrString/AttrInt/AttrFloat conversions, DurationSeconds, span lookup helpers, and agent identity extraction priority.
internal/evalmeasure/types_test.go

Documentation (9) +340 / -4

config.tsExpose Eval Measurements guide in docs navigation +1/-0

Expose Eval Measurements guide in docs navigation
• Adds an "Eval Measurements" entry to the infrastructure guides sidebar for discoverability.
docs/.vitepress/config.ts

0050-distributed-tracing-instrumentation.mdCross-link distributed tracing ADR to eval measurements ADR +8/-0

Cross-link distributed tracing ADR to eval measurements ADR
• Records the follow-on decision that online trace scoring writes eval-measurements.jsonl and (planned) reuses OTLP configuration for remote export.
docs/ADRs/0050-distributed-tracing-instrumentation.md

0087-eval-measurements-online-trace-scoring.mdAdd ADR 0087: online trace scoring and tool-agnostic export +114/-0

Add ADR 0087: online trace scoring and tool-agnostic export

• Introduces an accepted ADR defining eval measurements, fail-open same-job scoring, ownership split (fullsend engine vs agents manifests), and per-measurement versioning (id@version). Establishes EM-001 trace_fitness as the first scorer and documents planned OTLP export alignment with ADR 0050.

docs/ADRs/0087-eval-measurements-online-trace-scoring.md

architecture.mdDocument eval measurements as an observability artifact +4/-0

Document eval measurements as an observability artifact
• Adds eval measurements to the observability architecture section, including the planned OTLP export direction and linking to ADR 0087 and the new guide.
docs/architecture.md

glossary.mdClarify terminology for eval measurements vs eval scenarios +4/-4

Clarify terminology for eval measurements vs eval scenarios

• Refines glossary definitions to distinguish online/trend measurements on wild traces from curated functional eval fixtures, with direct links to ADR 0087/0051 and the guide.

docs/glossary.md

README.mdList Eval Measurements guide under infrastructure guides +1/-0

List Eval Measurements guide under infrastructure guides
• Adds a new entry pointing operators to the eval measurements guide.
docs/guides/README.md

cli-internals.mdDocument new eval-measure CLI subcommand and flags +4/-0

Document new eval-measure CLI subcommand and flags
• Adds 'fullsend eval-measure' to the CLI internals reference, including telemetry, registry, and output directory flags.
docs/guides/dev/cli-internals.md

distributed-tracing.mdAdd section describing eval measurements alongside tracing +12/-0

Add section describing eval measurements alongside tracing
• Documents that eval-measure runs after each managed run, produces eval-measurements.jsonl, and will reuse OTEL_EXPORTER_OTLP_* for portable remote export when implemented.
docs/guides/infrastructure/distributed-tracing.md

eval-measurements.mdAdd operator guide for eval measurements, manifests, and CLI usage +192/-0

Add operator guide for eval measurements, manifests, and CLI usage

• Introduces a full guide covering prerequisites, artifacts, ownership boundaries, manifest resolution (local override vs agents@v0), and usage of 'fullsend eval-measure'. Includes a future-facing declarative manifest sketch and clarifies fail-open semantics.

docs/guides/infrastructure/eval-measurements.md

Other (1) +46 / -0

action.ymlAdd fail-open eval-measure post-step with manifest resolution +46/-0

Add fail-open eval-measure post-step with manifest resolution

• Adds a GitHub Actions step that locates run-telemetry.jsonl, resolves an agent measurement manifest (local FULLSEND_DIR override or agents@v0 fetch), and runs 'fullsend eval-measure'. The step is 'continue-on-error' so measurement failures never fail the agent job.

action.yml

github-actions · 2026-08-10T11:46:33Z

Site preview

Preview: https://5a1aca36-site.fullsend-ai.workers.dev

Commit: 30564ef2a65df0b64cb70af8f6bb1884f72640ab

qodo-code-review · 2026-08-10T11:52:16Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (1)

1. ~~Ledger suppresses failed writes~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

MeasureAndExport calls RecordScored before AppendMeasurements, so if appending
eval-measurements.jsonl fails after the ledger write succeeds, the measurement is permanently
marked as done and will be skipped on future runs. This can silently lose measurement data and
prevent recovery without manual ledger repair.

Code

internal/evalmeasure/run.go[R44-48]

+			if err := RecordScored(ledgerPath, r.TraceID, r.Name, r.Version); err != nil {
+				return all, fmt.Errorf("record scored: %w", err)
+			}
+			if err := AppendMeasurements(measPath, []EvaluationResult{r}); err != nil {
+				return all, fmt.Errorf("append measurements: %w", err)

Relevance

●●● Strong
Clear reliability bug: ledger should not mark done before successful write; likely fixed.
PR-#1682

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code checks AlreadyScored against the ledger, then records the ledger entry and only afterward
writes the JSONL. Any write failure after recording the ledger will cause permanent suppression on
subsequent runs.

internal/evalmeasure/run.go[33-49]
internal/evalmeasure/export_local.go[47-65]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The idempotency ledger entry is recorded before the measurement is appended to `eval-measurements.jsonl`. If the append fails (e.g., ENOSPC/EIO/permission), the ledger still indicates success, preventing future re-writes.

## Issue Context
Idempotency should reflect successful persistence of the measurement record.

## Fix Focus Areas
- internal/evalmeasure/run.go[33-49]

## Suggested fix
Swap the order so `AppendMeasurements(...)` happens first, then `RecordScored(...)` only after append succeeds. Optionally, batch appends and then record ledger entries after the batch is flushed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Guides README not updated~~ ✓ Resolved 📜 Skill insight ⚙ Maintainability

Description

This PR adds a new guide under docs/guides/ but does not update docs/guides/README.md to index
it. This makes the guides index incomplete.

Code

docs/guides/infrastructure/eval-measurements.md[1]
+# Eval Measurements

Relevance

●●● Strong
Updating guides index/README when adding a new guide has clear accepted precedent.
PR-#5778

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The checklist requires updating docs/guides/README.md when adding any new guide under
docs/guides/. The PR adds eval-measurements.md as a new guide but does not include a
corresponding README index update in the diff.

docs/guides/infrastructure/eval-measurements.md[1-1]
Skill: writing-user-docs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new guide file was added under `docs/guides/`, but the guides index file `docs/guides/README.md` was not updated to include it.

## Issue Context
The index is the primary entry point for discovering guides; missing entries cause documentation drift.

## Fix Focus Areas
- docs/guides/infrastructure/eval-measurements.md[1-1]
- docs/guides/README.md[1-200]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Broken curl header args~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The Eval measurements step builds curl args via `${GH_TOKEN:+-H "Authorization: Bearer
${GH_TOKEN}"}`, but quotes produced by parameter expansion are not shell syntax and get passed
literally/word-split, so curl can fail whenever GH_TOKEN is non-empty and the remote manifest
fetch is needed. This causes eval measurement scoring to skip even when the agents manifest exists
upstream.

Code

action.yml[R500-503]

+          URL="https://raw.githubusercontent.com/fullsend-ai/agents/v0/eval/measurements/${AGENT}.yaml"
+          TMP="$(mktemp)"
+          if curl -fsSL ${GH_TOKEN:+-H "Authorization: Bearer ${GH_TOKEN}"} -o "${TMP}" "${URL}"; then
+            REGISTRY="${TMP}"

Relevance

●●● Strong
Shell quoting/arg-splitting robustness issues are routinely fixed; low-risk correctness fix.
PR-#2106

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR-added curl invocation relies on quotes embedded in a parameter expansion; since those quotes
are not interpreted as quoting, the -H value is split/broken and curl can fail, preventing remote
manifest retrieval.

action.yml[494-503]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`action.yml` uses bash parameter expansion to conditionally add a `curl -H "Authorization: Bearer …"` header. Because bash does not re-parse quotes introduced via expansion, the header is word-split into multiple argv entries and curl can fail when `GH_TOKEN` is set.

## Issue Context
This breaks the remote fallback manifest download path, which is the primary path until local manifests are present.

## Fix Focus Areas
- action.yml[500-503]

## Suggested fix
Replace the expansion with a safe conditional (or an argv array), e.g.:

```bash
CURL_ARGS=( -fsSL -o "${TMP}" "${URL}" )
if [[ -n "${GH_TOKEN:-}" ]]; then
 CURL_ARGS=( -fsSL -H "Authorization: Bearer ${GH_TOKEN}" -o "${TMP}" "${URL}" )
fi
if curl "${CURL_ARGS[@]}"; then
 REGISTRY="${TMP}"
else
 ...
fi
```

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View high (3)

4. ~~Guide missing prerequisites section~~ ✓ Resolved 📜 Skill insight ✧ Quality

Description

docs/guides/infrastructure/eval-measurements.md includes procedural CLI usage without a clearly
labeled Prerequisites section before the procedure. This violates the guide structure requirement.

Code

docs/guides/infrastructure/eval-measurements.md[R115-122]

+## CLI
+
+```bash
+fullsend eval-measure \
+  --telemetry path/to/run-telemetry.jsonl \
+  --registry path/to/agents/eval/measurements/review.yaml \
+  --out-dir path/to/output
+```

Relevance

●●● Strong
Docs guides commonly add explicit Prerequisites before procedures; precedent accepted.
PR-#2277
PR-#2663

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The checklist requires each documentation guide to include a prerequisites section before procedural
steps. The ## CLI section introduces how to run fullsend eval-measure without any preceding `##
Prerequisites` section.

docs/guides/infrastructure/eval-measurements.md[115-122]
Skill: writing-user-docs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new guide contains procedural instructions (CLI invocation) but does not include a clearly labeled `## Prerequisites` section before those steps.

## Issue Context
Compliance requires prerequisites to appear before step 1 of any procedure in guides.

## Fix Focus Areas
- docs/guides/infrastructure/eval-measurements.md[115-122]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~ADR 0087 multiple decisions~~ ✓ Resolved 📜 Skill insight ⚙ Maintainability

Description

ADR 0087 records multiple distinct decisions (mechanism, ownership, persistence, remote export,
first scorer, versioning) inside one ADR. This violates the requirement that each ADR record exactly
one decision.

Code

docs/ADRs/0087-eval-measurements-online-trace-scoring.md[R65-68]

+Introduce **eval measurements**: deterministic scorers that read
+`run-telemetry.jsonl` after `fullsend run` in the **same** managed job
+(`fullsend eval-measure` in `action.yml`), **fail-open**.
+

Relevance

●● Moderate
ADR splitting/“one decision” enforcement is subjective; no close accepted/rejected precedent found.
PR-#2743

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The checklist requires an ADR to contain exactly one decision. The Decision section in ADR 0087
introduces the core mechanism and then adds several additional decisions as separate bolded
sub-items, indicating multiple decisions in one ADR.

docs/ADRs/0087-eval-measurements-online-trace-scoring.md[65-75]
Skill: writing-adrs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ADR 0087` includes multiple distinct decisions within the Decision section. Compliance requires each ADR to record exactly one decision.

## Issue Context
The current Decision section contains multiple bolded sub-decisions (e.g., Ownership, Persistence, Remote export, First scorer, Versioning).

## Fix Focus Areas
- docs/ADRs/0087-eval-measurements-online-trace-scoring.md[65-88]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. ~~Planned OTLP noted without callout~~ ✓ Resolved 📜 Skill insight ≡ Correctness

Description

The docs describe a not-yet-implemented OTLP export path as "planned" without using the required `>
**Planned:**` callout format and without an issue link. This can mislead readers about current vs
future behavior.

Code

docs/architecture.md[290]

+- Eval measurements: fail-open same-job scoring of wild-run traces into `eval-measurements.jsonl` beside telemetry; portable remote score export is designed to use the same OTLP configuration as agent traces (MLflow Assessments adapter available now; OTLP path planned); backend-specific UI adapters are optional ([ADR 0087](ADRs/0087-eval-measurements-online-trace-scoring.md)). See [Eval Measurements](guides/infrastructure/eval-measurements.md).

Relevance

●● Moderate
Planned-feature callout/link rule seems inconsistently enforced; similar issue-link add was
rejected.
PR-#3903

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The checklist requires any mention of planned features to use the > **Planned:** callout format
and include an issue link. The added architecture line explicitly says the OTLP path is "planned"
but provides neither the callout format nor an issue link.

docs/architecture.md[290-290]
Skill: writing-user-docs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Documentation mentions a planned/not-yet-implemented feature without the required `> **Planned:**` callout format and without linking to a tracking issue.

## Issue Context
The checklist requires planned features to be clearly marked and traceable to an issue.

## Fix Focus Areas
- docs/architecture.md[290-290]
- docs/guides/infrastructure/eval-measurements.md[145-149]
- docs/ADRs/0087-eval-measurements-online-trace-scoring.md[80-83]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

7. Ledger keys on name 🐞 Bug ≡ Correctness ⭐ New

Description

MeasureAndExport uses r.Name (which can be overridden via manifest measurements[].name) as part
of the ledger key, so renaming a measurement without changing id@version causes the same trace to
be re-scored and appended again. This breaks idempotency and can duplicate eval-measurements.jsonl
rows for the same (trace_id, id@version).

Code

internal/evalmeasure/run.go[R40-41]

+			done, err := AlreadyScored(ledgerPath, r.TraceID, r.Name, r.Version)
+			if err != nil {

Relevance

●● Moderate
Seems like a real idempotency bug, but docs imply ledger intentionally keys on name; could be design
choice.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
MeasureAndExport passes r.Name into ledger lookups/writes, while registry allows overriding
Name independent of ID/version; since the ledger key includes the name string, cosmetic renames
change idempotency behavior and can create duplicates.
internal/evalmeasure/run.go[37-54]
internal/evalmeasure/export_local.go[43-45]
internal/evalmeasure/registry.go[59-75]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The idempotency ledger key currently includes `EvaluationResult.Name`, but `Name` can be a cosmetic display override (`measurements[].name`). This means changing only the display name invalidates the ledger entry and re-appends the same measurement for the same trace and `id@version`.

## Issue Context
- `MeasurementSpec.Name` is documented/implemented as an optional display override.
- `MeasurementSpec.versionString()` already produces the stable `id@version` identity.
- The ledger should key on stable measurement identity (e.g., `trace_id` + `id@version`), not presentation name.

## Fix Focus Areas
- internal/evalmeasure/run.go[37-53]
- internal/evalmeasure/export_local.go[43-45]
- internal/evalmeasure/registry.go[55-64]
- internal/evalmeasure/types.go[27-38]

### Suggested approach
- Change ledger key schema to drop `evalName` from the key, e.g. `traceID + "|" + version`.
- Alternatively, add a stable `MeasurementID` field to `EvaluationResult` (or pass `m.ID` through scoring) and key the ledger on `(traceID, measurementID@version)`.
- Update `AlreadyScored`/`RecordScored` call sites accordingly and adjust tests if needed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

8. ~~Export errors not fail-open~~ ⊘ Outdated 🐞 Bug ☼ Reliability

Description

ExportMLflowAssessments returns raw errors from trackingURIFromEnv() instead of wrapping them in
ExportError, so the CLI won’t classify them as warnings and fullsend eval-measure can exit
non-zero on export-related misconfiguration. This violates the documented “export failures warn and
exit 0” behavior.

Code

internal/evalmeasure/export_mlflow.go[R67-70]
+	base, err := trackingURIFromEnv()
+	if err != nil {
+		return err
+	}

Relevance

●●● Strong
Matches repo pattern: export-related misconfig should warn/fail-open, not abort CLI.
PR-#1682
PR-#1573

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The MLflow exporter returns a non-ExportError on URI derivation failure, while the CLI only treats
*ExportError as a warning; therefore this export failure path becomes fatal to the CLI command.
internal/evalmeasure/export_mlflow.go[61-70]
internal/cli/evalmeasure.go[42-50]
internal/cli/evalmeasure.go[64-67]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`trackingURIFromEnv()` derivation/parsing failures are export-related but are returned as plain errors, bypassing the CLI’s `ExportError` downgrade path.

## Issue Context
The CLI checks `errors.As(err, *ExportError)` to decide whether to warn-and-exit-0 vs fail. Export-related errors should consistently be `ExportError`.

## Fix Focus Areas
- internal/evalmeasure/export_mlflow.go[65-70]
- internal/cli/evalmeasure.go[42-49]

## Suggested fix
Change:
```go
base, err := trackingURIFromEnv()
if err != nil { return err }
```
To:
```go
base, err := trackingURIFromEnv()
if err != nil { return &ExportError{Err: err} }
```
(or, if you want explicit-vs-derived semantics, only wrap errors on the derived path but still keep the CLI behavior consistent with docs).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

9. ~~OTLP/OTEL jargon undefined~~ ✓ Resolved 📜 Skill insight ✧ Quality

Description

The new guide introduces jargon/acronyms (e.g., OTLP, OTEL_EXPORTER_OTLP_*) without an inline
definition or a glossary link on first use. This reduces readability for new users.

Code

docs/guides/infrastructure/eval-measurements.md[R19-22]

+  └─ always writes  output/**/run-telemetry.jsonl
+  └─ if OTEL_EXPORTER_OTLP_* set → live OTLP export of agent spans
+       (any compatible backend — ADR 0050)
+

Relevance

●●● Strong
Docs feedback to define terms/acronyms on first use has been accepted in similar guides.
PR-#5778

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The checklist requires jargon to be defined on first use via a glossary link or inline definition.
The guide uses OTEL_EXPORTER_OTLP_*/OTLP terminology immediately in the architecture diagram
without defining it or linking to the glossary.

docs/guides/infrastructure/eval-measurements.md[19-22]
Skill: writing-user-docs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The guide uses domain jargon/acronyms on first use without defining them inline or linking to `docs/glossary.md`.

## Issue Context
The checklist requires jargon to be defined on first use to keep guides accessible.

## Fix Focus Areas
- docs/guides/infrastructure/eval-measurements.md[19-28]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View medium (2)

10. ~~ADR 0050 edit not called out~~ ✗ Dismissed 📘 Rule violation § Compliance

Description

An already-accepted ADR (ADR 0050) was modified, but the PR description does not explicitly
mention ADR 0050 and summarize the change. This makes review and auditing of decision-history
edits harder.

Code

docs/ADRs/0050-distributed-tracing-instrumentation.md[R154-159]

+**2026-08-10 — Eval measurements ([ADR 0087](0087-eval-measurements-online-trace-scoring.md)):**
+online scoring of wild-run traces writes `eval-measurements.jsonl` beside
+telemetry; portable remote score export is designed to follow the same OTLP
+configuration as this ADR (MLflow Assessments adapter available now; OTLP
+path planned). Backend-specific UI adapters remain optional. Distinct from
+functional eval fixtures ([ADR 0051](0051-agent-eval-harness-for-test-infrastructure.md)).

Relevance

●● Moderate
They avoid silent edits to accepted ADRs, but no precedent for PR-description callout requirement.
PR-#5244

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist requires explicit PR-description callouts when an accepted ADR is edited. The diff
shows a substantive new annotation added to accepted ADR 0050, triggering the requirement.
Rule 1062059: Call out edits to accepted ADRs in PR descriptions
docs/ADRs/0050-distributed-tracing-instrumentation.md[154-159]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Accepted ADR `0050-distributed-tracing-instrumentation.md` was modified, but the PR description must explicitly call out the ADR identifier/filename and summarize what changed.

## Issue Context
The PR description currently discusses ADR 0087 but does not explicitly name ADR 0050.

## Fix Focus Areas
- docs/ADRs/0050-distributed-tracing-instrumentation.md[154-159]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

11. ~~ADR 0087 too many consequences~~ ✓ Resolved 📜 Skill insight ⚙ Maintainability

Description

ADR 0087's Consequences section contains 6 bullets, exceeding the required 3–5 bullets. This
breaks the standard ADR format and reduces readability.

Code

docs/ADRs/0087-eval-measurements-online-trace-scoring.md[R106-113]

+## Consequences
+
+- Wild runs produce a reviewable score file beside telemetry with or without a
+  remote backend.
+- Orgs that already set OTEL for traces get a portable score export contract
+  without a second auth scheme; optional adapters may enrich one product UI.
+- Missing manifests skip cleanly; measure failure never fails the agent job.
+- Functional scenarios (gate) and eval measurements (trend) stay separate;

Relevance

●● Moderate
Consequence bullet-count is a format nit; no strong precedent they enforce this strictly.
PR-#2582

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist requires 3–5 consequence bullets. The ADR includes six separate consequence bullets,
violating the requirement.
docs/ADRs/0087-eval-measurements-online-trace-scoring.md[106-119]
Skill: writing-adrs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR Consequences section must be 3–5 one-sentence bullet points, but this ADR has more.

## Issue Context
The current Consequences section includes 6 bullets.

## Fix Focus Areas
- docs/ADRs/0087-eval-measurements-online-trace-scoring.md[106-119]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

12. ~~Temp manifest not cleaned~~ ⊘ Outdated 🐞 Bug ☼ Reliability ⭐ New

Description

The GitHub Action downloads the upstream manifest into a mktemp file but never deletes it on the
success path (or on failures after the download succeeds). This leaves temporary files behind on
self-hosted or reused runners.

Code

action.yml[R439-445]

+          TMP="$(mktemp)"
+          CURL_ARGS=( -fsSL -o "${TMP}" )
+          if [[ -n "${GH_TOKEN:-}" ]]; then
+            CURL_ARGS+=( -H "Authorization: Bearer ${GH_TOKEN}" )
+          fi
+          if curl "${CURL_ARGS[@]}" "${URL}"; then
+            REGISTRY="${TMP}"

Relevance

●●● Strong
Team often accepts adding EXIT traps after mktemp to prevent temp-file leaks under set -euo
pipefail.
PR-#4864
PR-#5442
PR-#2196

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The step creates TMP and assigns it to REGISTRY on successful curl, but only rm -f occurs in
the curl failure branch; there is no trap or post-command cleanup.

action.yml[423-450]
action.yml[453-456]
PR-#4864

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`action.yml` creates a temporary file for the downloaded manifest (`TMP="$(mktemp)"`) but only removes it on the curl-failure branch. On success (and on later errors), the temp file is not removed.

## Issue Context
This is low-impact on ephemeral GitHub-hosted runners, but can accumulate on self-hosted runners or any environment that reuses workspaces.

## Fix Focus Areas
- action.yml[438-456]

### Suggested approach
- Immediately after `TMP="$(mktemp)"`, add a trap to delete it:
 - `trap 'rm -f -- "${TMP}"' EXIT`
- Ensure the trap is only set in the branch where `TMP` is created, so it doesn’t interfere with `LOCAL_REG` usage.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

13. ~~ADR 0087 context too long~~ ✓ Resolved 📜 Skill insight ⚙ Maintainability

Description

ADR 0087's Context section exceeds the required 1–3 short paragraphs and includes extended bullet
lists. This makes the ADR harder to scan and pushes problem-details into the ADR instead of linking
out.

Code

docs/ADRs/0087-eval-measurements-online-trace-scoring.md[R23-31]

+## Context
+
+Agent runs already emit OpenTelemetry traces as `run-telemetry.jsonl`, with
+optional live OTLP export when `OTEL_EXPORTER_OTLP_*` is set
+([ADR 0050](0050-distributed-tracing-instrumentation.md)). Separately,
+[ADR 0051](0051-agent-eval-harness-for-test-infrastructure.md) owns the
+**functional** eval harness: curated fixtures / scenarios in
+`fullsend-ai/agents` `eval/<agent>/` that gate agent PRs. Those fixtures do
+not score wild production runs.

Relevance

● Weak
Nearly identical “Context must be 1–3 paragraphs” compliance ask was explicitly rejected.
PR-#2582

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist requires the ADR Context section to be 1–3 short paragraphs. The added ADR includes
multiple paragraphs and a multi-item bullet list under Context, exceeding the constraint.
docs/ADRs/0087-eval-measurements-online-trace-scoring.md[23-52]
Skill: writing-adrs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR Context section is longer than allowed (must be 1–3 short paragraphs) and includes additional long-form content.

## Issue Context
The Context section contains multiple paragraphs plus an extended "Adjacent telemetry proposals" list.

## Fix Focus Areas
- docs/ADRs/0087-eval-measurements-online-trace-scoring.md[23-53]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

14. ~~ADR 0087 exceeds 100 lines~~ ✓ Resolved 📜 Skill insight ⚙ Maintainability

Description

ADR 0087 is 119 lines in this PR, exceeding the 100-line maximum (excluding frontmatter). This
suggests the ADR is too long and should be shortened or split.

Code

docs/ADRs/0087-eval-measurements-online-trace-scoring.md[R116-119]

+- Retro can recommend a **manifest scorer** or a **scenario fixture** — not
+  substitutes.
+- Richer telemetry (Level 3 / Status fixes) expands what scorers *can* assert;
+  it does not replace this same-job path.

Relevance

● Weak
ADR ≤100 lines enforcement was previously rejected when requested to shrink an ADR.
PR-#2582

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist sets a maximum of 100 lines of ADR content. The added ADR file in the diff is 119
lines long, exceeding that limit.
docs/ADRs/0087-eval-measurements-online-trace-scoring.md[1-119]
Skill: writing-adrs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR content exceeds the 100-line limit, indicating excessive scope or repeated context.

## Issue Context
The file added in this PR spans 119 lines; the checklist caps ADRs at 100 lines of content (excluding frontmatter).

## Fix Focus Areas
- docs/ADRs/0087-eval-measurements-online-trace-scoring.md[13-119]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View low (1)

15. eval-measurements.md wrong guide path 📜 Skill insight ⌂ Architecture

Description

A new guide was added under docs/guides/infrastructure/, but guides must live under either
docs/guides/admin/ or docs/guides/user/. This breaks the required guide directory convention and
makes guide organization inconsistent.

Code

docs/guides/infrastructure/eval-measurements.md[1]
+# Eval Measurements

Relevance

● Weak
Guide taxonomy violations (dev/infrastructure vs admin/user) have precedents being rejected.
PR-#5454

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The checklist mandates that every file under docs/guides/ be located in either admin/ or
user/. This PR adds a guide in docs/guides/infrastructure/, violating that placement
requirement.

docs/guides/infrastructure/eval-measurements.md[1-1]
Skill: writing-user-docs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new documentation guide was added at `docs/guides/infrastructure/eval-measurements.md`, but compliance requires guides to be placed under `docs/guides/admin/` or `docs/guides/user/`.

## Issue Context
This PR also links to the guide from VitePress config and other docs, so moving the file requires updating those links.

## Fix Focus Areas
- docs/guides/infrastructure/eval-measurements.md[1-1]
- docs/.vitepress/config.ts[262-262]
- docs/architecture.md[290-290]
- docs/guides/infrastructure/distributed-tracing.md[246-255]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

✅ Compliance rules (platform): 54 rules

✅ Skills: writing-user-docs, writing-adrs

Tip of the day

💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

fullsend-ai-review · 2026-08-10T12:02:29Z

Review

Findings

Medium

[edge-case] internal/evalmeasure/find.go:52 — The hostRunDirPattern regex ^agent-(.+)-([0-9]+)-([0-9]+)$ uses a greedy .+ capture group for the agent name. While greedy backtracking produces correct results for the current naming convention (e.g., agent-model-3-123-456 correctly yields m[1]="model-3"), the design relies on backtracking behavior that becomes fragile if agent names contain trailing numeric segments matching the PID/timestamp shape. Consider tightening the regex to require minimum lengths for PID/timestamp groups (e.g., PIDs are typically 1-7 digits, Unix timestamps are 10+ digits) to make the boundary unambiguous for all practical names.

Low

[injection] action.yml:431 — AGENT env var from inputs.agent is interpolated into git show path and MEASURE_FILE filename without shell-level validation. The binary's sanitizeMeasurementAgentName rejects /, \, and .., and git show fails safely for malformed paths. The value is always double-quoted. Defense-in-depth: consider adding a regex guard in the shell step.
[fail-open] action.yml:434 — When PR_BASE_SHA is empty (non-PR events), GITHUB_SHA fallback reads the pushed commit's manifest. Risk is limited to trend poisoning on non-PR pushes since eval measurements are fail-open and never gate delivery.
[scope-creep] internal/cli/run.go:2415 — PR includes changes beyond the core eval measurement engine (resolveWorkItemID expansion, agent name lowercasing in sandbox names, outputDirExcludeRel, GitLab scaffold breaking change). These are functionally co-dependent with eval measurements (agent name matching, work_item fitness, preventing output/ from entering the sandbox) and ship atomically for correctness.
[naming-convention] internal/cli/evalmeasure.go:78 — evalMeasureOpts has both outDir and outputDir fields, which are confusing near-synonyms. outDir (→ --out-dir) is where eval-measurements.jsonl is written; outputDir (→ --output-dir) is the CI output base containing runDirs. Consider renaming outDir to measureDir or evalOutDir.
[error-handling] internal/evalmeasure/run.go:29 — MeasureAndExport repurposes any non-nil parseErr as stats.Incomplete without verifying it is specifically a scanner truncation error. Currently correct (only scanner errors return alongside non-empty traces) but not robust against future changes to ParseTelemetryFile.
[edge-case] internal/cli/evalmeasure.go:120 — sanitizeMeasurementAgentName rejects /, \, and .. but permits spaces and shell metacharacters. The downstream filepath.IsLocal check in localMeasurementManifest provides a second defense layer.
[missing-doc] docs/contributing/documentation.md — New eval-measure CLI command not added to the documentation cross-reference table. The table tracks documentation touchpoints for each CLI command group.

Previous run

Review

Findings

Medium

[breaking-change] internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-agent.yml — The GitLab CI scaffold changes --output-dir from /tmp/fullsend-output to ${CI_PROJECT_DIR}/output. This is a breaking change for existing GitLab installations that requires scaffold re-sync. The PR title correctly signals this with ! and the body documents the migration path.
Remediation: Ensure release notes clearly describe the upgrade steps for GitLab users.

Low

[edge-case] action.yml — The git cat-file/git show commands use the raw ${AGENT} input without lowercasing. A mixed-case agent name will silently miss the local base-branch manifest and fall back to the SHA-pinned agents@v0 fetch. The fallback is itself trusted, but the override doesn't activate as intended.
[path-traversal] action.yml — ${AGENT} is interpolated into git show/git cat-file without shell-level validation. The Go CLI's sanitizeMeasurementAgentName validates, but the shell code runs first. Risk is minimal: git show only accesses the repo tree, inputs.agent is workflow-author-controlled, and continue-on-error: true limits blast radius. Defense-in-depth validation would close the gap.
[path-traversal] internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-agent.yml — Same pattern: ${STAGE} interpolated into git show without shell-level validation. HMAC verification covers STAGE when configured; git show only accesses the repo tree.
[scope-creep] internal/cli/run.go — resolveWorkItemID() now falls back to ORIGINATING_URL and GITLAB_ISSUE_URL and imports evalmeasure.UnknownSentinel. These changes affect all fullsend run invocations, not just eval-measure. The fallbacks close a real gap for retro/GitLab runs, but the cross-package dependency for a sentinel constant is worth noting.
[cross-repo-artifact-contract] internal/evalmeasure/export_local.go — The eval-measurements.jsonl JSONL schema (EvaluationResult fields) becomes a de facto cross-repo contract once consumed by downstream tooling. Per-measurement id@version handles scorer evolution, but the file-format-level shape is unversioned.

Previous run (2)

Review

Findings

Low

[naming/cross-package sentinel] internal/evalmeasure/fitness.go:13 — UnknownSentinel is exported from the evalmeasure package but encodes a CLI-layer concept (resolveWorkItemID fallback). The dependency direction (cli → evalmeasure) is valid and matches other imports, but the constant packages a runner concept in the scorer package. Consider placing shared sentinels in a neutral package.
[naming/flag inconsistency] internal/cli/evalmeasure.go:36 — --out-dir and --output-dir flags have distinct semantics (measurements output dir vs. CI output base for runDir discovery) but confusingly similar names. --output-dir reuses the existing fullsend run --output-dir convention, and flag descriptions are clear, so the risk is low.
[GitLab CI template contract] internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-agent.yml — The scaffold template changes --output-dir from /tmp/fullsend-output to ${CI_PROJECT_DIR}/output, adds the eval-measure step, and fixes GITLAB_ISSUE_URL to avoid fabricating .../issues/0. Existing installations retain their current template until re-install.
[ledger-integrity] internal/evalmeasure/export_local.go:44 — The ledger key uses pipe (|) as delimiter. traceID from OTLP JSONL is not validated against pipe/newline characters. A crafted run-telemetry.jsonl could cause a false ledger match. Impact is limited to self-DoS of idempotency since platform telemetry is written by the host recorder, not the sandbox.
[new-output-contract] internal/evalmeasure/types.go:38 — EvaluationResult defines the JSONL schema for eval-measurements.jsonl. WorkItemID uses omitempty; SpanID/Agent do not (intentional per code comment — Value omitempty avoided since a real fail can be 0.0). JSON field names should be treated as stable once consumers exist.
[scope-creep] internal/cli/run.go:2445 — resolveWorkItemID adds ORIGINATING_URL and GITLAB_ISSUE_URL fallbacks, changing fullsend.work_item_id span attribute behavior for existing runs. Well-motivated by EM-001's work_item fitness check.
[error-message-style] internal/cli/evalmeasure.go:107 — Validation errors use plain fmt.Errorf without %w wrapping. Acceptable for user-facing CLI messages not programmatically checked.

Previous run (3)

Review

Findings

Medium

[naming-consistency] internal/cli/evalmeasure.go — The eval-measure command has both --out-dir and --output-dir flags with different semantics (one for eval-measurements.jsonl placement, one for CI output base). The codebase uses --output-dir consistently elsewhere (fullsend run). Having near-identical flag names is confusing for users.
Remediation: Rename --out-dir to something more descriptive that avoids collision, such as --measurements-dir or --score-dir.
[missing-authorization] — This PR introduces a significant new feature (new CLI command, new Go package, new ADR, GHA/GitLab CI wiring, documentation) but has no directly linked issue. The PR body references Track agents@v0 release cut for eval measurement manifests #6384 for tracking the agents@v0 release cut, but that tracks a subsequent release step, not the eval measurements feature itself.
Remediation: Link or create an issue that covers the eval measurements feature.

Low

[edge-case] internal/evalmeasure/find.go:47 — hostRunDirPattern regex ^agent-(.+)-([0-9]+)-([0-9]+)$ uses greedy .+ for the agent name capture. For a hypothetical all-digit agent name (e.g., 123), directory agent-123-456-789 would parse as agent=123-456 due to greedy backtracking. All current stock agents use alphabetic names, so this is latent.
[cross-package-coupling] internal/evalmeasure/fitness.go:11 — UnknownSentinel is defined in the scoring package but its meaning originates in the CLI layer (resolveWorkItemID). The dependency direction is inverted, though both packages are under internal/ and cli already imports evalmeasure for other types.
[missing-doc-comment] internal/evalmeasure/export_local.go:13 — Exported constants MeasurementsFile and LedgerFile lack doc comments, while the sibling PlatformTelemetryFile in find.go has one.
[api-shape] internal/evalmeasure/run.go:23 — MeasureFile wraps MeasureAndExport but silently discards ParseStats. Only used in tests; consider removing or surfacing stats.
[exported-api-surface] internal/evalmeasure/export_local.go:18 — AppendMeasurements, AlreadyScored, and RecordScored are exported but only called within the evalmeasure package. Since the package is under internal/, this has no external impact but could be unexported for API hygiene.
[test-naming] internal/cli/evalmeasure_test.go — Helper writeTwoTraceTelemetry has the same name (different signature) as a helper in internal/evalmeasure/run_test.go. Valid Go but confusing when grepping.
[error-handling] internal/cli/evalmeasure.go:108 — evalMeasureFetchContext workspace fallback path: if UserCacheDir() and MkdirAll both fail, subsequent fetch operations will silently skip. This is correct fail-open behavior but the warn could note that manifest resolution will be skipped.
[race-condition] internal/evalmeasure/run.go:54 — The idempotency flow (AlreadyScored → AppendMeasurements → RecordScored) is not atomic. Concurrent processes could produce duplicate JSONL rows. Acknowledged in test comments; CI runs once per job.

Info

[provenance-warning] — Prior review context discarded: provenance validation failed (unverifiable-wrong-app). This review treats all findings as first-time assessments.

Previous run (4)

Review

Findings

Low

[data-quality-on-partial-failure] internal/evalmeasure/run.go:65 — If AppendMeasurements succeeds but RecordScored fails, the measurement is persisted to eval-measurements.jsonl but the ledger is not updated. On retry, the same measurement is appended again (duplicate line). Documented in TestMeasureFile_AppendBeforeLedger; consumers can deduplicate on (trace_id, name, version).
[ledger-key-injection] internal/evalmeasure/export_local.go:51 — The ledger key format is traceID|evalName|version. Registry validation rejects pipe and newline in evalName fields, but traceID comes from parsed OTLP data and is not validated for pipe or newline characters. A malformed traceID containing | could cause a false-positive match in AlreadyScored, skipping a legitimate measurement. In practice, OTLP trace IDs are 32-character hex strings, and FindPlatformTelemetry ensures only host-written (top-of-runDir) files are scored.
[edge-case] internal/evalmeasure/find.go:37 — FindPlatformTelemetry returns at most one telemetry path when searching child runDirs (the newest by modification time). If an outputDir legitimately contains telemetry from multiple agent runDirs for different agents in the same job, only the newest is scored. This is documented and intentional.
[naming-inconsistency] internal/cli/evalmeasure.go — The CLI flag --out-dir uses a different naming convention than --output-dir. Both are defined on the same command with subtly different semantics (measurements output directory vs CI output base), creating potential confusion.
[duplicate-constant] internal/evalmeasure/find.go:13 — PlatformTelemetryFile duplicates telemetry.TelemetryFile. A test (TestPlatformTelemetryFileMatchesRecorder) guards the invariant, but importing the canonical constant would be cleaner.
[stale-example] docs/guides/dev/tracing.md:132 — Lists gen_ai.system as an agent span end attribute without noting the OTel GenAI semconv v1.37 deprecation in favor of gen_ai.provider.name. The PR updates the infrastructure tracing reference to show both names, but this dev guide only mentions the old name.

Previous run (5)

Review

Findings

Low

[data-quality-on-partial-failure] internal/evalmeasure/run.go:65 — If AppendMeasurements succeeds but RecordScored fails, the measurement is persisted to eval-measurements.jsonl but the ledger is not updated. On retry, the same measurement is appended again (duplicate line). Documented in TestMeasureFile_AppendBeforeLedger; consumers can deduplicate on (trace_id, name, version).
[ledger-key-injection] internal/evalmeasure/export_local.go:51 — The ledger key format is traceID|evalName|version. Registry validation rejects pipe and newline in evalName fields, but traceID comes from parsed OTLP data and is not validated for pipe or newline characters. A malformed traceID containing | could cause a false-positive match in AlreadyScored, skipping a legitimate measurement. In practice, OTLP trace IDs are 32-character hex strings, and FindPlatformTelemetry ensures only host-written (top-of-runDir) files are scored.
[edge-case] internal/evalmeasure/find.go:37 — FindPlatformTelemetry returns at most one telemetry path when searching child runDirs (the newest by modification time). If an outputDir legitimately contains telemetry from multiple agent runDirs for different agents in the same job, only the newest is scored. This is documented and intentional.
[naming-inconsistency] internal/cli/evalmeasure.go — The CLI flag --out-dir uses a different naming convention than --output-dir. Both are defined on the same command with subtly different semantics (measurements output directory vs CI output base), creating potential confusion.
[cross-package-sentinel] internal/evalmeasure/fitness.go — UnknownSentinel is exported from internal/evalmeasure and imported by internal/cli/run.go for resolveWorkItemID(). The sentinel is a property of the run telemetry contract, not specific to eval measurements. Consider moving to a shared location (e.g. internal/telemetry).
[duplicate-constant] internal/evalmeasure/find.go — PlatformTelemetryFile duplicates telemetry.TelemetryFile. A test (TestPlatformTelemetryFileMatchesRecorder) guards the invariant, but importing the canonical constant would be cleaner.
[stale-example] docs/guides/dev/tracing.md:132 — Lists gen_ai.system as an agent span end attribute without noting the OTel GenAI semconv v1.37 deprecation in favor of gen_ai.provider.name. The PR updates the infrastructure tracing reference to show both names, but this dev guide only mentions the old name.

Previous run (6)

Review

Findings

Low

[data-quality-on-partial-failure] internal/evalmeasure/run.go:65 — If AppendMeasurements succeeds but RecordScored fails, the measurement is persisted to eval-measurements.jsonl but the ledger is not updated. On retry, the same measurement is appended again (duplicate line). Documented in TestMeasureFile_AppendBeforeLedger; consumers can deduplicate on (trace_id, name, version).
[ledger-key-injection] internal/evalmeasure/export_local.go:51 — The ledger key format is traceID|evalName|version. Registry validation rejects pipe and newline in evalName fields, but traceID comes from parsed OTLP data and is not validated for pipe or newline characters. A malformed traceID containing | could cause a false-positive match in AlreadyScored, skipping a legitimate measurement. In practice, OTLP trace IDs are 32-character hex strings, and FindPlatformTelemetry ensures only host-written (top-of-runDir) files are scored.
[edge-case] internal/evalmeasure/find.go:37 — FindPlatformTelemetry returns at most one telemetry path when searching child runDirs (the newest by modification time). If an outputDir legitimately contains telemetry from multiple agent runDirs for different agents in the same job, only the newest is scored. This is documented and intentional.
[naming-inconsistency] internal/cli/evalmeasure.go — The CLI flag --out-dir uses a different naming convention than --output-dir. Both are defined on the same command with subtly different semantics (measurements output directory vs CI output base), creating potential confusion.
[cross-package-sentinel] internal/evalmeasure/fitness.go — UnknownSentinel is exported from internal/evalmeasure and imported by internal/cli/run.go for resolveWorkItemID(). The sentinel is a property of the run telemetry contract, not specific to eval measurements. Consider moving to a shared location (e.g. internal/telemetry).
[duplicate-constant] internal/evalmeasure/find.go — PlatformTelemetryFile duplicates telemetry.TelemetryFile. A test (TestPlatformTelemetryFileMatchesRecorder) guards the invariant, but importing the canonical constant would be cleaner.
[stale-example] docs/guides/dev/tracing.md:132 — Lists gen_ai.system as an agent span end attribute without noting the OTel GenAI semconv v1.37 deprecation in favor of gen_ai.provider.name. The PR updates the infrastructure tracing reference to show both names, but this dev guide only mentions the old name.

Previous run (7)

Review

Findings

Low

[data-quality-on-partial-failure] internal/evalmeasure/run.go:46 — If AppendMeasurements succeeds but RecordScored fails, the measurement is persisted to eval-measurements.jsonl but the ledger is not updated. On retry, the same measurement is appended again (duplicate line). Documented in TestMeasureFile_AppendBeforeLedger; consumers can deduplicate on (trace_id, name, version).
[ledger-key-injection] internal/evalmeasure/export_local.go:51 — The ledger key format is traceID|evalName|version. Registry validation rejects pipe and newline in evalName fields, but traceID comes from parsed OTLP data and is not validated for pipe or newline characters. A malformed traceID containing | could cause a false-positive match in AlreadyScored, skipping a legitimate measurement. In practice, OTLP trace IDs are 32-character hex strings, and FindPlatformTelemetry ensures only host-written (top-of-runDir) files are scored.
[edge-case] internal/evalmeasure/find.go:37 — FindPlatformTelemetry returns at most one telemetry path when searching child runDirs (the newest by modification time). If an outputDir legitimately contains telemetry from multiple agent runDirs for different agents in the same job, only the newest is scored. This is documented and intentional.
[naming-inconsistency] internal/cli/evalmeasure.go:643 — The CLI flag --out-dir uses a different naming convention than --output-dir. Both are defined on the same command with subtly different semantics (measurements output directory vs CI output base), creating potential confusion.

Previous run (8)

Review

Findings

Low

[data-quality-on-partial-failure] internal/evalmeasure/run.go:46 — If AppendMeasurements succeeds but RecordScored fails, the measurement is persisted to eval-measurements.jsonl but the ledger is not updated. On retry, the same measurement is appended again (duplicate line). Documented in TestMeasureFile_AppendBeforeLedger; consumers can deduplicate on (trace_id, name, version).
[non-determinism] action.yml:426 — The find command uses -print -quit to locate run-telemetry.jsonl, which returns the first match without guaranteeing order. In practice, fullsend run produces a single telemetry file, so this is unlikely to matter.
[ledger-key-injection] internal/evalmeasure/export_local.go:51 — The ledger key format is traceID|evalName|version. The registry validation rejects pipe and newline characters in ID, scorer, and name fields, but traceID comes from parsed OTLP data and is not validated for pipe or newline characters. A malformed traceID containing | could cause a false-positive match in AlreadyScored, skipping a legitimate measurement. In practice, OTLP trace IDs are 32-character hex strings, so this is unlikely outside adversarial input.
[GHA-workflow-command-injection] action.yml:444 — The printf statement uses %q for ${AGENT} (good), but %s for ${LOCAL_REG} and ${URL}, both of which contain the unsanitized ${AGENT} value. All known callers hardcode simple identifiers, keeping practical risk low.
[path-traversal] action.yml:433 — The AGENT input is interpolated into a filesystem path (${FULLSEND_DIR}/eval/measurements/${AGENT}.yaml) and a GitHub raw-content URL without path-component sanitization. LoadRegistry's strict schema validation limits exploitability to denial-of-service.

Previous run (9)

Review

Findings

Low

[data-quality-on-partial-failure] internal/evalmeasure/run.go:46 — If AppendMeasurements succeeds but RecordScored fails, the measurement is persisted to eval-measurements.jsonl but the ledger is not updated. On retry, the same measurement is appended again (duplicate line). Documented in TestMeasureFile_AppendBeforeLedger; consumers can deduplicate on (trace_id, name, version).
[non-determinism] action.yml:426 — The find command uses -print -quit to locate run-telemetry.jsonl, which returns the first match without guaranteeing order. In practice, fullsend run produces a single telemetry file, so this is unlikely to matter.
[GHA-workflow-command-injection] action.yml:444 — The printf statement uses %q for ${AGENT} (good), but %s for ${LOCAL_REG} and ${URL}, both of which contain the unsanitized ${AGENT} value. All known callers hardcode simple identifiers, keeping practical risk low.
[path-traversal] action.yml:433 — The AGENT input is interpolated into a filesystem path (${FULLSEND_DIR}/eval/measurements/${AGENT}.yaml) and a GitHub raw-content URL without path-component sanitization. LoadRegistry's strict schema validation limits exploitability to denial-of-service.

Previous run (10)

Review

Findings

High

[logic-error] action.yml:532 — The PR appends a new "Upload fullsend artifacts" step at the end of the composite action, but the original upload-artifact step at line 412 is left in place. Both use artifact name fullsend-${{ inputs.agent }} and path ${{ github.workspace }}/output. The existing upload runs BEFORE fullsend eval-measure, so eval-measurements.jsonl is never included in the artifact. The second upload will fail with a duplicate artifact name error (upload-artifact v7 default overwrite: false), and since it lacks continue-on-error: true, the failure affects job status.
Remediation: Move the eval-measure step to run before the existing upload step at line 412 and remove the duplicate upload step at the end.

Low

[data-quality-on-partial-failure] internal/evalmeasure/run.go:46 — If AppendMeasurements succeeds but RecordScored fails, the measurement is persisted to eval-measurements.jsonl but the ledger is not updated. On retry, the same measurement is appended again (duplicate line). For a local JSONL artifact this is low-severity since consumers can deduplicate on (trace_id, name, version).
[GHA-workflow-command-injection] action.yml:521 — The echo statement interpolates ${AGENT} directly into stdout. While inputs.agent is workflow-author-controlled (not event-payload-sourced) and all known callers hardcode simple identifiers, the composite action is a public contract where external consumers could theoretically wire untrusted data.
[unused-parameter-idiom] internal/evalmeasure/run.go:18 — MeasureAndExport accepts context.Context but immediately discards it with _ = ctx. The parameter is reserved for future OTLP export per the doc comment.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (11)

Review

Findings

Low

[data quality on partial failure] internal/evalmeasure/run.go:48 — If AppendMeasurements succeeds but RecordScored fails, the measurement is persisted to eval-measurements.jsonl but the ledger is not updated. On retry, the same measurement is appended again (duplicate line). For a local JSONL artifact this is low-severity since consumers can deduplicate on (trace_id, name, version).
[error handling gap] internal/evalmeasure/run.go:37 — When the scoring loop encounters a write error, MeasureAndExport returns partial results alongside the error. The CLI handler discards results on error, so measurements already written to disk are not printed to stdout.
[test adequacy] internal/evalmeasure/run_test.go — No test covers partial-failure scenarios where AppendMeasurements or RecordScored fails mid-batch.
[unused parameter idiom] internal/evalmeasure/run.go:20 — MeasureAndExport accepts context.Context but immediately discards it with _ = ctx. The parameter is reserved for future OTLP export per the doc comment.
[cross-repo-schema] internal/evalmeasure/registry.go:27 — The measurement manifest YAML schema is introduced without a formal schema version field. ADR 0087 designs per-measurement versioning (id@version), and the YAML parser silently ignores unknown keys, so adding new optional fields later will not break existing binaries.
[cross-repo-compatibility] action.yml — The eval measurements step hardcodes the agents-repo manifest path convention (eval/measurements/${AGENT}.yaml) and pins to v0. This is consistent with other agents-repo fallbacks in this codebase.

Previous run (12)

Review

Findings

Low

[missing-docs-for-new-public-symbol] docs/guides/dev/cli-internals.md:112 — The CLI command tree documents every top-level subcommand but does not include the new eval-measure subcommand or its flags (--telemetry, --registry, --out-dir). The tree is already stale (missing poll), and eval-measure has its own dedicated guide.
[missing-docs-for-new-public-symbol] docs/cli/README.md:24 — The "Additional commands" table does not include eval-measure. Other CI-internal commands (post-review, post-comment, reconcile-status, poll) are also absent, suggesting the table intentionally covers only user-facing commands.
[data quality on partial failure] internal/evalmeasure/run.go:44 — If AppendMeasurements succeeds but RecordScored fails, the measurement is persisted to eval-measurements.jsonl but the ledger is not updated. On retry, the same measurement is appended again (duplicate line). For a local JSONL artifact this is low-severity since consumers can deduplicate on (trace_id, name, version).
[error handling gap] internal/evalmeasure/run.go:33 — When the scoring loop encounters a write error, MeasureAndExport returns partial results alongside the error. The CLI handler discards results on error, so measurements already written to disk are not printed to stdout.
[test adequacy] internal/evalmeasure/run_test.go — No test covers partial-failure scenarios where AppendMeasurements or RecordScored fails mid-batch.
[path traversal] action.yml:478 — AGENT input is interpolated into file paths and URLs without sanitization. Risk is minimal: the step has continue-on-error: true, AGENT comes from trusted workflow inputs (not PR content), and the worst case is a failed file check or 404.
[cross-repo-schema] internal/evalmeasure/registry.go:27 — The measurement manifest YAML schema is introduced without a formal schema version field. ADR 0087 designs per-measurement versioning (id@version), and the YAML parser silently ignores unknown keys, so adding new optional fields later will not break existing binaries.
[cross-repo-compatibility] action.yml:35 — The eval measurements step hardcodes the agents-repo manifest path convention (eval/measurements/${AGENT}.yaml) and pins to v0. This is consistent with other agents-repo fallbacks in this codebase.
[unused parameter idiom] internal/evalmeasure/run.go:17 — MeasureAndExport accepts context.Context but immediately discards it with _ = ctx. The parameter is reserved for future OTLP export per the doc comment.

Previous run (13)

Review

Findings

Critical

[reusable-workflow-contract-break] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — The PR removes the entire with: block from the composite action step (uses: ./.defaults/) in all 6 reusable workflows, deleting the required agent input along with version, fullsend-dir, run-url, status-repo, status-number, and mint-url. The composite action declares agent as required: true with no default in action.yml. Without the with: block, inputs.agent resolves to an empty string, causing fullsend run "" to execute with no agent name. Every consumer repo calling these reusable workflows will break. This change is also unrelated to the eval measurements feature — it is not mentioned in the PR title, body, or ADR 0087.
Remediation: Restore the with: blocks on all 6 reusable workflow files. If the removal is intentional (e.g., moving input passing to a different mechanism), it should be landed as a separately scoped PR with an explanation of the alternative mechanism.

High

[protected-path] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — Six protected workflow files under .github/ are modified. The PR has no linked issue and the description does not explain the with: block removals from these governance and infrastructure files. Human approval is always required for changes to protected paths.

Medium

[data loss on partial failure] internal/evalmeasure/run.go:42 — RecordScored (ledger write) is called before AppendMeasurements (measurement write). If AppendMeasurements fails after RecordScored succeeds, the ledger marks the trace as already-scored but the measurement was never persisted. On retry, AlreadyScored returns true and the measurement is permanently lost. The code was rewritten since the prior review but the ordering bug persists.
Remediation: Swap the order — call AppendMeasurements before RecordScored so the ledger entry is only written after the measurement is safely persisted. A duplicate measurement line on retry is recoverable; a lost measurement is not.
[missing-docs-for-new-public-symbol] docs/cli/README.md:24 — The "Additional commands" table lists fullsend run, lock, and scan but does not include the new fullsend eval-measure subcommand.
Remediation: Add a row for fullsend eval-measure with description pointing to the eval measurements guide.
[missing-docs-for-new-public-symbol] docs/guides/dev/cli-internals.md:112 — The CLI command tree documents every subcommand but does not include the new eval-measure subcommand or its flags (--telemetry, --registry, --out-dir). The Key Source Files table also omits internal/cli/evalmeasure.go and internal/evalmeasure/.
Remediation: Add eval-measure with its flags to the CLI command tree and add the new source files to the Key Source Files table.

Low

[missing-authorization] — No linked issue for a non-trivial change (1,700+ new lines of Go code, new CLI subcommand, new ADR, GHA post-step wiring, six reusable workflow modifications). ADR 0087 provides architectural authorization but an issue link would improve traceability.
[error handling gap] internal/evalmeasure/run.go:33 — When the scoring loop encounters a ledger or write error, MeasureAndExport returns partial results alongside the error. The CLI handler treats any non-nil error as fatal, so partial results already written to disk are not printed to stdout. The user sees an error but no indication of which measurements succeeded.
[test adequacy] internal/evalmeasure/run_test.go — No test covers partial-failure scenarios where RecordScored or AppendMeasurements fails mid-batch. The idempotency test covers the happy-path retry but not the failure-then-retry path where the ordering of RecordScored vs AppendMeasurements matters.
[naming specificity] internal/cli/evalmeasure.go:57 — The helper printResults is more generic than existing CLI helpers in this package (printResolvedDeps, printStatusTable, printSkipGuidance). Consider renaming to printMeasurementResults to match the established print<Domain> pattern.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (14)

Review

Findings

High

[protected-path] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — Six protected workflow files under .github/ are modified. The PR has no linked issue; human approval is always required for changes to governance and infrastructure files. Affected protected files: .github/workflows/reusable-code.yml, .github/workflows/reusable-fix.yml, .github/workflows/reusable-prioritize.yml, .github/workflows/reusable-retro.yml, .github/workflows/reusable-review.yml, .github/workflows/reusable-triage.yml.

Medium

[data loss on partial failure] internal/evalmeasure/run.go:42 — RecordScored (ledger write) is called before AppendMeasurements (measurement write). If AppendMeasurements fails after RecordScored succeeds, the ledger marks the trace as already-scored but the measurement was never persisted. On retry, AlreadyScored returns true and the measurement is permanently lost.
Remediation: Swap the order — call AppendMeasurements before RecordScored so the ledger entry is only written after the measurement is safely persisted.
[missing consumer for new feature] .github/workflows/reusable-dispatch.yml — reusable-dispatch.yml (per-repo dispatch workflow) was not updated to forward MLFLOW_TRACKING_URI, MLFLOW_TRACKING_USERNAME, or MLFLOW_TRACKING_PASSWORD. It also does not declare MLFLOW_TRACKING_PASSWORD in its secrets section. All six reusable-{stage}.yml files were updated but this file was missed. Per-repo installations using reusable-dispatch.yml will silently skip MLflow Assessment export even when the org has the secrets configured. Per docs/contributing/workflow-contracts.md, both installation-mode chains must be updated when a reusable workflow adds a new secrets: entry.
Remediation: Add MLFLOW_TRACKING_PASSWORD to the secrets section of reusable-dispatch.yml and forward MLFLOW_TRACKING_URI, MLFLOW_TRACKING_USERNAME, MLFLOW_TRACKING_PASSWORD in the env block of each action invocation.
[missing-docs-for-new-public-symbol] docs/cli/README.md:24 — The "Additional commands" table lists fullsend run, lock, and scan but does not include the new fullsend eval-measure subcommand.
Remediation: Add a row for fullsend eval-measure with description pointing to the eval measurements guide.
[missing-docs-for-new-public-symbol] docs/guides/dev/cli-internals.md:112 — The CLI command tree documents every subcommand but does not include the new eval-measure subcommand or its flags (--telemetry, --registry, --out-dir).
Remediation: Add an eval-measure entry to the CLI command tree.

Low

[missing-authorization] — No linked issue for a non-trivial change (1,700+ new lines of Go code, new CLI subcommand, new ADR, GHA post-step wiring, six reusable workflow modifications). ADR 0087 provides architectural authorization but an issue link would improve traceability.
[error handling gap] internal/evalmeasure/run.go:33 — On non-ExportError failures in the scoring loop, the CLI handler returns the error without printing partial results that were already written to disk.
[credential-scope] internal/evalmeasure/export_mlflow.go:97 — When MLFLOW_TRACKING_URI is unset, trackingURIFromEnv() derives the MLflow base URL from OTEL endpoints. If OTEL points to a non-MLflow backend, Basic Auth credentials would be sent to an unintended host (requires both a derived URI mismatch AND the password being set).
[comment consistency across workflows] .github/workflows/reusable-triage.yml:180 — The triage workflow's MLFLOW comment is 3 lines (includes "URI defaults from OTEL endpoint when unset") while the other five workflows use a 2-line comment.
[test seam pattern] internal/evalmeasure/export_mlflow.go:32 — The assessmentHTTPDo test seam var is annotated "Not safe for t.Parallel()" but there is no enforcement mechanism beyond the comment.
[test adequacy] internal/evalmeasure/run_test.go — No test covers partial-failure scenarios where RecordScored or AppendMeasurements fails mid-batch, which is particularly relevant given the ordering bug in the write path.
[missing-cross-reference] docs/guides/user/tracing-with-mlflow.md:97 — The MLflow tracing guide's "See also" section does not cross-reference the new eval measurements guide, which documents the MLFLOW_TRACKING_* env vars for the Assessments adapter.

Labels: PR adds new Go eval-measure package (internal/evalmeasure/), modifies CI workflows (.github/workflows/), and adds documentation (docs/guides/, docs/ADRs/)

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review · 2026-08-10T13:34:18Z

🤖 Review · ❌ Terminated · Started 1:34 PM UTC · Ended 1:54 PM UTC

Commit: 49f0d19 · View workflow run →

codecov · 2026-08-10T13:48:05Z

Codecov Report

❌ Patch coverage is 88.13793% with 86 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/evalmeasure/parse.go	75.47%	20 Missing and 6 partials ⚠️
internal/cli/evalmeasure.go	86.66%	12 Missing and 10 partials ⚠️
internal/evalmeasure/fitness.go	91.56%	8 Missing and 6 partials ⚠️
internal/evalmeasure/export_local.go	78.57%	5 Missing and 4 partials ⚠️
internal/evalmeasure/run.go	85.36%	3 Missing and 3 partials ⚠️
internal/evalmeasure/find.go	92.50%	2 Missing and 1 partial ⚠️
internal/evalmeasure/registry.go	95.45%	1 Missing and 1 partial ⚠️
internal/fetch/fetch.go	50.00%	2 Missing ⚠️
internal/sandbox/sandbox.go	92.85%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

fullsend-ai-review · 2026-08-10T13:54:44Z

🤖 Finished Review · ✅ Success · Started 1:34 PM UTC · Completed 1:54 PM UTC

Commit: 49f0d19 · View workflow run →

ralphbean · 2026-08-10T15:15:50Z

Heads up — this looks adjacent to #5524 (ADR 0075, harness-snapshot.json). Both build on ADR 0050 and add a new file riding alongside run-telemetry.jsonl. ADR 0087 already lists #5947/#5944/#2423 as adjacent proposals — wonder if 0075 belongs there too?

- Fix data-loss bug: swap AppendMeasurements before RecordScored so ledger only marks scored after measurement is persisted - Fix broken curl header args in action.yml using bash array instead of parameter expansion (word-split safe) - Add Prerequisites section to eval-measurements guide - Add OTEL/OTLP inline definitions on first use in guide - Use > **Planned:** callout format for unimplemented OTLP export - Update docs/guides/README.md with eval measurements entry - Consolidate ADR 0087 Decision into single paragraph (was multiple sub-decisions); trim Consequences to 5 bullets - ADR 0050 cross-reference uses Planned callout format - Rename printResults → printMeasurementResults Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fullsend-ai-review · 2026-08-11T01:04:59Z

🤖 Review · ⚠️ Cancelled · Started 1:04 AM UTC · Ended 1:14 AM UTC

Commit: 43a6ebd · View workflow run →

fullsend-ai-review · 2026-08-11T01:15:57Z

🤖 Finished Review · ✅ Success · Started 1:15 AM UTC · Completed 1:33 AM UTC

Commit: 4bf9b26 · View workflow run →

Superseded by updated review

fullsend-ai-review · 2026-08-20T13:43:50Z

🤖 Finished Review · ✅ Success · Started 1:43 PM UTC · Completed 2:04 PM UTC

Commit: b7a6591 · View workflow run →

waynesun09

Review-only pass: 2 findings on this PR (scaffold breaking-change labeling, eval-measure manifest trust boundary). See inline comments.

Managed GHA/GitLab eval-measure no longer prefers MR/PR-tree .fullsend/eval overrides; use default/base tip via --registry or SHA-pinned agents@v0. Label the GitLab output-dir default move as breaking per COMMITS.md. BREAKING CHANGE: GitLab fullsend-agent scaffold default --output-dir now writes under ${CI_PROJECT_DIR}/output (was an ephemeral tmp path) so eval-measurements.jsonl can be retained as a CI artifact. Re-sync adds a top-level output/ directory, excludes it from the sandbox upload/git when nested under --target-repo, and keeps telemetry as a 1-week artifact. Jobs that depended on the old tmp layout must adopt the new path or pin an older scaffold. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Per-org installs use workspace-root eval/measurements/, not .fullsend/eval/… — derive the git-show path from inputs.fullsend-dir while still reading bytes from the PR base / GITHUB_SHA tip. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>

fullsend-ai-review · 2026-08-20T14:41:36Z

🤖 Finished Review · ✅ Success · Started 2:41 PM UTC · Completed 3:00 PM UTC

Commit: 903c0a4 · View workflow run →

waynesun09

Approving at head 903c0a42 after multiple review rounds. Design assessment:

Architecture/intention — sound.

Clean layering: eval measurements (online, deterministic, fail-open, never gating) are correctly separated from the ADR 0051 functional eval harness, and from primary OTEL facts (measurements are a derived sibling artifact that never rewrites the trace). The id@version ledger keying gives trend continuity a real contract.
The ownership seam matches the repo split: executable logic (parser/scorers/CLI/CI step) in fullsend because it reads what fullsend writes; per-agent enablement policy in fullsend-ai/agents manifests; org policy via local override. The change-guide table in the PR body makes the seam actionable.
Tool-agnostic export is the right call: portable JSONL now, remote export reusing the ADR 0050 OTEL_EXPORTER_OTLP_* path later — no vendor Assessments coupling in core.
The manifest trust model is now sound after 8f3564d6/903c0a42: managed jobs never read measurement policy from the MR/PR working tree (trend-poisoning closed); overrides come from the default/base tip via git show + --registry (mirroring kill-switch config trust), else SHA-pinned agents@v0 via GetRef — and the GITHUB_SHA fallback only engages on non-PR events, where it is trusted by definition.
Known failure modes are handled deliberately: skip semantics for pre-script-skipped/incomplete traces, gen_ai.system → gen_ai.provider.name semconv-rename tolerance in modelOK, retro work-item gap closed via ORIGINATING_URL, and the breaking GitLab --output-dir move is properly labeled with a migration path.

Non-blocking notes (fine as follow-ups):

The per-org claim in 903c0a42 doesn't hold end-to-end: reusable-dispatch passes fullsend-dir: "" in org mode, and the measure step's shell default is .fullsend (the GITHUB_WORKSPACE equality case also derives an empty MEASURE_REL), so a workspace-root eval/measurements/ override never activates — it silently falls back to agents@v0. Fail-safe, and org mode is deprecated (ADR 0044), but either fix the derivation or drop the per-org wording from docs/comments so nobody expects it to work.
Two-step activation (#6384): nothing enforces the release cut after agents#722 merges, so merged manifests can silently no-op until v0 re-points. The tracking issue is the mitigation — keep it pinned until the release lands.
Agreed with the bot lows: shell-level ${AGENT}/${STAGE} validation before the git show interpolation would be cheap defense-in-depth (the Go CLI sanitizes, but the shell path runs first), and the base-tip manifest lookup is case-sensitive where ScoreTrace matching is not.
eval-measurements.jsonl becomes a de-facto cross-repo contract once dashboards consume it — worth a short normative note (like normalized-event v1) before a second consumer appears.

Prior rounds (output/ exclusion scoping, UserCacheDir fetch cache, exporter→parser round-trip test, architecture-doc consistency, BREAKING labeling, MR-tree manifest trust) are all verified fixed at this head. CI green; review bot approved.

Keep the eval-measurements branch current with main. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>

fullsend-ai-review · 2026-08-20T15:30:54Z

🤖 Finished Review · ✅ Success · Started 3:30 PM UTC · Completed 3:51 PM UTC

Commit: 30564ef · View workflow run →

fullsend-ai-retro · 2026-08-20T16:07:02Z

🤖 Finished Retro · ✅ Success · Started 4:07 PM UTC · Completed 4:24 PM UTC

Commit: 30564ef · View workflow run →

fullsend-ai-retro · 2026-08-20T16:24:23Z

Retro: PR #6036 — feat(eval-measurements): online trace scoring

Workflow shape: Human-authored PR (49 files, +3943/−112) reviewed by fullsend-ai-review[bot] (19 runs over 10 days), qodo-code-review[bot] (2 runs), and human reviewer waynesun09 (8 review rounds). No code/fix/triage agents dispatched. 26 additional workflow triggers resolved to "no stage matched." Agents repo: fullsend-ai/agents@v0 (commit 6bdcab691824).

Review quality gap

The human reviewer identified 9 HIGH and 31+ MEDIUM findings; the review agent identified 1 HIGH and 2 MEDIUM (plus ~18 LOW). The agent's unique HIGH (duplicate artifact upload step causing name collision) was valid and not caught by the human. However, the agent missed every security-critical and cross-cutting finding:

Supply-chain security: floating v0 curl with no SHA pinning (human HIGH)
Trust-boundary violation: unbounded find can score agent-planted telemetry (human HIGH) — the agent flagged the same find as LOW "non-determinism" without recognizing the attack surface
Scorer correctness: 4 structural gaps where EM-001 produces fail for cases that should be skip (human HIGH)
Cross-forge gaps: 5+ GitLab-specific issues (missing GH_TOKEN, ephemeral output path, unset env vars, artifact retention, offline mode)
Upstream spec staleness: gen_ai.system renamed in OTel semconv v1.37.0 — code ships already-stale
Fix verification failures: human found 3 fix commits that didn't achieve their stated goals; agent re-reviewed without catching this

Token cost

19 review agent runs (each reviewing a 49-file, 3943-line diff with Opus) plus 26 "no stage matched" workflow triggers. The agent produced largely similar LOW findings across the last ~8 runs with minimal progression in finding quality. 4 runs on Aug 20 alone within 3 hours.

Evidence for existing issues

fullsend#898 (review agent misses security-critical findings on large architectural PRs): This PR is strong evidence — the agent missed all security-critical findings on a 49-file PR.
agents#858 (security sub-agent should trace data flow across file boundaries): The agent failed to connect find -print -quit non-determinism with agent-writable files in the same directory tree.
agents#856 (security sub-agent should compose individually-low findings into aggregate risk): The find non-determinism (LOW) + agent-writable files = spoofable telemetry (HIGH) — a composition the agent didn't make.
agents#659 (security sub-agent should cross-reference sandbox capabilities): Understanding that sandbox.ExtractOutputFiles copies agent-writable files alongside platform telemetry would have elevated the find finding.
agents#343 (scope re-review to finding verification): The agent re-reviewed the full diff 19 times instead of focusing on whether prior findings were resolved.
agents#685 (explicitly resolve or persist prior findings on re-review): The agent didn't track which prior-round findings were resolved vs. still open.
fullsend#5007 (deduplicate inline comments across force-push re-reviews): 43 inline comments with many duplicates across re-review cycles.
fullsend#1418 / fullsend#893 / fullsend#2994 (reduce redundant dispatches): 26 "no stage matched" triggers wasted compute.
fullsend#6275 (automate cross-forge harness passthrough variable parity test): Human found 5+ GitLab-specific gaps by manually comparing scaffold templates.
fullsend#3532 (review agent should cross-check commit message claims against actual code): 3 fix commits that didn't achieve their stated goals went uncaught by the agent.

Proposals filed

Review agent should verify upstream specification currency when code references external standards (in fullsend-ai/agents)
Review agent correctness sub-agent should verify fix efficacy against prior-round findings on re-review (in fullsend-ai/agents)

ascerra requested a review from a team as a code owner August 10, 2026 11:44

github-actions Bot deployed to site-preview August 10, 2026 11:46 View deployment

ascerra marked this pull request as draft August 10, 2026 11:50

qodo-code-review Bot reviewed Aug 10, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added type/feature New capability request component/ci CI pipelines and checks go Pull requests that update go code component/docs User-facing documentation labels Aug 10, 2026

ascerra mentioned this pull request Aug 10, 2026

feat(eval): add measurement manifests for online scoring fullsend-ai/agents#722

Merged

4 tasks

github-actions Bot deployed to site-preview August 10, 2026 13:35 View deployment

This comment was marked as outdated.

Sign in to view

ralphbean mentioned this pull request Aug 10, 2026

docs(adr): accept ADR 0090 harness snapshot for agent traces #5524

Open

5 tasks

github-actions Bot deployed to site-preview August 11, 2026 01:06 View deployment

github-actions Bot deployed to site-preview August 11, 2026 01:16 View deployment

This comment was marked as outdated.

Sign in to view

waynesun09 reviewed Aug 20, 2026

View reviewed changes

Comment thread internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-agent.yml

Comment thread internal/cli/evalmeasure.go

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed requires-manual-review Review requires human judgment labels Aug 20, 2026

ascerra and others added 2 commits August 20, 2026 10:35

ascerra changed the title ~~feat(eval): add eval measurements and EM-001 trace_fitness scorer~~ feat(eval)!: add eval measurements and EM-001 trace_fitness scorer Aug 20, 2026

ascerra requested a review from waynesun09 August 20, 2026 14:39

ascerra temporarily deployed to dev August 20, 2026 14:39 — with GitHub Actions Inactive

github-actions Bot deployed to site-preview August 20, 2026 14:41 View deployment

fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed ready-for-merge All reviewers approved — ready to merge labels Aug 20, 2026

waynesun09 approved these changes Aug 20, 2026

View reviewed changes

chore: merge origin/main into feat/eval-measurements

30564ef

Keep the eval-measurements branch current with main. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>

ascerra had a problem deploying to dev August 20, 2026 15:28 — with GitHub Actions Failure

github-actions Bot deployed to site-preview August 20, 2026 15:30 View deployment

ascerra temporarily deployed to dev August 20, 2026 15:39 — with GitHub Actions Inactive

ascerra added this pull request to the merge queue Aug 20, 2026

Merged via the queue into main with commit fc74fab Aug 20, 2026
24 of 25 checks passed

ascerra deleted the feat/eval-measurements branch August 20, 2026 16:05

fullsend-ai-retro Bot mentioned this pull request Aug 20, 2026

Review agent should verify upstream specification currency when code references external standards fullsend-ai/agents#893

Open

This was referenced Aug 20, 2026

Review agent correctness sub-agent should verify fix efficacy against prior-round findings on re-review fullsend-ai/agents#894

Open

Review agent should verify documentation claims against explicitly-coupled companion PRs fullsend-ai/agents#895

Open

dhshah13 mentioned this pull request Aug 20, 2026

feat(telemetry): implement ADR 0050 Level 3 content capture #6429

Open

Uh oh!

Conversation

ascerra commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Breaking change

Ownership (please read)

Tool-agnostic export

Related

Changes

Testing

Checklist

Notes for reviewers

Uh oh!

fullsend-ai-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary by Qodo

Uh oh!

github-actions Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Site preview

Uh oh!

qodo-code-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Low

Review

Findings

Medium

Low

Info

Review

Findings

Low

Review

Findings

Low

Review

Findings

Low

Review

Findings

Low

Review

Findings

Low

Review

Findings

Low

Review

Findings

High

Low

Review

ascerra commented Aug 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 10, 2026 •

edited

Loading

qodo-code-review Bot commented Aug 10, 2026 •

edited

Loading

github-actions Bot commented Aug 10, 2026 •

edited

Loading

qodo-code-review Bot commented Aug 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 10, 2026 •

edited

Loading

codecov Bot commented Aug 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 11, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 11, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 20, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 20, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 20, 2026 •

edited

Loading

fullsend-ai-retro Bot commented Aug 20, 2026 •

edited

Loading