refactor(source-control): make the babysit JSON predicates TypeGuards, deleting 15 casts

[kyle-sexton]

Closes #3448

Summary

babysit_util.is_json_object and is_json_array returned a plain bool. A true
result therefore told the type checker nothing, so every caller that acted on one had
to restate the fact with cast(dict[str, Any], ...) or cast(list[Any], ...), or with
an inline isinstance test paired with the same cast. Both predicates now return a
typing.TypeGuard, and the casts the narrowing makes redundant are gone.

Count correction. The issue estimated "~12 casts in merge/gh"; triage counted 17
cast sites in the merge module. The real number deleted is 15: 14 in
babysit_merge.py and 1 in babysit_util.dig. babysit_gh.py contains no cast at
all, so nothing was deleted there, which matches the triage comment's own correction.
Three casts remain in babysit_merge.py (lines 208, 968, 1088); none of them restates
a predicate result, so none is in scope. Ten inline isinstance tests that existed
only to pair with a cast now call the shared predicate instead.

Fix

What each guard's runtime check validates, versus what it asserts. This is the
whole risk of the change: once a predicate is a TypeGuard, the compiler trusts it and
stops warning about the thing the cast made visible. Neither guard asserts more than
its own check earns.

Helper	Runtime check	Asserted type	Fit
is_json_object	isinstance(value, dict)	TypeGuard[dict[Any, Any]]	Exact. The check establishes the container and inspects no key. JSON grammar does guarantee string keys, but this function never reads one, so the guard deliberately does not claim dict[str, Any].
is_json_array	isinstance(value, list)	TypeGuard[list[Any]]	Exact. The check establishes the container and inspects no element, which is precisely what an Any element type asserts.

Both guards are weaker than the casts they replace only in what they assert: every
deleted cast(dict[str, Any], ...) asserted string keys with nothing behind it, and the
guard that replaces it asserts only dict-ness. That is not a type-system barrier, though,
because dict[Any, Any] is gradually assignable to dict[str, Any]. Concretely, at
babysit_merge.py:552, the ref that is_json_object narrows flows straight into
_ref_repo(ref: dict[str, Any]) and pyright raises no complaint. That is harmless here,
since ref was Any before this change, but it means the weaker assertion does not
block a stronger downstream use the way an actual type-system boundary would. No call
site needed dict[str, Any] for its own purposes: they all index with string literals,
which dict[Any, Any] already permits. TypeGuard (not TypeIs) is used on purpose,
so the negative branch is left unnarrowed.

The stronger argument for safety is runtime, not the type checker: cast returns its
argument unchanged, it is a runtime identity with no effect of its own, and the 10
inline isinstance tests this PR swapped for the shared predicate run the same check
the predicate now runs. So the runtime behavior at every deleted-cast site is provably
byte-identical to before the change, regardless of what pyright or any other type
checker reports.

Each deletion was checked for guard dominance at the site rather than pattern-matched on
the cast's presence: the guarded-if-then-return/continue sites (repository_default_branch,
head_committed_at, the branch_rules loop, dig) dominate their uses by early exit; the
conditional-expression sites (unresolved_threads, pr / data / params /
required_context_list) narrow inside the true arm only; and the one comprehension site
narrows the element expression from its own if clause.

One collateral fix: at babysit_merge.py:557 the sharper narrowing made pyright see
int(number) receive Any | None. The except (TypeError, ValueError) around that call
was already the real validation, so number is annotated Any to say the shape is
decided there. No behavior change.

Test coverage added. babysit_util.py mapped to no test suite, which
scripts/affected-tests.sh treats as an error rather than an empty selection. A new
tests/test_babysit_util.py (12 cases) pins the runtime side of the two guards and of
dig.

Version chain: source-control 0.55.61 -> 0.55.62 plus the CHANGELOG entry. 0.55.62 was
confirmed free against current main (1b68186, itself 0.55.61) and against every open
PR that bumps this plugin (#3873 claims 0.55.61, #3871 0.55.60, #3774 0.55.58, #3740
0.55.54).

Verification

Run in a dedicated worktree at 93a6ef96, branched from origin/main 1b681862.

• Type check (primary evidence). pyright 1.1.408 over the babysit script tree:
  130 errors -> 92, with zero new diagnostics (comm against a baseline taken
  from a clean git archive origin/main export). Over the three files the issue names,
  babysit_util.py / babysit_merge.py / babysit_gh.py: 12 errors -> 0. The 38
  errors that disappear are reportOptionalMemberAccess on .get(...) calls the
  predicates already guarded at runtime. Note: the repo has no pyproject.toml and no
  checked-in pyright config, so this is pyright's default (basic) mode and is not a
  CI gate here; it is the bundled python ecosystem's check-cmd tool.
• Non-vacuity for the type-level work (revert a guard to plain bool, errors must
  return at the deleted cast sites):
  • is_json_array -> bool: 2 errors return at babysit_merge.py:980
    (reportOptionalIterable, reportGeneralTypeIssues), which is exactly the use of
    required_context_list, whose cast(list[Any], ...) this PR deletes.
  • is_json_object -> bool: 8 errors return in babysit_merge.py. One of them,
    line 202 (author_object.get("login")), is directly downstream of a deleted cast in
    unresolved_threads. Honest qualifier: the other 7 are the pre-existing errors
    this PR fixes, not deleted-cast sites, and several deleted casts (the
    gh_json-returns-Any sites at 228/759-766) do not produce a compiler error when
    the guard is reverted, because Any swallows it. Those casts were ceremony rather
    than compiler-required; removing them is safe but is not proven by this mutation.
• Mutation proof for the added runtime tests (all four mutants caught, baseline OK):
  is_json_object also accepts a list -> 3 cases fail; is_json_array rejects an empty
  list -> 1 fails; is_json_object rejects an empty dict -> 1 fails; dig passes a
  non-dict level through instead of returning None -> 1 fails.
• Tests. plugins/source-control/skills/babysit-prs/scripts/engine.test.sh:
  662 tests, OK (650 before this PR's 12 new cases), plus its ruff pass and all 11
  guarded-wrapper behavior checks PASS. scripts/test_check_contract_clause_coverage.py:
  24 tests, OK.
• bash scripts/affected-tests.sh --run: exit 3 (success: 3 shell suites PASS, 10
  Python suites SELECTED as NOT RUN for a runner it will not guess; those are the
  engine.test.sh tree, run above). No unmapped file remains: adding
  tests/test_babysit_util.py closed the one UNMAPPED report the first run produced.
• Lint / format. scripts/run-ruff.sh check on the babysit scripts tree: all checks
  passed. scripts/run-ruff.sh format --check on all three changed Python files: already
  formatted. markdownlint-cli2 on the CHANGELOG: 0 issues.
• Version parity, all four modes green at 93a6ef96: --check, --check-order,
  --check-bump origin/main, --check-preserved origin/main (237 headings compared).
• No em dashes introduced in any repo-tracked surface.

Unmet / out of scope. Nothing in the acceptance criteria is unmet, with one wording
correction recorded above: the criteria say "the repository's configured Python type
checker", and this repo configures none, so pyright-default is reported as the evidence
it is. Sibling issue #3449 (the triplicated guarded-mutation preamble in the babysit
scripts) is untouched. Predicates were not introduced anywhere else in the plugin, and no
merge-module logic changed.

Related

• Closes source-control: babysit_util type-check helpers should be TypeGuards, deleting ~12 casts in merge/gh #3448
• Sibling, deliberately not in this PR: source-control: triplicated guarded-mutation preamble in babysit scripts wants a reviewed hoist #3449 (guarded-mutation preamble consolidation)
• Filed by the /code-tidying:batch-simplify closeout (Phase 8)
• Version-collision check against open source-control PRs: perf(source-control): read the worktree gates fields in one process each #3873, perf(source-control): cut pr-issue-linkage gate spawns by redirect placement #3871, fix(source-control): survive the pinned-GraphQL restriction, fail closed on thread resolution #3774, fix(hook-utils): a hook payload cut short at EOF is a loud allow; a stall stays a block #3740

🤖 Generated with Claude Code

https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob

---

Generated by Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-07T01:12:48.117656Z	93a6ef9	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 2m 22s —— View job

---

CI code review

• Gather context (PR diff, changed files, CLAUDE.md/AGENTS.md)
• Apply skip gate (open, not draft, meaningful diff → proceed)
• Review changed files for correctness/maintainability findings
• Post inline findings (none met the high-signal bar)
• Post final summary

Scope: plugins/source-control/skills/babysit-prs/scripts/babysit_util.py,
babysit_merge.py, the new tests/test_babysit_util.py, plus the version/CHANGELOG bump.
Security concerns are out of scope for this lane (see /review:security-review).

Findings: none. I traced every TypeGuard-narrowed call site the diff touches
(unresolved_threads, repository_default_branch, the branch_rules loop and its
required_status_checks comprehension, evaluate_decision_default_veto's number
handling, head_committed_at, evaluate's pr/required_context_list, allowed_method,
and dig) against the pre-change cast/isinstance version and confirmed each is a
byte-identical runtime check with only the static type improved. The three casts the PR
says it deliberately left alone (
babysit_merge.py:208,
:968,
:1088
) do check out as unpaired with any predicate, so leaving them is consistent with the
PR's own stated scope. New tests in test_babysit_util.py cover the accept/reject shape
of both guards, including the truthiness-shortcut traps (empty container, None) and
dig's non-dict bail-out.

Everything in the "Verification" section of the PR body (pyright counts, mutation
testing, engine.test.sh, ruff, version-parity) is author-claimed and not
independently re-run by this review — this sandbox's Bash tool required an approval
gate on python3 -m unittest ... that wasn't available in this lane, so I could not
execute the new test file myself. Static reading of the test file did not surface any
assertion that looks wrong against the implementation.

One thing double-checked because it looked like a possible version-floor risk but
resolved cleanly: TypeGuard requires Python 3.10+, and this module already imports
from datetime import UTC (3.11+) on an unrelated pre-existing line, so the added
import doesn't lower the plugin's effective floor.

No inline comments posted — no findings cleared the high-signal bar for this lane.

[claude]

Claude finished @kyle-sexton's task in 1m 9s —— View job

---

Security review complete

• Gather context (PR diff, changed files)
• Apply skip gate
• Review changed files for security-relevant surface
• Post findings

No security issues found.

This PR is a pure typing refactor of babysit_util.is_json_object / is_json_array (making them TypeGuards) and the removal of the cast(...) calls their narrowing makes redundant in babysit_merge.py, plus a new unit test file. Reviewed:

• plugins/source-control/skills/babysit-prs/scripts/babysit_util.py
• plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py
• plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_util.py (new test file)
• plugins/source-control/.claude-plugin/plugin.json, CHANGELOG.md (version bump + changelog only)

Every replaced cast() call was already downstream of the same isinstance(value, dict) / isinstance(value, list) check that the TypeGuard predicates run — cast is a runtime no-op, so this is provably behavior-preserving at every call site, matching the PR's own verification notes. No new external input parsing, no authorization/access-control logic, no injection surface (command, SQL, path, template), and no token/secret/credential handling is touched. No GitHub Actions workflow files are in this diff.

No findings to report.

[github-actions]

Last security-reviewed head: 93a6ef96423328882c1e42e929a4ad5953d637f1. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[github-actions]

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.