mirror of
https://github.com/alirezarezvani/claude-skills.git
synced 2026-08-28 04:24:58 +00:00
fix(deep-learning-book): word-boundary keyword matching and typed input validation
Fourth review on PR #994 raised two findings against this plugin's scripts. Both reproduced before fixing; the review's other findings are against marketing/linkedin, which this branch carries from main but does not touch (see the PR comment). 1. reading_path_planner.py matched keywords by bare substring, so "rag" matched inside "storage", "lora" inside "exploratory", "conv" inside "converge" and "text" inside "context". Confirmed: --goal "train models for image storage and retrieval" exited 3, confidently refused as out-of-scope RAG work, and "an exploratory look at optimization" exited 3 citing LoRA. A tool whose stated design is to refuse rather than guess was guessing, and doing it with certainty. Matching is now word-boundary anchored with an optional plural, plus an explicit surface-form table for the few tokens whose inflections a word-boundary match would otherwise miss (fine-tuning, prompting, agentic). Verified: both goals above now route correctly (exit 0 / the optimization lane), "converge" reaches the optimization lane rather than vision, and the real refusals still refuse — RLHF, LoRA fine-tuning, RAG pipelines and prompt/agent goals all still exit 3. 2. model_arithmetic.py documented exit 4 for a spec it cannot parse but only caught SpecError and ShapeError, so malformed input escaped as a traceback with exit 1. Confirmed across five cases: a non-dict top-level JSON, a non-dict layer entry, stride 0, groups 0, and a non-numeric filters value. Numeric fields now go through a checked accessor that rejects non-integer and non-positive values, the input layer's shape is validated, and the top-level spec and every layer entry are type-checked. All five now exit 4 with a message naming the layer and field. Regression battery over eight goals routes exactly as before; the convnet sample still reports 545,098 parameters and the transformer asset 7,087,872. Gates green: compileall, check_paths, check_frontmatter, check_dual_publish, check_model_freshness, smoke_scripts (696 passed), derive_counters --check, check_skill_names, check_plugin_json, book_skill_validator, and --help + --sample --output json on all four tools. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BswsZp5zrJWFAGU6KWNA1s
This commit is contained in:
Claude
parent
eeb3cb9ad6
commit
e923237360
2 changed files with 81 additions and 14 deletions
|
|
@ -57,11 +57,37 @@ class ShapeError(ValueError):
|
|||
|
||||
|
||||
def _require(layer: dict, key: str, index: int):
|
||||
if not isinstance(layer, dict):
|
||||
raise SpecError(f"layer {index} is {type(layer).__name__}, not an object")
|
||||
if key not in layer:
|
||||
raise SpecError(f"layer {index} ({layer.get('type', '?')}) is missing '{key}'")
|
||||
return layer[key]
|
||||
|
||||
|
||||
def _positive_int(layer: dict, key: str, index: int, default: int | None = None) -> int:
|
||||
"""Read an integer field, rejecting non-numeric and non-positive values.
|
||||
|
||||
Without this, a non-numeric "filters" raised ValueError and a zero "stride" or
|
||||
"groups" raised ZeroDivisionError — both escaping as tracebacks rather than the
|
||||
documented exit 4.
|
||||
"""
|
||||
raw = layer.get(key, default) if isinstance(layer, dict) else default
|
||||
if raw is None:
|
||||
raise SpecError(f"layer {index} ({layer.get('type', '?')}) is missing '{key}'")
|
||||
try:
|
||||
value = int(raw)
|
||||
except (TypeError, ValueError):
|
||||
raise SpecError(
|
||||
f"layer {index} ({layer.get('type', '?')}): '{key}' must be an integer, "
|
||||
f"got {raw!r}"
|
||||
) from None
|
||||
if value <= 0:
|
||||
raise SpecError(
|
||||
f"layer {index} ({layer.get('type', '?')}): '{key}' must be positive, got {value}"
|
||||
)
|
||||
return value
|
||||
|
||||
|
||||
def _prod(shape: tuple[int, ...]) -> int:
|
||||
total = 1
|
||||
for dim in shape:
|
||||
|
|
@ -74,10 +100,24 @@ def step(layer: dict, shape: tuple[int, ...], index: int) -> tuple[tuple[int, ..
|
|||
kind = _require(layer, "type", index)
|
||||
|
||||
if kind == "input":
|
||||
return tuple(_require(layer, "shape", index)), 0, 0
|
||||
raw_shape = _require(layer, "shape", index)
|
||||
if not isinstance(raw_shape, (list, tuple)) or not raw_shape:
|
||||
raise SpecError(f"layer {index} (input): 'shape' must be a non-empty list")
|
||||
dims = []
|
||||
for dim in raw_shape:
|
||||
try:
|
||||
dim = int(dim)
|
||||
except (TypeError, ValueError):
|
||||
raise SpecError(
|
||||
f"layer {index} (input): shape entries must be integers, got {dim!r}"
|
||||
) from None
|
||||
if dim <= 0:
|
||||
raise SpecError(f"layer {index} (input): shape entries must be positive")
|
||||
dims.append(dim)
|
||||
return tuple(dims), 0, 0
|
||||
|
||||
if kind == "linear":
|
||||
units = int(_require(layer, "units", index))
|
||||
units = _positive_int(layer, "units", index)
|
||||
bias = bool(layer.get("bias", True))
|
||||
if len(shape) == 2:
|
||||
# Per-token (position-wise) linear over a (seq, features) sequence: one
|
||||
|
|
@ -97,9 +137,9 @@ def step(layer: dict, shape: tuple[int, ...], index: int) -> tuple[tuple[int, ..
|
|||
return (units,), params, shape[0] * units
|
||||
|
||||
if kind == "conv2d":
|
||||
filters = int(_require(layer, "filters", index))
|
||||
kernel = int(_require(layer, "kernel", index))
|
||||
stride = int(layer.get("stride", 1))
|
||||
filters = _positive_int(layer, "filters", index)
|
||||
kernel = _positive_int(layer, "kernel", index)
|
||||
stride = _positive_int(layer, "stride", index, 1)
|
||||
padding = layer.get("padding", "same")
|
||||
if len(shape) != 3:
|
||||
raise ShapeError(
|
||||
|
|
@ -121,7 +161,7 @@ def step(layer: dict, shape: tuple[int, ...], index: int) -> tuple[tuple[int, ..
|
|||
f"reduces {height}x{width} to {out_h}x{out_w} — the kernel is larger "
|
||||
"than the feature map."
|
||||
)
|
||||
groups = int(layer.get("groups", 1))
|
||||
groups = _positive_int(layer, "groups", index, 1)
|
||||
if channels % groups or filters % groups:
|
||||
raise SpecError(
|
||||
f"layer {index} (conv2d): groups={groups} does not divide "
|
||||
|
|
@ -133,8 +173,8 @@ def step(layer: dict, shape: tuple[int, ...], index: int) -> tuple[tuple[int, ..
|
|||
return (filters, out_h, out_w), params, macs
|
||||
|
||||
if kind == "pool2d":
|
||||
size = int(layer.get("size", 2))
|
||||
stride = int(layer.get("stride", size))
|
||||
size = _positive_int(layer, "size", index, 2)
|
||||
stride = _positive_int(layer, "stride", index, size)
|
||||
if len(shape) != 3:
|
||||
raise ShapeError(f"layer {index} (pool2d) needs a 3-D input, got {shape}")
|
||||
channels, height, width = shape
|
||||
|
|
@ -150,8 +190,8 @@ def step(layer: dict, shape: tuple[int, ...], index: int) -> tuple[tuple[int, ..
|
|||
return (_prod(shape),), 0, 0
|
||||
|
||||
if kind == "embedding":
|
||||
vocab = int(_require(layer, "vocab", index))
|
||||
dim = int(_require(layer, "dim", index))
|
||||
vocab = _positive_int(layer, "vocab", index)
|
||||
dim = _positive_int(layer, "dim", index)
|
||||
seq = int(layer.get("seq_len", shape[0] if shape else 1))
|
||||
return (seq, dim), vocab * dim, 0 # a lookup, not a matmul
|
||||
|
||||
|
|
@ -169,7 +209,7 @@ def step(layer: dict, shape: tuple[int, ...], index: int) -> tuple[tuple[int, ..
|
|||
f"layer {index} (mha) needs a 2-D input (seq_len, d_model), got {shape}"
|
||||
)
|
||||
seq, d_model = shape
|
||||
heads = int(layer.get("heads", 8))
|
||||
heads = _positive_int(layer, "heads", index, 8)
|
||||
if d_model % heads:
|
||||
raise SpecError(
|
||||
f"layer {index} (mha): d_model={d_model} is not divisible by heads={heads}"
|
||||
|
|
@ -187,7 +227,7 @@ def step(layer: dict, shape: tuple[int, ...], index: int) -> tuple[tuple[int, ..
|
|||
f"layer {index} ({kind}) needs a 2-D input (seq_len, features), got {shape}"
|
||||
)
|
||||
seq, features = shape
|
||||
units = int(_require(layer, "units", index))
|
||||
units = _positive_int(layer, "units", index)
|
||||
gates = 4 if kind == "lstm" else 3
|
||||
params = gates * (features * units + units * units + 2 * units)
|
||||
macs = seq * gates * (features * units + units * units)
|
||||
|
|
@ -198,9 +238,14 @@ def step(layer: dict, shape: tuple[int, ...], index: int) -> tuple[tuple[int, ..
|
|||
|
||||
|
||||
def analyse(spec: dict, dtype: str, convention: str) -> dict:
|
||||
if not isinstance(spec, dict):
|
||||
raise SpecError(f"spec must be a JSON object, got {type(spec).__name__}")
|
||||
layers = spec.get("layers")
|
||||
if not isinstance(layers, list) or not layers:
|
||||
raise SpecError("spec must contain a non-empty 'layers' list")
|
||||
for index, layer in enumerate(layers):
|
||||
if not isinstance(layer, dict):
|
||||
raise SpecError(f"layer {index} is {type(layer).__name__}, not an object")
|
||||
if layers[0].get("type") != "input":
|
||||
raise SpecError("the first layer must be of type 'input'")
|
||||
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ from __future__ import annotations
|
|||
|
||||
import argparse
|
||||
import json
|
||||
import re
|
||||
import sys
|
||||
|
||||
# --------------------------------------------------------------------------- data
|
||||
|
|
@ -186,6 +187,27 @@ OUT_OF_SCOPE: dict[str, str] = {
|
|||
"reinforcement": "Reinforcement learning — mentioned only in passing (ch12).",
|
||||
}
|
||||
|
||||
# Tokens whose real surface forms a word-boundary match would otherwise miss.
|
||||
# Everything else matches itself, optionally pluralized.
|
||||
SURFACE_FORMS: dict[str, tuple[str, ...]] = {
|
||||
"fine-tun": ("fine-tuning", "fine-tune", "fine-tuned", "finetuning", "finetune"),
|
||||
"prompt": ("prompt", "prompting", "prompts"),
|
||||
"agent": ("agent", "agents", "agentic"),
|
||||
}
|
||||
|
||||
|
||||
def _matches(token: str, text: str) -> bool:
|
||||
"""True when token appears in text as a whole word (optionally pluralized).
|
||||
|
||||
Substring matching is wrong here and was a real defect: "rag" appears inside
|
||||
"storage", "lora" inside "exploratory", "conv" inside "converge", and "text"
|
||||
inside "context" — each one producing a confident false refusal or a wrong lane.
|
||||
"""
|
||||
for form in SURFACE_FORMS.get(token, (token,)):
|
||||
if re.search(rf"\b{re.escape(form)}(?:s|es)?\b", text):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- logic
|
||||
|
||||
|
|
@ -237,7 +259,7 @@ def score_lanes(goal: str) -> list[tuple[str, int]]:
|
|||
text = goal.lower()
|
||||
scored = []
|
||||
for key, lane in LANES.items():
|
||||
matched = [kw for kw in lane["keywords"] if kw in text]
|
||||
matched = [kw for kw in lane["keywords"] if _matches(kw, text)]
|
||||
if matched:
|
||||
scored.append((key, len(matched), max(len(kw) for kw in matched)))
|
||||
scored.sort(key=lambda row: (-row[1], -row[2], row[0]))
|
||||
|
|
@ -246,7 +268,7 @@ def score_lanes(goal: str) -> list[tuple[str, int]]:
|
|||
|
||||
def out_of_scope_hits(goal: str) -> list[str]:
|
||||
text = goal.lower()
|
||||
return [note for token, note in OUT_OF_SCOPE.items() if token in text]
|
||||
return [note for token, note in OUT_OF_SCOPE.items() if _matches(token, text)]
|
||||
|
||||
|
||||
def plan(goal: str, background: str, hours_per_week: float,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue