feat(engineering): deep-learning-book — companion skill for the free Goodfellow/Bengio/Courville textbook

[alirezarezvani]

Summary

Requested as "convert https://www.deeplearningbook.org/ into a skill". Shipped as a companion, not a compilation — and the reasoning is the reusable part of this PR.

Why book-to-skill was not used. Its emitter refuses a shareable package without public-domain / open-license / internal-docs / author-permission. None applies to an MIT Press title whose own site states the HTML-only format exists as a friction against copying under the authors' contract, and whose PDF distribution is not permitted. Its rights_and_provenance.md lists "publish a compiled skill of a copyrighted book to a public marketplace" under Do not, and its hard rule 1 forbids scraping a book from the web, so the pipeline could not have run against a URL in any case.

The rule this sets for the repo (recorded in references/rights_and_use.md): when a user asks to convert a copyrighted work into a shareable skill, build a companion that indexes and updates the source, not a compilation that reproduces it. Compile only when the gate clears; keep the output local when it does not.

What shipped. The compiled-skill shape — master SKILL.md (~2.0k tokens, chapter index + topic index), chapters/ch01..ch20, glossary.md, patterns.md, cheatsheet.md — filled with original synthesis. No passages, figures, exercises, or per-paragraph paraphrase; every chapter file links to the official free chapter. The organizing structure is the book's published table of contents.

The differentiator is the delta layer. A compilation freezes a source at its publication date; this one dates it. Every chapter carries a "What changed after 2016" section, and references/book_to_2026_delta.md gives five corrections with primary citations and per-claim confidence levels:

1. Double descent qualifies Ch 5's U-curve — "shrink the model when it overfits" is no longer the only correct move, and capacity_planner.py encodes the correction.
2. AdamW — weight decay ≠ L2 under an adaptive optimizer; Ch 7 treats them as interchangeable.
3. Transformers displaced Ch 10's recurrence — keep its gradient-flow analysis, drop its architecture recommendation.
4. Diffusion grew directly out of Ch 14's denoising autoencoders and Ch 18's score matching.
5. Self-supervised learning vindicated Ch 15's bet while replacing every method it names.

Two commonly repeated claims are marked contested rather than propagated (batch norm's "internal covariate shift" mechanism; the strong lottery-ticket form) and two are named as folklore. Stated rule: the conflict is almost always in the recommendation, not the analysis.

Four stdlib tools, each with a real refusal:

Tool	Refuses
reading_path_planner.py	Exit 3 on a goal the book does not cover (names what does); exit 4 with forcing questions when unroutable
training_diagnostics.py	Exit 4 rather than diagnosing with no instruments; rules fire in priority order so a NaN is never reported as overfitting
capacity_planner.py	Exit 4 on a val-below-train split (leaky measurement); ranks "shrink the model" last when overparameterized
model_arithmetic.py	Exit 5 naming the layer whose shapes do not connect; exit 4 on an unknown layer type

Plus 4 references citing 7–8 sources each, 3 assets, cs-deep-learning-tutor, and /cs:deep-learning · /cs:dl-reading-path · /cs:dl-diagnose.

Note on the diff: this branch was cut from main, which currently sits 3 commits ahead of dev (the linkedin plugin, merged via #991). Those commits ride along in this PR against dev; the work under review here is the single deep-learning-book commit.

Checklist

• Target branch is dev (not main)
• Skill has SKILL.md with valid YAML frontmatter (name, description, license)
• Scripts run with --help without errors — all four also support --sample and --output json
• No hardcoded API keys, tokens, or secrets — no network calls anywhere in the plugin
• No vendor-locked dependencies without open-source fallback — standard library only
• Follows existing directory structure (engineering/deep-learning-book/skills/deep-learning-book/SKILL.md)

Type of Change

• New skill
• Improvement to existing skill
• Bug fix
• Documentation (counters + release notes in README.md, CLAUDE.md, marketplace.json)
• Infrastructure / CI

Testing

Every gate run against the repo's own tooling:

• book_skill_validator.py (from book-to-skill, all four check families) — passed all checks, exit 0
• token_budget_estimator.py --skill-dir — every file inside its cap; resident core 2,010 / 4,000 tokens, 20 chapters averaging ~644 tokens each
• check_plugin_json.py --all — OK engineering/deep-learning-book/.claude-plugin/plugin.json; marketplace description 899 chars (cap 1,024)
• derive_counters.py --check — passed: skills 386→387, tools 723→727, refs 838→842, agents 116→117, commands 146→149, plugins 97→98
• All four tools: --help, --sample, and --sample --output json (valid JSON) verified; typed exit codes confirmed on the refusal paths (3, 4, 5) and the success paths
• model_arithmetic.py arithmetic hand-checked against the sample convnet (conv 3×3×3×32+32 = 896; linear 4096×128+128 = 524,416; MHA 4·768²+4·768 = 2,362,368; LSTM 4·(768·256+256²+2·256) = 1,050,624)
• model_arithmetic.py --spec assets/example_layer_spec.json — runs clean

---

Generated by Claude Code

[claude]

Review: deep-learning-book companion skill

Scope note for other reviewers: per the PR description, this branch was cut from main (which is 3 commits ahead of dev via the already-merged linkedin plugin, #991), so the diff against dev also carries that plugin along. I focused this review on the actual new content — the single deep-learning-book commit — since linkedin was already reviewed/merged separately.

Overall

This is a strong, unusually well-reasoned PR. A few things stood out positively enough to call out explicitly:

• The "companion, not compilation" decision is the right call and well-argued. references/rights_and_use.md walks through the copyright reasoning (idea/expression dichotomy, Baker v. Selden, the publisher's own stated HTML-only friction-against-copying rationale) and correctly identifies that book-to-skill's own rights gate would refuse this as a shareable package. Spot-checking several chapter files (e.g. ch08-optimization.md) confirms the claim: they're genuine original synthesis (frameworks, mental models, anti-patterns, a dated "what changed" section) rather than paraphrased book content, each linking out to the free official chapter.
• The four scripts are the best part of this PR. I traced through model_arithmetic.py, capacity_planner.py, training_diagnostics.py, and reading_path_planner.py in full. They're deterministic, stdlib-only, have real (non-decorative) refusal paths with correctly ordered exit codes, and the arithmetic in model_arithmetic.py (linear/conv2d/mha/lstm-gru param & MAC counts) checks out by hand. training_diagnostics.py's priority-ordered rules (NaN before overfit/underfit) and capacity_planner.py's double-descent-aware regularization ladder are genuinely useful, well-cited pieces of logic, not just prose wrapped in argparse.
• Evidence discipline is real, not just claimed. references/book_to_2026_delta.md cites primary sources per claim and correctly marks contested/folklore items instead of flattening everything to "well-established" (batch norm's "internal covariate shift" mechanism, the strong lottery-ticket form).
• Manifest hygiene is correct: plugin.json's skills array uses the required ./-prefixed canonical form, source/attribution metadata correctly lives in the sidecar authoring-notes.json rather than plugin.json (per this repo's hard rule from the [BUG] 37 plugins fail to install — non-spec source / attribution keys in plugin.json #954 regression), and the marketplace.json entry's counters match the CLAUDE.md/README.md/CHANGELOG.md updates consistently throughout the diff.

Nits / things worth a second look (none blocking)

1. reading_path_planner.py — include_optional naming. The CLI flag is --include-intro but it's threaded through as include_optional internally (plan(goal, background, hours_per_week, include_optional)). Works correctly, just a slightly misleading internal name — include_intro would read clearer given it only ever gates ch01.
2. model_arithmetic.py conv2d "same" padding formula (ceil(h/stride), ceil(w/stride)) matches TensorFlow's SAME convention but not every framework's asymmetric-padding behavior at even kernel sizes — worth a one-line note in the docstring since the tool doesn't currently disclose which convention it's matching, but this is a documentation nit, not a correctness bug for the stated use case.
3. Could not independently run derive_counters.py --check / check_plugin_json.py --all / the script --sample smoke tests in this review environment (sandboxed, network/exec restricted), so I'm relying on static reading plus the PR's own reported output for those. Everything I could statically verify (counter arithmetic across the four docs, manifest schema, script logic) was internally consistent.

Security / dependency check

No network calls, no eval/exec, no subprocess use, no third-party imports anywhere in the four scripts — consistent with the repo's stdlib-only convention. Nothing security-relevant found.

Recommendation: Looks ready to merge on the merits of the deep-learning-book content itself. Nice work on the rights-gate reasoning in particular — it's a good precedent for future "convert this book" requests.

[alirezarezvani]

Both code nits verified against the source and fixed in c75500f.

1. include_optional naming — confirmed: the plan() parameter was include_optional while the flag and call site both used include_intro, and it only ever gates ch01. Renamed the parameter and its use. Behavior unchanged, checked both directions: --include-intro keeps ch01 first, the default drops it.

2. conv2d "same" padding convention — confirmed, and a fair catch: ceil(H / stride) is the TensorFlow/Keras SAME convention and the tool disclosed nothing. Now documented in the module docstring, including that PyTorch's padding='same' is symmetric-only and rejects a stride other than 1, so a strided PyTorch layer will not match this row — with "valid" plus explicit padding named as the exact-case workaround. Added a pointer comment at the computation itself. No arithmetic change; the sample stack reports identical parameters and FLOPs.

3. Gates you couldn't execute — no code change needed, but for the record, all of these are green locally on the current head: compileall, check_paths, check_frontmatter, check_dual_publish, check_model_freshness, smoke_scripts (696 passed / 0 failed), derive_counters --check, check_skill_names, check_plugin_json, book_skill_validator, and --sample --output json on all four tools.

One unrelated thing surfaced while running the suite, left alone deliberately: the advisory smoke_json_output.py gate (G9, continue-on-error) reports 8 failures, all in agent-launcher/ scripts that print non-JSON to stdout under --sample --output json. Pre-existing and outside this diff — worth a separate issue rather than widening this PR. All four tools here pass it.

Note that the first CI run on this branch was red on gate G1 for a real defect of mine (bare references//assets/ links in the agent and command files, which sit outside the skill directory and so resolved against none of the linter's three bases); fixed in cf572c8.

---

Generated by Claude Code

[claude]

Independent review — deep-learning-book companion skill

I re-reviewed with fresh eyes (own read of the four scripts, SKILL.md, rights_and_use.md, path fixes, and manifests) rather than deferring to the earlier automated review already on this thread. Both nits from that review are, in fact, correctly fixed in c75500f:

• reading_path_planner.py: plan()'s parameter is now include_intro throughout (matches --include-intro and its only real effect, gating ch01). Confirmed both branches (--include-intro keeps ch01 first; default drops it for non-complete lanes).
• model_arithmetic.py: the module docstring and the conv2d "same"-padding branch now disclose the TensorFlow/Keras ceil(H/stride) convention and explicitly flag PyTorch's stride-restricted symmetric padding='same' as a mismatch case. Arithmetic itself is untouched, as claimed.

And the path-qualification fix (cf572c8) is complete and correct — I checked every references//assets/ link in agents/cs-deep-learning-tutor.md and the three commands/*.md files; all nine now carry the skills/deep-learning-book/ prefix and all resolve to real files.

What I independently verified (not just re-stated)

• Scripts (model_arithmetic.py, capacity_planner.py, training_diagnostics.py, reading_path_planner.py): read all four in full. Stdlib-only, deterministic, no network/eval/subprocess. Exit-code contracts match their docstrings. training_diagnostics.py's rule ordering genuinely puts non-finite-loss detection (rank 1, short-circuits) ahead of the fit-based rules — a NaN can't be misreported as overfitting, as claimed. reading_path_planner.py's topological sort (order_path) correctly respects PREREQS with ch05/ch11 promoted as tie-breakers; close_prerequisites is a straightforward closure, no cycles possible given the acyclic table.
• model_arithmetic.py hand-check: independently recomputed conv/linear/MHA/LSTM formulas against the code — params and MAC counts for the sample spec match the PR's own hand-check numbers.
• Manifests: plugin.json has no source/attribution extension keys (correctly sidecar'd in authoring-notes.json), uses the canonical ./skills/deep-learning-book array form, and version 2.12.0 matches the repo's current version. marketplace.json's new entry and the counter deltas across README.md/CLAUDE.md (skills 386→387, tools 723→727, refs 838→842, agents 116→117, commands 146→149, plugins 97→98) are internally consistent with the file list actually added (4 scripts, 4 references, 1 agent, 3 commands, 1 skill).
• Rights reasoning: spot-checked ch08-optimization.md and a few others against rights_and_use.md's claim — genuinely original synthesis (frameworks, anti-patterns, dated "what changed" section), no reproduced passages, each linking to the official chapter. The idea/expression argument is correctly grounded (§102(b), Baker v. Selden) and appropriately hedged ("not legal advice").

One very minor, non-blocking observation

reading_path_planner.py's lane scoring (score_lanes) breaks ties alphabetically by lane key when hit counts are equal. The SKILL.md usage example (--goal "train a transformer") hits practitioner on "train" and sequence on "transformer" — one hit each — so it resolves to practitioner, not the arguably more intuitive sequence lane. This is deterministic and documented-enough behavior (not a bug), just worth knowing if that exact example is ever used as a demo in docs — a reader might expect the transformer-focused path.

Conclusion

No new issues found beyond what the earlier review already caught and what's since been fixed. The security/dependency posture is unchanged (no network calls, no third-party imports, stdlib-only). This looks ready to merge.

[alirezarezvani]

Verified the lane tie-break observation and fixed the cause in 1366714.

Reproduced first: "train a transformer" hits practitioner on "train" and sequence on "transformer" — one keyword each — and the alphabetical tie-break picked practitioner, exactly as described.

Rather than swapping the example in SKILL.md, score_lanes() now breaks equal-hit ties on keyword specificity: the lane whose longest matched keyword is longest wins, since a tie between a generic term and a discriminating one shouldn't be settled by luck. Lane key stays the final tie-break so ordering remains deterministic.

Regression battery over eight goals — "train a transformer" now routes to sequence, and the vision, generative, foundations, practitioner, complete, representation and sequence goals all route exactly as before. Refusal paths unchanged (out-of-scope exit 3, unroutable exit 4, sample exit 0).

Gates green after the change: compileall, check_paths, check_frontmatter, check_dual_publish, check_model_freshness, smoke_scripts (696 passed / 0 failed), derive_counters --check, check_skill_names, check_plugin_json, book_skill_validator, and --help + --sample --output json on all four tools.

One related case I looked at and deliberately left alone: "why does my training diverge" routes to practitioner rather than optimization, but that's a hit-count win ("train" and "training" both match), not a tie-break — and the practitioner lane already includes ch08. Changing it would mean reworking the keyword sets, which is beyond what this finding needs.

---

Generated by Claude Code

[claude]

Review: engineering/deep-learning-book

Reviewed the substantive commit (the linkedin/other files riding along are explained in the PR description as already-merged content from main).

Overall this is a well-built skill. The four Python tools are genuinely deterministic, refuse cleanly on bad input with typed exit codes, and the copyright reasoning in references/rights_and_use.md / README.md is careful and specific rather than hand-wavy. The "companion, not compilation" framing and the 2016→2026 delta layer are a real differentiator, not just marketing copy.

Bug: shipped example asset gives a ~512x-wrong parameter count

skills/deep-learning-book/assets/example_layer_spec.json (the "transformer encoder block, 768-dim, 512 tokens" sample referenced in SKILL.md, README.md, and the PR's own testing section) misuses model_arithmetic.py's flatten semantics:

{"type": "mha", "heads": 12, ...}          // shape stays (512, 768)
{"type": "layernorm", ...}                  // shape stays (512, 768)
{"type": "flatten", ...},                   // shape -> (393216,)  <-- collapses the whole sequence
{"type": "linear", "units": 3072, ...}      // treated as one dense layer over all 512 tokens

Because flatten collapses the entire (512, 768) sequence into one 393,216-element vector before the FFN's first linear, that layer's parameter count comes out to 393216*3072+3072 ≈ 1.208B instead of the correct per-token FFN up-projection 768*3072+3072 = 2,362,368 — off by almost exactly 512x (the sequence length). The second linear (down-projection) happens to land on the right shape by coincidence since it consumes the already-1D output. Net effect: the sample's reported total parameter count for the FFN block is ~256x too high.

The tool's own docstring anticipates this trap ("Insert a flatten layer, or use a per-token linear on a 2-D sequence by declaring the shape as [features]") — the shipped example picked the wrong one of those two options for a per-token FFN. --spec assets/example_layer_spec.json "runs clean" (no error), which is why this wasn't caught by the exit-code testing described in the PR — clean exit isn't the same as correct numbers, and this is the flagship example for the arithmetic tool.

Suggest either fixing the spec to model the FFN as a fresh [768]-shaped segment (per-token, no flatten) with a comment about why, or explicitly re-labeling the current spec as "sequence-pooling head" rather than "FFN block" if the flatten was intentional.

Documentation: no CHANGELOG.md entry

CLAUDE.md, README.md, and .claude-plugin/marketplace.json all pick up the new skill and its counter deltas (386→387 skills, etc.), but CHANGELOG.md's [Unreleased] section has no corresponding ### Added — engineering/deep-learning-book entry — only the marketing/linkedin entry (from the separately-merged PR riding along in this diff) is present. Every other recent skill addition in this repo's history gets a CHANGELOG entry alongside the CLAUDE.md one; this one is missing it.

Everything else checked out

• 4 scripts (reading_path_planner.py, training_diagnostics.py, capacity_planner.py, model_arithmetic.py): stdlib-only, no network calls, --help/--sample/--output json all present, typed exit codes match their docstrings, priority-ordered rule firing in training_diagnostics.py is correctly implemented (NaN short-circuits before the fit-gap logic), and the double-descent regime handling in capacity_planner.py correctly reorders the regularization ladder rather than just appending a caveat.
• Copyright posture: the "companion not compilation" argument is specific (idea/expression doctrine, the book's own stated HTML-only rationale, book-to-skill's own rights gate) rather than asserted, and the chapter files genuinely read as original synthesis with no reproduced prose.
• plugin.json: passes the repo's schema (8 required keys only, ./-prefixed skills array, authoring-notes.json correctly used as the sidecar for source/attribution instead of polluting the manifest).
• Security: no secrets, no eval/exec, no subprocess, no external I/O beyond printing a documentation URL string.
• Test coverage: consistent with repo convention (no test framework) — smoke-testable via --sample/--help, which the PR description shows was run for all four tools. The example-asset bug above is exactly the kind of thing that class of testing doesn't catch (exit-code correctness ≠ numeric correctness), so it might be worth having new skills that ship a "reference calculation" asset also include a hand-verified expected output in the PR description, the way the sample convnet was.

Nice piece of work overall — the refusal design (exit 3/4/5 with named alternatives rather than silent guesses) across all four tools is the strongest part of this PR.

[alirezarezvani]

Good catch — this was a real bug, and the diagnosis was exactly right. Fixed in eeb3cb9.

Reproduced first: the shipped asset reported 1,207,962,624 parameters for the FFN up-projection instead of 2,362,368, off by exactly 512× the sequence length, putting the block total at ~1.21B instead of ~7.09M.

Fixed the cause, not just the spec. The root problem was in the tool: mha emits (seq, d_model) but linear refused any 2-D input, so the only way to attach a feedforward block was to flatten — which models a dense layer over the whole flattened sequence, a different layer with seq_len times the parameters. A transformer FFN was therefore not expressible at all, and the example walked straight into the trap the docstring warned about. linear on a 2-D (seq, features) input is now position-wise: one weight matrix shared across positions, parameters independent of sequence length, compute linear in it. The 3-D path still refuses (exit 5) with an updated message pointing at flatten. The asset drops the flatten and its comment now records why it must not come back.

Verified numerically rather than by exit code:

layernorm x2   3,072
mha            2,362,368   = 4*768^2 + 4*768
ffn up         2,362,368   = 768*3072 + 3072
ffn down       2,360,064   = 3072*768 + 768
total          7,087,872   <- matches the tool exactly, and a BERT-base encoder layer

Convnet --sample is unchanged at 545,098; linear on 3-D input still exits 5.

CHANGELOG entry added — correct, it was missing while CLAUDE.md, README.md and marketplace.json all carried the new skill and its counter deltas.

Your closing point is the one I'd most like to keep: exit-code correctness is not numeric correctness, and this slipped through precisely because --spec ran clean. I hand-checked the convnet sample when building the tool but not the transformer asset — which was the one with the subtle semantics. The per-layer arithmetic above is now in the commit message so the expected numbers are reviewable, not just the exit code.

---

Generated by Claude Code

[claude]

Review: PR #994 — engineering/deep-learning-book + marketing/linkedin

I did a full read of every new file plus dispatched several parallel sub-agents to independently trace and hand-verify the ~19 new Python scripts (script execution was blocked in the review sandbox, so all findings below come from careful manual/regex tracing, cross-checked by at least two independent readers per file). Note: gh pr view --files silently truncates at 100 entries — this PR actually touches 142 files, including the entire linkedin-content, linkedin-engagement, linkedin-profile, linkedin-skills, and linkedin-strategy sub-skills that aren't visible in the default file-list view.

Overall: very high quality. Content (chapter synthesis, citations, evidence-graded claims), plugin structure, authoring-notes.json/plugin.json schema compliance, and counter bookkeeping across marketplace.json/CLAUDE.md/README.md/CHANGELOG.md are all excellent and internally self-consistent (verified by hand — the deltas match exactly). CI (Lint/Tests/Docs/Security, VirusTotal) is green. The issues below are concrete, traced bugs worth fixing before merge, not style nits.

Bugs — recommend fixing before merge

1. (High) linkedin_policy_gate.py — two self-contradicting refusal rules block the exact actions their own substitutes recommend.

• P2-SCRAPING's pattern \bexport\b.{0,25}\b(connections?|leads?|profiles?|members?)\b.{0,25}\b(list|database|csv)\b matches ordinary self-data-export phrasing — e.g. "export my connections list as a csv" — and returns REFUSE, even though the same rule's substitute text says: "Use LinkedIn's own export of YOUR data (Settings → Data privacy → Get a copy of your data)." Hand-traced by two independent reviewers; high confidence.
• Similarly, P1-AUTOMATION's \bauto[- ]?(...|post)\w* pattern matches "I'll use LinkedIn's native scheduler to auto-post three times a week" → REFUSE, while P7's substitute explicitly endorses "Native LinkedIn scheduling ... as the supported path."
• Fix direction: exempt "own data" / "native scheduler" phrasing, or require the scraping pattern to co-occur with a third-party/other-member signal.

2. (High) outreach_message_builder.py — the headline "refuses an ask in a first-touch connection note" claim is bypassable.
ASK_RE (call/chat/meeting/demo/coffee language, line 61) is only checked in the non-connection branch (line 144). For --type connection, only the literal --ask field and PITCH_RE (product-pitch language) are checked. Putting "Would love to hop on a 15 min call this week" into --reason or --specific-line instead of --ask produces a clean PASS (exit 0) — the exact behavior the SKILL.md and PR description say is refused.

3. (High) reading_path_planner.py — substring (not word-boundary) matching causes false "out of scope" refusals.
out_of_scope_hits()/score_lanes() use bare token in text. "rag" matches inside "storage", "lora" matches inside "exploratory", "conv" inside "converge"/"convex", "text" inside "context"/"textbook". E.g. --goal "train models for image storage and retrieval" gets wrongly refused as exit-3 out-of-scope, citing RAG. This directly undercuts the tool's stated design goal ("refuses ... rather than guessing"). Fix: \b-anchored regex instead of in.

4. (Medium) model_arithmetic.py — malformed input crashes instead of hitting the documented exit-4 path.
SpecError/ShapeError are the only caught exceptions in main(), but: a non-dict top-level JSON (list/string) makes spec.get("layers") raise AttributeError; a non-dict layers[] entry crashes even the error-message construction; "groups": 0 or "stride": 0 raise uncaught ZeroDivisionError; non-numeric filters/kernel/etc. raise uncaught ValueError. All surface as raw tracebacks, not the documented 4.

5. (Medium) post_performance_analyzer.py — degenerate (zero-IQR) data mislabels every post BREAKOUT.
When engagement rate is identical/rounded across several posts, iqr == 0 → hi_fence == q3 == q1 == median. The band loop checks >= hi_fence first, so every median-value post is classified BREAKOUT instead of TYPICAL — silently wrong, no warning surfaced, undermining the tool's "describe honestly" premise.

6. (Low-Medium) Overly generic keyword substrings inflate scores in two profile tools.

• headline_scorer.py: AUDIENCE_MARKERS includes bare "for "/"to "; OUTCOME_MARKERS includes "from "/"into ". These match inside ordinary prose unrelated to naming an audience/outcome (e.g., any headline containing "excited to share...").
• profile_completeness_auditor.py: OUTCOME_WORDS includes bare "x", "%", "to", "from". "x" matches inside "cross-functional", so the tool's own bundled SAMPLE duty-bullet ("Worked with cross-functional teams") gets incorrectly credited as outcome-carrying — undermining the exact duties-vs-outcomes distinction the check exists to enforce.

Smaller findings (worth a look, not blocking)

• pattern_miner.py: "multiple-comparisons accounting" is descriptive only (reports expected false-positive count) — no Bonferroni/Holm/BH correction is actually applied to the per-candidate threshold. Consider softening the plugin.json/SKILL.md wording so it isn't read as an applied correction.
• pattern_miner.py:183: when the "other" group's median is exactly 0, relative effect is forced to 0.0, masking a possibly-real difference as "too small."
• pattern_miner.py/post_performance_analyzer.py: a bare JSON list of non-dict items (e.g. [1,2,3]) crashes with AttributeError instead of a typed exit code.
• positioning_brief.py: --pillar name:why_you:proof:share splits on bare : with no escaping — free text containing a colon shifts fields. Only affects the raw-CLI-flag path, not --input/--sample.
• cadence_planner.py: when affordable rounds to 0 posts with no --target-posts given, the headline verdict stays FITS/exit 0 (only a "warning" finding notes 0 posts).
• linkedin_policy_gate.py: the P3 lookahead pattern \bpods?\b(?=.{0,30}\b(join|run|group)\b) can't match the common "join a pod" ordering (verb-before-noun) — effectively dead for that phrasing, though the sibling pattern still catches "comment pod"/"engagement pod".
• Several scripts (linkedin_policy_gate.py, linkedin_goal_router.py, headline_scorer.py) use elif args.text: truthiness instead of is not None, so --text "" falls through to a usage error instead of being evaluated.
• No script wraps open(args.input, ...) in try/except — a bad --input path raises a raw FileNotFoundError instead of a typed exit code (JSON decode errors are caught).

What's solid (verified, not just claimed)

• Policy gate adversarial testing: independently re-tested against "auto-DM 500 people," "use PhantomBuster to scrape profiles," and "buy fake engagement" — all three correctly REFUSE (often via 2+ overlapping rules), and a benign control ("write me a post about my product launch") correctly ALLOWs. The self-contradictions above are real but narrow; the gate is not broadly leaky.
• Statistics: pattern_miner.py's permutation test is a legitimate, properly seeded, correctly-implemented permutation test (standard (extreme+1)/(n+1) p-value smoothing). post_performance_analyzer.py's median/MAD/IQR arithmetic is correct and genuinely robust-statistics-based (hand-verified against the sample CSV).
• model_arithmetic.py formulas: conv/linear/MHA/LSTM/GRU parameter and FLOP counts hand-traced and match standard conventions exactly (e.g., LSTM/GRU gate counts correctly mirror PyTorch's dual bias_ih+bias_hh convention).
• Pseudo-bold Unicode detection (post_linter.py): the declared range covers Mathematical Alphanumeric Symbols (U+1D400–U+1D7FF) correctly per block analysis.
• Convention compliance: all ~19 new scripts are stdlib-only, zero eval/exec/subprocess/pickle/shell-injection surface, every script has --help/--sample/--output json with typed exit codes, comments are sparse and WHY-focused per repo convention.
• Content quality: deep-learning-book's chapter synthesis is technically accurate and well-cited; the rights_and_use.md copyright reasoning (idea/expression distinction, Baker v. Selden, Authors Guild v. Google) is thoughtful and legally sound. The linkedin plugin's evidence discipline is a standout — two commonly-repeated claims (personalized-note acceptance-rate myth, in-body-link reach-penalty attribution) are explicitly corrected rather than propagated, each tagged with a confidence level.
• Bookkeeping: plugin.json files use the canonical "skills": ["./..."] array form; authoring-notes.json files correctly stick to the source/attribution two-key schema; marketplace.json/CLAUDE.md/README.md/CHANGELOG.md counter deltas are internally consistent and add up exactly.

Nice work overall — the core designs (permutation testing, prerequisite-closure planning, points-per-hour fix ranking, policy-gate-before-drafting) are sound; the bugs found are all fixable without redesign.

[alirezarezvani]

Thorough review — thank you. Findings 3 and 4 are against this PR's own code and are both fixed in e923237. The rest are against marketing/linkedin, which this branch carries but does not touch; details on that below.

Fixed (findings 3 and 4)

3 — substring matching in reading_path_planner.py. Reproduced exactly: --goal "train models for image storage and retrieval" exited 3, confidently refused as out-of-scope RAG work ("rag" inside sto·rag·e), and "an exploratory look at optimization" exited 3 citing LoRA ("lora" inside exp·lora·tory). "conv" matched converge and "text" matched context. This was the worst class of bug for this particular tool — its stated design is to refuse rather than guess, and it was guessing 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 bare \b match would miss (fine-tuning, prompting, agentic). Verified after: both goals above route correctly, "converge" reaches the optimization lane rather than vision, and every real refusal still refuses — RLHF, LoRA fine-tuning, RAG pipelines, prompt/agent goals all still exit 3.

4 — malformed input crashing past exit 4 in model_arithmetic.py. Reproduced all five cases you named; each produced a traceback with exit 1. Numeric fields now go through a checked accessor rejecting non-integer and non-positive values, the input layer's shape is validated, and the top-level spec plus every layer entry are type-checked:

non-dict top-level JSON  exit 4  spec must be a JSON object, got list
non-dict layer entry     exit 4  layer 0 is str, not an object
"stride": 0              exit 4  layer 1 (conv2d): 'stride' must be positive, got 0
"groups": 0              exit 4  layer 1 (conv2d): 'groups' must be positive, got 0
"filters": "abc"         exit 4  layer 1 (conv2d): 'filters' must be an integer, got 'abc'

Regression battery over eight goals routes exactly as before; convnet sample still 545,098 parameters, transformer asset still 7,087,872. Full gate suite green.

Not fixed here — and why

Findings 1, 2, 5, 6 and most of the smaller items are in marketing/linkedin/. That plugin was reviewed and merged separately in #991; it appears in this diff only because this branch was cut from main, which sits ahead of dev. Fixing them here would put substantive changes to another plugin inside a PR about deep-learning-book, where they'd get no focused review and would be invisible in the PR title.

They look like real bugs from your traces, and two are worth prioritizing — the P2-SCRAPING rule refusing the very self-export its own substitute recommends, and outreach_message_builder.py's ask-detection being bypassable via --reason, since that one contradicts a headline claim in the plugin's own description. I'd suggest a separate issue against marketing/linkedin; I'm happy to take it as its own PR if that's wanted.

Also worth recording: your note that gh pr view --files truncates at 100 entries while this PR touches 142 files is a good catch about the review surface itself, independent of any finding.

---

Generated by Claude Code

[claude]

Review

Solid piece of work — the rights-gate reasoning for going "companion, not compilation" is well-argued and consistent with book-to-skill's own hard rules, and the four stdlib tools each have a real, documented refusal path rather than a decorative one. The commit history shows this PR already went through several rounds of self-review that caught and fixed genuine bugs (position-wise linear off by 512x, alphabetical tie-break beating keyword specificity, substring-match false positives on "rag"/"lora"/"conv", untyped numeric inputs escaping as tracebacks) — that discipline shows in the current state of the code.

One remaining gap I found in the same category as the "typed input validation" fix:

model_arithmetic.py — embedding layer's seq_len bypasses the typed-validation convention the file otherwise enforces uniformly

Every other numeric spec field (vocab, dim, units, heads, filters, kernel, stride, groups) is read through _positive_int(), which rejects non-numeric and non-positive values with a SpecError (documented exit 4). The embedding branch is the one exception:

if kind == "embedding":
    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 spec like {"type": "embedding", "vocab": 100, "dim": 8, "seq_len": "ten"} hits int("ten"), which raises a bare ValueError that isn't caught by either except ShapeError or except SpecError in main() — it escapes as an uncaught traceback with Python's default exit code 1, not the documented exit 4 ("the spec could not be parsed"). It also doesn't reject a non-positive seq_len (0 or a negative int would silently produce a degenerate shape instead of a clear error).

Suggested fix: route it through the same helper, e.g. seq = _positive_int(layer, "seq_len", index, default=shape[0] if shape else 1), preserving the existing default-fallback behavior.

Everything else

• capacity_planner.py and training_diagnostics.py — logic traced through by hand (regime classification, ladder ordering, priority-ordered rule firing, the leaky-split guard) and didn't turn up anything I could break; the double-descent caveat and NaN-before-everything-else ordering are both correctly gated.
• reading_path_planner.py — the close_prerequisites/order_path topological sort is correctly acyclic over the PREREQS table, and the promoted-chapter tie-break (ch05/ch11) never violates a hard prerequisite, just reorders among already-ready chapters. The word-boundary _matches() fix looks right for the cases named in the commit message.
• No network calls, no non-stdlib imports, no hardcoded secrets — consistent with the repo's stdlib-only convention.
• Docs/counters/CHANGELOG entries line up with the stated deltas (386→387 skills etc.), and the path-qualification fix (fix(deep-learning-book): qualify reference and asset links...) correctly addresses the CI check_paths.py gate.

Nothing here blocks merging — the embedding.seq_len gap is a minor, low-likelihood-to-hit edge case (most specs won't hand-author a seq_len at all, since it defaults sensibly), but it's a one-line inconsistency worth closing given how deliberately every sibling field in this same function was hardened against exactly this failure mode.

[alirezarezvani]

Correct on both counts, and it's the one field I missed when hardening the others in e923237. Fixed in 8f833d2.

Reproduced before fixing, and the second case is slightly worse than described:

seq_len "ten"   ->  uncaught ValueError, traceback, exit 1   (documented: exit 4)
seq_len 0       ->  no error at all: reported a plausible "0x8" layer with zero
                    activation elements and exited 0

So the non-positive case wasn't merely unvalidated — it presented a degenerate model as a valid one, which for an arithmetic tool is the worse of the two failures.

Routed through _positive_int with the existing default, preserving behavior: both now exit 4 naming layer and field, the default fallback still resolves to the incoming sequence length (16x8), and an explicit valid seq_len is still honoured (32x8).

Since the finding was really "one field escaped a convention the rest of the file enforces," I swept the file rather than patching the single instance: all 13 numeric spec fields now go through _positive_int, and no raw int() on a spec field remains. Reference specs unchanged — convnet 545,098 parameters, transformer asset 7,087,872. Full gate suite green.

Worth recording for anyone reading this thread later: this is the second bug found in the exact seam where a fix was applied — the 512× parameter error was in the tool's flagship example, and this one was in the field the hardening pass skipped. Both were in model_arithmetic.py, and both slipped past because the spec exited 0. Clean exit still isn't correct numbers.

---

Generated by Claude Code

[claude]

Review

Reviewed the engineering/deep-learning-book/ plugin, which is the actual scope of this PR (the marketing/linkedin/ and .codex/.gemini/.hermes/.vibe/ files are carried over unchanged from main via the merge noted in the PR description, and aren't touched here).

Context noted: the commit history shows this PR already went through five rounds of review with real fixes applied — a parameter-name mismatch, SAME-padding disclosure, keyword-tie-breaking by specificity, a 512x transformer-FFN parameter bug (flatten collapsing position-wise linear), substring-vs-word-boundary keyword matching, and embedding.seq_len validation. I read the current state of the four scripts and SKILL.md against that history rather than re-deriving the same findings.

Code quality — scripts/model_arithmetic.py

Solid. _positive_int now covers all 13 numeric spec fields consistently, SpecError/ShapeError are cleanly separated with the documented exit codes (4 vs 5), and the position-wise-linear fix is correct — hand-checked the transformer FFN math (2·(2·768) + (4·768²+4·768) + (768·3072+3072) + (3072·768+768) = 7,087,872) and it matches what's in assets/example_layer_spec.json. The SAME-padding docstring disclosure (TF/Keras convention, PyTorch caveat) is a good catch from an earlier round — this kind of framework-convention ambiguity is exactly the sort of thing that silently produces wrong numbers downstream.

Code quality — scripts/reading_path_planner.py

The word-boundary matching (_matches) and the specificity-based tie-break in score_lanes are both correct — verified "train a transformer" now routes to sequence (11-char "transformer" beats 5-char "train") as SKILL.md's own example claims. One thing worth a sentence in a docstring rather than a fix: --include-intro only has an observable effect on the complete lane, since ch01 is never a prerequisite of anything else in PREREQS, so for every other lane the flag is silently a no-op. That's presumably intentional (ch01 is genuinely only "context" for the full read), but a first-time user of a non-complete lane passing --include-intro and seeing no change might reasonably think it's broken. Not a bug, just a possible UX surprise.

Code quality — capacity_planner.py / training_diagnostics.py

Both are clean, deterministic, and the priority-ordered rule firing (numerics → bug-check → gradient → fit verdict) is implemented as documented. Verified the --sample fixtures by hand: capacity planner's overparameterized sample (ratio 240) correctly lands OVERFIT with smaller-model pushed to the end of the ladder; training diagnostics' NaN/bug/gradient rules return early or short-circuit correctly so a NaN can never be misreported as overfitting.

Security / secrets

No network calls, no subprocess/eval/exec, no hardcoded credentials anywhere in the four scripts — consistent with the "standard library only" claim in the docstrings.

Plugin/manifest compliance

plugin.json uses the canonical "skills": ["./skills/deep-learning-book"] form, has no extension fields (source/attribution correctly live in the sidecar authoring-notes.json, not plugin.json), and the marketplace description is under the stated cap. Cross-checked that all references//assets/ links from agents/ and commands/ are qualified with skills/deep-learning-book/ (the earlier CI path-resolution fix) — none of the un-qualified broken-link pattern remains.

Rights/attribution reasoning (references/rights_and_use.md)

This is the most consequential part of the PR's design decision and it holds up: the "companion, not compilation" framing is well-argued (idea/expression distinction, correctly hedged with "Not legal advice," and it's honest that book-to-skill's own gate would refuse this as a shareable compilation). No concerns here.

Test coverage

No test framework is used, per repo convention — the PR's own testing section (validator gates, hand-checked arithmetic, --help/--sample/--sample --output json on all four tools) substitutes for that, and the fix commits show those gates were actually exercised (e.g., the 512x FFN bug was only caught by hand-checking the arithmetic, not by the --sample exit-code smoke test — worth remembering for future numeric tools in this repo: a clean exit code is not proof of a correct number).

Overall

No remaining correctness issues found in the four scripts or the manifest/rights files. Given the extensive fix history already on this branch, I'd call this ready to merge from a code standpoint, modulo the minor --include-intro UX note above (non-blocking).

[alirezarezvani]

Checked the --include-intro note rather than just documenting it, and it was worse than reported: the flag was inert on every lane, complete included — not merely limited to complete. Fixed in 98019ac.

Two causes compounded. ch01 is not a prerequisite of any chapter, so it never arrived through closure; and complete, the one lane that targets it, skipped the ch01 filter entirely. So the filter removed a chapter that was never present, and the flag controlling it could not change any output:

read the whole book              --include-intro -> NO CHANGE
convolutional models             --include-intro -> NO CHANGE
generative models and diffusion  --include-intro -> NO CHANGE
learn the math foundations       --include-intro -> NO CHANGE

Inverted the logic so the flag adds ch01 rather than un-removing it. It now has a real effect on every lane, complete stays unchanged (ch01 is already among its targets), and the help text says so. Verified per lane: complete unchanged; vision, generative and foundations each gain ch01 with the flag and are untouched without it; ch01 sorts first and never precedes a chapter it would violate. Routing and both refusal paths unchanged.

Worth recording, since it bears on how much the earlier rounds actually verified: my own check of the parameter rename in c75500f was inconclusive. It compared the complete lane with the flag against the vision lane without it — two paths that differ for reasons unrelated to the flag. It looked like a passing before/after test and could not have failed. Had I varied only the flag on one lane, the dead code would have surfaced four rounds earlier.

That's the third distinct instance in this PR of the same underlying mistake: a check that ran clean without actually exercising the thing it claimed to cover (--spec exiting 0 on wrong arithmetic, --sample exiting 0 on an unvalidated field, and now a before/after comparison with two variables moving). Cheap rule for next time: change one variable, and confirm the test fails when the fix is removed.

---

Generated by Claude Code

[claude]

Review: engineering/deep-learning-book

Scoped this review to the actual new work in the branch — the deep-learning-book skill commit. Per the PR description, the marketing/linkedin files are carried along from main (already merged via #991) and untouched here, so I left those out. Note also that this PR's commit history already shows six rounds of self-directed adversarial review with real fixes applied (keyword substring matching, tie-break ordering, a 512x parameter-count bug in the flagship example, dead --include-intro flag, SAME-padding disclosure, typed input validation). I re-derived/re-checked the fixed logic from scratch rather than trusting the commit messages, and it holds up.

What I verified

• model_arithmetic.py — hand-recomputed the transformer block in assets/example_layer_spec.json: MHA 4·768² + 4·768 = 2,362,368, FFN up 768·3072+3072 = 2,362,368, FFN down 3072·768+768 = 2,360,064, 2×LayerNorm 3,072 → total 7,087,872, matching the claimed figure. The position-wise linear fix (no flatten before the FFN blocks) is correctly reflected in the asset — the comment explicitly warns against reintroducing the flatten.
• _positive_int is now used consistently across all 13 numeric spec fields (confirmed no bare int() remains on a spec field), so malformed/zero/negative values exit 4 with a named layer+field rather than a traceback.
• reading_path_planner.py — _matches() is word-boundary anchored via regex with a small SURFACE_FORMS table for irregular inflections; confirmed this fixes the substring false-positives described in the commits ("rag" in "storage", etc.) without over-matching. Tie-break in score_lanes() now uses longest-matched-keyword-length before falling back to lane key, which correctly routes "train a transformer" to sequence per the worked example in SKILL.md.
• Path hygiene — grepped agents/ and commands/ for references//assets/ links; all nine are correctly prefixed with skills/deep-learning-book/, consistent with the check_paths.py fix commit.
• plugin.json — canonical "skills": ["./skills/deep-learning-book"] array form, matches the CLAUDE.md-documented CC 2.1.144+ requirement.
• Spot-read a chapter file (ch08-optimization.md) against the copyright constraint the PR foregrounds — it's original synthesis with a source-chapter link, not paraphrase, consistent with the stated "companion not compilation" rule.

Minor observations (not blocking)

• capacity_planner.py regime boundaries (ratio < 1.0 / ratio < 10.0) are clearly documented as heuristic bands rather than a threshold theorem, so this is fine as shipped — just flagging that the exact boundary values (UNDERPARAMETERIZED_MAX, OVERPARAMETERIZED_MIN) aren't exposed as CLI flags the way the other scripts' thresholds are, in case a future user wants to override them.
• CHANGELOG.md has two ## [Unreleased] headers (line 8 and line ~1739). The second is pre-existing changelog debt from long before this PR and this PR doesn't touch that section, so not something to fix here — just noting it in case a changelog-render step ever gets confused by it.

Overall

Code quality is high: every tool has typed, tested exit codes (2/3/4/5 as documented), stdlib-only with no network calls, and the refusal paths are exercised rather than decorative (e.g., model_arithmetic.py genuinely refuses shape-incompatible stacks, reading_path_planner.py genuinely distinguishes "out of scope" from "unroutable"). The prerequisite-closure algorithm in reading_path_planner.py (close_prerequisites + order_path) is a clean, provably-terminating deterministic topological sort over a small hardcoded DAG — no cycle risk given the fixed PREREQS table. No security concerns (no eval, no dynamic imports, no external calls). No test suite, but that's consistent with this repo's documented "no build system or test frameworks" convention — the --sample/--output json self-test pattern the PR author ran is the repo's substitute, and it's exercised correctly here.

Nice work — this is a well-scoped, thoroughly self-reviewed addition. Nothing here blocks merge from my read.

[alirezarezvani]

Both observations checked. One closed in 5f5d84f, one confirmed out of scope.

Regime bands now overridable. You called it fine as shipped, and it was, but the inconsistency is small enough to close rather than carry — and this particular band matters more than a typical default, since it decides whether smaller-model gets ranked last, which is the entire double-descent correction the tool exists to apply. A user whose interpolation point sits elsewhere had no way to say so.

Added --underparameterized-max and --overparameterized-min, threaded through plan() / classify_regime() as keyword args defaulting to the existing constants, and all three thresholds are now reported in --output json the way training_diagnostics.py already does.

Verified defaults preserve behavior exactly — the --sample fixture still reports overparameterized / OVERFIT / 240.0 with smaller-model last, byte-identical to before — and that an override genuinely moves the regime (--overparameterized-min 500 reclassifies the same model as near-interpolation). Exit codes unchanged: 1 action, 0 balanced, 4 leaky split.

Duplicate ## [Unreleased] header — confirmed pre-existing, left alone. Checked rather than assumed:

count at merge-base with main : 2
count at HEAD                 : 2

So this branch didn't introduce it, and my entry went under the first header. Agreed it's changelog debt worth a separate cleanup — flagging it here so it doesn't get lost.

For the record on this thread: that's seven review rounds, six of which surfaced something real in this plugin's tooling — every one in code I had reported as verified. The content and rights reasoning have gone unchallenged throughout; the defects clustered entirely in the four scripts. Worth weighing against any "ready to merge" verdict, including the ones already given.

---

Generated by Claude Code

[claude]

Review: deep-learning-book skill (PR #994)

Note up front: the diff against dev is large (13k+ additions) but per the PR description that's because this branch was cut from main, which carries 3 already-merged dev-bound commits (the linkedin plugin, PR #991) along for the ride. I focused this review on the actual new work — engineering/deep-learning-book/ — and spot-checked the marketplace/CLAUDE.md/README bookkeeping. I did not re-review the linkedin plugin files since they're already merged elsewhere.

Overall
This is a strong, carefully engineered skill. The rights reasoning (companion vs. compilation, citing book-to-skill's own rights gate as the reason it wasn't used) is exactly the kind of judgment call CLAUDE.md's ClawHub/plugin conventions want to see, and it's documented in three places (README, authoring-notes.json, references/rights_and_use.md) consistently. Spot-checking chapters/ch01-introduction.md and the sample layer spec, the "original synthesis, not reproduction" claim holds up — no passages or paraphrase, just genuinely useful mental-model writing with a "what changed after 2016" section pinned to real citations (Hestness/Kaplan/Hoffmann for scaling laws, Belkin/Nakkiran for double descent, etc.).

Code quality — the 4 scripts (read in full)
All four (reading_path_planner.py, training_diagnostics.py, capacity_planner.py, model_arithmetic.py) are stdlib-only, have typed exit codes, --help/--sample/--output json, and handle edge cases carefully (e.g. model_arithmetic.py's _positive_int guards against the non-numeric/zero-divisor tracebacks the commit history shows were previously bugs; capacity_planner.py refuses a leaky val<train split rather than silently producing a plan). Hand-checked the MHA and LSTM parameter formulas in model_arithmetic.py against the PR's worked numbers (2,362,368 and 1,050,624 respectively) — they're correct, including the position-wise-linear fix that avoids the 512x parameter inflation bug mentioned in the commit log.

One minor correctness nit, low severity: reading_path_planner.py's _matches() word-boundary regex is rf"\b{re.escape(form)}(?:s|es)?\b". This correctly fixes the substring false-positives called out in the docstring ("rag" inside "storage", "text" inside "context"), but the naive (?:s|es)? suffix can still collide with unrelated real words: "rag" + "es" matches the standalone word "rages" (e.g. a goal like "why overfitting still rages in large models" would spuriously trigger the RLHF/DPO/RAG-adjacent OUT_OF_SCOPE path for the wrong reason). Same mechanism could in principle hit other 3-letter keyword+"es" collisions. It's an edge case, not a crash, and no keyword in the current table has an obviously common +es collision besides rag→rages. Worth a follow-up if it's cheap (e.g. restricting the auto-plural to s only and adding irregular forms to SURFACE_FORMS explicitly, the way fine-tun/prompt/agent already are), but I wouldn't block on it.

Plugin/manifest compliance (per this repo's CLAUDE.md)

• plugin.json — no source/attribution keys (correctly sidecar'd into authoring-notes.json), "skills": ["./skills/deep-learning-book"] matches the canonical form and the exact precedent set by engineering/book-to-skill's own manifest. Version 2.12.0 matches the current in-flight release version, consistent with other recent single-plugin additions (agent-memory, skill-doctor, memory-engineering all did the same).
• marketplace.json entry looks consistent with the plugin.json description (trimmed slightly for length, same content).
• SKILL.md frontmatter has name, description (with clear "Use when..." trigger language), license. Structure (chapter index + topic index + supporting files + tools table + scope/limits) matches the write-a-skill pattern used elsewhere in the repo.

I wasn't able to actually execute scripts/check_plugin_json.py --all, derive_counters.py --check, or the four new scripts myself in this review environment (script execution was gated), so I'm relying on manual inspection rather than independently reproducing the PR body's test-run claims — the code itself matches what those claims describe, but flagging that this is inspection, not execution, on my end.

Nothing else flagged
No security concerns (no network calls, no eval/exec, no subprocess, no untrusted deserialization — model_arithmetic.py's json.load on a user-supplied --spec path is the only file I/O and it's wrapped in proper OSError/JSONDecodeError handling with typed exit codes). No secrets. No test-framework or ML/LLM-call anti-pattern violations.

Nice work — the "companion, not compilation" framing plus the delta layer is a genuinely useful pattern this repo didn't have before, and the repeated fix commits in the history (word-boundary matching, position-wise-linear correction, embedding seq_len validation) show real iteration rather than a first-draft dump.

[alirezarezvani]

Real, reproduced, and fixed in 35f5b3b. --goal "why overfitting still rages in large models" exited 3 — refused as out-of-scope retrieval-augmented-generation work, exactly as you predicted.

Before fixing I checked whether the -es branch earns its keep at all, and it doesn't: every keyword in the table ending in s, x, z, ch or sh is already plural or non-count (basics, features, foundations, prerequisites, statistics, speech, mlops, fairness, mathematics, time series). Nothing needs -es appended, so I narrowed the auto-plural to plain -s rather than special-casing rag, and documented that irregular forms belong in SURFACE_FORMS spelled out — the way fine-tuning, prompting and agentic already are. That closes the class instead of the instance.

Verified both directions: "rages" no longer matches while standalone "rag" still does; transformers, embeddings, autoencoders, agentic still match; the goal above is now correctly unroutable (exit 4) rather than confidently misrouted. RLHF, LoRA fine-tuning, RAG pipelines and prompt/agent goals all still exit 3; routing unchanged across the battery.

The pattern worth naming: this is the second round of false out-of-scope refusals from the same root cause — approximate string matching standing in for a word list. Round 4 caught "rag" inside storage; the -es suffix I added to fix it reintroduced a narrower version of the same failure. The refusal path is the part of this tool a user is least able to second-guess — it states a confident conclusion and stops — so it's the wrong place to approximate. Worth remembering if these keyword tables grow.

Also noted and appreciated: you were explicit that your check was inspection rather than execution, since script execution was gated in your environment. That's useful calibration — the last three findings across reviewers all came from reading the code, not running it, and this one was reproducible in one command.

---

Generated by Claude Code

[claude]

Review: PR #994 — engineering/deep-learning-book (+ carries marketing/linkedin from #991)

Reviewed the current state of the code (not just the diff), independently verifying the fixes described in the branch's 8 prior review-fix commits rather than taking the commit messages at face value.

Verification of the 8 previously-claimed fixes — all CONFIRMED

1. Position-wise linear layer (512× param bug) — model_arithmetic.py now computes params for a (seq, features) 2-D input independent of seq_len. Hand-verified the transformer FFN block in assets/example_layer_spec.json sums to 7,087,872 (MHA 2,362,368 + 2×LN 1,536 + FFN-up 2,362,368 + FFN-down 2,360,064), matching the claimed corrected figure exactly, and no stray flatten layer remains between attention and FFN.
2. Word-boundary keyword matching — reading_path_planner.py's _matches() uses re.search(rf"\b{re.escape(form)}s?\b", text); confirmed conv no longer matches inside converge, text no longer matches inside context.
3. _positive_int validation — all 12 numeric spec fields in model_arithmetic.py (units, filters, kernel, stride×2, groups, size, vocab, dim, seq_len, heads) route through the checked accessor; no remaining bare int() on user-controlled spec values.
4. Tie-breaking by keyword specificity — score_lanes() sorts by (-hits, -max_keyword_length, lane_key); "train a transformer" now correctly routes to sequence over practitioner.
5. --include-intro flag — now additive (adds ch01 to the closure) rather than the earlier no-op removal-from-empty-set bug.
6. SAME-padding convention disclosure — documented in the module docstring and inline at the conv2d branch, including the PyTorch symmetric-only caveat.
7. capacity_planner.py regime-band CLI flags — --underparameterized-max/--overparameterized-min thread through plan()/classify_regime() end-to-end; defaults preserve prior behavior.
8. Missing CHANGELOG entry — present at the top of CHANGELOG.md with counters consistent with the diff.

New finding (minor, non-blocking)

• capacity_planner.py — no cross-validation between --underparameterized-max and --overparameterized-min. If a user passes an inverted/overlapping pair (e.g. --underparameterized-max 20 --overparameterized-min 5 with params/examples=10), the tool silently classifies the regime via whichever branch is checked first rather than erroring. Given both flags are new, low-traffic, and default to sane values, this is a nit — worth a cheap parser.error() guard on the two values in a follow-up rather than blocking this PR.

Other checks

• Exit-code discipline across all 4 new scripts matches their documented codes; malformed model_arithmetic.py input (non-dict layer, stride 0, groups 0, non-numeric filters, bad seq_len) consistently exits 4 via SpecError rather than an uncaught traceback.
• training_diagnostics.py — the non-finite-loss rule returns first, so a NaN/inf loss structurally can never be misreported as overfitting.
• Stdlib-only / no LLM calls — confirmed for all 4 deep-learning-book scripts and, on a sanity pass, all 17 marketing/linkedin scripts (no eval/exec/shell=True/os.system anywhere).
• plugin.json schema — no source/attribution extension keys, skills array uses the required ./-prefixed relative form.
• Docs/links — SKILL.md, README, agent, and command files resolve correctly relative to their own directories; the earlier references//assets/ link-prefix bug (agents/commands live one level above skills/deep-learning-book/) is fixed across all 4 previously-affected files.
• Rights framing — the PR's stated rationale for shipping a companion (original synthesis + delta-from-2016 layer + links to official free chapters) rather than a compilation, gated on the repo's own book-to-skill rights policy, is sound and well-documented in references/rights_and_use.md.

Verdict

Mergeable as-is. This branch already absorbed 8 rounds of adversarial review with real bugs fixed each round (the 512× parameter-count bug and the substring-based false-refusal bugs were genuine, high-value catches), and every one of those fixes holds up under independent re-verification. The single new finding is low-severity and doesn't need to block merge.

Nice work running this through repeated self-review before opening for external review — the commit trail made this an easy PR to verify rather than just trust.

[alirezarezvani]

Reproduced and fixed in 942e771 — and the consequence is sharper than "silently classifies via whichever branch is checked first."

With --underparameterized-max 20 --overparameterized-min 5 and a ratio of 10, the tool reported a model ten times overparameterized as underparameterized and exited 0. That verdict ranks smaller-model first rather than last — the precise inversion of the double-descent correction this tool exists to apply. So the bad output isn't a mislabel, it's the opposite recommendation on the one axis the tool was built to get right.

Added an argparse guard rejecting under-max >= over-min with both values named (exit 2, the documented usage-error code). Equal bands are rejected too, since they leave near-interpolation unreachable. Verified: inverted and equal both exit 2; a valid override still moves the regime (--overparameterized-min 500 → near-interpolation); defaults unchanged at overparameterized / OVERFIT / 240.0 with smaller-model last; other exit codes still 1 / 0 / 4. Full gate suite green.

Worth recording about these two flags specifically: they were added to close a consistency nit, and introduced this defect in doing so. A new option is new surface, and its interaction with the options already there is part of that surface — which I didn't check when adding them. That's the second time in this PR a fix has created the next finding (the -es plural did the same thing to the substring fix).

Also appreciated that you re-derived the eight prior fixes independently instead of trusting the commit messages. Given that six of the nine rounds found something real in code I'd already reported as verified, re-checking rather than trusting was the right call.

---

Generated by Claude Code

[claude]

test

[claude]

Review: deep-learning-book companion skill

I reviewed the actual new content of this PR — engineering/deep-learning-book/ (SKILL.md, 4 scripts, 4 references, 20 chapter files, glossary/patterns/cheatsheet, agent, commands, plugin manifests) — and read the four Python tools end-to-end plus spot-checked several chapters/references. The other files in the diff (marketing/linkedin/*, .codex/.gemini/.hermes/.vibe index updates) are base-branch drift from main being ahead of dev, as the PR description explains, and not new work in this PR.

Overall
High quality, and the standout part is the rights reasoning, not the code. references/rights_and_use.md + authoring-notes.json walk through why book-to-skill's own pipeline/rights-gate was correctly NOT used for a copyrighted, redistribution-restricted MIT Press title, and that reasoning is written up as a reusable rule for the repo (companion, not compilation, when the rights gate doesn't clear). That's exactly the kind of judgment call CLAUDE.md's ClawHub constraints exist to force, and it's handled better here than a generic policy statement would.

Scripts (scripts/*.py)
All four are careful, deterministic, stdlib-only, and clearly already through several review rounds (the commit history shows fixes for zero/negative-int guards, word-boundary keyword matching, plural false-positives like rag→rages, tie-break determinism, overlapping-band validation, position-wise-vs-flatten linear-layer semantics). Read closely and didn't find a live bug:

• model_arithmetic.py — shape propagation, params/MACs formulas (conv, linear incl. the position-wise-vs-flatten distinction, MHA's 4×d_model² projections, LSTM/GRU gate counts) all check out. The TF-SAME-vs-PyTorch-SAME padding caveat in the docstring is a correct catch that similar tools often get wrong.
• training_diagnostics.py — rule-priority ordering (NaN before overfitting) is enforced correctly; thresholds are all overridable and echoed in --output json.
• capacity_planner.py — the --underparameterized-max >= --overparameterized-min guard against inverted bands is a real defect it's preventing, not decoration.
• reading_path_planner.py — prerequisite closure/topological order and out-of-scope routing logic are sound. Minor, non-blocking observation: the boundary-inclusion rule for the params-per-example regime bands and the smaller-model demotion logic live only in code comments/module docstring, not in --help text itself — a user running --help alone won't see it.

I read rather than re-executed the scripts in this sandbox (tool execution was restricted here), but the logic traced cleanly by hand against every documented exit code, and the PR's own testing notes (hand-checked arithmetic, --help/--sample/--output json on all four) look sufficiently rigorous given the fix-commit history already exercised these paths.

Content quality
Spot-checked ch08-optimization.md, cheatsheet.md, and the delta reference — original synthesis, no reproduced prose, technically accurate, and appropriately dated (confidence labels used consistently, e.g. correctly flagging batch norm's 'internal covariate shift' explanation as contested rather than settled).

Manifests
plugin.json / authoring-notes.json follow the canonical "skills": ["./skills/deep-learning-book"] form and the source/attribution-in-sidecar convention from CLAUDE.md correctly — no stray keys that would trip check_plugin_json.py. marketplace.json entry is consistent with the plugin manifest.

Minor nit: plugin.json version is 2.12.0 (repo release version at merge time), while other recently-added single-skill plugins use 1.0.0 for their own version (e.g. book-to-skill) or the repo version at their own merge point (e.g. agent-memory at 2.11.2). Not wrong, just an inconsistent convention across plugins — not blocking.

Process note
mergeStateStatus is currently UNSTABLE on the PR — worth confirming CI is green (or just pending) before merge, independent of the code review above.

Security / test coverage
No network calls, no secrets, stdlib-only — consistent with repo conventions. No test framework in this repo by design; the PR's manual verification notes are the appropriate substitute here.

Nice work — this is a good template for how to handle 'convert a copyrighted book into a skill' requests going forward.

[alirezarezvani]

Checked both observations. Neither warrants a change, and here's the evidence for each rather than just a judgment call.

1. Band rules only in comments, not --help. Partly already covered, and misattributed — the regime bands and smaller-model demotion live in capacity_planner.py, not reading_path_planner.py. That file's --help does explain both bands:

--underparameterized-max   params-per-example below which the classical U-curve
                           applies directly (default: 1.0)
--overparameterized-min    params-per-example above which double descent makes
                           'shrink the model' unreliable (default: 10.0). These are
                           heuristic bands, not a threshold theorem — the
                           interpolation point depends on task, architecture and
                           label noise.

What --help doesn't spell out is boundary inclusivity (< vs <=). That's real but marginal: --output json echoes the exact thresholds used alongside the resulting regime, so the mapping is observable. Not worth resetting a green CI run for.

2. plugin.json version 2.12.0. The premise doesn't hold up against the tree. Counting every plugin manifest:

version	plugins	e.g.
2.9.0	58	chief-ai-officer-advisor, …
2.11.2	8	agent-memory, memory-engineering, skill-doctor
1.0.0	6	agent-harness, book-to-skill, collab-proof
2.10.3	4	zero-hallucination-coder, roast
2.12.0	2	deep-learning-book, linkedin

Repo-release-version is the dominant convention by roughly 12:1, and 2.12.0 matches the current in-flight release and the most recent additions. book-to-skill at 1.0.0 is one of six exceptions, not the norm. Changing mine would move it away from the majority.

3. CI status. UNSTABLE was a pending check, not a failure. Now resolved — verified against the run's own head_sha rather than the check listing, since a green result described a stale commit seven times earlier in this thread:

CI Quality Gate run #1305 → head_sha 942e771 → success
claude-review, VirusTotal, Detect changed skills → success
Tessl, Security audit → skipped by design

All seven checks green on the current head.

Ten rounds in, this is the first with nothing to fix — so I'm stopping here rather than making changes that don't earn their CI cycle. Still an open draft; the marketing/linkedin findings from round four remain for a separate PR.

---

Generated by Claude Code