Skip to content

record's --target Undocumented in local-strict-review, and check's --target Help Text Stale After #1236 #1248

Description

@ptr727

Two small drifts surfaced by an adversarial review of #1222 (canonical_review.py record now
stamping hubCommit as a merge-base against --target), both out of scope for that PR since
fixing them needs files outside its assignment.

1. local-strict-review's "Recording the Pass" example omits --target on record

.agents/skills/local-strict-review/SKILL.md's "Recording the Pass" section gives:

python3 "$engine" status --target <target>        # JSON, take contentDigest
python3 "$engine" record --reviewer agent-skill --target <target> --expect-digest <digest> [--findings N]

for local_review.py, and its own text says a branch based on main needs --target passed
explicitly or the digest is measured against a fork point nobody read.

canonical_review.py record gained the same --target flag in #1222 (default develop), for
the same reason: hubCommit is now stamped as the merge-base against the target, and leaving
--target off silently anchors it at develop even on a branch based on main. But "The
Carried-Content Pass" section's own example still reads:

python3 scripts/canonical_review.py check --target <target>
python3 scripts/canonical_review.py record --reviewer agent-skill --unit '<key>=<digest>' [--findings N]

--target on check, not on record. Followed literally on a main-based branch, check
measures the real fork point while record stamps a merge-base against develop, which is
neither the base check measured nor a commit the reviewed content ever existed at.

Fix: add --target <target> to the record line in that example, matching the local_review.py
one above it. .agents/skills/ is hub-canonical content outside #1222's assigned file list.

2. check's own --target help string is stale after #1236

scripts/canonical_review.py's check subcommand still reads:

p_check.add_argument(
    "--target",
    default=None,
    help=f"target branch (default {DEFAULT_TARGET}), resolved as origin/<value> first",
)

local_review.target_ref (which canonical_review.py calls into) changed its resolution order
in #1236 ("Prefer an Explicitly Named Remote-Tracking Ref Over origin/ in local_review.py"): it
now tries refs/remotes/<value> before origin/<value>, so a value that is already a
remote-tracking ref (e.g. upstream/main) is used as written rather than being prefixed with
origin/. The help string's claim that it is "resolved as origin/ first" is therefore no
longer accurate. #1222 gave record's own new --target a different, accurate phrasing rather
than repeating the stale claim, which is what surfaced this.

Fix: reword check's --target help string (and cross-check local_review.py's own --target
help strings for the same drift) to match target_ref's current resolution order, documented in
its own docstring.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions