24da72b7f4
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m55s
REVIEW.md, from the suggestions of the first review of #460: - stacked-PR pitfall: fs_commands.rs compiles two CSV files of src/__fixtures__/samples/ through include_str!, outside the paths of check-rust; - high level: .claude/settings.json and .mcp.json, a pull_request_target trigger, an install step that changes the toolchain or the PATH (the check-workflow list is now given as examples of the principle), a lock origin whose host or scheme changes even with a new version, and a diff that removes or bypasses a consent (price consent lives on the front); - db.ts and dataExportService.ts stay normal (Max's decision of 2026-10-18), and the data zone now names the lock, the profile switch and the restore; - release zone: cargo-xwin and the Rust toolchain, installed unpinned at each tag, also run with the signing key; - windows-sys guard: EXPECTED joins CRATES and PIN; - identifier: changing it orphans the databases and the app_data_dir files (license key, activation token, token fallback); the keychain service is a constant and does not follow. scripts/review-levels.py is the calibration script of the PR body, now in the repo: pages of 50 files read until an empty page (the instance caps pages at 50), the new rules above, comment lines ignored by the workflow checks, and --check, which compares its patterns with the Haut table without network access. Fresh run: 3/28/8 on the 39 reference PRs and 6/33/12 on the 51, unchanged. Refs #456 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
393 lines
17 KiB
Python
Executable file
393 lines
17 KiB
Python
Executable file
#!/usr/bin/env python3
|
|
"""Replay the review levels of REVIEW.md on merged pull requests (#456).
|
|
|
|
Usage:
|
|
python3 scripts/review-levels.py [FIRST LAST] (default: 344 452)
|
|
python3 scripts/review-levels.py --check (no network)
|
|
|
|
The first form reads the Forgejo API (token in $FORGEJO_TOKEN, else
|
|
~/.forgejo-token; read-only calls) and prints each merged PR of the range
|
|
with its trial level (pilot document, section 4.3) and its REVIEW.md level,
|
|
then the distributions for the 39 reference PRs and for the whole range.
|
|
--check compares the patterns of this script with the first column of the
|
|
Haut table of REVIEW.md and exits 1 if they differ: a PR that changes the
|
|
Niveaux section updates this script in the same diff.
|
|
|
|
The content conditions of the Haut table are approximated from the diff:
|
|
lib.rs (only generate_handler![] entries are exempt), Tauri configuration
|
|
and manifests (only the app's version is exempt), lock files (a package
|
|
added or removed, an origin changed), check workflows (a change that can
|
|
turn a failing check green, read against the PR's merge base), network
|
|
calls (reqwest in Rust, fetch to an absolute URL in TS), consent files.
|
|
"""
|
|
import itertools
|
|
import json
|
|
import os
|
|
import re
|
|
import sys
|
|
import urllib.request
|
|
|
|
API = "https://git.lacompagniemaximus.com/api/v1/repos/maximus/simpl-resultat"
|
|
REVIEW_MD = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "REVIEW.md")
|
|
|
|
# The 39 reference PRs of the pilot document, frozen by number: the listing
|
|
# that produced them (closed PRs, limit=40, sort=recentupdate) is not stable.
|
|
REF39 = [344, 375, 377, 378, 379, 380, 381, 382, 383, 386, 387, 388, 394, 402, 403,
|
|
407, 412, 413, 414, 415, 416, 417, 418, 420, 428, 433, 434, 435, 436, 438,
|
|
439, 440, 441, 442, 443, 444, 445, 448, 452]
|
|
|
|
TRIAL_HIGH = [
|
|
"src-tauri/src/commands/{auth_commands,token_store,license_commands,entitlements,account_cache,backup_commands,fs_commands,export_import_commands}.rs",
|
|
"src-tauri/src/lib.rs", "src-tauri/src/database/**", "src-tauri/tauri.conf.json",
|
|
"src-tauri/capabilities/**", ".forgejo/workflows/**", ".cargo/**", "src-tauri/Cargo.{toml,lock}",
|
|
"package.json", "package-lock.json",
|
|
"src/services/{authService,licenseService,db,feedbackService,dataExportService}.ts",
|
|
"src/services/entitlements*", "scripts/build-latest-json*",
|
|
]
|
|
TRIAL_LOW = ["**/*.md", "docs/**", "reports/**", "tasks/**", "public/CHANGELOG*", "LICENSE"]
|
|
|
|
# REVIEW.md, table Haut: rows without a condition.
|
|
HIGH_ALWAYS = [
|
|
"src-tauri/src/database/**",
|
|
"src-tauri/src/commands/profile_commands.rs",
|
|
"src-tauri/src/commands/{license_commands,entitlements,auth_commands,token_store,account_cache}.rs",
|
|
"src/services/{licenseService,authService}.ts",
|
|
".claude/skills/issue-license/**",
|
|
"src-tauri/src/commands/export_import_commands.rs",
|
|
"src-tauri/src/commands/feedback_commands.rs", "src/services/feedbackService.ts",
|
|
"src-tauri/capabilities/**",
|
|
".forgejo/workflows/release.yml", "scripts/build-latest-json*", "scripts/check-latest-json.jq",
|
|
".claude/skills/release/**",
|
|
"src-tauri/build.rs", "{vite,vitest,postcss}.config.*", ".postcssrc*", ".npmrc", "**/rust-toolchain*",
|
|
"Cargo.{toml,lock}",
|
|
"**/.cargo/**",
|
|
".claude/settings.json", ".mcp.json",
|
|
]
|
|
CONSENT_FILES = {"src/components/balance/PriceFetchControl.tsx",
|
|
"src/components/settings/PriceFetchConsentToggle.tsx"}
|
|
TAURI_CONFIG = ["src-tauri/{tauri*.conf.json*,Tauri*.toml}"]
|
|
CHECK_WORKFLOWS = {".forgejo/workflows/check-rust.yml", ".forgejo/workflows/check-frontend.yml"}
|
|
SHIPPED = ["src/**", "src-tauri/src/**", "index.html", "public/**"]
|
|
LOW = ["**/*.md", "docs/**", "LICENSE", "tasks/**", "reports/**"]
|
|
NEVER_LOW = ["CLAUDE.md", "REVIEW.md", "docs/adr/**", "docs/architecture.md", "docs/qa-update-cycle.md", ".claude/**"]
|
|
|
|
|
|
def token():
|
|
return os.environ.get("FORGEJO_TOKEN") or open(os.path.expanduser("~/.forgejo-token")).read().strip()
|
|
|
|
|
|
def get(path, raw=False):
|
|
req = urllib.request.Request(API + path, headers={"Authorization": "token " + token()})
|
|
data = urllib.request.urlopen(req).read()
|
|
return data if raw else json.loads(data)
|
|
|
|
|
|
def expand_braces(pat):
|
|
m = re.search(r"\{([^{}]*)\}", pat)
|
|
if not m:
|
|
return [pat]
|
|
return list(itertools.chain.from_iterable(
|
|
expand_braces(pat[:m.start()] + alt + pat[m.end():]) for alt in m.group(1).split(",")))
|
|
|
|
|
|
def glob_re(pat):
|
|
# '**/' spans zero or more directories, '*' and '?' never cross '/'
|
|
i, out = 0, ""
|
|
while i < len(pat):
|
|
if pat.startswith("**/", i):
|
|
out += "(?:.*/)?"; i += 3; continue
|
|
if pat.startswith("**", i):
|
|
out += ".*"; i += 2; continue
|
|
c = pat[i]
|
|
out += "[^/]*" if c == "*" else "[^/]" if c == "?" else re.escape(c)
|
|
i += 1
|
|
return re.compile("^" + out + "$")
|
|
|
|
|
|
def match(pats, path):
|
|
# '{a,b}' alternates, wildcards allowed inside
|
|
return any(glob_re(p).match(path) for pat in pats for p in expand_braces(pat))
|
|
|
|
|
|
def file_blocks(diff):
|
|
out = {}
|
|
for block in re.split(r"(?m)^diff --git ", diff)[1:]:
|
|
out[block.split("\n", 1)[0].split(" b/")[-1]] = block
|
|
return out
|
|
|
|
|
|
def meaningful(text):
|
|
t = text.strip()
|
|
return t != "" and not t.startswith(("#", "//"))
|
|
|
|
|
|
def changed(block):
|
|
return [l for l in block.split("\n")
|
|
if l[:1] in "+-" and not l.startswith(("+++", "---")) and meaningful(l[1:])]
|
|
|
|
|
|
def lock_adds_or_removes(name, block):
|
|
cargo = name.endswith("Cargo.lock")
|
|
lines = block.split("\n")
|
|
if cargo and any(re.match(r'^[+-]name = ', l) for l in lines):
|
|
return True
|
|
if not cargo and any(re.match(r'^[+-]\s+"node_modules/[^"]+": \{', l) for l in lines):
|
|
return True
|
|
source = r'^([+-])source = "([^"]+)"' if cargo else r'^([+-])\s+"resolved": "([^"]+)"'
|
|
digest = r'^\+(checksum) = ' if cargo else r'^\+\s+"(integrity)": '
|
|
version = r'^\+version = ' if cargo else r'^\+\s+"version": '
|
|
for hunk in re.split(r"(?m)^@@", block)[1:]:
|
|
hl = hunk.split("\n")
|
|
# an origin whose host or scheme changes, with or without a new version
|
|
before, after = set(), set()
|
|
for l in hl:
|
|
m = re.match(source, l)
|
|
if m:
|
|
(before if m.group(1) == "-" else after).add(origin_of(m.group(2)))
|
|
if after - before:
|
|
return True
|
|
# a checksum or integrity that changes without a new version
|
|
if any(re.match(digest, l) for l in hl) and not any(re.match(version, l) for l in hl):
|
|
return True
|
|
return False
|
|
|
|
|
|
def origin_of(url):
|
|
m = re.match(r"^([a-z+]+)://([^/]+)", url)
|
|
return m.group(1) + "://" + m.group(2) if m else url
|
|
|
|
|
|
STEP = re.compile(r"^\s*- (name|uses):")
|
|
VERDICT = re.compile(r"npm ci|npm run build|npm test|cargo check|cargo test|cargo audit|cargo-audit|Verify", re.I)
|
|
|
|
|
|
def regions(base):
|
|
lines = base.split("\n")
|
|
region, top, job_key = {}, None, None
|
|
for i, l in enumerate(lines, 1):
|
|
m = re.match(r"^([A-Za-z_][\w-]*):", l)
|
|
if m:
|
|
top = m.group(1)
|
|
m = re.match(r"^ ([A-Za-z_][\w-]*):", l)
|
|
if m:
|
|
job_key = m.group(1)
|
|
elif re.match(r"^\S", l) or re.match(r"^ \S", l):
|
|
job_key = None
|
|
if top in ("on", "true"):
|
|
region[i] = "on"
|
|
elif top == "permissions" or job_key in ("env", "container", "permissions"):
|
|
region[i] = "jobcfg"
|
|
else:
|
|
region[i] = "other"
|
|
starts = [i for i, l in enumerate(lines, 1) if STEP.match(l)]
|
|
for k, s in enumerate(starts):
|
|
e = starts[k + 1] - 1 if k + 1 < len(starts) else len(lines)
|
|
if VERDICT.search("\n".join(lines[s - 1:e])):
|
|
for i in range(s, e + 1):
|
|
if region[i] == "other":
|
|
region[i] = "verdict"
|
|
return region, lines
|
|
|
|
|
|
def weakening(block, base):
|
|
"""Ways a check-workflow diff could turn a failing check green (empty = none)."""
|
|
region, lines = regions(base)
|
|
reasons, old, plus = set(), 0, []
|
|
|
|
def flush(pos):
|
|
texts = [p[1:] for p in plus]
|
|
active = [t for t in texts if meaningful(t)] # comments never count
|
|
if any(re.search(r"continue-on-error|\|\|\s*true", t) for t in active):
|
|
reasons.add("continue-on-error or || true added")
|
|
if any(re.search(r"GITHUB_ENV|GITHUB_PATH|npm pkg set|git checkout|git restore", t) for t in active):
|
|
reasons.add("writes the job environment or the repository")
|
|
if any(re.search(r"^\s*(- )?uses:", t) and not re.search(r"Swatinem/rust-cache|actions/cache", t) for t in active):
|
|
reasons.add("action added (not a cache)")
|
|
if any(re.search(r"rustup|toolchain|\bPATH\b|/bin/", t) for t in active):
|
|
reasons.add("install step changes the toolchain or the PATH")
|
|
if any(re.match(r"^(\s{4})?(if|defaults):", t) for t in active):
|
|
reasons.add("job-level if/defaults changed")
|
|
first = next((t for t in texts if meaningful(t)), None)
|
|
if first is None or STEP.match(first):
|
|
return
|
|
prev, nxt = region.get(pos - 1, "other"), region.get(pos, "other")
|
|
if "on" in (prev, nxt):
|
|
if any(re.search(r"branches|paths-ignore", t) for t in active):
|
|
reasons.add("trigger narrowed")
|
|
j = pos - 1
|
|
while j > 0 and not re.match(r"^\s*[\w-]+:", lines[j - 1]):
|
|
j -= 1
|
|
if j > 0 and "paths-ignore" in lines[j - 1] and any(re.match(r"^\s*- ", t) for t in active):
|
|
reasons.add("paths-ignore entry added")
|
|
elif prev == nxt == "verdict":
|
|
reasons.add("verdict step modified")
|
|
elif "jobcfg" in (prev, nxt):
|
|
reasons.add("job env/container/permissions modified")
|
|
|
|
for line in block.split("\n"):
|
|
m = re.match(r"^@@ -(\d+)(?:,\d+)? \+\d+(?:,\d+)? @@", line)
|
|
if m:
|
|
flush(old); plus = []
|
|
old = int(m.group(1)); continue
|
|
if line.startswith("+") and not line.startswith("+++"):
|
|
plus.append(line); continue
|
|
flush(old); plus = []
|
|
if line.startswith("-") and not line.startswith("---"):
|
|
if meaningful(line[1:]):
|
|
if re.match(r"^(\s{4})?(if|defaults):", line[1:]):
|
|
reasons.add("job-level if/defaults changed")
|
|
if re.search(r"rustup|toolchain|\bPATH\b|/bin/", line[1:]):
|
|
reasons.add("install step changes the toolchain or the PATH")
|
|
r = region.get(old, "other")
|
|
if r == "on":
|
|
reasons.add("trigger removed or changed")
|
|
elif r == "verdict":
|
|
reasons.add("verdict step modified or removed")
|
|
elif r == "jobcfg":
|
|
reasons.add("job env/container/permissions modified")
|
|
old += 1
|
|
elif line.startswith(" "):
|
|
old += 1
|
|
flush(old)
|
|
return sorted(reasons)
|
|
|
|
|
|
def adds_network_call(path, block):
|
|
if not match(SHIPPED, path) or re.search(r"test", path):
|
|
return False
|
|
for l in block.split("\n"):
|
|
if l.startswith("+") and not l.startswith("+++"):
|
|
if path.endswith(".rs") and "reqwest" in l:
|
|
return True
|
|
if re.search(r"\.(ts|tsx)$", path) and re.search(r"fetch\(\s*[`'\"]https?://", l):
|
|
return True
|
|
return False
|
|
|
|
|
|
def trial_level(paths):
|
|
if any(match(TRIAL_HIGH, p) for p in paths):
|
|
return "haut"
|
|
return "bas" if all(match(TRIAL_LOW, p) for p in paths) else "normal"
|
|
|
|
|
|
def review_level(number, files, diff, merge_base):
|
|
blocks = file_blocks(diff)
|
|
triggers = []
|
|
for f in files:
|
|
b = blocks.get(f["filename"], "")
|
|
for path in filter(None, {f["filename"], f.get("previous_filename")}):
|
|
if match(HIGH_ALWAYS, path):
|
|
triggers.append(path)
|
|
elif path == "src-tauri/src/lib.rs":
|
|
if [l for l in changed(b) if not re.match(r"^[+-]\s*commands::\w+,?\s*$", l)]:
|
|
triggers.append(path)
|
|
elif path == "src-tauri/src/commands/balance_commands.rs":
|
|
hunks = re.split(r"(?m)^(?=@@)", b)[1:]
|
|
if any(re.search(r"fetch_price|activation", h.split("\n")[0]) or
|
|
any(re.search(r"fetch_price|activation", l) for l in changed(h)) for h in hunks):
|
|
triggers.append(path)
|
|
elif match(TAURI_CONFIG, path) or path in ("src-tauri/Cargo.toml", "package.json"):
|
|
if [l for l in changed(b) if not re.match(r'^[+-]\s*"?version"?\s*[:=]', l)]:
|
|
triggers.append(path)
|
|
elif path in ("src-tauri/Cargo.lock", "package-lock.json"):
|
|
if lock_adds_or_removes(path, b):
|
|
triggers.append(path)
|
|
elif path == ".forgejo/workflows/audit.yml" and changed(b):
|
|
triggers.append(path)
|
|
elif path in CHECK_WORKFLOWS:
|
|
wf = path.split("/")[-1]
|
|
base = get(f"/raw/.forgejo/workflows/{wf}?ref={merge_base}", raw=True).decode()
|
|
if weakening(b, base):
|
|
triggers.append(path)
|
|
elif path.startswith(".forgejo/workflows/") and f["status"] == "added":
|
|
triggers.append(path + " (new workflow)")
|
|
elif path == "REVIEW.md":
|
|
triggers.append(path + " (check the « Niveaux » section by hand)")
|
|
if adds_network_call(path, b):
|
|
triggers.append(path + " (network call)")
|
|
added = [l for l in b.split("\n") if l.startswith("+") and not l.startswith("+++")]
|
|
if path.startswith(".forgejo/workflows/") and any(re.search(r"\bsecrets\b|pull_request_target", l) for l in added):
|
|
triggers.append(path + " (secrets or pull_request_target added)")
|
|
if path in CONSENT_FILES and any(re.search(r"consent", l, re.I) for l in changed(b)):
|
|
triggers.append(path + " (consent)")
|
|
parts = path.split("/")
|
|
top_level = len(parts) == 1 or (parts[0] == "src-tauri" and len(parts) == 2)
|
|
new_hidden = f["status"] == "added" and any(
|
|
x.startswith(".") for x in parts[:-1]) and parts[0] not in (".forgejo", ".claude", ".cargo", ".github")
|
|
# Forgejo reports an edited file as "changed"
|
|
if (f["status"] not in ("changed", "modified") and top_level and not path.endswith(".md")) or new_hidden:
|
|
triggers.append(path + " (" + f["status"] + " at top level)")
|
|
paths = [f["filename"] for f in files]
|
|
if triggers:
|
|
return "haut", sorted(set(triggers))
|
|
if all(match(LOW, p) and not match(NEVER_LOW, p) for p in paths):
|
|
return "bas", []
|
|
return "normal", []
|
|
|
|
|
|
CONDITIONAL = {
|
|
"src-tauri/src/lib.rs", "src-tauri/src/commands/balance_commands.rs",
|
|
"src-tauri/Cargo.toml", "package.json", "src-tauri/Cargo.lock", "package-lock.json",
|
|
".forgejo/workflows/**", ".forgejo/workflows/audit.yml",
|
|
".forgejo/workflows/{check-rust,check-frontend}.yml", "REVIEW.md",
|
|
}
|
|
|
|
|
|
def check():
|
|
"""Compare this script's patterns with the first column of the Haut table."""
|
|
text = open(REVIEW_MD, encoding="utf-8").read()
|
|
table = text.split("### Haut", 1)[1].split("\n\n", 2)[1]
|
|
documented = set()
|
|
for row in table.split("\n"):
|
|
if not row.startswith("| ") or row.startswith("| Motif") or row.startswith("| fichier à la racine"):
|
|
continue
|
|
documented.update(re.findall(r"`([^`]+)`", row.split(" | ")[0]))
|
|
coded = set(HIGH_ALWAYS) | set(TAURI_CONFIG) | set(SHIPPED) | CONDITIONAL
|
|
for label, extra in (("REVIEW.md only", documented - coded), ("script only", coded - documented)):
|
|
for pattern in sorted(extra):
|
|
print(f"{label}: {pattern}")
|
|
if documented != coded:
|
|
return 1
|
|
print(f"ok: {len(coded)} patterns, identical to the Haut table of REVIEW.md")
|
|
return 0
|
|
|
|
|
|
def main():
|
|
if sys.argv[1:] == ["--check"]:
|
|
sys.exit(check())
|
|
first, last = (int(sys.argv[1]), int(sys.argv[2])) if len(sys.argv) == 3 else (344, 452)
|
|
merged, page = [], 1
|
|
while True:
|
|
batch = get(f"/pulls?state=closed&limit=50&page={page}")
|
|
if not batch:
|
|
break
|
|
merged += [p for p in batch if p.get("merged") and first <= p["number"] <= last]
|
|
page += 1
|
|
merged.sort(key=lambda p: p["number"])
|
|
rows = []
|
|
for p in merged:
|
|
n = p["number"]
|
|
files, fpage = [], 1
|
|
while True:
|
|
# the instance caps a page at 50 items: read until an empty page
|
|
fb = get(f"/pulls/{n}/files?limit=50&page={fpage}")
|
|
if not fb:
|
|
break
|
|
files += fb
|
|
fpage += 1
|
|
diff = get(f"/pulls/{n}.diff", raw=True).decode("utf-8", "replace")
|
|
merge_base = get(f"/pulls/{n}")["merge_base"]
|
|
trial = trial_level([f["filename"] for f in files])
|
|
level, triggers = review_level(n, files, diff, merge_base)
|
|
rows.append((n, trial, level, triggers))
|
|
print(f"#{n}\t{'ref' if n in REF39 else ' '}\t{trial}\t{level}\t{'; '.join(triggers)}")
|
|
for label, keep in (("39 reference PRs", lambda n: n in REF39), (f"{len(rows)} PRs #{first}-#{last}", lambda n: True)):
|
|
sel = [r for r in rows if keep(r[0])]
|
|
for col, name in ((1, "trial"), (2, "REVIEW.md")):
|
|
counts = {lvl: sum(1 for r in sel if r[col] == lvl) for lvl in ("bas", "normal", "haut")}
|
|
share = 100 * counts["haut"] / len(sel)
|
|
print(f"{label}, {name}: {counts} -> haut {share:.1f} %")
|
|
|
|
|
|
if __name__ == "__main__":
|
|
main()
|