feat(plugin): you altered the shape of something — here is everything that reads it (#4215)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 57s
CI & Build / Python tests (push) Successful in 1m36s
CI & Build / Build & push image (push) Successful in 16s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 57s
CI & Build / Python tests (push) Successful in 1m36s
CI & Build / Build & push image (push) Successful in 16s
Milestone 419 step 4. Five of the milestone's seven misses were the same move: acting on the thing in hand without reading the contract around it. Lesson #4207 says so in words, and was written by its author hours before a structurally identical mistake, having been surfaced twice in the turns between. Text delivered at the moment of acting is too weak a carrier for a reflex that has to change what the act IS. This looks it up instead. Rule 33 one scope down: its checks are between layers, and the same question exists between a definition and its callers. THREE KINDS OF EXPOSED NAME, because a contract breaks three ways that look nothing alike in source — the defined symbol (a rename or removal), its parameter names (arity), and the quoted keys of its dict literals (the shape of what it returns). THE THIRD IS THE ONE A SIGNATURE-WATCHER MISSES, and it is in because of the miss that produced this step. Two commits ago `get_writepath_config` gained one dict key; three arms read that dict inside a fail-open `except`, every one silently became a no-op, and ten tests went red with nothing pointing at the cause. No signature changed. Run against that exact edit, the check now names tests/helpers.py — the actual root cause — among six files, before the write. TWO GATES, AND THE SECOND IS WHAT MAKES IT USABLE. A change to the exposed set is necessary but not sufficient: a definition nothing else references has no contract to break, so the readers lookup runs second and an empty result ends it silently. Body-only edits say nothing, a subject is named once per session, and the ledger lives in the swept directory under the `.ids` convention, so the existing compaction-clear guards cover it — checked against test_session_ledger_clear's own parsers rather than assumed. LOCAL AND SERVERLESS, like the duplicate-name arm beside it. It needs the working tree and nothing else; the server has no checkout, so this is the only place the question can be asked. It is a NUDGE: scribe_prior_art.sh still returns no permissionDecision, which is the operator's recorded decision that a recall aid may not stand in the way of a write. A test asserts that here as well as in test_write_path_trigger.py, because this is the arm most likely to tempt someone into making it a gate — it reports something that may already be broken. The `sym` half delegates to `scribe_defs` rather than repeating its patterns: those cover nine languages and have been corrected several times, and a second copy would inherit today's version and quietly stop agreeing with it (#3497). Verified by lifting the test file's own helpers and driving all 19 cases against the real shell over a fixture git repo. Two of my own errors were caught that way and are fixed: the fixtures were arriving as single lines because Python `repr` inside bash single quotes leaves `\n` as two characters (the extractor is line-oriented, so the tests would have gone green against input no editor can produce), and the no-readers case put its subject in a file that was not the excluded one, so it had a reader and tested the opposite of its name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
This commit is contained in:
@@ -367,6 +367,164 @@ scribe_defs() {
|
||||
# $1 repo root, $2 repo-relative path of the file being written (excluded from
|
||||
# the grep — it would always match itself on an Edit). Definitions on stdin.
|
||||
# Prints one "> - `name` is already defined in N other file(s): …" per hit.
|
||||
# ── The contract around a change (#4215, milestone 419) ───────────────────
|
||||
#
|
||||
# WHAT THIS ANSWERS. "You altered the arity, name or shape of something — here
|
||||
# is everything that reads it." Rule 33's interface-contract check, one scope
|
||||
# down: not between layers but between a definition and its callers.
|
||||
#
|
||||
# WHY IT IS A CHECK AND NOT A LESSON. #4207 was written — "widening a tuple is
|
||||
# an interface change to every unpack site, and the compiler will not tell
|
||||
# you" — hours before a structurally identical mistake was made by its author,
|
||||
# and it was surfaced twice in the turns before. Text delivered at the moment
|
||||
# of acting is too weak a carrier for a reflex that has to change what the act
|
||||
# IS. This looks it up instead.
|
||||
#
|
||||
# `scribe_exposed` — the names a CALLER can depend on, from a blob of code.
|
||||
# Three kinds, because a contract breaks three ways and they look nothing
|
||||
# alike in the source:
|
||||
#
|
||||
# sym what is defined rename / removal
|
||||
# arg its parameter names arity and order
|
||||
# key quoted keys of dict literals the shape of what it RETURNS
|
||||
#
|
||||
# The third is here because of the miss that produced this step. A config
|
||||
# function gained one dict key; three arms read that dict inside a fail-open
|
||||
# `except`, so every one of them silently became a no-op and ten tests went
|
||||
# red at once with nothing pointing at the cause. No signature changed. A
|
||||
# check that only watched signatures would have watched the wrong thing.
|
||||
scribe_exposed() {
|
||||
# The `sym` half DELEGATES to scribe_defs rather than repeating its patterns.
|
||||
# Those patterns cover nine languages and have been corrected several times
|
||||
# (the Go receiver form, the `type` import-specifier false positive, the
|
||||
# dunder skip); a second copy here would inherit today's version and then
|
||||
# quietly stop agreeing with it, which is #3497's history for the two rule
|
||||
# arms. One reader, called twice.
|
||||
local blob
|
||||
blob=$(cat)
|
||||
{
|
||||
printf '%s' "$blob" | scribe_defs
|
||||
printf '%s' "$blob" | awk '
|
||||
function emit(kind, name) {
|
||||
if (name != "" && name !~ /^__.*__$/) print kind "\t" name
|
||||
}
|
||||
{
|
||||
line = $0; sub(/^[[:space:]]+/, "", line)
|
||||
sub(/^((pub(\([a-z]+\))?|export|default|private|internal|protected|public|static|suspend|async|open|sealed|data|abstract|final|inline|unsafe|extern|override)[[:space:]]+)*/, "", line)
|
||||
|
||||
# Parameter names, from whatever announces a definition. Taken from the
|
||||
# FIRST parenthesis only: a default value can itself contain parens and
|
||||
# a greedy match would swallow the body of a one-liner.
|
||||
if (match(line, /^(function|def|func|fun|fn|sub)[[:space:]]+[A-Za-z_$][A-Za-z0-9_$]*[[:space:]]*\(/) \
|
||||
|| match(line, /^(const|let)[[:space:]]+[A-Za-z_$][A-Za-z0-9_$]*[[:space:]]*=[[:space:]]*(async[[:space:]]*)?\(/)) {
|
||||
args = line
|
||||
sub(/^[^(]*\(/, "", args)
|
||||
sub(/\).*$/, "", args)
|
||||
n = split(args, parts, ",")
|
||||
for (i = 1; i <= n; i++) {
|
||||
a = parts[i]
|
||||
gsub(/^[[:space:]]+|[[:space:]]+$/, "", a)
|
||||
# Strip a type annotation, a default, and the * / ** / & markers.
|
||||
sub(/[:=].*$/, "", a)
|
||||
gsub(/^[*&]+/, "", a)
|
||||
gsub(/[[:space:]]/, "", a)
|
||||
# `self` and `cls` are not part of anything a caller passes.
|
||||
if (a != "" && a != "self" && a != "cls" && a ~ /^[A-Za-z_$][A-Za-z0-9_$]*$/)
|
||||
emit("arg", a)
|
||||
}
|
||||
}
|
||||
|
||||
# Quoted keys of a dict / object literal. Anchored on the quote so a
|
||||
# dictionary ACCESS (`cfg["k"]`) does not read as a definition of one —
|
||||
# only `"k":` counts, which is the writing position.
|
||||
rest = $0
|
||||
while (match(rest, /["'\''][A-Za-z_][A-Za-z0-9_]*["'\''][[:space:]]*:/)) {
|
||||
tok = substr(rest, RSTART, RLENGTH)
|
||||
rest = substr(rest, RSTART + RLENGTH)
|
||||
gsub(/["'\'']/, "", tok); sub(/[[:space:]]*:$/, "", tok)
|
||||
emit("key", tok)
|
||||
}
|
||||
}
|
||||
' 2>/dev/null
|
||||
} | sort -u
|
||||
}
|
||||
|
||||
# Who references `name` anywhere else in the repo. Word-bounded, so `cfg` does
|
||||
# not match `cfg_path`, and the defining file is excluded — a definition is
|
||||
# always its own first mention and listing it says nothing.
|
||||
#
|
||||
# `|| true` INSIDE the substitution for the reason #4042 records at
|
||||
# scribe_local_dups: under `pipefail` a `head` that exits early kills the
|
||||
# still-writing git grep, and an outer fallback then wipes the hits head had
|
||||
# already printed.
|
||||
scribe_contract_readers() {
|
||||
local root="$1" rel="$2" name="$3"
|
||||
[ -n "$root" ] && [ -n "$name" ] || return 0
|
||||
git -C "$root" grep -I -l -w -e "$name" -- . ":(exclude)${rel}" 2>/dev/null \
|
||||
| head -6 || true
|
||||
}
|
||||
|
||||
# The whole check, rendered. Kept here rather than inline in the hook so it
|
||||
# can be exercised against a pair of blobs with no event, no server and no
|
||||
# session — which is how every case in test_contract_around_the_change.py is
|
||||
# written.
|
||||
#
|
||||
# Arguments: root, repo-relative path, the subject definition, the old text,
|
||||
# the new text, and the session ledger (may be empty).
|
||||
#
|
||||
# TWO GATES, AND THE SECOND IS WHAT KEEPS THIS QUIET. A change to the exposed
|
||||
# set is necessary but not sufficient: a definition NOTHING else references
|
||||
# has no contract to break, so the readers lookup runs second and an empty
|
||||
# result ends it silently. On a repo of any size most edits touch something
|
||||
# local, so most edits say nothing here — and a hint that fires on everything
|
||||
# is one that gets skipped.
|
||||
scribe_contract_block() {
|
||||
local root="$1" rel="$2" subject="$3" old_text="$4" new_text="$5" ledger="$6"
|
||||
local changed gained lost readers count
|
||||
[ -n "$root" ] && [ -n "$subject" ] || return 0
|
||||
[ -n "$old_text" ] && [ -n "$new_text" ] || return 0
|
||||
|
||||
# Named once per session per subject. A second edit to the same definition
|
||||
# is the SAME contract question, and answering it again would punish the
|
||||
# ordinary rhythm of getting a change right over several passes.
|
||||
if [ -n "$ledger" ] && [ -f "$ledger" ]; then
|
||||
grep -qxF "$subject" "$ledger" 2>/dev/null && return 0
|
||||
fi
|
||||
|
||||
# One pass, no process substitution: `comm` would need /dev/fd, and this
|
||||
# also keeps the two sides' extraction visibly identical.
|
||||
changed=$(
|
||||
{
|
||||
printf '%s' "$old_text" | scribe_exposed | sed 's/^/O\t/'
|
||||
printf '%s' "$new_text" | scribe_exposed | sed 's/^/N\t/'
|
||||
} | awk -F'\t' '
|
||||
NF >= 3 { k = $2 "\t" $3; side[k] = side[k] $1 }
|
||||
END {
|
||||
for (k in side)
|
||||
if (side[k] == "O") print "lost\t" k
|
||||
else if (side[k] == "N") print "gained\t" k
|
||||
}
|
||||
' 2>/dev/null
|
||||
)
|
||||
[ -n "$changed" ] || return 0
|
||||
|
||||
readers=$(scribe_contract_readers "$root" "$rel" "$subject")
|
||||
[ -n "$readers" ] || return 0
|
||||
count=$(printf '%s\n' "$readers" | grep -c . 2>/dev/null || printf '0')
|
||||
|
||||
gained=$(printf '%s\n' "$changed" | awk -F'\t' '$1=="gained" {printf "%s%s %s", (n++?", ":""), $2, $3}')
|
||||
lost=$(printf '%s\n' "$changed" | awk -F'\t' '$1=="lost" {printf "%s%s %s", (n++?", ":""), $2, $3}')
|
||||
|
||||
printf '> The contract around `%s` changed, and %s other file(s) reference it (`git grep -w`; a nudge, not a gate):\n' \
|
||||
"$subject" "$count"
|
||||
[ -n "$gained" ] && printf '> gained: %s\n' "$gained"
|
||||
[ -n "$lost" ] && printf '> lost: %s\n' "$lost"
|
||||
printf '> read by: %s\n' "$(printf '%s' "$readers" | tr '\n' ' ' | sed 's/ $//')"
|
||||
printf '> A caller that passes or reads the old shape keeps compiling and fails only when that line runs (lesson #4207). Read them before moving on.\n'
|
||||
[ -n "$ledger" ] && printf '%s\n' "$subject" >> "$ledger" 2>/dev/null
|
||||
return 0
|
||||
}
|
||||
|
||||
scribe_local_dups() {
|
||||
local root="$1" rel="$2" kind name pat hits count label files
|
||||
while IFS=$'\t' read -r kind name; do
|
||||
|
||||
Reference in New Issue
Block a user