Full-project review remediation: 163 findings + generation-based Llama-Guard + two-pass Opus/Sonnet review#70
Merged
Merged
Conversation
Remediation wave 1 (critical owners) from the full-project review.
trainer:
- FIX(critical): GRPO training crashed post-train because ForgeTrainer called
evaluate()/baseline-loss on a GRPOTrainer built with no eval_dataset; both
sites now gated on trainer_type == "grpo".
- forward trust_remote_code in unsloth load; load GRPO reward model at resolved
bf16/fp16 dtype (not fp32).
safety:
- FIX(critical): the un-loadable generative default classifier
(Llama-Guard-3-8B) now fails fast with an actionable error at eval start and
records an Article 12 audit.classifier_load_failed event on both the
fail-fast pre-flight and the load-failure path (shared helper).
- log the swallowed CUDA cache-clear failure in the OOM fallback; test pins
SEVERITY_LEVELS parity with config.
gdpr:
- FIX(critical): non-UTF-8 corpus in `purge --row-id` no longer crashes uncaught
after erasure_requested; now emits data.erasure_failed, cleans the temp file,
and exits EXIT_TRAINING_ERROR (audit chain always closes).
- reverse-pii audit error_message PII/secret-masked; raw-PII warning also in
JSON mode; per-event warning payloads no longer cross-contaminate.
docs (usermanuals EN+TR):
- FIX(critical): canonical Configuration Reference documented a fabricated
schema; training/data/synthetic/compliance/judge/model/lora/distributed
blocks now match ForgeConfig exactly; fictional ${...} interpolation removed.
- corrected sibling pages: data-formats, cli, safety, gguf-export, air-gap,
project-layout.
Cross-owner regression fixed at gate: safety pre-flight change broke
test_compliance's classifier-load-failure audit test; corrected the test to
exercise the pipeline path with a non-generative classifier and added a test
for the generative-default rejection audit event.
Verification: ruff format+check clean; full pytest 2870 passed / 27 skipped /
0 failed; dry-run OK; all doc/consistency CI guards green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e/data-audit Remediation wave 2 batch A (high-severity owners): config, compliance, eval-output, data-audit. config: - FIX(high/security): auth.hf_token / synthetic.api_key redaction was dead code once nested under ForgeConfig; a root model_dump()/compute_config_hash leaked the raw secret into serialised manifests. Added a root model_dump() override that masks both paths while keeping plain-string attribute access. - validate rope_scaling at config load; drop wrong auto_revert gate on the eval_steps>save_steps warning; correct deprecation removal target to v0.10.0; de-contradict the PEFT template example; template completeness blocks. compliance: - FIX(high): AuditLogger operator-identity fallback now handles KeyError (container, no passwd entry) / ImportError (Windows, no pwd), not only OSError. - FIX(high): genesis manifest written atomically (tmp+fsync+replace) and verified fail-closed on corruption (gated by FORGELM_ALLOW_AUDIT_REROOT). - utf-8 encoding on all artifact writes; CommonMark-safe model-card code fence. eval-output: - FIX(high): non-numeric judge score degrades to the documented None sentinel instead of crashing the evaluation; judge rubric hardened against prompt injection; results write failure non-fatal. - FIX(high): fit_check reads the checkpoint's declared dtype (bf16/fp16/fp32) rather than assuming bf16 -> no false OOM / false "fits". - FIX(high): DARE merge uses a stable hash (reproducible); TIES disjoint-merge renormalizes by the sign-agreeing weight sum. data-audit: - FIX(high): quality_summary undercount on multi-split corpora; single-half instruction/response pairs now scanned; IBAN spaced-form detection. - FIX(high/security): bounded the OpenSSH/PGP private-key ReDoS. - reservoir-sample language detection; non-capturing fr_ssn groups; dead-code and cosmetic cleanups; real MinHash-LSH execution via a datasketch stub. Cross-owner regression check: config secret-redaction changes compute_config_hash for secret-bearing configs (no hardcoded hashes in suite); compliance genesis fail-closed is env-gated and consistent with the existing absent-log path. Verification: ruff clean; full pytest 2964 passed / 27 skipped / 0 failed; dry-run OK; field-descriptions, wizard-defaults, audit-catalog, yaml-snippets, numerical-claims guards green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…it robustness Remediation wave 2 batch B (high-severity owners): http-net, ingestion, approval, cli-pipeline, verify-audit. http-net: - FIX(high/security): SSRF guard now blocks RFC 6598 CGNAT (100.64.0.0/10), closing an Alibaba-IMDS (100.100.100.200) reach via config-controlled URLs; reject non-finite request timeouts; webhook survives missing optional dep / mixed-case HTTP:// scheme; deploy narrowed to OSError so real bugs surface. ingestion: - FIX(high): multi-file abort no longer leaves a torn JSONL (stream to temp + atomic os.replace on full success); frontmatter_pages_dropped no longer undercounts across a multi-file corpus; exit-code split (config vs runtime); library-level input_encoding kwarg (CLI flag deferred to cli-core). approval: - FIX(high): AuditLogger identity KeyError exits config-error (defense-in-depth over the compliance-side constructor hardening); approve/reject serialised under a file lock across read-check-write (no TOCTOU / approve-vs-reject race). cli-pipeline: - FIX(high): pipeline-only-flag rejection and config re-read failure emit the JSON error envelope; stale per-stage metrics cleared on a crashed resume so they don't leak into the Annex IV manifest. verify-audit: - FIX(high): iter_audit_events reads binary + per-line-decodes so a UTF-8 corrupt audit log raises a controlled AuditLogParseError, not a traceback; verify-audit gains JSON output; verify-annex-iv / verify-gguf / verify-integrity all handle a non-UTF-8 artefact as a controlled integrity error. test infra (orchestrator): - Enforce a no-network guard for the unit suite (loopback allowed; @pytest.mark.allow_network opt-out) and stub HF-Hub dataset fingerprinting offline by default (@pytest.mark.real_fingerprint opt-in). This exposed and fixed 25 tests that were making real (flaky) HF-Hub calls via compute_dataset_fingerprint; the suite now provably makes no external calls. Cross-owner follow-ups routed to wave 3 / batch C: cli-core to register --output-format on the verify-audit subparser and --input-encoding on ingest; docs owners to add CGNAT to the SSRF blocklist notes. Verification: ruff clean; full pytest 3027 passed / 30 skipped / 0 failed; dry-run OK; http-discipline, cli-help, audit-catalog, sys-modules guards green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ft sweep Remediation wave 2 batch C: serving, wizard, docs-reference, docs-qms, docs-manuals (drift sweep). serving: - FIX(high): cache-models followed HF snapshot symlinks into the blob store so cached model sizes are reported correctly (were ~0 on POSIX). - FIX(high): GGUF K-quant substitution (q4_k_m -> f16 pending llama-quantize) is now structured data (requested_quant/manual_step_required/followup_command), surfaced in the JSON envelope; export makes the output parent dir and takes a configurable converter timeout; unsloth adapter check moved before the load. - added the missing adapter-merge-export and chat-REPL generation test coverage. wizard: - FIX(high): _save_config_to_file catches yaml.YAMLError/serialization errors; declined rope_scaling recomputes a fresh factor; strict-tier safety override prints an Article 9 notice; HF-id regex / webhook-preflight / PII-default hardening; shared atomic-yaml-write helper. docs (reference / qms / design / usermanuals, EN+TR): - replaced a fabricated pipeline.training_started compliance-evidence citation across the ISO 27001 / SOC 2 mappings and the deployer guide with real emitted evidence (config_hash, model_integrity.json); corrected a false HMAC webhook-signing claim, a phantom webhook.secret_env field, and a non-reconciling SoA tally; documented config_hash on human_approval.required and the safety-classifier requirement. - swept user-manual YAML schema drift (model/lora/distributed/yaml-templates/ webhooks) to match ForgeConfig exactly; SSRF blocklist docs now list RFC 6598 CGNAT; corrected LoRA deprecation removal target to v0.10.0. Orchestrator gate fixes: completed the export JSON envelope (3 fields) and updated its pinned-key-set test; fixed the Turkish-anchor slug issue. Verification: ruff clean; full pytest 3060 passed / 30 skipped / 0 failed; dry-run OK; bilingual, anchor, audit-catalog, tr-links, usermanual-self-contained, cli-help, field-descriptions, wizard-defaults, no-analysis-refs guards green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ning Remediation wave 3 batch A (medium owners): cli-core, data-core, lib-api, tooling. cli-core: - non-training mode flags are now a mutually-exclusive argparse group (were silent-first-wins); wizard Ctrl-C exits wizard-cancelled (5) not a traceback; verify-audit --output-format works post-subcommand; ingest --input-encoding wired to the library seam; --default-probes help matches the 51-prompt/18-cat bundle; deploy --vendor gains choices; facade re-exports all 19 registrars. data-core: - unsupported dataset extension fails fast with an actionable error (was an opaque load_dataset error + offline network lookup); native test split popped not aliased; PUA glyph classification narrowed off legitimate CJK/Indic text; column-length mismatch raises the module's own message; synthetic generation flushes incrementally so a crash keeps completed rows. lib-api: - manage_checkpoints catches tarfile.TarError (removes partial archive) + writes a sha256 sidecar; HF-token read utf-8; public registries are immutable maps. tooling: - every CI job declares least-privilege permissions: contents: read; check_bilingual_code_blocks widened to the user manuals; docker-compose config path + pinned tensorboard image; MinHash-LSH real-library CI leg (ingestion-scale); NEW check_usermanual_schema_drift.py guard validating usermanual YAML keys against ForgeConfig (report-only for now — surfaced 38 pre-existing fabricated keys to be cleaned in a docs pass, then flipped to --strict). Orchestrator gate fix: added the missing TR --fit-check shell block in usermanuals/tr/training/sft.md (the widened bilingual-code-blocks guard caught the pre-existing EN/TR drift). Verification: ruff clean; full pytest 3127 passed / 24 skipped / 0 failed; dry-run OK; cli-help, bilingual (parity + code-blocks), anchor, field-descriptions, notebook-pins guards green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rd --strict Remediation wave 3 batch B (docs owners): docs-toplevel, docs-standards, notebooks, and a usermanual schema-drift cleanup. usermanual schema drift (docs/usermanuals, EN+TR): - cleared all 38 fabricated-key instances the new guard surfaced across 8 page-pairs (deployment/model-merging merge.* -> real MergeConfig fields; evaluation.trend / evaluation.max_length / synthetic.teacher.* / training.optimizer / model.name / benchmark.output removed or corrected). - flipped tools/check_usermanual_schema_drift.py to --strict in ci.yml and added it to the CLAUDE.md/AGENTS.md gauntlet (guard-wiring test kept in sync). docs-toplevel: - README doc table no longer hides the fully-translated TR guides; CLAUDE.md / AGENTS.md drop the gitignored docs/marketing/ links and show the wizard/ sub-package; CONTRIBUTING gauntlet matches canonical; site/README structure regenerated. (CHANGELOG legacy-header conformance + site binary assets favicon/OG left for the maintainer — real design work.) docs-standards: - reconciled the rulebook's CI-guard wiring/inventory claims with the real .github/workflows + tools/ (site-chrome parity IS CI-wired; ~19 extra guards run; stale bilingual-pair counts reworded to self-updating); regex.md paths updated to the post-Phase-12 data_audit split. notebooks: - safety_evaluation.ipynb results cell reads the real safety_results.json schema (was a KeyError); galore / data_curation framing corrected; the stale "Pinned to v0.8.0" comment fixed across all 11 notebooks (pins unchanged). Verification: ruff clean; full pytest 3132 passed / 24 skipped / 0 failed; dry-run OK; all doc/consistency guards green including schema-drift --strict. Note: an untracked safety_config.yaml stray appeared in the working tree during this batch; left in place (not committed) pending the maintainer's confirmation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r now works) The shipped default safety classifier meta-llama/Llama-Guard-3-8B is a generative checkpoint with no trained sequence-classification head, so it could never be scored through the text-classification pipeline — a prior remediation added a fail-fast that rejected it. This adds the generation path so the default works out of the box. - config: new SafetyConfig.classifier_mode (auto|classification|generation, default auto) + config_template example. - safety.py: _resolve_classifier_mode (auto -> generation for a known-generative Llama-Guard checkpoint, else classification); _load_generative_guard (causal LM load, trust_remote_code=False, transformers-5 dtype=, audit-on-failure); _generate_guard_verdict (Llama-Guard chat template, greedy, OOM fail-closed); _parse_guard_verdict (safe/unsafe + S-codes; malformed -> unsafe + low confidence, never silently safe); _classify_responses_generative returns the exact same classified dict shape so gates/SafetyResult are unchanged. The fail-fast reject now fires only for classification mode + a generative checkpoint; both paths keep the Article-12 audit.classifier_load_failed emit. - callers: trainer.py + _safety_eval.py thread classifier_mode through. - tests: verdict parser (safe/unsafe/malformed), generative aggregation + fail-closed, mode resolution/routing, load-failure audit; repurposed the old default-rejection tests to assert auto now succeeds and classification+generative still fails fast + audits (no GPU / no network — generate is mocked). - docs (EN+TR): configuration.md, safety_eval_subcommand.md, usermanuals safety + configuration pages document classifier_mode and the working default. Note: under generation scoring, confidence_weighted degenerates to safe_ratio (Llama-Guard emits a categorical verdict, no probability head) — documented. Verification: ruff clean; full pytest 3157 passed / 24 skipped / 0 failed; dry-run OK; field-descriptions, wizard-defaults, bilingual (parity + code-blocks), schema-drift --strict, anchor, cli-help, audit-catalog guards green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ediation diff Comprehensive Opus review of main..development (16 units, adversarially verified) surfaced 0 critical/high and 40 medium/low/info findings — the remediation held up. All fixed: config: - REGRESSION: the new rope_scaling validator falsely rejected valid `longrope` (needs short_factor/long_factor, factor optional) and `rope_type`-keyed configs (transformers 5 accepts rope_type|type). Now type-conditional + both keys. - SECURITY: model_dump_json() bypassed the root secret-redaction override (model_dump() was covered but not the JSON path) — added a redacting override. - documented that confidence_weighted degenerates to a safe-ratio under generation mode (the new default). safety/docs: - confidence_weighted caveat threaded through safety.py + safety manuals (EN+TR). - standardised the classifier_load_failed audit event on Article 15 (was mixed 12/15 across safety.py/_safety_eval/tests); catalog was already correct. - corrected the safety_results.json doc example to match the real gate math / S-code severity / zero-filled severity distribution. judge / data-audit: - NaN/Inf judge scores now degrade to the None sentinel (were clipped to 10.0). - IBAN false-positives on all-caps prose eliminated via ISO 7064 mod-97 checksum; PGP private-key body bound raised to 64KiB (the 8KiB bound clipped real keys). - rubric prompt-injection delimiter escaping; cross-split name mis-parse. ingestion / cli / wizard / compliance / trainer / lib-api: - invalid `--input-encoding` codec fails fast (EXIT_CONFIG_ERROR) instead of a silent empty-but-successful run; PermissionError/IsADirectory routed to exit 1. - gdpr warning events now carry the catalog-mandated completed fields; genesis manifest fsyncs its parent dir; GRPO reward classifier falls back to fp32 on CPU; approve/reject no longer materialise a non-existent output_dir; export text mode prints the manual-quantize follow-up; wizard DNS preflight bounds the resolver in a worker thread; BYOD message + registry docstrings corrected. - `--input-encoding` documented in cli.md/ingestion.md (EN+TR). tooling/docs: schema-drift guard docstring + CI step name reflect --strict; CLAUDE.md/AGENTS.md gauntlet count (16) + CONTRIBUTING gauntlet synced; conftest no-network scope documented; safety.py grandfathered in the module-size guard (generation feature crossed 1000 LOC; forgelm/safety/ split planned). Verification: ruff clean; full pytest 3206 passed / 24 skipped / 0 failed; dry-run OK; all 14 doc/consistency guards green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eview An independent Sonnet second-pass review of main..development (16 units, adversarially verified) caught 4 high / 12 medium / 12 low / 3 info — including two cross-cutting regressions the Opus pass missed. All fixed: compliance (HIGH): - the audit genesis-manifest reroot opt-in (FORGELM_ALLOW_AUDIT_REROOT) never regenerated the manifest, so `verify-audit` stayed permanently invalid after a break-glass recovery. The reroot now force-writes a fresh genesis manifest (atomic tmp+fsync+replace), re-establishing the hash-chain root; still an explicit, audited opt-in. Parent-dir fsync failure no longer misreported as a failed write. wizard (HIGH x2, cross-interaction regression): - the tightened rope_scaling validator (longrope now needs short_factor/ long_factor) made the wizard's longrope collector emit a config that failed validation, and the "keep existing" path wrongly rejected valid longrope blocks. The collector now collects short_factor/long_factor for longrope and validates keep-existing blocks through the real TrainingConfig validator; parametrized tests run every rope type's wizard output through ForgeConfig(). config / safety / trainer: - model_dump_json() passes ensure_ascii=False (codebase convention); the generation guard truncates its moderation prompt (truncation=True, max_length) like the classification path; classifier_mode trainer threading gained test coverage. ingestion / cli / data-audit / merging / http: - `ingest --output <a-directory>` fails fast again (was fail-slow after the atomic-write refactor); verify-audit docstring corrected; purge warning/ completed payloads share one field set; IBAN check-digit group made ASCII; TIES logs a warning on negative adapter weights; SSRF docstring clarified. docs / test-gaps: - model-merging.md no longer claims `--benchmark-only` runs safety gates (it ignores evaluation.safety) — points at `safety-eval` for the safety gate; frontmatter_pages_dropped scalar-vs-sample divergence documented; testing.md documents the no-network guard + markers; new tests/test_conftest_network_guard.py; schema-drift + bilingual-code-blocks guards gained real-corpus coverage tests. tooling/docs (orchestrator): CONTRIBUTING gauntlet count (16); safety.py grandfather in the module-size guard tracked in docs/roadmap/risks-and-decisions.md; CLAUDE.md/AGENTS.md count + schema-drift guard prose; conftest no-network scope doc. Verification: ruff clean; full pytest 3234 passed / 24 skipped / 0 failed; dry-run OK; all 14 doc/consistency guards + guard-wiring + module-size tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 192 files, which is 42 over the limit of 150. To get a review, narrow the scope: Upgrade to Pro+ to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (192)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant updates to the ForgeLM project, including a new interactive wizard for configuration generation, improved safety evaluation with generation-based Llama-Guard scoring, and various compliance and security hardening measures. It also adds a new CI guard (check_usermanual_schema_drift.py) to validate documentation examples against the schema. My review identified a numbering error in the troubleshooting guide where step 3 was skipped, which should be corrected.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+75
to
91
| 4. **Reduce max sequence length**: | ||
| ```yaml | ||
| model: | ||
| max_length: 1024 # down from 2048 | ||
| ``` | ||
|
|
||
| 4. **Use DeepSpeed ZeRO-3 for large models**: | ||
| 5. **Use DeepSpeed ZeRO-3 for large models**: | ||
| ```yaml | ||
| distributed: | ||
| strategy: "deepspeed" | ||
| deepspeed_config: "zero3_offload" | ||
| ``` | ||
|
|
||
| 5. **Reduce LoRA rank**: | ||
| 6. **Reduce LoRA rank**: | ||
| ```yaml | ||
| lora: | ||
| r: 8 # down from 16 |
There was a problem hiding this comment.
The numbering in this list of troubleshooting steps seems to have an error. It jumps from step 2 directly to step 4, skipping step 3. Please ensure the numbering is sequential.
Collaborator
Author
There was a problem hiding this comment.
Thanks — I checked this one carefully and it looks like a false positive from the diff view rather than a real numbering gap.
In the file at HEAD, the "CUDA Out of Memory (OOM)" list is sequential 1–6:
| line | item |
|---|---|
| 50 | 1. Enable 4-bit quantization |
| 56 | 2. Enable automatic OOM recovery |
| 68 | 3. Reduce batch size manually |
| 75 | 4. Reduce max sequence length |
| 81 | 5. Use DeepSpeed ZeRO-3 for large models |
| 88 | 6. Reduce LoRA rank |
I also scanned every numbered list in the file programmatically (13 numbered items total) and found no gaps.
What the diff shows is this PR fixing the numbering: the file previously had duplicate step numbers (3., 3., 4., 5.), which were renumbered to 3.–6.. Because the old 3. line is removed and a new one added in the same hunk, the interleaved +/- lines can read as a 2 → 4 jump in the diff view even though the resulting file is correct.
Happy to be corrected if you are seeing a different list — if so, please point at the rendered file line and I will fix it.
Collaborator
Author
|
@coderabbitai review all |
|
✅ Action performedFull review finished. |
CI review follow-up on PR #70. lint (required check) was failing: `ruff format --check .` covers the whole repo including .ipynb code cells, and notebooks/safety_evaluation.ipynb was left unformatted after its results-cell correction (the local gauntlet only formatted forgelm/ tests/ tools/). Ran `ruff format` over notebooks/ — the only change is comment indentation in one cell; the notebook stays valid JSON and its version pins are untouched. Also addressed two CodeFactor (bandit) findings on lines this PR introduced — CodeFactor is advisory here (the repo's own gate, tools/check_bandit.py, fails only on HIGH severity), but both are genuine hygiene: - B104: dropped "0.0.0.0" from the no-network guard's allowed-host set. It is the wildcard/unspecified address, not loopback; nothing needed it, so this tightens the guard as well as silencing the warning. testing.md's documented allow-list updated to match. - B108: the two cache-subcommand text-summary tests used a hardcoded "/tmp/cache" display path; they now use the tmp_path fixture per testing.md. Not changed, deliberately: bandit B615 (HF from_pretrained/load_dataset without revision pinning) — ForgeLM pins no upstream Hub revision anywhere today, so the new Llama-Guard loader follows the existing pattern; real remediation is a `revision` config surface threaded through every loader, tracked as a follow-up rather than bolted onto the lines this PR happened to touch. The two "Complex Method" notices are pre-existing, as is test_trainer.py's /tmp literal. Verification: ruff format --check . clean (240 files); ruff check clean; full pytest 3234 passed / 24 skipped / 0 failed; bilingual + anchor guards green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A systematic, multi-dimensional review of the whole project surfaced 163 verified findings; this branch remediates all of them, adds a user-requested safety feature, and then re-reviews the entire diff twice (independent Opus + Sonnet passes) and fixes what those passes found.
Test suite: 2868 → 3234 passing (+366 regression tests), 0 failing. Coverage ~81.8% → 84.5%.
What changed
Remediation of the 163 review findings (3 severity waves)
trainer.py); the shipped default safety classifier could never load; a non-UTF-8 corpus crashedpurge --row-idleaving a dangling GDPR audit chain; the canonical Configuration Reference documented a fabricated YAML schema.100.64.0.0/10);auth.hf_token/synthetic.api_keyare redacted from root-level config dumps/hashes (incl.model_dump_json); ReDoS-bounded secret detectors; the human-approval gate is concurrency- and container-safe; the audit/verification toolbelt no longer crashes on corrupt logs.ingestoutput, judge None-sentinel + prompt-injection hardening, reproducible DARE merges, TIES renormalization, dataset-extension validation, and much more.New feature — generation-based Llama-Guard scoring
The default
meta-llama/Llama-Guard-3-8Bis a generative checkpoint and could not be scored through thetext-classificationpipeline. It is now loaded as a causal LM, moderated via the Llama-Guard chat template, and parsed (safe/unsafe\nS<n>, fail-closed on malformed output) into the same safety report — so the default works out of the box. Newevaluation.safety.classifier_mode(auto|classification|generation).Systemic guardrails (prevent regression)
--strictin CI) — validates every fenced YAML key underdocs/usermanuals/against the realForgeConfigschema; cleared 38 pre-existing fabricated-key examples.permissions: contents: read; the MinHash-LSH backend is exercised against the real library in CI.Two-pass Opus + Sonnet review of the diff
Each pass reviewed the full
main..developmentdiff across correctness, regression, integration, security, test adequacy, and doc accuracy, adversarially verified. The Opus pass fixed 40 findings (e.g. arope_scalinglongropefalse-reject regression, themodel_dump_jsonredaction gap, NaN/Inf judge scores, an IBAN false-positive + a PGP-recall regression). The Sonnet second pass caught two cross-cutting regressions the first pass missed: the audit genesis-manifest reroot never regenerated the manifest (break-glass recovery leftverify-auditpermanently invalid), and the tightenedrope_scalingvalidator made the wizard'slongropeoutput invalid — both fixed.Documentation
Reference / user-manual / guide / QMS / standards docs (EN + TR mirrors), notebooks, and the top-level docs were corrected against the code throughout. Site version/claim parity guards are green (no site content change required beyond
site/README).Verification
ruff format+ruff checkclean · fullpytest3234 passed / 24 skipped / 0 failed ·--dry-runOK · all ~20 doc/consistency CI guards green (bilingual parity + code-blocks, anchor resolution, CLI↔docs help, audit-event catalog, field descriptions, wizard defaults, usermanual self-contained + schema-drift, module size, notebook pins, site version, …).Notes for reviewers
forgelm/safety.pycrossed the 1000-LOC soft ceiling with the generation feature and is grandfathered in the module-size guard; aforgelm/safety/sub-package split is tracked as a follow-up.forgelm/verify.pymodule extraction, standalonesafety-evalCLI threshold flags) are non-blocking and documented in the working notes.🤖 Generated with Claude Code