Skip to content

fix(tests): route-specific command telemetry must not move the score - #3516

Merged
dmetzgar merged 2 commits into
mainfrom
fix/non-gating-command-telemetry-is-weightless
Sep 24, 2026
Merged

dmetzgar merged 2 commits into
mainfrom
fix/non-gating-command-telemetry-is-weightless

Conversation

@dmetzgar

@dmetzgar dmetzgar commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Revised after review. The first pass keyed on criterion type — every non-gating command_executed — and that was wrong; it swept in solution init, flow validate, flow debug and flow eval, which both routes run. It now keys on the command. Details in "What changed after review" below.

The bug

pass_threshold: 0 was read as "this criterion is advisory", and test_v1_only_authoring_commands_match_the_temporary_allowlist enforces only that. It is half the idiom — coder_eval's own field docs carry the other half:

weight=0 excludes from the score but NOT from the pass/fail gate … To make a criterion truly non-gating, also set pass_threshold=0.

So pass_threshold: 0 with weight: 1.5 never fails a task and always moves its score.

That matters where the command itself is route-specific. In the 2026-09-23 same-ground-ab run those criteria scored 1.0 for v1 and 0.0 for v2, dragging tasks that passed every graded check down with them. datafabric_integration_create_get returned SUCCESS at 0.55 — three flow node add advisories weighing 5.0 against two graded criteria at 3.0.

The rule

A criterion is in scope only when its command has no counterpart in the other route. Four families, each with its reason recorded beside it in ROUTE_SPECIFIC_COMMANDS:

family why count
flow node add|configure|remove|update, flow edge … v1 mutates the graph a node at a time; the SDK loop writes .flow.ts 6
agent init|refresh --inline-in-flow|--conversational v1 scaffolds the sidecar with the CLI; conversationalAgent() emits agent.json itself 5
is connections list, is resources run list, is triggers objects|describe registry prepare picks the connection, pages the collection and writes bindings in one call 4
flow registry pull the manifest refresh the SDK loop has no need of 8

23 criteria across 17 files. A new gate, test_route_specific_command_telemetry_is_weightless, keeps it true. Three criteria had no weight: line at all — which defaults to 1.0 — so silence is treated as a violation, not as compliance.

What is deliberately not in scope

solution init, flow init, flow validate, flow debug, flow eval …, --output json, is webhooks config, ixp projects create — shared lifecycle commands both routes run on the same artifact. 31 of the 54 observed criteria were BOTH PASS, these among them.

flow registry get|search|list is out for the same measured reason: the SDK arm passes those in the IxP tasks, so the node registry is not a v1-only surface. Only the refresh step is.

Where an arm fails one of the survivors — flow debug in slack_channel_description_simulated, solution init in webhook_waitfor_parallel — that is now a finding about the arm, which is the point.

stop_early criteria are exempt: there weight is load-bearing for the pass-stop floor, not just the score (ixp/routing.yaml's sentinel says so verbatim), so the gate skips them rather than trusting one reading of one installed coder-eval version.

Effect, replayed against the run's own recorded results

arm before after
v1 0.959 0.959
v2 0.919 0.933

Arm delta narrows −0.041 → −0.027. Eight v2 tasks improve, five to a clean 1.00. v1's mean does not move at all and no task in either arm goes down — the signature of a correction that is actually neutral.

Pass/fail counts do not move, because weight never gated. This is a score correction, not a pass-rate one.

What changed after review

first pass now
keyed on criterion type (command_executed + pass_threshold: 0) the command's route-specificity
criteria 61 across 38 files 23 across 17 files
v2 mean 0.935 0.933
v1 mean 0.960 (moved) 0.959 (unmoved)
tasks regressed 2 per arm none
allowlist needed yes — registry_discovery would have scored 0.0 with no weight anywhere no — only its registry pull is in scope, leaving 4.5 of weight

The two regressions the broad sweep caused (e2e_devcon_expense_approval 0.37 → 0.23, interactive_customer_escalation_triage 0.45 → 0.38) came from zeroing flow validate and solution init telemetry those tasks were passing. Both files are untouched now.

Verification

  • pytest tests/tasks/uipath-maestro-flow — 1361 pass.
  • The new gate verified to catch a violation: restoring one weight: 0 fails it with the offending path named.
  • All 17 changed YAMLs parse; no task is left with zero total weight.
  • Score effects above are a replay of the recorded success_criteria_results against the new weights, not an estimate.

🤖 Generated with Claude Code

`pass_threshold: 0` was read as "this criterion is advisory", and
`test_v1_only_authoring_commands_match_the_temporary_allowlist` enforces only
that. It is half the idiom. coder_eval's own field documentation spells out the
other half:

    weight=0 excludes from the score but NOT from the pass/fail gate ...
    To make a criterion truly non-gating, also set pass_threshold=0.

So a criterion with `pass_threshold: 0` and `weight: 1.5` never fails a task and
always moves its score — it sits in both halves of the weighted mean.

That is not neutral between arms. A `command_executed` grades the SHELL COMMAND
an arm ran, and the two arms run different commands by construction. In the
2026-09-23 same-ground run, 15 such criteria across 10 flow tasks scored 1.0 for
v1 and 0.0 for v2 — `flow node add`, `flow registry get`, `agent init
--conversational`, `is connections list`, `is resources run list` — and dragged
tasks that passed every graded check down with them.
`datafabric_integration_create_get` returned SUCCESS at 0.55: three advisories
at weights 1.5/2.0/1.5 against two graded criteria at 3.0.

The outcome each of those stands in for is already graded by a `run_command`
against the artifact, which is route-blind. `slack_channel_description_simulated`
says so in its own comment: the run_command "Supersedes a static connector-key
grep ... THIS is the authoritative proof". So the telemetry keeps reporting and
stops scoring.

61 criteria across 38 tasks get `weight: 0` — three of them had no `weight` line
at all, which defaults to 1.0. `single_node/outlook_trigger_inbox` already did
this and is the shape the rest now match.

Two exclusions, both deliberate:

- `ixp/routing.yaml` — its two criteria carry `stop_early`, where weight is
  load-bearing for the pass-stop floor rather than for the score. Its own
  sentinel comment says so. The new gate skips `stop_early` criteria.
- `smoke/registry_discovery.yaml` — allowlisted. It "deliberately produces no
  artifact", so all four of its criteria grep the shell; zeroing them leaves a
  total weight of 0 and `calculate_weighted_score` reports 0.0 for that. It needs
  an outcome-graded criterion over the agent's report, which is a task redesign,
  not a reweight.

Replaying the run's recorded criterion results against the new weights: the flow
suite's v2 mean score goes 0.919 -> 0.935 and v1 0.959 -> 0.960, narrowing the
arm delta from -0.041 to -0.025. Pass/fail counts do not move, because weight
never gated. Seven v2 tasks reach a clean 1.00.

Two tasks per arm go DOWN, which is the point working: passing telemetry was
padding a failing grade. `e2e_devcon_expense_approval` drops 0.37 -> 0.23 in v2
and now reads as the HITL outcome-port failure it is.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @dmetzgar's task in 2m 27s —— View job


PR Review: fix(tests): non-gating command telemetry must not move the score either

  • Read code review guidelines and project conventions
  • Read the full diff (39 files, +117/−56)
  • Read the new test gate in test_same_ground_corpus.py
  • Spot-check YAML changes for correctness
  • Check for consistency across all 38 changed task files
  • Post review findings

Summary

Sets weight: 0 on 57 command_executed criteria (across 38 task YAMLs) that already had pass_threshold: 0, closing a gap where "advisory" telemetry criteria still influenced the weighted score. Adds a structural gate (test_non_gating_command_telemetry_is_weightless) to enforce the invariant going forward, with a documented allowlist for smoke/registry_discovery.yaml (which needs a task redesign) and a stop_early exemption for ixp/routing.yaml (where weight is load-bearing for the pass-stop floor).

Change-by-Change Review

1. tests/tasks/uipath-maestro-flow/_shared/test_same_ground_corpus.py — New gate

Severity: OK

Well-implemented structural test. Key observations:

  • The NON_GATING_WEIGHT_ALLOWLIST with smoke/registry_discovery.yaml is well-justified — the comment explains that zeroing all weights there would collapse the score to 0.0 and the real fix is a task redesign.
  • The stop_early exemption is correct. Verified that ixp/routing.yaml:100 and ixp/routing.yaml:125 carry stop_early, and the sentinel comment at line 118 explicitly warns against zeroing the weight: "a zero-weight armed criterion collapses the pass-stop floor."
  • The gate correctly catches absent weight lines (which default to 1.0 in coder_eval), treating silence as non-compliance — line 242 checks weight is None or float(weight.group(1)) != 0.
  • Docstring is thorough and explains the measured cost, the root cause, and the reasoning for the stop_early exemption.

2. 38 YAML task files — weight: N → weight: 0

Severity: OK

All 38 files follow the same mechanical pattern: changing weight: <non-zero> to weight: 0 on command_executed criteria that already had pass_threshold: 0 (or pass_threshold: 0.0). Three specific cases verified:

  • ixp/routing_listing.yaml: This file had no weight line at all — it gains weight: 0 (line 68). Correct — this is one of the three files the PR description mentions where "silence is not compliance."
  • ixp/e2e_03_project_creation_handoff.yaml: Both non-stop_early advisory criteria get weight: 0. The stop_early criterion at line 121 is untouched. Correct.
  • Multi-criteria files (e.g., integration_create_get.yaml with 3 criteria, voice_outbound_call.yaml with 3): all matching criteria are zeroed consistently.

No pass_threshold values were changed. No non-command_executed criteria were touched. No criteria with stop_early were touched. The change is mechanical and correct.

3. Deliberate exclusions

Severity: OK

  • ixp/routing.yaml — excluded via the stop_early check in the gate. Both its command_executed criteria carry stop_early. The sentinel comment at line 118 documents why weight must remain non-zero. Correct.
  • smoke/registry_discovery.yaml — allowlisted. All 4 criteria are command_executed with pass_threshold: 0.0 and non-zero weights. The task "deliberately produces no artifact" so zeroing all weights leaves total weight of 0. The allowlist comment explains this and flags the task redesign needed.

4. Scope check: generate_schema.yaml

Severity: OK

The one command_executed with pass_threshold: 0.0 and weight: 3.0 outside this PR's scope is in connector_features/generate_schema.yaml, which is tagged uipath-platform (not uipath-maestro-flow). The new gate filters on the uipath-maestro-flow tag so it correctly excludes this file. Worth noting for a future sweep, but not in scope here.

What's Missing

Nothing identified. The PR's own description notes the registry_discovery task redesign as future work — that's correctly scoped as a separate change.

Area Ratings

Area Status Notes
Frontmatter N/A No skill files changed
E2E Tests OK Test gate added, all YAMLs still parse
Skill Body N/A No skill files changed
References & Assets N/A No references or assets changed
Repo Hygiene OK All changes scoped to tests/, no secrets, no unrelated changes

Issues for Manual Review

  • generate_schema.yaml (uipath-platform tag): has the same pass_threshold: 0 + weight: 3.0 pattern on a command_executed criterion. Not covered by the new gate (different tag). Consider whether it needs the same fix in a follow-up.
  • The PR description claims 1361 pytest passes — I was unable to run the test suite to independently verify.

Conclusion

Clean, well-scoped fix backed by measured data. The mechanical YAML changes are consistent, the new gate is correctly implemented with appropriate exemptions, and the exclusions are well-documented. No issues found — approve.

@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @dmetzgar's task in 2m 34s —— View job


Coder-eval task lint (advisory)

38 task YAMLs modified; 0 Critical, 0 High, 0 Medium, 0 Low, 38 OK.

Rubric: .claude/commands/lint-task.md. This check is advisory and never blocks merge.

Evidence of passing run

✅ Author confirms passing run: "pytest tests/tasks/uipath-maestro-flow — 1361 pass." and "New gate verified to actually catch a violation: reverting one weight: 0 fails it with the offending path named."

Per-task lint

Every change across all 38 files is identical in shape: weight: <non-zero> → weight: 0 on command_executed criteria that already carry pass_threshold: 0 (or pass_threshold: 0.0). One file (ixp/routing_listing.yaml) adds an explicit weight: 0 line where the field was previously absent (implicit default 1.0). No prompts, criteria types, patterns, or other fields were modified.

This change does not trigger any rubric axis — it fixes a pre-existing scoring inconsistency where advisory criteria still carried weight in the score calculation.

All 38 files — verdict: OK (click to expand)
File Verdict
connector_features/datafabric_connector/integration_create_get.yaml OK
connector_features/datafabric_connector/smoke_error.yaml OK
connector_features/datafabric_connector/smoke_query.yaml OK
connector_features/enum.yaml OK
connector_features/generic_dynamic_node/generic_dynamic_node.yaml OK
connector_features/jdbc_databricks_query/jdbc_databricks_query.yaml OK
connector_features/paginated_reference_lookup.yaml OK
connector_features/path_params.yaml OK
connector_features/slack-http-fallback/slack_http_fallback.yaml OK
connector_features/testmanager_attachments/testmanager_attachments.yaml OK
connector_features/testmanager_crud_grounded/testmanager_crud_grounded.yaml OK
connector_features/testmanager_execution_results/testmanager_execution_results.yaml OK
connector_features/testmanager_generic_records/testmanager_generic_records.yaml OK
connector_features/testmanager_requirement_lifecycle/testmanager_requirement_lifecycle.yaml OK
connector_features/testmanager_testcase_lifecycle/testmanager_testcase_lifecycle.yaml OK
connector_features/testmanager_testset_lifecycle/testmanager_testset_lifecycle.yaml OK
connector_trigger/webhook_waitfor_parallel.yaml OK
conversational/conversational_chat_loop.yaml OK
e2e/devcon_expense_approval.yaml OK
e2e/jira_lifecycle/jira_lifecycle.yaml OK
evaluate/evaluator_type_choice.yaml OK
evaluate/local_crud.yaml OK
evaluate/no_auto_upload.yaml OK
evaluate/simulation/simulation_crud.yaml OK
interactive/customer_escalation_triage/customer_escalation_triage.yaml OK
interactive/slack_channel_description_simulated/slack_channel_description_simulated.yaml OK
ixp/e2e_02_project_selection.yaml OK
ixp/e2e_03_project_creation_handoff/e2e_03_project_creation_handoff.yaml OK
ixp/e2e_04_build_mechanics.yaml OK
ixp/integration_handle_routing.yaml OK
ixp/routing_listing.yaml OK
ixp/scaffold_minimal.yaml OK
ixp/scaffold_multinode.yaml OK
node_features/datafabric_native/integration_native_read_create.yaml OK
smoke/init_maestro_automate.yaml OK
smoke/init_validate.yaml OK
voice/voice_inbound_call.yaml OK
voice/voice_outbound_call.yaml OK

Within-PR duplicates

No duplicate clusters detected. (No new task YAMLs were added.)

Conclusion

✅ All changed tasks pass the rubric. Evidence of passing run confirmed. The change is a targeted score-correction: advisory command_executed criteria (already non-gating via pass_threshold: 0) are made truly weightless (weight: 0) so they stop biasing the weighted mean between arms. The new gate test_non_gating_command_telemetry_is_weightless enforces this as an invariant going forward.


Review caught this, and the review was right. The first pass keyed on criterion
TYPE — every non-gating `command_executed` — on the reasoning that such a
criterion grades the shell command an arm ran, and the two arms run different
commands. That reasoning does not survive the data.

Of the 54 criteria it swept that the 2026-09-23 run actually observed, 31 were
BOTH PASS: `solution init` (5), `flow validate` (4), `flow debug` (3),
`flow eval ...` (3), `flow init` (2), `--output json`, `is webhooks config`,
`ixp projects create`. Those are lifecycle commands both routes run on the same
artifact, and both routes do run them. Zeroing them removed real, satisfiable
signal and bought no neutrality — and it moved v1's own mean, which a neutrality
correction has no business doing.

So the rule is now about the COMMAND: a criterion is in scope only when its
command has no counterpart in the other route. Four families, each with its
reason recorded next to it:

  - `flow node add|configure|remove|update`, `flow edge ...` — v1 mutates the
    graph a node at a time; the SDK loop writes `.flow.ts`.
  - `agent init|refresh --inline-in-flow|--conversational` — v1 scaffolds the
    sidecar with the CLI; `conversationalAgent()` emits `agent.json` itself.
  - `is connections list`, `is resources run list`, `is triggers objects|describe`
    — `registry prepare` picks the connection, pages the collection and writes
    bindings in one call.
  - `flow registry pull` — the manifest refresh the SDK loop has no need of.

`flow registry get|search|list` is deliberately NOT a family: the SDK arm passes
those in the IxP tasks, so the node registry is not a v1-only surface. Only the
refresh step is.

23 criteria across 17 files, down from 61 across 38.

Replayed against the run's recorded results: v2's mean goes 0.919 -> 0.933 and
the arm delta -0.041 -> -0.027, within a thousandth of the broad sweep's 0.935.
v1's mean does not move at all and no task in either arm goes down — where the
first pass dropped `e2e_devcon_expense_approval` 0.37 -> 0.23 and
`interactive_customer_escalation_triage` 0.45 -> 0.38 by zeroing validate and
solution-init telemetry those tasks were passing.

`smoke/registry_discovery` no longer needs an allowlist: only its `registry
pull` criterion is in scope, leaving 4.5 of weight behind, so the zero-total
score the broad sweep would have produced cannot arise.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmetzgar dmetzgar changed the title fix(tests): non-gating command telemetry must not move the score either fix(tests): route-specific command telemetry must not move the score Sep 23, 2026
@dmetzgar
dmetzgar merged commit 5db8dd8 into main Sep 24, 2026
39 checks passed
@dmetzgar
dmetzgar deleted the fix/non-gating-command-telemetry-is-weightless branch September 24, 2026 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants