Skip to content

Commit 9c359db

Browse files
jamesadevineCopilot
andcommitted
fix: clean up review findings from TypeScript gate evaluator PR
Update stale doc comments referencing Python evaluator to TypeScript. Fix fnmatch reference in filter-ir.md to describe custom glob semantics. Hoist toLowerCase() map outside includes() for value_in_set/value_not_in_set predicates to avoid per-call array allocation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4c1c209 commit 9c359db

4 files changed

Lines changed: 17 additions & 13 deletions

File tree

docs/filter-ir.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ supports these predicate types:
113113

114114
| Predicate | Bash Shape | Example |
115115
|-----------|-----------|---------|
116-
| `GlobMatch { fact, pattern }` | `fnmatch(value, pattern)` | Title matches `*[review]*` |
116+
| `GlobMatch { fact, pattern }` | Simple glob (`*` any chars, `?` single char) | Title matches `*[review]*` |
117117
| `Equality { fact, value }` | `[ "$VAR" = "value" ]` | Draft is `false` |
118118
| `ValueInSet { fact, values, case_insensitive }` | `echo "$VAR" \| grep -q[i]E '^(a\|b)$'` | Author in allow-list |
119119
| `ValueNotInSet { fact, values, case_insensitive }` | Inverse of `ValueInSet` | Author not in block-list |

scripts/ado-script/src/gate/predicates.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,19 @@ export function evaluatePredicate(p: PredicateSpec, facts: Map<string, unknown>)
7878
return String(facts.get(p.fact) ?? "") === p.value;
7979
case "value_in_set": {
8080
const value = String(facts.get(p.fact) ?? "");
81-
return p.case_insensitive
82-
? p.values.map((v) => v.toLowerCase()).includes(value.toLowerCase())
83-
: p.values.includes(value);
81+
if (p.case_insensitive) {
82+
const lower = p.values.map((v) => v.toLowerCase());
83+
return lower.includes(value.toLowerCase());
84+
}
85+
return p.values.includes(value);
8486
}
8587
case "value_not_in_set": {
8688
const value = String(facts.get(p.fact) ?? "");
87-
return p.case_insensitive
88-
? !p.values.map((v) => v.toLowerCase()).includes(value.toLowerCase())
89-
: !p.values.includes(value);
89+
if (p.case_insensitive) {
90+
const lower = p.values.map((v) => v.toLowerCase());
91+
return !lower.includes(value.toLowerCase());
92+
}
93+
return !p.values.includes(value);
9094
}
9195
case "numeric_range": {
9296
const value = Number(facts.get(p.fact) ?? 0);

src/compile/extensions/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -613,7 +613,7 @@ pub fn collect_extensions(front_matter: &FrontMatter) -> Vec<Extension> {
613613
}
614614

615615
// ── Trigger filters (ExtensionPhase::Tool) ──
616-
// Activated when Tier 2/3 filters require the Python evaluator.
616+
// Activated when filters require the gate evaluator (TypeScript gate.js).
617617
let pr_filters = front_matter.pr_filters().cloned();
618618
let pipeline_filters = front_matter.pipeline_filters().cloned();
619619
if TriggerFiltersExtension::is_needed(

src/compile/filter_ir.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -919,8 +919,8 @@ pub enum PredicateSpec {
919919
/// Generate the JSON Schema for the gate spec.
920920
///
921921
/// This schema is the formal contract between the Rust compiler and the
922-
/// Python evaluator. It should be shipped in `scripts/gate-spec.schema.json`
923-
/// alongside the evaluator.
922+
/// TypeScript gate evaluator. It is used to generate `types.gen.ts` in
923+
/// the `scripts/ado-script` workspace.
924924
pub fn generate_gate_spec_schema() -> String {
925925
let schema = schemars::schema_for!(GateSpec);
926926
serde_json::to_string_pretty(&schema).expect("schema serialization")
@@ -929,13 +929,13 @@ pub fn generate_gate_spec_schema() -> String {
929929
// ─── Codegen ────────────────────────────────────────────────────────────────
930930

931931
// The inline heredoc evaluator has been removed in favor of external script delivery.
932-
// See TriggerFiltersExtension for the external path and compile_gate_step_inline for Tier 1.
932+
// See TriggerFiltersExtension for the external path (bundled TypeScript gate.js).
933933

934934
impl Fact {
935935
/// ADO macro exports required by this fact.
936936
///
937-
/// Returns `(env_var_name, ado_macro)` pairs that must be exported in
938-
/// the bash shim for the Python evaluator to read.
937+
/// Returns `(env_var_name, ado_macro)` pairs that must be set in the
938+
/// step's `env:` block for the gate evaluator to read.
939939
pub fn ado_exports(&self) -> Vec<(&'static str, &'static str)> {
940940
match self {
941941
Fact::PrTitle => vec![("ADO_PR_TITLE", "$(System.PullRequest.Title)")],

0 commit comments

Comments
 (0)