fix(sdk-review): batch-5 grep -c idiom, precedence, self-skip, portability

- check-pr-size / check-deps-supply: replace 'grep -c … || echo 0' idiom
  (returns '0\n0' and breaks -eq/-gt) with 'grep … | wc -l | tr -d \" \"'
- check-pr-size: resolve BASE_SHA from origin/${GITHUB_BASE_REF} merge-base
- check-testing-depth: replace [ -z … ] || [ … ] && continue with explicit
  if — the previous form's short-circuit hid errors under set -e
- check-deletion-hygiene: replace \b with portable (^|[^A-Za-z0-9_]) since
  grep -E doesn't universally honor \b
- check-constants: filter skill files out of AST scan (self-skip)

Author: tiagoek

.claude/scripts/check-constants.sh

@@ -4,6 +4,7 @@
 set -euo pipefail
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 source "$SCRIPT_DIR/lib/json-emit.sh"
+source "$SCRIPT_DIR/lib/skill-self-skip.sh"
 
 LANGUAGE="${LANGUAGE:-python}"
 DIFF_FILE="${DIFF_FILE:-/dev/stdin}"
@@ -16,9 +17,16 @@ diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
 # CON-01 via AST on Python files
 if [ "$LANGUAGE" = "python" ]; then
   changed=$(echo "$diff_content" | grep -oE '^\+\+\+ b/src/.*\.py' | sed 's|^+++ b/||' | sort -u)
-  if [ -n "$changed" ]; then
+  # Filter out skill files (self-review protection)
+  filtered=""
+  while IFS= read -r f; do
+    [ -z "$f" ] && continue
+    if [ "$(is_skill_file "$f")" = "true" ]; then continue; fi
+    filtered="$filtered $f"
+  done <<< "$changed"
+  if [ -n "$filtered" ]; then
     # shellcheck disable=SC2086
-    python3 "$SCRIPT_DIR/lib/ast_python_checks.py" con-01 $changed 2>/dev/null >> "$findings" || true
+    python3 "$SCRIPT_DIR/lib/ast_python_checks.py" con-01 $filtered 2>/dev/null >> "$findings" || true
   fi
 fi
 

.claude/scripts/check-deletion-hygiene.sh

@@ -25,8 +25,9 @@ if [ "$LANGUAGE" = "python" ]; then
   ')
   while IFS= read -r sym; do
     [ -z "$sym" ] && continue
-    # search codebase (excluding the file where it was removed)
-    hits=$(grep -rEIln "\b${sym}\b" "$REPO_ROOT/src" 2>/dev/null | wc -l | tr -d ' ')
+    # search codebase (excluding the file where it was removed).
+    # grep -E doesn't universally honor \b — use portable (^|non-word) anchors.
+    hits=$(grep -rEIln "(^|[^A-Za-z0-9_])${sym}([^A-Za-z0-9_]|$)" "$REPO_ROOT/src" 2>/dev/null | wc -l | tr -d ' ')
     if [ "$hits" -gt 0 ]; then
       emit_finding "DEL-01" "BLOCK" "src/" 1 \
         "Symbol '$sym' removed from __all__ but still referenced in $hits files" "" >> "$findings"

.claude/scripts/check-deps-supply.sh

@@ -18,10 +18,14 @@ if echo "$diff_content" | grep -qE '\+.*(index-url|extra-index-url).*int\.reposi
   emit_finding "DEP-04" "BLOCK" "config" 1 \
     "Internal artifactory --index-url reference in public artifact" "" >> "$findings"
 fi
+# Helper: `grep -c` returns "0" + exit 1 on zero matches → || echo 0 emits
+# "0\n0" and breaks -eq. Use wc -l instead.
+count_lines() { echo "$1" | grep -E "$2" 2>/dev/null | wc -l | tr -d ' '; }
+
 # DEP-03: pyproject.toml deps changed but uv.lock not
 if [ "$LANGUAGE" = "python" ]; then
-  pyproject_deps_changed=$(echo "$diff_content" | grep -cE '^\+.*"[a-z][a-z0-9_-]*[><=~!]+' || echo 0)
-  uv_lock_changed=$(echo "$diff_content" | grep -c '^\+\+\+ b/uv\.lock' || echo 0)
+  pyproject_deps_changed=$(count_lines "$diff_content" '^\+.*"[a-z][a-z0-9_-]*[><=~!]+')
+  uv_lock_changed=$(count_lines "$diff_content" '^\+\+\+ b/uv\.lock')
   if [ "$pyproject_deps_changed" -gt 0 ] && [ "$uv_lock_changed" -eq 0 ]; then
     # only fire if pyproject.toml dep table (not [project] version) changed
     if echo "$diff_content" | grep -qE '^\+.*\[project\.(dependencies|optional-dependencies)\]|^\+[[:space:]]+"[a-z][a-z0-9_-]*[><=~!].*"'; then

.claude/scripts/check-pr-size.sh

@@ -6,42 +6,57 @@ source "$SCRIPT_DIR/lib/json-emit.sh"
 
 LANGUAGE="${LANGUAGE:-python}"
 DIFF_FILE="${DIFF_FILE:-/dev/stdin}"
-BASE_SHA="${BASE_SHA:-HEAD~10}"
+# Resolve BASE_SHA from the PR base ref rather than HEAD~10.
+if [ -z "${BASE_SHA:-}" ]; then
+  base_ref="${GITHUB_BASE_REF:-main}"
+  if git rev-parse --verify "origin/${base_ref}" >/dev/null 2>&1; then
+    BASE_SHA=$(git merge-base HEAD "origin/${base_ref}" 2>/dev/null || echo "HEAD~10")
+  elif git rev-parse --verify "$base_ref" >/dev/null 2>&1; then
+    BASE_SHA=$(git merge-base HEAD "$base_ref" 2>/dev/null || echo "HEAD~10")
+  else
+    BASE_SHA="HEAD~10"
+  fi
+fi
 HEAD_SHA="${HEAD_SHA:-HEAD}"
 
 STARTED=$(now_iso)
 findings=$(mktemp); trap 'rm -f "$findings"' EXIT
 
 diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
 
+# Helper: `grep -c` returns "0" AND exit 1 on no match. Under `set -e` /
+# pipefail the || echo 0 idiom concatenates both, producing "0\n0" and
+# breaking arithmetic. Route through wc -l instead.
+count_lines() { echo "$1" | grep -E "$2" 2>/dev/null | wc -l | tr -d ' '; }
+
 # PR-SIZE-01: additions > 800
-additions=$(echo "$diff_content" | grep -cE '^\+[^+]' || echo 0)
+additions=$(count_lines "$diff_content" '^\+[^+]')
 if [ "$additions" -gt 800 ]; then
   emit_finding "PR-SIZE-01" "FLAG" "." 1 \
     "PR has $additions additions (>800) — consider splitting into stacked PRs for easier review" \
     "See docs/CONTRIBUTING.md § Incremental Delivery for stacked-PR workflow" >> "$findings"
 fi
 
 # PR-SIZE-02: > 15 files
-files_touched=$(echo "$diff_content" | grep -cE '^diff --git' || echo 0)
+files_touched=$(count_lines "$diff_content" '^diff --git')
 if [ "$files_touched" -gt 15 ]; then
   emit_finding "PR-SIZE-02" "FLAG" "." 1 \
     "PR touches $files_touched files (>15) — consider splitting by concern" "" >> "$findings"
 fi
 
 # PR-SIZE-03: > 3 modules
 if [ "$LANGUAGE" = "python" ]; then
-  mods_count=$(echo "$diff_content" | grep -oE 'src/sap_cloud_sdk/[a-z_]+/' | sed 's|src/sap_cloud_sdk/||; s|/$||' | sort -u | wc -l | tr -d ' ')
+  mods_count=$(echo "$diff_content" | grep -oE 'src/sap_cloud_sdk/[a-z_]+/' 2>/dev/null | sed 's|src/sap_cloud_sdk/||; s|/$||' | sort -u | wc -l | tr -d ' ')
 else
-  mods_count=$(echo "$diff_content" | grep -oE 'src/main/java/com/sap/cloud/sdk/[a-z_]+/' | sed 's|src/main/java/com/sap/cloud/sdk/||; s|/$||' | sort -u | wc -l | tr -d ' ')
+  mods_count=$(echo "$diff_content" | grep -oE 'src/main/java/com/sap/cloud/sdk/[a-z_]+/' 2>/dev/null | sed 's|src/main/java/com/sap/cloud/sdk/||; s|/$||' | sort -u | wc -l | tr -d ' ')
 fi
 if [ "$mods_count" -gt 3 ]; then
   emit_finding "PR-SIZE-03" "FLAG" "." 1 \
     "PR modifies $mods_count modules (>3) — consider one PR per module" "" >> "$findings"
 fi
 
-# PR-SIZE-05: > 30 commits
-commit_count=$(git log "${BASE_SHA}..${HEAD_SHA}" --oneline 2>/dev/null | grep -v -c '^Merge' || echo 0)
+# PR-SIZE-05: > 30 commits (exclude merge commits by looking at parents)
+commit_count=$(git log "${BASE_SHA}..${HEAD_SHA}" --no-merges --oneline 2>/dev/null | wc -l | tr -d ' ')
 if [ "$commit_count" -gt 30 ]; then
   emit_finding "PR-SIZE-05" "FLAG" "." 1 \
     "PR has $commit_count commits (>30) — consider squashing or splitting" "" >> "$findings"

.claude/scripts/check-testing-depth.sh

@@ -27,7 +27,13 @@ if [ "$LANGUAGE" = "python" ]; then
   # TD-10: New module → integration test required
   new_modules=$(echo "$diff_content" | awk '/^diff --git/ { flag=0 } /^new file mode/ { flag=1 } flag && /^\+\+\+ b\/src\/sap_cloud_sdk\/[a-z_]+\/[^/]+\.py/ { print }' | grep -oE 'src/sap_cloud_sdk/[a-z_]+/' | sed 's|src/sap_cloud_sdk/||; s|/$||' | sort -u)
   while IFS= read -r mod; do
-    [ -z "$mod" ] || [ "$mod" = "core" ] && continue
+    # Both conditions should skip the loop iteration. The previous
+    # A || B && continue form parses as (A || B) && continue, which is
+    # correct — but the `&& continue` under `set -e` short-circuits the
+    # loop body's exit status and hides errors. Explicit if is safer.
+    if [ -z "$mod" ] || [ "$mod" = "core" ]; then
+      continue
+    fi
     has_integration=$(echo "$diff_content" | grep -qE "tests/$mod/integration/" && echo yes || echo no)
     if [ "$has_integration" = "no" ]; then
       emit_finding "TD-10" "BLOCK" "tests/$mod/integration/" 1 \