quality follow-ups (preflight git rev-parse, schema_version on graders, shellcheck pass, fix latent SC2259 in submodule pipeline)

Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>

Author: vedika-saravanan

.claude/evals/aggregate.py

@@ -86,6 +86,31 @@ def _cohen_kappa(a: list[bool | None], b: list[bool | None]) -> dict[str, Any]:
     }
 
 
+# Bump GRADING_SCHEMA_VERSION whenever the on-disk grading.*.json shape
+# changes incompatibly. Readers below only warn on mismatch (older
+# workspaces remain partially readable); add migration logic here when
+# you bump.
+GRADING_SCHEMA_VERSION = "1"
+
+
+def _check_schema(path: Path, payload: dict) -> None:
+    """Warn (don't fail) if a grading file was produced by a different schema.
+
+    Keeps the loop resilient: an old workspace with no version field still
+    aggregates, just with a one-line note pointing at the producer.
+    """
+    found = payload.get("schema_version")
+    if found is None:
+        sys.stderr.write(
+            f"warning: {path} has no schema_version field "
+            f"(expected {GRADING_SCHEMA_VERSION!r}); produced by an older "
+            f"grader. Some fields may be missing.\n")
+    elif found != GRADING_SCHEMA_VERSION:
+        sys.stderr.write(
+            f"warning: {path} schema_version={found!r}, expected "
+            f"{GRADING_SCHEMA_VERSION!r}. Aggregation may misread fields.\n")
+
+
 def _load_iteration(iter_dir: Path) -> dict[str, dict[str, dict]]:
     """Return {config_name: {grader_name: grading_dict}}."""
     out: dict[str, dict[str, dict]] = {}
@@ -95,7 +120,9 @@ def _load_iteration(iter_dir: Path) -> dict[str, dict[str, dict]]:
         cfg_record: dict[str, dict] = {}
         for path in sorted(cfg.glob("grading.*.json")):
             grader = path.stem.split(".", 1)[1]
-            cfg_record[grader] = json.loads(path.read_text())
+            payload = json.loads(path.read_text())
+            _check_schema(path, payload)
+            cfg_record[grader] = payload
         if cfg_record:
             out[cfg.name] = cfg_record
     return out

.claude/evals/graders/executable.py

@@ -213,6 +213,7 @@ def grade(skill: str, responses: dict, assertions_path: Path) -> dict:
     skipped = sum(1 for s in scenarios if s["status"] == "skipped")
 
     return {
+        "schema_version": "1",
         "grader": "executable",
         "skill": skill,
         "assertions_path": str(assertions_path),

.claude/evals/graders/judge.py

@@ -247,6 +247,7 @@ def grade(skill: str, prompts: list[dict], assertions: dict,
            (len(have_scores) * 8) if have_scores else 0.0)
 
     return {
+        "schema_version": "1",
         "grader": "judge",
         "skill": skill,
         "model": model,

.claude/evals/graders/programmatic.py

@@ -150,6 +150,7 @@ def grade(skill: str, responses: dict[str, str], assertions_path: Path) -> dict:
         max(len(scenarios), 1))
 
     return {
+        "schema_version": "1",
         "grader": "programmatic",
         "skill": skill,
         "assertions_path": str(assertions_path),

.claude/evals/viewer/generate_review.py

@@ -25,6 +25,25 @@
 EVALS_ROOT = Path(__file__).resolve().parents[1]
 PROMPTS_DIR = EVALS_ROOT / "prompts"
 
+# Keep in sync with aggregate.py::GRADING_SCHEMA_VERSION. Duplicated rather
+# than imported so the viewer stays standalone.
+GRADING_SCHEMA_VERSION = "1"
+
+
+def _check_schema(path: Path, payload: dict) -> None:
+    """Warn (don't fail) when a grading file was produced under a different
+    schema. The viewer is best-effort: it'll still render what it can."""
+    found = payload.get("schema_version")
+    if found is None:
+        sys.stderr.write(
+            f"warning: {path} has no schema_version field "
+            f"(expected {GRADING_SCHEMA_VERSION!r}). Some columns may be blank.\n"
+        )
+    elif found != GRADING_SCHEMA_VERSION:
+        sys.stderr.write(f"warning: {path} schema_version={found!r}, "
+                         f"expected {GRADING_SCHEMA_VERSION!r}.\n")
+
+
 # Skill discovery: each iteration directory carries a `skill.txt` or, failing
 # that, we try to pull it from the first grading file.
 
@@ -58,7 +77,9 @@ def _load_iteration(iter_dir: Path) -> dict[str, dict]:
         gradings: dict[str, dict] = {}
         for path in sorted(cfg.glob("grading.*.json")):
             grader = path.stem.split(".", 1)[1]
-            gradings[grader] = json.loads(path.read_text())
+            payload = json.loads(path.read_text())
+            _check_schema(path, payload)
+            gradings[grader] = payload
         if responses or gradings:
             out[cfg.name] = {
                 "responses": responses,

.claude/skills/_shared/scripts/preflight.sh

@@ -19,10 +19,18 @@ set -u
 JSON=0
 [[ "${1:-}" == "--json" ]] && JSON=1
 
-# Locate repo root (the dir that contains scripts/doctor.sh).
+# Locate repo root. Prefer `git rev-parse` so moving this script up or down
+# a directory level does not silently break path arithmetic. Fall back to the
+# legacy 4-levels-up relative path when not in a git checkout (e.g. extracted
+# tarball, or running outside the repo).
 HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
-# .claude/skills/_shared/scripts/ -> repo root is 4 levels up.
-REPO_ROOT="$(cd "$HERE/../../../.." && pwd)"
+if command -v git >/dev/null 2>&1 \
+    && REPO_ROOT="$(git -C "$HERE" rev-parse --show-toplevel 2>/dev/null)" \
+    && [[ -n "$REPO_ROOT" ]]; then
+  :
+else
+  REPO_ROOT="$(cd "$HERE/../../../.." && pwd)"
+fi
 
 # ---------------------------------------------------------------------------
 # Helpers
@@ -46,7 +54,9 @@ OS_KERNEL="$(uname -srm 2>/dev/null || echo unknown)"
 OS_ID=""
 OS_VERSION=""
 if [[ -f /etc/os-release ]]; then
+  # shellcheck source=/dev/null
   OS_ID="$(. /etc/os-release && echo "${ID:-}")"
+  # shellcheck source=/dev/null
   OS_VERSION="$(. /etc/os-release && echo "${VERSION_ID:-}")"
 fi
 CPU_COUNT="$(nproc 2>/dev/null || echo 0)"
@@ -194,8 +204,13 @@ BUILD_DIR_EXISTS=false
 
 SUBMODULES_JSON='[]'
 if [[ -f "$REPO_ROOT/.gitmodules" ]] && command -v git >/dev/null 2>&1; then
+  # Note: must use `python3 -c` rather than `python3 - <<HEREDOC` here,
+  # because the heredoc redirect would override the piped stdin from
+  # git submodule status and the script would never see any submodule
+  # lines (silently emits '[]' regardless). Currently no submodules in
+  # this repo, so the bug was latent until a future submodule lands.
   SUBMODULES_JSON=$(cd "$REPO_ROOT" && git submodule status 2>/dev/null \
-    | python3 - <<'PY'
+    | python3 -c '
 import json, sys
 out = []
 for line in sys.stdin:
@@ -211,8 +226,7 @@ for line in sys.stdin:
             "initialized": initialized,
         })
 print(json.dumps(out))
-PY
-  )
+')
 fi
 
 # ---------------------------------------------------------------------------

scripts/build_one_lib.sh

@@ -81,12 +81,21 @@ fi
 # Configure (only on first run, unless --reconfigure)
 if [[ ! -d "$BUILD" || $RECONFIGURE -eq 1 ]]; then
   echo "Configuring $LIB ($BUILD_TYPE) ..."
+  if [[ $DO_TESTS -eq 1 ]]; then
+    TESTS_FLAG="-DCUDAQX_INCLUDE_TESTS=ON"
+  else
+    TESTS_FLAG="-DCUDAQX_INCLUDE_TESTS=OFF"
+  fi
+  if [[ $NO_PYTHON -eq 1 ]]; then
+    PYTHON_FLAG="-DCUDAQX_BINDINGS_PYTHON=OFF"
+  else
+    PYTHON_FLAG="-DCUDAQX_BINDINGS_PYTHON=ON"
+  fi
   cmake -G Ninja -S "$SRC" -B "$BUILD" \
     -DCUDAQ_DIR="$CUDAQ_DIR_VAL" \
     -DCMAKE_INSTALL_PREFIX="$CUDAQX_INSTALL_PREFIX_VAL" \
     -DCMAKE_BUILD_TYPE="$BUILD_TYPE" \
-    $( [[ $DO_TESTS -eq 1 ]] && echo "-DCUDAQX_INCLUDE_TESTS=ON" || echo "-DCUDAQX_INCLUDE_TESTS=OFF" ) \
-    $( [[ $NO_PYTHON -eq 1 ]] && echo "-DCUDAQX_BINDINGS_PYTHON=OFF" || echo "-DCUDAQX_BINDINGS_PYTHON=ON" )
+    "$TESTS_FLAG" "$PYTHON_FLAG"
 fi
 
 echo "Building $LIB ..."