feat(criteria): accept glob patterns in criterion path fields by jiyangzh · Pull Request #65 · UiPath/coder_eval

jiyangzh · 2026-07-29T23:02:51Z

Problem

A criterion path is a literal, so a task must hardcode where an artifact lands. When the prompt does not pin that location, a correct artifact scores 0.0 on the path alone.

Real case from the UiPath/skills smoke gate. uip maestro flow init <Name> scaffolds a wrapper solution directory whose name the prompt never specifies. The agent chose InvoiceApprovalSolution/, the task asserted InvoiceApproval/InvoiceApproval/InvoiceApproval.flow:

Criterion	Result
`run_command` — validate the discovered flow	pass — `"Status": "Valid"`
`file_exists` — hardcoded path	0.0 — "does not exist"
`file_contains` — hardcoded path	0.0 — "does not exist"

Task scored 0.375 on a valid artifact and failed the ≥95% pass-rate gate. The workaround was a per-repo helper script that globs and shells out — one more thing to maintain, and it only fixes the tasks that adopt it.

Change

path resolves through a new Sandbox.resolve_files. A pattern containing *, ?, or [ expands against the sandbox root; a literal path is untouched.

- type: "file_contains"
  path: "**/*.flow"
  includes: ['"core.logic.decision"']

Because every path-based criterion already routed through sandbox.file_exists / sandbox.get_file_content, they all inherit this from one change point: file_exists, file_contains, file_matches_regex, file_check, json_check, import_check.

Semantics:

file_exists passes on at least one match.
Content reads require exactly one match. More than one raises ValueError listing every match — ValueError is not in _ESCALATING_EXCEPTIONS, so handle_criterion_errors captures it as a scored-0.0 result with the message. An ambiguous pattern is reported, never silently resolved to one arbitrary file.
Matches are sorted so grading is deterministic; directories are dropped so a glob cannot resolve to something unreadable.
A literal path behaves exactly as before — no existing task changes meaning.

Verification

13 new tests in tests/test_glob_paths_in_file_criteria.py — unit coverage of resolve_files (literal, glob hit, glob miss, ambiguous, directory-skipping, sort order, unset sandbox) plus end-to-end coverage through SuccessChecker for file_exists / file_contains / file_check, including the ambiguous-glob 0.0-with-message path.
Full suite: 3744 passed, 2 skipped (both environment-gated: no ANTHROPIC_API_KEY, Windows-only).
ruff format --check, ruff check, pyright (0 errors), and the custom-lint suite (166 passed) all clean.

Follow-up in UiPath/skills

Once this releases and the tests/.coder-eval-version pin moves off 0.8.8, the interim helper script is deleted and ~19 task files with hardcoded <Name>/<Name>/<Name>.flow paths collapse to path: "**/*.flow".

A criterion `path` is a literal, so a task must hardcode where an artifact lands. When the prompt does not pin that location — a scaffolding tool that creates a wrapper directory the agent names itself — a correct artifact in an unexpected directory scores 0.0 on the path alone, while a sibling criterion that discovers the file reports the run as valid. Resolve `path` through `Sandbox.resolve_files`, which expands a pattern containing `*`, `?`, or `[` against the sandbox root and leaves a literal path untouched. Every path-based criterion inherits it: file_exists, file_contains, file_matches_regex, file_check, json_check, import_check. `file_exists` passes on at least one match. Content reads require exactly one match and otherwise raise, listing every match — an ambiguous pattern is captured as a scored-0.0 result rather than silently grading one arbitrary file. Matches are sorted for determinism and directories are dropped.

github-actions · 2026-07-29T23:03:07Z

uipreliga

Review: coder_eval — pr:65

Scope: pr:65 · branch feat/glob-paths-in-file-criteria · 1e2ca2e · 2026-07-30T17:45Z · workflow variant

Change class: complex — changes the path-resolution semantics of shared Sandbox APIs (file_exists, get_file_content) that every path-based criterion consumes, adding new control flow and a new raise path that can change a task's score

coder_eval is in excellent shape overall (9.4/10 — clean style, strong typing, no security or error-handling defects, well-factored plugin/merge architecture), but the new glob seam in Sandbox.resolve_files (src/coder_eval/sandbox.py:970-1034) is the real risk: globbing is inferred from the presence of *?[ rather than opted into, and expands unfiltered over a sandbox root containing .venv/node_modules/copied starter files, so identical agent output can score differently — fix the two harness defects before trusting any run for grading, and treat the rest as documentation and robustness polish.

Summary

Axis	Score	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	10 / 10	0	0	0	—
2. Type Safety	8.4 / 10	0	3	1	Path fields that gained glob semantics (json_check.path/json_schema, classification_match.path) still carry pre-glob Field descriptions (schema SSOT drift)
3. Test Health	9.9 / 10	0	0	1	`test_matches_are_sorted` is self-referential and passes vacuously on an empty result
4. Security	10 / 10	0	0	0	—
5. Architecture & Design	9.9 / 10	0	0	1	reference_comparison.agent_file is a sandbox-relative path field that bypasses the new seam, so path semantics now silently differ per criterion
6. Error Handling & Resilience	10 / 10	0	0	0	—
7. API Surface & Maintainability	9.5 / 10	0	1	0	Ambiguous-glob ValueError enumerates every match unbounded and is duplicated into details/error (persisted to task.json, injected into judge prompts)
8. Evaluation Harness Quality	7.5 / 10	2	1	0	String-sniffing `*?[` in resolve_files silently reinterprets a literal path as a glob (no literal-first fallback, no regression test)

Overall Score: 9.4 / 10 · Weakest Axis: Evaluation Harness Quality at 7.5 / 10
Totals: 🔴 0 · 🟠 2 · 🟡 5 · 🔵 3 across 8 axes.

Blockers

[Axis 8] String-sniffing *?[ in resolve_files silently reinterprets a literal path as a glob (no literal-first fallback, no regression test) (src/coder_eval/sandbox.py:990) — resolve_files classifies a path as a glob purely by character scan:

if not any(c in path for c in "*?["):
    candidate = self.sandbox_dir / path
    return [candidate] if candidate.exists() else []

return sorted(p for p in self.sandbox_dir.glob(path) if p.is_file())

There is no escape hatch and no author opt-in — any path that contains a metacharacter is routed to Path.glob, where brackets become a character class. Verified on this machine (python 3.14 pathlib, same semantics as 3.13): with report[2024].json (real, content "literal") and report2.json (content "decoy") both present in the sandbox, d.glob("report[2024].json") returns ['report2.json']. So a criterion written as a plain literal path: "report[2024].json" now resolves to exactly one match — the WRONG file — and get_file_content silently grades the decoy with no error and no ambiguity warning. This is precisely the failure the PR's commit message says it prevents ("rather than silently grading one arbitrary file"). The inverse also verified: d.glob("logs[1]") returns [] while (d/'logs[1]').exists() is True, so file_exists flips True->False for an unchanged literal path and the criterion scores 0.0 for identical agent output.

Blast radius is wider than hand-written YAML: orchestration/task_loader.py:429-430 substitutes ${row.<field>} into every string leaf of success_criteria (data["success_criteria"] = [_substitute_row_in_tree(c, row) for c in data["success_criteria"]]), so a dataset row value carrying a [, *, or ? injects glob semantics into a path the task author wrote as a literal, per-row and invisibly.

Fix: make globbing an explicit opt-in rather than inferred from content — e.g. a separate path_glob field, or a glob: true flag on the criterion — or, at minimum, keep the literal-first fallback: if (sandbox_dir / path).exists() treat it as a literal regardless of metacharacters, and only glob when the literal does not exist. Add a regression test with a bracketed literal filename plus a decoy that the character class would match.
2. [Axis 8] Glob resolution walks the whole sandbox root with no ignore list, containment/symlink guard, or bound (src/coder_eval/sandbox.py:994) — return sorted(p for p in self.sandbox_dir.glob(path) if p.is_file()) walks the entire sandbox root with no exclusions. Verified empirically: d.glob("**/*.py") over a tree containing a.py, .hidden/b.py, __pycache__/c.py, .venv/lib/dep.py returns all four — pathlib's glob matches hidden directories too.

That root is not agent-authored territory. Sandbox._setup_virtualenv sets self.venv_dir = self.sandbox_dir / ".venv" (sandbox.py:520) for any task with a python: block (tasks/token_check.yaml:7, tasks/fibonacci_with_template.yaml:15, tasks/inline_starter_example.yaml:12, tasks/python_cli_simulated_judged/echo_simulated_judged.yaml:28, tasks/internal/session_resumption.yaml:20), and template/starter files are copied in before the agent runs. Two score-correctness consequences for IDENTICAL agent output:

Fairness: file_exists with path: "**/*.py" returns True off .venv/lib/python3.13/site-packages/... or a copied starter file even when the agent wrote nothing. path: "**/*.json" is worse — node_modules is wall-to-wall package.json.
Determinism: a content criterion (file_contains / file_check / json_check) that resolves to exactly one agent-authored file on a cold sandbox resolves to many once the agent (or a retry, or a flaky uv pip install / npm install) materializes dependencies, and get_file_content then raises the ambiguity ValueError -> score 0.0. Same deliverable, different score, decided by incidental filesystem state.

The module already imports the mechanism to fix this: from .resources import get_ignore_patterns, should_ignore_path (sandbox.py:18), whose default set includes .venv, __pycache__, node_modules, dist, build, *.pyc (src/coder_eval/resources/__init__.py:73,127). Filter glob matches through should_ignore_path(...) in resolve_files. Note the new test fixture deliberately sidesteps this: Sandbox(SandboxConfig(driver="tempdir", python=None), ...) at tests/test_glob_paths_in_file_criteria.py:26 creates no venv and no template, so the case that changes scores has no coverage.

Non-blocking, but please consider before merge

[Axis 2] Path fields that gained glob semantics (json_check.path/json_schema, classification_match.path) still carry pre-glob Field descriptions (schema SSOT drift) (src/coder_eval/models/criteria.py:381) — The PR updated four descriptions (FileExistsCriterion.path L197, FileContainsCriterion.path L209, FileMatchesRegexCriterion.path L282, FileCheckCriterion.path L413) but left every other field that flows through the same Sandbox.resolve_files untouched:

    path: str = Field(description="Path to the JSON file (relative to sandbox root)")                       # L381
    json_schema: str | None = Field(default=None, description="Path to JSON Schema file (relative to sandbox root)")  # L382

and ClassificationMatchCriterion.path at L603:

    path: str = Field(description="Path to the file (relative to sandbox) containing the agent's predicted label")

All three reach the new code: criteria/json_check.py:72,81 (path) and :144,147 (schema_path), criteria/classification_match.py:56. docs/TASK_DEFINITION_GUIDE.md:564 even advertises json_check as covered, so the doc and the model now contradict each other, and classification_match gained undocumented glob behavior. CLAUDE.md's design principle is "Single Source of Truth: Schema models are the authoritative source for parameter definitions" — update these three descriptions (and note the exactly-one-match rule for the content-reading ones) in the same edit as the other four.
2. [Axis 2] is_file() filtering only on the glob branch: directories match via a literal path but not via a glob, contradicting the new docstring (src/coder_eval/sandbox.py:994) — The docstring promises "directories are dropped so a glob cannot resolve to something unreadable" and the return contract is "Sorted matching files", but only one of the two branches honors it:

        if not any(c in path for c in "*?["):
            candidate = self.sandbox_dir / path
            return [candidate] if candidate.exists() else []   # L990-992: directories kept, unsorted

        return sorted(p for p in self.sandbox_dir.glob(path) if p.is_file())  # L994: directories dropped

With a directory build.flow in the sandbox: file_exists("build.flow") → True, file_exists("build.*") → False — the same target, opposite verdicts, decided by whether the string happens to contain a metacharacter. And get_file_content("build.flow") reaches matches[0].read_text(...) (L1023) on a directory and raises IsADirectoryError, which is absent from the Raises: block just added at L1006-1009 (RuntimeError / FileNotFoundError / ValueError). Either apply is_file() uniformly (return [candidate] if candidate.is_file() else []) — which also makes file_exists mean what its name says — or document the split; the current asymmetry makes the list[Path] return type mean "files" or "paths" depending on the argument. Note tests/test_glob_paths_in_file_criteria.py:63 covers only the glob half (test_glob_skips_directories).
3. [Axis 2] Absolute path containing a metacharacter makes file_exists non-total — Path.glob NotImplementedError escapes a documented -> bool API (src/coder_eval/sandbox.py:1034) — file_exists documents a pure boolean contract and previously could not raise:

        Returns:
            True if at least one file matches, False otherwise
        """
        return bool(self.resolve_files(path))

but it now delegates to Path.glob, which on Python 3.13.11 raises for non-relative patterns — verified: Path(tmp).glob('/etc/hosts') → NotImplementedError: Non-relative patterns are unsupported. Absolute criterion paths are a supported, in-use shape: the module NOTE at L965-968 says path validation is deliberately skipped, and tasks/byod_smoke_test.yaml:37 (path: /opt/byod_marker) plus tasks/dockerfile_build_example/dockerfile_build_example.yaml:66,73 (/opt/greeting.txt, /opt/secret_check.txt) use them today. A user following the new doc line ("Every path-based criterion ... accepts a glob in path") and writing path: /opt/*.txt gets an exception, not False — inside a criterion it is swallowed by handle_criterion_errors into a 0.0-scored error= result, and at evaluation/judge_context.py:291 the file_exists call sits outside the try that guards the subsequent get_file_content. Either handle the absolute case explicitly in resolve_files (glob from Path(path).anchor with the relative remainder, or return []/raise a typed ValueError with a clear message) and add it to the documented contract, or keep the function total by catching NotImplementedError/ValueError from glob and returning [].
4. [Axis 7] Ambiguous-glob ValueError enumerates every match unbounded and is duplicated into details/error (persisted to task.json, injected into judge prompts) (src/coder_eval/sandbox.py:1020) — The message interpolates the full match list with no cap:

            listed = ", ".join(str(p.relative_to(self.sandbox_dir)) for p in matches)
            raise ValueError(
                f"Pattern '{path}' matches {len(matches)} files — refusing to guess which to grade: {listed}"
            )

The sandbox venv lives inside the graded root (sandbox.py:520: self.venv_dir = self.sandbox_dir / ".venv"), so a natural author pattern like path: "**/*.json" or "**/*.py" enumerates the whole .venv (and node_modules for JS tasks) — thousands of paths in one string. That string lands verbatim in CriterionResult.error and details (criteria/base.py:80-83), which is persisted into task.json/run.json and rendered by the report writers, and it is injected untruncated into an LLM judge prompt via evaluation/judge_context.py:299 (FileBlock(path=path, content=f"<error reading file: {e}>") — the truncate() call is only on the success branch at line 302). Cap the listing, e.g. show the first 10 sorted matches plus … and {len(matches) - 10} more; the count alone already tells the author the pattern is too broad.
5. [Axis 8] Glob docs section names a nonexistent import_check criterion and omits real glob-inheriting surfaces (classification_match, json_schema, judge artifact paths) (docs/TASK_DEFINITION_GUIDE.md:564) — The new section states: "Every path-based criterion (file_exists, file_contains, file_matches_regex, file_check, json_check, import_check) accepts a glob in path." import_check does not exist anywhere in the repo — grep -rn import_check over *.py/*.md/*.yaml hits only an unrelated example string in .claude/commands/coder-eval-implement-plan.md:124, and there is no criteria/import_check.py. A task author who copies that name gets a Pydantic discriminator failure.

The list is also under-inclusive in the direction that matters: three more surfaces route through the rewritten Sandbox methods and therefore now accept globs with no documentation and no author signal — classification_match (src/coder_eval/criteria/classification_match.py:56 raw = sandbox.get_file_content(criterion.path)), uipath_eval (src/coder_eval/criteria/uipath_eval.py:100), and the judge file lists (src/coder_eval/evaluation/judge_context.py:291,296 sandbox.file_exists(path) / sandbox.get_file_content(path), fed by LLMJudgeCriterion.files at src/coder_eval/models/criteria.py:694 and AgentJudgeCriterion.files at :856). classification_match is the sharpest of these: an ambiguous glob raises ValueError, which handle_criterion_errors converts to a plain CriterionResult (criteria/base.py:80-86) rather than the ClassificationCriterionResult the suite P/R/F1 overlay needs — the row drops out of the confusion matrix instead of scoring.

Finally, the model descriptions the doc claims parity with were only updated for four of the six listed types: JsonCheckCriterion.path still reads description="Path to the JSON file (relative to sandbox root)" (src/coder_eval/models/criteria.py:381), as does json_schema (:382). Fix the list (drop import_check, add the three real inheritors or explicitly scope them out), and update JsonCheckCriterion.path / json_schema / the two files descriptions to match.

Nits

[Axis 2] resolve_files returns [] for an un-setup sandbox while get_file_content raises RuntimeError for the identical precondition (src/coder_eval/sandbox.py:987) — The new public API silently absorbs a programming error:

        if not self.sandbox_dir:
            return []

so resolve_files("**/*.flow") on an un-setup() sandbox is indistinguishable from a sandbox where nothing matched, while get_file_content (L1011-1012) raises RuntimeError("Sandbox not set up") for the identical state — two entry points, two contracts, one condition (tests/test_glob_paths_in_file_criteria.py:83-89 pins both). The -> list[Path] signature gives a caller no way to tell the cases apart, and any future consumer that reports "no file matched <pattern>" will mis-report a setup bug as an agent failure. Prefer raising the same RuntimeError from resolve_files and letting file_exists decide how to degrade (this mirrors, rather than extends, the pre-existing lenient file_exists behavior).
2. [Axis 3] test_matches_are_sorted is self-referential and passes vacuously on an empty result (tests/test_glob_paths_in_file_criteria.py:81) — FAILURE: a future edit drops the sorted(...) at src/coder_eval/sandbox.py:994; on a filesystem enumerating a/ before z/ this test still passes, and the determinism guarantee the docstring makes at line 977 ("Matches are sorted so grading is deterministic") ships unguarded. The single assertion is assert sandbox.resolve_files("**/*.flow") == sorted(sandbox.resolve_files("**/*.flow")) — it never states the expected order, so it can only fail if resolve_files stops sorting AND Path.glob's iteration order happens to be reversed on the test machine, making its regression-catching power filesystem-order-dependent. FIX: replace with an explicit expected list, e.g. assert sandbox.resolve_files("**/*.flow") == [sandbox.sandbox_dir / "a" / "first.flow", sandbox.sandbox_dir / "z" / "last.flow"], which is deterministic and documents the sort key.
3. [Axis 5] reference_comparison.agent_file is a sandbox-relative path field that bypasses the new seam, so path semantics now silently differ per criterion (src/coder_eval/models/criteria.py:447) — agent_file: str = Field(description="Path to agent's generated file (relative to sandbox root)") (models/criteria.py:447) reads through raw pathlib, not the seam: agent_path = sandbox.sandbox_dir / criterion.agent_file / if not agent_path.exists(): (criteria/reference_comparison.py:66-67) and agent_code = agent_path.read_text(encoding="utf-8") (:76). Before this PR every sandbox-relative path field behaved identically; now some glob-expand and one does not, with nothing in the schema distinguishing them — a user who tries agent_file: "**/*.py" gets error="Agent file not found: **/*.py" and a 0.0. Fix: either route reference_comparison through the same seam or add a note on the field that it is literal-only. This disappears entirely under finding 1's explicit-opt-in design, where a field either declares glob support or doesn't.

What's Missing

Parallel paths:

🟠 sandbox.py already owns two "which files count" filters — should_ignore_path for template copying (sandbox.py:490-494, patterns in resources/default_ignore_patterns.yaml: .venv, __pycache__, node_modules, dist, build) and the _EXCLUDED set for capture_to (sandbox.py:25) — but the new resolve_files (sandbox.py:994) extends neither, so grading globs walk .venv/node_modules/starter files that both sibling paths deliberately exclude. (trigger: src/coder_eval/sandbox.py) (restates: Axis 8: Glob resolution walks the whole sandbox root with no ignore list)
🟡 reference_comparison is the one path-based criterion left reading through raw pathlib (criteria/reference_comparison.py:66-76, sandbox.sandbox_dir / criterion.agent_file + .exists() + .read_text()), so after this PR agent_file: "**/*.py" errors with Agent file not found while the identical pattern in file_contains resolves — path semantics now differ per criterion with nothing in the schema signalling it. (trigger: src/coder_eval/sandbox.py) (restates: Axis 5: reference_comparison.agent_file bypasses the new seam)
🟡 Three surfaces silently inherited glob behavior without any accompanying update: classification_match (criteria/classification_match.py:56), uipath_eval (criteria/uipath_eval.py:100), and the judge file lists (evaluation/judge_context.py:291,296 fed by LLMJudgeCriterion.files / AgentJudgeCriterion.files) — none are documented, none got a field-description update, and classification_match only catches FileNotFoundError, so the new ambiguity ValueError escapes. (trigger: docs/TASK_DEFINITION_GUIDE.md) (restates: Axis 8: Glob docs section names a nonexistent import_check and omits real glob-inheriting surfaces)

Display & mapping dicts:

🟡 The per-criterion parameter tables in the same guide were not extended for the new capability: file_check's path row (docs/TASK_DEFINITION_GUIDE.md:641) and json_check's path / json_schema rows (:694-695) still read "Path to the file (relative to sandbox root)" with no glob or exactly-one-match note, and the llm_judge / agent_judge / classification_match sections (:821, :882, :959) say nothing at all — a reader who lands on a criterion section instead of the new :562 section sees the pre-PR contract. (trigger: docs/TASK_DEFINITION_GUIDE.md) (restates: Axis 2: Path fields that gained glob semantics still carry pre-glob Field descriptions)

Tests:

🟠 The new suite covers only 3 of the 6 criteria the docs advertise: there is no glob test for file_matches_regex (criteria/file_matches_regex.py:43,51) or json_check (criteria/json_check.py:72,81), and none at all for json_check.json_schema — a second path field that now globs (criteria/json_check.py:144,147) and whose ambiguity ValueError would surface as a schema-load failure rather than a path failure. (trigger: tests/test_glob_paths_in_file_criteria.py)
🟠 No test pins the behavior of a literal path that happens to contain [, *, or ? (e.g. app/[id]/page.tsx, report[2024].json) — the exact case where resolve_files (sandbox.py:990) changes an existing task's verdict, and the case a regression test would have to encode before any future literal-first fallback can be added safely. (trigger: tests/test_glob_paths_in_file_criteria.py) (restates: Axis 8: String-sniffing *?[ in resolve_files silently reinterprets a literal path as a glob)
🟡 Zero absolute-path coverage (no /opt, /etc case) even though in-tree tasks use absolute criterion paths (tasks/byod_smoke_test.yaml:37, tasks/dockerfile_build_example/…:66,73) and Path.glob raises NotImplementedError for non-relative patterns — a test would have caught that file_exists stopped being total. (trigger: tests/test_glob_paths_in_file_criteria.py) (restates: Axis 2: Absolute path containing a metacharacter makes file_exists non-total)
🟡 The fixture constructs SandboxConfig(driver="tempdir", python=None) (tests/test_glob_paths_in_file_criteria.py:26,84), which creates no .venv and copies no template/starter files — so every glob test runs against a tree containing only agent-written files, and the one configuration that actually changes scores (a venv-bearing task where **/*.py or **/*.json matches hundreds of dependency files) is never exercised. (trigger: tests/test_glob_paths_in_file_criteria.py) (restates: Axis 8: Glob resolution walks the whole sandbox root with no ignore list)
🟡 No test covers the judge-context path (evaluation/judge_context.py:291,296), which now globs llm_judge.files / agent_judge.files entries: neither the happy case (a pattern resolving to a file) nor the ambiguous case, where file_exists returns True, get_file_content raises, and the full untruncated match listing is embedded into the judge prompt as <error reading file: …> (judge_context.py:300, no truncate() on that branch). (trigger: tests/test_glob_paths_in_file_criteria.py)
🔵 test_glob_skips_directories (:63) asserts only the glob half of the directory contract; there is no counterpart asserting what the literal branch does with a directory (file_exists("build.flow") -> True, then get_file_content raising IsADirectoryError), so the branch asymmetry the docstring denies is unpinned in either direction. (trigger: tests/test_glob_paths_in_file_criteria.py) (restates: Axis 2: is_file() filtering only on the glob branch)
🔵 All TestGlobThroughCriteria cases call the sync SuccessChecker(...).check(...), but production grading goes exclusively through check_all_async (orchestrator.py:1418,1476,1568) — the async surface CLAUDE.md calls primary has no glob coverage, so a future criterion that overrides _check_impl_async directly would ship its glob path untested. (trigger: tests/test_glob_paths_in_file_criteria.py)

Downstream consumers:

🟡 Nothing downstream records which file a glob resolved to: file_exists.py:44 and file_contains.py:42 interpolate criterion.path (the pattern) into details/error, and judge_context.py:292,300,302 builds FileBlock(path=path) from the pattern — so a passing **/*.flow criterion in task.json, the reports, and the judge prompt is indistinguishable from any other, and a grading dispute can't be reconstructed from the run record. Resolving the pattern into details ("matched Sol/Proj/Proj.flow") is the cheap fix. (trigger: src/coder_eval/sandbox.py)
🟡 get_file_content gained a new exception in its contract (ValueError on ambiguity, sandbox.py:1017-1021) but no caller was audited for it: classification_match.py:56 catches only FileNotFoundError, so the ValueError degrades to a plain CriterionResult and the row is filtered out of the suite confusion matrix by the isinstance(r, ClassificationCriterionResult) guard in _classification_aggregate.py:44 — a suite's accuracy / P / R / F1 denominators silently shrink instead of scoring the row 0.0. (trigger: src/coder_eval/sandbox.py) (restates: Axis 8: Glob docs section names a nonexistent import_check and omits real glob-inheriting surfaces)
🔵 The unbounded ambiguity message reaches three sinks with different caps and none was adjusted: reports.py truncates at _FAILURE_REASON_MAX_LEN = 240 (reports.py:35,861), criteria/base.py:81-82 persists it twice (both details and error) untruncated into task.json, and judge_context.py:300 injects it untruncated into an LLM prompt. (trigger: src/coder_eval/sandbox.py) (restates: Axis 7: Ambiguous-glob ValueError enumerates every match unbounded)

Daily/nightly:

🟠 The PR rewrites Sandbox.file_exists / get_file_content — the grading path every task in the nightly suite runs through (68 path: declarations across tasks/) — with no statement of nightly impact. I verified none of those 68 in-repo paths currently contains *, ?, or [, so in-tree nightly scores are unchanged today; that safety argument belongs in the PR body, and the un-audited part (out-of-tree coder-eval-uipath task suites, plus ${row.<field>} substitution injecting metacharacters into criterion paths per-row at orchestration/task_loader.py:430) is where the blast radius actually is. (trigger: src/coder_eval/sandbox.py)
🟡 Sandbox is public API for out-of-tree plugins (the coder_eval.plugins SPI / coder_eval_uipath), and this PR changes its contract in two observable ways — get_file_content now raises a new ValueError and a reworded FileNotFoundError ("No file matches …" instead of the OSError text), and file_exists is no longer total — with no migration note in docs/ and no mention in the commit message that third-party criteria catching only FileNotFoundError need updating. (trigger: src/coder_eval/sandbox.py)

Harness & Lint Improvements

Static checks (lint / type):

[ce-lint] CE032 — no raw sandbox path joins outside the Sandbox seam. New rule tests/lint/rules/ce032_no_raw_sandbox_path_join.py (a BaseRule, wired into ALL_RULES in tests/lint/runner.py), scoped to src/coder_eval/criteria/ and src/coder_eval/evaluation/: forbid sandbox.sandbox_dir / <expr> and Path(sandbox.sandbox_dir, …); all sandbox file access must go through Sandbox.resolve_files / file_exists / get_file_content. Verified feasibility: a grep over src/coder_eval/ at HEAD returns exactly ONE violation — src/coder_eval/criteria/reference_comparison.py:66 (agent_path = sandbox.sandbox_dir / criterion.agent_file) — so the rule lands green after the one fix, with # noqa: CE032 available for a deliberate literal-only reader. Prevents: reference_comparison.agent_file bypassing the new glob seam so path semantics silently differ per criterion (models/criteria.py:447 / criteria/reference_comparison.py:66-76), and any future criterion re-introducing a second, divergent path-resolution path.
[ce-lint] CE033 — path fields on criterion models must state their path semantics. Registry-driven check (not an AST BaseRule; wire like CE030 as tests/lint/doc_schema_parity.py-style module + tests/test_custom_lint.py::TestCE033PathFieldSemantics): for every member of the SuccessCriterion union, every str / list[str] field whose name matches path|_file$|^files$|dir|schema must have a Field(description=…) containing the word glob or literal. I enumerated the affected set — 9 fields today (FileExistsCriterion.path, FileContainsCriterion.path, FileMatchesRegexCriterion.path, FileCheckCriterion.path, JsonCheckCriterion.path, JsonCheckCriterion.json_schema, ReferenceComparisonCriterion.agent_file, ClassificationMatchCriterion.path, LLMJudgeCriterion.files, AgentJudgeCriterion.files) — small enough to annotate once; the int max_file_chars fields are excluded by the type filter. This makes the CLAUDE.md "schema models are the authoritative source" rule mechanical instead of aspirational. Prevents: The schema SSOT drift finding: JsonCheckCriterion.path/json_schema (models/criteria.py:381-382) and ClassificationMatchCriterion.path (:603) keeping pre-glob descriptions while their code paths (criteria/json_check.py:72,81,144,147; criteria/classification_match.py:56) gained glob semantics; also the undocumented glob behavior of the two judge files fields (:694, :856).
[ce-lint] CE034 — criterion type names in docs must exist in CriterionRegistry. New doc-surface check tests/lint/doc_criterion_names.py + tests/test_custom_lint.py::TestCE034DocCriterionNames (same non-BaseRule wiring as CE027–CE031, since it reasons over Markdown). Algorithm, prototyped and measured: scan docs/**/*.md for runs of comma/and/|-separated backticked snake_case tokens; when ≥2 tokens in a run ARE registered criterion types, every other snake_case token in that run must also be registered. Measured on main: 0 findings across all of docs/ (no false positives). Run against the PR's sentence it reports exactly ['import_check']. Cheap variant/extension: also validate - type: <name> lines inside fenced YAML blocks against the registry (complements CE029, which only checks whole-document examples). Prevents: docs/TASK_DEFINITION_GUIDE.md:564 advertising a nonexistent import_check criterion (a task author copying it gets a Pydantic discriminator failure), and the same class of stale/fabricated criterion name in any future doc list.
[ce-lint] CE035 — no self-referential assertions in tests. New rule tests/lint/rules/ce035_no_self_referential_assert.py wired into tests/lint/runner.py, scoped to tests/: flag assert A == B where one side contains an ast.Call structurally identical (by ast.dump) to a call on the other side — e.g. assert f(x) == sorted(f(x)), assert sorted(g()) == sorted(g()). No existing gate covers this: ruff's enabled set is E/F/I/N/W/UP/B/SIM/RUF, and B015 only fires on a bare comparison statement, never inside an assert; flake8-pytest-style is not enabled and has no equivalent either. Fix guidance in the message: state the expected value explicitly. Prevents: tests/test_glob_paths_in_file_criteria.py:81 (assert resolve_files(p) == sorted(resolve_files(p))), which passes vacuously on an empty result and can only fail if the OS happens to enumerate in reverse — so the documented sort-determinism guarantee (sandbox.py:977) ships effectively unguarded.
[ce-lint] Shared glob helper + CE036 — non-literal glob patterns must go through it. Extract glob_relative(root: Path, pattern: str) -> Iterator[Path] into src/coder_eval/path_utils.py, handling the absolute-pattern case the way src/coder_eval/cli/run_helpers.py:88 ALREADY does (Path(pattern.anchor).glob(pattern.relative_to(anchor))) — the fix for the absolute-glob crash is literally already in-tree, just not shared. Then add tests/lint/rules/ce036_glob_through_helper.py (wired in tests/lint/runner.py) forbidding .glob(<non-literal>) / .rglob(<non-literal>) anywhere in src/coder_eval/ except inside that helper. Migration cost measured: 4 non-literal call sites (sandbox.py:994, cli/run_helpers.py:88,90, agents/codex_agent.py:1802, logging_config.py:373), each either migrated or # noqa: CE036 with a reason. Side benefit: the metacharacter/literal policy then lives in one audited place instead of being re-decided per call site. Prevents: The absolute-path finding — file_exists('/opt/*.txt') raising NotImplementedError: Non-relative patterns are unsupported out of a documented -> bool API (sandbox.py:1034), for a path shape shipped tasks use today (tasks/byod_smoke_test.yaml:37, tasks/dockerfile_build_example/…:66,73) — and it centralizes the *?[ sniffing behind one reviewable helper.
[ce-lint] CE037 (low priority) — uniform self.sandbox_dir precondition in sandbox.py. Rule: an if not self.sandbox_dir: guard whose body returns a falsy default instead of raise RuntimeError("Sandbox not set up") is a violation, requiring an explicit # noqa: CE037 — <why lenient>. Honest cost/benefit note: this is NOT green today — file_exists (sandbox.py:998) and list_files (:1012) are pre-existing lenient guards, so adopting it means one policy decision plus two annotated exceptions. Its value is exactly that: it converts an accidental split contract into a declared one. Prevents: The finding that resolve_files returns [] for an un-setup() sandbox while get_file_content raises RuntimeError for the identical precondition (sandbox.py:987 vs :1011) — an un-setup bug that a future "no file matched <pattern>" message would mis-report as an agent failure.

Harness improvements (not statically reachable):

Adversarial-filename round-trip test for resolve_files. In tests/test_glob_paths_in_file_criteria.py, add a parametrized fixture that creates real files whose names contain glob metacharacters — report[2024].json alongside a decoy report2.json, app/[id]/page.tsx (a Next.js dynamic route), logs[1], what?.txt, star*.md — and assert the invariant: for every file created, file_exists(<its own relative path>) is True and get_file_content(<same>) returns that file's own content (never a decoy's). Why not static: The defect is pathlib's runtime matching semantics against real on-disk names; no AST rule can know that [2024] in a YAML string will be reinterpreted as a character class, and the decoy-grading path needs a filesystem to exist at all. Prevents: The high-severity literal-reinterpretation finding at sandbox.py:990 — a plain literal path containing [/*/? silently grades the wrong file (one match, so the ambiguity guard never trips) or flips True→False; amplified per-row by ${row.*} substitution at orchestration/task_loader.py:430.
Literal-vs-glob branch equivalence test (directories + error contract). Assert that for the same target the two resolve_files branches agree: with a directory build.flow present, file_exists("build.flow") and file_exists("build.*") must return the same value, and get_file_content on a directory must raise one of the documented exceptions in the Raises: block (sandbox.py:1006-1009), not a bare IsADirectoryError. Why not static: It is a behavioral equivalence between two code paths over real inode types (file vs. directory); a lint rule cannot tell that .exists() and .is_file() diverge only for directories. Prevents: The literal/glob is_file() asymmetry finding (sandbox.py:990-994) where the docstring's "directories are dropped" and "Returns: Sorted matching files" contract is honored by only one branch — currently covered on the glob side only (test_glob_skips_directories, tests/…:63).
Realistic sandbox fixture for the glob tests (venv / node_modules / template files) + a scoring-determinism assertion. The current fixtures use SandboxConfig(driver="tempdir", python=None) (tests/test_glob_paths_in_file_criteria.py:26,84), which materializes no .venv and no starter files — precisely the state in which the defect cannot appear. Add a fixture with python: set (or a synthetic .venv/lib/.../dep.py, node_modules/pkg/package.json, __pycache__/, plus copied starter files) and assert: (a) file_exists("**/*.py") is False when the agent wrote nothing; (b) a file_contains/json_check criterion scores identically before and after dependencies are installed into the same sandbox. Why not static: Requires a materialized dependency tree and two grading passes over changing filesystem state — the whole point is that the score changes for identical agent output depending on incidental sandbox contents. Prevents: The high-severity unfiltered-glob finding (sandbox.py:994 walks the graded root with no should_ignore_path filter, while self.venv_dir = self.sandbox_dir / ".venv" lives inside it, sandbox.py:520) — both the fairness half (false True off .venv/starter files) and the determinism half (a content criterion flipping to the ambiguity ValueError → 0.0 after a uv pip install).
Cap CriterionResult.error / details at the single sink, with a size-budget test. Truncate in criteria/base.py::_failed_result (reuse evaluation/judge_context.truncate) and add a test asserting no error/details string persisted to task.json exceeds a documented budget; separately cap the listing in sandbox.py:1018-1020 to the first ~10 sorted matches plus … and N more. Why not static: I prototyped the source-side lint ("unbounded ", ".join(...) interpolated into a raise") and it flags 7 existing sites in src/, all of which join over closed sets (schema field names, variant ids) and are legitimately bounded. Distinguishing an FS-derived collection from a closed one needs runtime knowledge, so a lint rule here would be pure false-positive noise — the sink cap is the correct enforcement point. Prevents: The unbounded ambiguous-glob message finding: **/*.json over a sandbox containing .venv/node_modules enumerates thousands of paths into one string, stored twice in task.json (criteria/base.py:81-82) and injected untruncated into an LLM-judge prompt via evaluation/judge_context.py:300.
Absolute-path coverage in the glob tests. tests/test_glob_paths_in_file_criteria.py contains no absolute-path case (zero occurrences of /opt, /etc, or "absolute"), yet shipped tasks use absolute criterion paths and the module deliberately skips path validation (NOTE at sandbox.py:965-968). Add cases for an absolute literal (must stay total → False) and an absolute glob (must return [] or raise a typed, documented error — never NotImplementedError). Why not static: Totality of a -> bool function against a third-party library's runtime exception is not expressible in the type system; pyright sees Path.glob as non-raising. Prevents: The absolute-glob finding at sandbox.py:1034, where a documented-legal input degrades into an opaque error="NotImplementedError: Non-relative patterns are unsupported" 0.0-scored result at both the criterion and judge-context call sites.
Judge-prompt golden test for the file-block error branch. Add a test that builds a judge context where get_file_content raises (ambiguous glob), and assert the resulting FileBlock content is bounded by the same max_file_chars budget as the success branch — today evaluation/judge_context.py:300 interpolates the raw exception while only line 302 truncates. Why not static: A lint rule requiring "both branches call truncate" is too narrow to justify a CE number and would not generalize; the real assertion is about the rendered prompt, which needs the context builder to actually run. Prevents: The judge-prompt half of the unbounded-error finding, and any future error string reaching a paid LLM call untruncated.

Top 5 Priority Actions

Restore literal-first resolution in resolve_files (src/coder_eval/sandbox.py:990): a plain path containing [, *, or ? — e.g. a Next.js route app/[id]/page.tsx, or a ${row.<field>} value substituted per-row at src/coder_eval/orchestration/task_loader.py:430 — is silently reinterpreted as a pattern, reproducibly grading a decoy file or flipping file_exists True→False for unchanged agent output; make globbing an explicit opt-in (glob: true / path_glob) or fall back to the literal when (sandbox_dir / path).exists(), plus a bracketed-literal-with-decoy regression test.
Filter glob matches through the already-imported should_ignore_path (src/coder_eval/sandbox.py:994; import at :18, patterns in src/coder_eval/resources/default_ignore_patterns.yaml): **/*.py / **/*.json currently match .venv (created at sandbox.py:520 for any task with a python: block) and node_modules, so file_exists passes on files the agent never wrote, and a content criterion that resolved to one file on a cold sandbox hits the ambiguity ValueError → 0.0 once dependencies materialize.
Make classification_match glob-safe (src/coder_eval/criteria/classification_match.py:56): it catches only FileNotFoundError, so an ambiguous-glob ValueError escapes to @handle_criterion_errors and yields a plain CriterionResult (src/coder_eval/criteria/base.py:116-117, 77-83) instead of a ClassificationCriterionResult, which the isinstance filter at src/coder_eval/criteria/_classification_aggregate.py:44 drops entirely — silently removing the row from suite P/R/F1 rather than scoring it.
Harden the resolve_files contract (src/coder_eval/sandbox.py:987-1020): apply is_file() on the literal branch too (990-992) so directories don't pass file_exists only when the path lacks a metacharacter, keep the function total for absolute globs like /opt/*.txt (documented-legal per tasks/byod_smoke_test.yaml:37, but Path.glob raises NotImplementedError → opaque 0.0 error= result at :1034 and src/coder_eval/evaluation/judge_context.py:291), raise instead of returning [] on an un-setup sandbox (:987), and cap the ambiguity message's match listing (:1018-1020) which is persisted twice into task.json and injected untruncated into judge prompts (judge_context.py:300).
Fix the doc/schema single-source-of-truth drift: docs/TASK_DEFINITION_GUIDE.md:564 advertises a nonexistent import_check criterion (a copy-paste yields a Pydantic discriminator failure), omits the real new glob inheritors (classification_match, uipath_eval, json_schema, and the llm_judge/agent_judge files lists), and steers authors toward broad **/* globs with no .venv/node_modules caveat, while JsonCheckCriterion.path/json_schema (src/coder_eval/models/criteria.py:381-382), ClassificationMatchCriterion.path (:603), and the literal-only ReferenceComparisonCriterion.agent_file (:447) still carry pre-glob descriptions; also replace the vacuous self-referential sort assertion at tests/test_glob_paths_in_file_criteria.py:81 with an explicit expected ordering and add venv/absolute-path coverage.

Stats: 0 🔴 · 2 🟠 · 5 🟡 · 3 🔵 across 8 axes reviewed.

bai-uipath

Right seam to put this on, and requiring exactly one match for content reads is the correct call over max/min-over-files, which would invent a scoring rule and let a staged fixture satisfy a check. One thing needs to land before merge; the rest are optional.

Major

The glob runs unfiltered over the sandbox root at grading time. pathlib doesn't skip dotdirs, and the skills post_run prune of node_modules/.venv runs in teardown, after evaluation, so a **/*.json pattern matches vendored files and grades or hard-fails on them; **/*.flow works today only because the extension is rare, which makes this latent rather than absent. Fix: filter the glob branch through the existing get_ignore_patterns/should_ignore_path helpers in the resources module (already used for the template copy), passing the sandbox-relative path since they match on any path component. Worth a line in the guide that dist and build are in that default set, with ignore_patterns: ["!dist"] as the escape hatch.

Fix if you agree, otherwise lgtm

The follow-up migration should glob away only the segment the prompt leaves free. The failure is an unknown wrapper prefix, not an unknown filename, so **/<Name>.flow drops the prefix and stays unique; a blanket **/*.flow turns exactly-one into a hard 0.0 on any task carrying a second flow file.
Nothing records which file was graded. With exactly-one semantics the message is most of the feature: put the resolved path in details on the passing path too, and cap the ambiguity list at ~10 with +N more since it persists into task.json.
Minor: try the literal path first and only fall back to globbing, so a real filename containing [ or ? isn't silently reinterpreted; import_check in the guide isn't a criterion type, while json_check and classification_match inherited the behavior and still carry pre-glob field descriptions; reference_comparison's agent file reads the sandbox directly and bypasses the new seam; the sortedness test asserts against sorted() of its own output.

jiyangzh requested review from akshaylive, bai-uipath, tmatup and uipreliga as code owners July 29, 2026 23:02

uipreliga reviewed Jul 30, 2026

View reviewed changes

bai-uipath approved these changes Jul 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(criteria): accept glob patterns in criterion path fields - #65

feat(criteria): accept glob patterns in criterion path fields#65
jiyangzh wants to merge 1 commit into
mainfrom
feat/glob-paths-in-file-criteria

jiyangzh commented Jul 29, 2026

Uh oh!

github-actions Bot commented Jul 29, 2026 •

edited

Loading

Uh oh!

uipreliga left a comment

Uh oh!

bai-uipath left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jiyangzh commented Jul 29, 2026

Problem

Change

Verification

Follow-up in UiPath/skills

Uh oh!

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

Uh oh!

📋 Review Task List

Uh oh!

uipreliga left a comment

Choose a reason for hiding this comment

Review: coder_eval — pr:65

Summary

Blockers

Non-blocking, but please consider before merge

Nits

What's Missing

Harness & Lint Improvements

Top 5 Priority Actions

Uh oh!

bai-uipath left a comment

Choose a reason for hiding this comment

Major

Fix if you agree, otherwise lgtm

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jul 29, 2026 •

edited

Loading