Skip to content

Commit 85dc7ce

Browse files
huangruitenghuangruiteng
authored
Merge pull request #5359 from karenchuu/codex/benchmark-experiment-identity
refactor(benchmark): decide the experiment identity in one owner
2 parents 64a35a1 + 1704893 commit 85dc7ce

8 files changed

Lines changed: 631 additions & 54 deletions

File tree

‎loopx/capabilities/benchmark_toolkit/concurrency_envelope.py‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
from __future__ import annotations
22

33
import json
4-
import re
54
from collections.abc import Mapping
65
from datetime import UTC, datetime, timedelta
76
from pathlib import Path
87
from typing import Any
98

9+
from .experiment_identity import (
10+
ARM_ROLES,
11+
experiment_token_text as _token,
12+
)
1013
from ...domain_state import default_domain_state_file_path
1114
from ...file_lock import exclusive_file_lock
1215
from ...registry import atomic_write_json
@@ -17,8 +20,6 @@
1720
)
1821
BENCHMARK_CONCURRENCY_ENVELOPE_FILENAME = "concurrency-envelope.json"
1922

20-
_TOKEN_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.:@+-]{0,127}$")
21-
_ARM_ROLES = {"baseline", "control", "treatment", "explore"}
2223
_RESOURCE_HEADROOM_KINDS = {
2324
"file_descriptors",
2425
"memory",
@@ -69,13 +70,6 @@ def _timestamp(value: Any, *, field: str) -> str:
6970
return parsed.isoformat().replace("+00:00", "Z")
7071

7172

72-
def _token(value: Any, *, field: str) -> str:
73-
text = str(value or "").strip()
74-
if not _TOKEN_RE.fullmatch(text):
75-
raise ValueError(f"{field} must be a compact public-safe token")
76-
return text
77-
78-
7973
def _bounded_int(value: Any, *, field: str, minimum: int = 0) -> int:
8074
if isinstance(value, bool) or not isinstance(value, int) or value < minimum:
8175
qualifier = "positive" if minimum == 1 else "non-negative"
@@ -275,7 +269,7 @@ def _normalize_active_run(value: Mapping[str, Any]) -> dict[str, str]:
275269
raise TypeError("active run must be an object")
276270
_reject_unknown_fields(value, allowed=_ACTIVE_RUN_FIELDS, field="active_run")
277271
arm_role = _token(value.get("arm_role"), field="arm_role")
278-
if arm_role not in _ARM_ROLES:
272+
if arm_role not in ARM_ROLES:
279273
raise ValueError("arm_role is unsupported")
280274
return {
281275
"run_id": _token(value.get("run_id"), field="run_id"),

‎loopx/capabilities/benchmark_toolkit/experiment_board.py‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,16 @@
22

33
import json
44
import math
5-
import re
65
from collections.abc import Iterable, Mapping
76
from datetime import datetime
87
from pathlib import Path
98
from typing import Any
109

1110
from ...domain_state import default_domain_state_file_path, upsert_domain_state_jsonl
11+
from .experiment_identity import (
12+
ARM_ROLES,
13+
experiment_token_text as _token,
14+
)
1215
from .factorial_contrast import (
1316
build_benchmark_factorial_contrasts,
1417
build_benchmark_metric_delta,
@@ -18,8 +21,6 @@
1821
BENCHMARK_EXPERIMENT_BOARD_SCHEMA_VERSION = "benchmark_experiment_board_v0"
1922
BENCHMARK_EXPERIMENT_BOARD_LEDGER_FILENAME = "experiment-board.jsonl"
2023

21-
_TOKEN_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.:@+-]{0,127}$")
22-
_ARM_ROLES = {"baseline", "control", "treatment", "explore"}
2324
_RUN_STATUSES = {"planned", "running", "completed", "runner_invalid", "cancelled"}
2425
_RUN_STATUS_TRANSITIONS = {
2526
"planned": _RUN_STATUSES,
@@ -78,13 +79,6 @@ def _reject_unknown_fields(
7879
raise ValueError(f"{field} contains unsupported fields: {', '.join(unknown)}")
7980

8081

81-
def _token(value: Any, *, field: str) -> str:
82-
text = str(value or "").strip()
83-
if not _TOKEN_RE.fullmatch(text):
84-
raise ValueError(f"{field} must be a compact public-safe token")
85-
return text
86-
87-
8882
def _optional_token(value: Any, *, field: str) -> str | None:
8983
if value in (None, ""):
9084
return None
@@ -267,7 +261,7 @@ def normalize_benchmark_experiment_board_row(
267261
raise ValueError("benchmark experiment board row schema mismatch")
268262

269263
arm_role = _token(payload.get("arm_role"), field="arm_role")
270-
if arm_role not in _ARM_ROLES:
264+
if arm_role not in ARM_ROLES:
271265
raise ValueError("arm_role is unsupported")
272266
status = _token(payload.get("status"), field="status")
273267
if status not in _RUN_STATUSES:
@@ -744,12 +738,12 @@ def build_benchmark_experiment_board(
744738
]
745739
role_counts = {
746740
role: sum(1 for row in normalized if row["arm_role"] == role)
747-
for role in sorted(_ARM_ROLES)
741+
for role in sorted(ARM_ROLES)
748742
}
749743
comparison_arm_role_counts = _comparison_lane_counts(
750744
comparisons,
751745
field="candidate_arm_role",
752-
values=_ARM_ROLES - {"baseline"},
746+
values=ARM_ROLES - {"baseline"},
753747
)
754748
comparison_claim_scope_counts = _comparison_lane_counts(
755749
comparisons,

‎loopx/capabilities/benchmark_toolkit/experiment_identity.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
"""One owner for the two vocabularies a benchmark experiment identity uses.
2+
3+
An arm role and a public-safe token are validated at the package boundary, at
4+
the study projection, at the concurrency envelope and at the CLI that admits a
5+
case slot. Each of those places restated the answer, so widening a token in one
6+
file left the others rejecting the same value, and the CLI's ``--arm-role``
7+
choices could disagree with the envelope that admits the run.
8+
9+
``ARM_ROLES`` is the set every checker compares against. ``ARM_ROLE_CHOICES``
10+
is the same four names in the order ``argparse`` shows them to a human; the
11+
order is part of the help text, so it is stated rather than derived.
12+
13+
``experiment_token_text`` is the reject path that goes with the token shape.
14+
Five modules carried a byte-identical private ``_token`` helper, so the shape and
15+
its rejection message travelled together in five copies.
16+
"""
17+
18+
from __future__ import annotations
19+
20+
import re
21+
from typing import Any
22+
23+
ARM_ROLES = frozenset({"baseline", "control", "treatment", "explore"})
24+
ARM_ROLE_CHOICES = ("baseline", "control", "treatment", "explore")
25+
26+
EXPERIMENT_TOKEN_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.:@+-]{0,127}$")
27+
28+
29+
def experiment_token_text(value: Any, *, field: str) -> str:
30+
text = str(value or "").strip()
31+
if not EXPERIMENT_TOKEN_PATTERN.fullmatch(text):
32+
raise ValueError(f"{field} must be a compact public-safe token")
33+
return text

‎loopx/capabilities/benchmark_toolkit/factorial_contrast.py‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
from __future__ import annotations
44

5-
import re
65
from collections.abc import Iterable, Mapping
76
from typing import Any
87

8+
from .experiment_identity import experiment_token_text as _token
99
from .four_arm_contract import (
1010
BENCHMARK_FOUR_ARM_CONTRACT_SCHEMA_VERSION,
1111
BENCHMARK_FOUR_ARM_QUALIFICATION_SCOPE,
@@ -14,17 +14,9 @@
1414

1515
BENCHMARK_FACTORIAL_CONTRAST_SCHEMA_VERSION = "benchmark_factorial_contrast_v0"
1616

17-
_TOKEN_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.:@+-]{0,127}$")
1817
_FACTOR_CELLS = {(False, False), (True, False), (False, True), (True, True)}
1918

2019

21-
def _token(value: Any, *, field: str) -> str:
22-
text = str(value or "").strip()
23-
if not _TOKEN_RE.fullmatch(text):
24-
raise ValueError(f"{field} must be a compact public-safe token")
25-
return text
26-
27-
2820
def _optional_token(value: Any, *, field: str) -> str | None:
2921
if value in (None, ""):
3022
return None

‎loopx/capabilities/benchmark_toolkit/study_projection.py‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import json
77
import math
88
import os
9-
import re
109
import statistics
1110
import tempfile
1211
from collections.abc import Iterable, Mapping
@@ -24,6 +23,10 @@
2423
normalize_benchmark_experiment_board_row,
2524
preview_benchmark_experiment_board_upsert,
2625
)
26+
from .experiment_identity import (
27+
ARM_ROLES,
28+
experiment_token_text as _token,
29+
)
2730
from .four_arm_contract import BENCHMARK_FOUR_ARM_CONTRACT_SCHEMA_VERSION
2831
from .runtime_observation import (
2932
BENCHMARK_RUNTIME_OBSERVATION_SCHEMA_VERSION,
@@ -41,8 +44,6 @@
4144
)
4245
BENCHMARK_STUDY_DASHBOARD_SCHEMA_VERSION = "benchmark_study_dashboard_v0"
4346

44-
_TOKEN_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.:@+-]{0,127}$")
45-
_ARM_ROLES = {"baseline", "control", "treatment", "explore"}
4647
_METRIC_ROLES = {"primary", "guardrail", "supporting"}
4748
_RECORD_KINDS = {
4849
"study_manifest",
@@ -63,13 +64,6 @@ def _reject_unknown_fields(
6364
raise ValueError(f"{field} contains unsupported fields: {', '.join(unknown)}")
6465

6566

66-
def _token(value: Any, *, field: str) -> str:
67-
text = str(value or "").strip()
68-
if not _TOKEN_RE.fullmatch(text):
69-
raise ValueError(f"{field} must be a compact public-safe token")
70-
return text
71-
72-
7367
def _optional_token(value: Any, *, field: str) -> str | None:
7468
if value in (None, ""):
7569
return None
@@ -230,7 +224,7 @@ def normalize_benchmark_study_manifest(
230224
)
231225
arm_id = _token(raw_arm.get("arm_id"), field="arm.arm_id")
232226
arm_role = _token(raw_arm.get("arm_role"), field="arm.arm_role")
233-
if arm_role not in _ARM_ROLES:
227+
if arm_role not in ARM_ROLES:
234228
raise ValueError("arm.arm_role is unsupported")
235229
raw_assignments = raw_arm.get("factor_assignments")
236230
if not isinstance(raw_assignments, Mapping):

‎loopx/capabilities/benchmark_toolkit/traex_evidence.py‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,25 +4,17 @@
44

55
import hashlib
66
import json
7-
import re
87
from collections.abc import Iterable, Mapping
98
from pathlib import Path
109
from typing import Any
1110

11+
from .experiment_identity import experiment_token_text as _token
1212
from ...registry import atomic_write_json
1313

1414
TRAE_BENCHMARK_EVIDENCE_SCHEMA_VERSION = "benchmark_trae_evidence_capture_v0"
1515
BENCHMARK_MODEL_ROUTE_RECEIPT_SCHEMA_VERSION = "benchmark_model_route_receipt_v0"
1616
ATIF_SCHEMA_VERSION = "ATIF-v1.7"
1717

18-
_PUBLIC_TOKEN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.:@+-]{0,127}$")
19-
20-
21-
def _token(value: Any, *, field: str) -> str:
22-
text = str(value or "").strip()
23-
if not _PUBLIC_TOKEN.fullmatch(text):
24-
raise ValueError(f"{field} must be a compact public-safe token")
25-
return text
2618

2719

2820
def _canonical_json(value: Any) -> str:

‎loopx/cli_commands/benchmark_concurrency.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from pathlib import Path
88
from typing import Any
99

10+
from ..capabilities.benchmark_toolkit.experiment_identity import ARM_ROLE_CHOICES
1011
from ..capabilities.benchmark_toolkit import (
1112
admit_benchmark_case,
1213
build_benchmark_adaptive_concurrency_policy,
@@ -102,7 +103,7 @@ def register_benchmark_concurrency_commands(
102103
admit_parser.add_argument(
103104
"--arm-role",
104105
required=True,
105-
choices=["baseline", "control", "treatment", "explore"],
106+
choices=ARM_ROLE_CHOICES,
106107
)
107108
admit_parser.add_argument(
108109
"--resource-headroom-json",

0 commit comments

Comments
 (0)