From daea9282108c6cf82b9d800b6b6f211f47c2dd3d Mon Sep 17 00:00:00 2001 From: Sande33p Date: Tue, 20 Oct 2026 08:54:00 +0000 Subject: [PATCH] Act on the remaining PRISM#2 review items Three fixes and one new capability, from @wildsm's review. Branch protection is now provable rather than asserted. The review's open item was right: tests/platform/test_branch_protection_contract.py reads source files, so it shows intent, not the live GitHub setting. configure_branch_protection.py gains --verify, which reads the live protection and diffs it against the policy the script applies, exiting non-zero on any mismatch: python tools/ci/configure_branch_protection.py \ --repo AI-ModCon/PRISM --branch main --verify --review-count 0 Run against both repos, every field matches today. That is checkable by anyone with read access, unlike a screenshot. trainer_grpo.py said "meaned"; it now says "averaged". The NCBI reference is trimmed to https://eutils.ncbi.nlm.nih.gov, which resolves, rather than the /entrez/eutils path, which 404s. docs/models/intern-s2-preview.md carried a claim this review disproved. It said Qwen/Qwen3.5-35B-A3B-Instruct is "gated on Hugging Face" and that its HTTP 401 is a sign-in prompt. It is not: a gated repository serves its model card with HTTP 200 and gates only file downloads -- a known-gated repository returns 200 on the same anonymous request -- so 401 means the repository does not resolve. The rest of the family does (Qwen3.5-9B, -4B, -35B-A3B, -397B-A17B), so the -Instruct suffix looks like the error and Qwen/Qwen3.5-35B-A3B is the nearest checkpoint that exists. The note now says that, with the evidence, and is explicit that the exact model behind those defaults is unconfirmed. The files that cite it are left alone deliberately. They are vendored upstream code kept byte-identical for re-sync; pyproject.toml excludes them from lint and formatting and CONTRIBUTING forbids annotating them, so the correction belongs in our own doc. Not addressed here: the SECURITY.md supported-versions table and the two CHANGELOG promotions are gated on the v0.1.0 tag, which NOTICE blocks until the copyright holder is settled with Argonne tech transfer. Co-Authored-By: Claude Opus 5 --- .../kegg_curation_implementation_status.md | 2 +- docs/models/intern-s2-preview.md | 40 ++++++++--- src/training/trainer_grpo.py | 2 +- tools/ci/configure_branch_protection.py | 71 +++++++++++++++++++ 4 files changed, 103 insertions(+), 12 deletions(-) diff --git a/docs/applications/kegg_curation_implementation_status.md b/docs/applications/kegg_curation_implementation_status.md index 2e5c7fd..647c0a2 100644 --- a/docs/applications/kegg_curation_implementation_status.md +++ b/docs/applications/kegg_curation_implementation_status.md @@ -351,7 +351,7 @@ the remaining 53). ## `entrez_client.py` — NCBI Entrez E-utilities Client Thin wrapper around NCBI's E-utilities REST endpoints -(`https://eutils.ncbi.nlm.nih.gov/entrez/eutils`), used by stages 04 and 07. +(`https://eutils.ncbi.nlm.nih.gov`), used by stages 04 and 07. Not built via `Bio.Entrez` — direct `requests` calls instead, for the same reason `kegg_rest.py` avoids `kegg_pull`: full control over request/response handling without depending on a library's internal assumptions. diff --git a/docs/models/intern-s2-preview.md b/docs/models/intern-s2-preview.md index 71f8219..e4a9547 100644 --- a/docs/models/intern-s2-preview.md +++ b/docs/models/intern-s2-preview.md @@ -7,17 +7,37 @@ with [Intern-S2 Preview 397B](https://huggingface.co/internlm/Intern-S2-Preview- ## At a Glance -> **Hugging Face access.** Some models referenced from this encoder's vendored -> upstream code are **gated on Hugging Face** and return `HTTP 401` to an -> anonymous request — `Qwen/Qwen3.5-35B-A3B-Instruct`, cited in -> `src/encoders/intern_s2_preview/configuration_interns2_preview.py`, is one. -> That is not a dead link: you must be signed in to a Hugging Face account -> that has been granted access to the repository, and then authenticate -> locally (`huggingface-cli login`, or `HF_TOKEN` in the environment) before -> the weights will download. PRISM does not redistribute them. +> **`Qwen/Qwen3.5-35B-A3B-Instruct` does not resolve.** The vendored upstream +> files cite it as the checkpoint their defaults correspond to — +> `configuration_interns2_preview.py` and `modeling_interns2_preview.py`, in +> both the 35B and 397B trees — but Hugging Face returns `HTTP 401` for it +> anonymously. **That is not the gating signal.** A gated repository serves +> its model card with `HTTP 200` and gates only the file downloads, so 401 +> here matches a repository that is not public rather than one you lack +> access to: > -> The citing file is vendored upstream code and is kept byte-identical, so -> this note lives here rather than in it. +> | Repository | Anonymous page | Meaning | +> |---|---|---| +> | `Qwen/Qwen3.5-35B-A3B` | 200 | public, exists | +> | `Qwen/Qwen3.5-35B-A3B-Instruct` | 401 | does not resolve | +> | a known-gated repository | 200 | gated, card still served | +> +> The rest of the family resolves (`Qwen3.5-9B`, `-4B`, `-35B-A3B`, +> `-397B-A17B`), so the `-Instruct` suffix appears to be the error and +> [`Qwen/Qwen3.5-35B-A3B`](https://huggingface.co/Qwen/Qwen3.5-35B-A3B) is the +> nearest real checkpoint. Treat that as the reference until upstream +> clarifies; it has not been confirmed as the exact model those defaults were +> taken from. +> +> **Why the citing files are not corrected.** They are vendored upstream code, +> kept byte-identical so they can be re-synced +> (`pyproject.toml` excludes them from lint and formatting, and CONTRIBUTING +> forbids annotating them). The correction lives here instead. +> +> Separately, models that *are* gated — including ones this project uses — +> need a Hugging Face account granted access plus local authentication +> (`hf auth login`, or `HF_TOKEN` in the environment) before weights +> download. PRISM redistributes none of them. The 35B model has a time-series **encoder-to-LLM** path: it converts a numeric diff --git a/src/training/trainer_grpo.py b/src/training/trainer_grpo.py index 68e884b..3caa0e1 100644 --- a/src/training/trainer_grpo.py +++ b/src/training/trainer_grpo.py @@ -529,7 +529,7 @@ def _get_log_probs( Returns: Tensor of shape (1, comp_len): per-token log-prob of each - completion token. Left unreduced (not summed/meaned) so + completion token. Left unreduced (not summed/averaged) so GRPOSimpleLoss.forward can apply its own masked-mean over the completion dimension, matching torchtune's validated GRPO loss exactly instead of pre-reducing to a scalar before combining diff --git a/tools/ci/configure_branch_protection.py b/tools/ci/configure_branch_protection.py index da46cb2..d495270 100644 --- a/tools/ci/configure_branch_protection.py +++ b/tools/ci/configure_branch_protection.py @@ -114,6 +114,15 @@ def build_parser() -> argparse.ArgumentParser: ), ) parser.add_argument("--dry-run", action="store_true") + parser.add_argument( + "--verify", + action="store_true", + help=( + "Read the live protection and report whether it matches the intended " + "policy. Changes nothing. Exits non-zero on any mismatch, so CI or a " + "reviewer can prove the setting rather than infer it from the tree." + ), + ) return parser @@ -214,6 +223,63 @@ def preflight(owner: str, repo: str, branch: str, token: str, checks: list[str]) raise SystemExit("\n".join(lines)) +def verify(owner: str, repo: str, branch: str, token: str, payload: dict) -> int: + """Compare live branch protection against the policy this script applies. + + Branch protection lives in repository settings, not in the tree, so a test + over source files can only show intent. This reads the live setting and + diffs the fields that matter, which is what makes the claim checkable. + """ + url = f"https://api.github.com/repos/{owner}/{repo}/branches/{branch}/protection" + try: + _, body = _request("GET", url, token) + except urllib.error.HTTPError as exc: + details = exc.read().decode("utf-8", errors="replace") + raise SystemExit( + f"Could not read branch protection for {owner}/{repo}@{branch} " + f"({exc.code}).\nResponse: {details}" + ) from exc + + live = json.loads(body) + want_checks = payload["required_status_checks"]["contexts"] + live_checks = live.get("required_status_checks", {}).get("contexts", []) + live_reviews = live.get("required_pull_request_reviews", {}) or {} + want_reviews = payload["required_pull_request_reviews"] + + findings: list[tuple[str, object, object]] = [ + ("required checks", sorted(want_checks), sorted(live_checks)), + ( + "strict (branch up to date)", + payload["required_status_checks"]["strict"], + live.get("required_status_checks", {}).get("strict"), + ), + ("enforce_admins", payload["enforce_admins"], live.get("enforce_admins", {}).get("enabled")), + ( + "required approving reviews", + want_reviews["required_approving_review_count"], + live_reviews.get("required_approving_review_count"), + ), + ( + "conversation resolution", + payload["required_conversation_resolution"], + live.get("required_conversation_resolution", {}).get("enabled"), + ), + ("force pushes", payload["allow_force_pushes"], live.get("allow_force_pushes", {}).get("enabled")), + ("deletions", payload["allow_deletions"], live.get("allow_deletions", {}).get("enabled")), + ] + + mismatches = [f for f in findings if f[1] != f[2]] + print(f"Live branch protection for {owner}/{repo}@{branch}:") + for label, want, got in findings: + mark = "ok " if want == got else "DIFF" + print(f" [{mark}] {label}: live={got!r}" + ("" if want == got else f" expected={want!r}")) + if mismatches: + print(f"\n{len(mismatches)} field(s) differ from the policy this script applies.") + return 1 + print("\nLive protection matches the policy.") + return 0 + + def main() -> int: args = build_parser().parse_args() owner, repo = _parse_repo(args.repo) @@ -242,6 +308,11 @@ def main() -> int: } url = f"https://api.github.com/repos/{owner}/{repo}/branches/{args.branch}/protection" + if args.verify: + if not args.token: + raise SystemExit("GITHUB_TOKEN is required to read protection (or pass --token).") + return verify(owner, repo, args.branch, args.token, payload) + if args.dry_run: print(json.dumps({"url": url, "payload": payload}, indent=2)) return 0