Repository navigation
feat(wave-5): Faz 30 full sweep — final pre-release documentation + tooling closure - #34
Conversation
…Wave 5 Wave 5 = Faz 30 full sweep (Path B chosen by maintainer over a v0.5.6-deferred Path A). Closure plan §2 status table updated: Wave 4 marked DONE (PR #33 merged via 01e40ba), Wave 5 declared in-progress on `closure/wave5-integration` covering the remaining Faz 30 deliverables (Task A doc triplets, Task J new tool, Task N anchor strict + cleanup, Tasks B/C/D site rebuild, Tasks E/F/G final pass, Tasks H/I/K/L/M guard runs). This commit clears the four Tier 1 ghost-feature drift items that Wave 4 round-1 left deferred. Each fix verified live against the shipped parser surface before editing. GH-11 — `forgelm benchmark --model "..."` (subcommand form) does not exist; `forgelm --help` lists no `benchmark` subcommand. The shipping form is `forgelm --config <yaml> --benchmark-only <path>`. - docs/usermanuals/en/evaluation/benchmarks.md L66-78 — rewritten to show the YAML config + `--benchmark-only` flag invocation. - docs/usermanuals/tr/evaluation/benchmarks.md — TR mirror. GH-16 — `--export-bundle` was renamed to `--compliance-export` in an earlier wave; QMS SOPs + roadmap doc still cited the old flag. - docs/qms/sop_model_training.md:66 — flag renamed. - docs/qms/roles_responsibilities.md:66 — flag renamed. - docs/roadmap/completed-phases.md:346 — flag renamed; output ext `.zip` → directory (matches `--compliance-export DIR` semantics). GH-18 — `forgelm deploy --target` parser only accepts `{ollama,vllm,tgi,hf-endpoints}`. KServe + Triton entries in the deploy-targets table + their dedicated example sections were removed (EN+TR). Replaced with a one-paragraph note pointing KServe / Triton operators at hand-authored manifests. GH-20 — Ingest flag drift was the largest. The shipped `forgelm ingest` parser uses: - `--strategy {sliding,paragraph,markdown}` (NOT `tokens`/`sentence`). - `--chunk-tokens N` (NOT `--max-tokens`). - `--overlap N` / `--overlap-tokens N`. - No `--language` / `--include` / `--exclude` / `--format` flags (those were earlier-design names that never landed). - No `--pii-locale` flag — PII ML language hints live on the audit subcommand as `--pii-ml-language LANG`. Affected files (EN+TR): - docs/usermanuals/{en,tr}/data/ingestion.md — strategies table rewritten, CLI flag table rewritten to the actual parser surface, removed the `--format raw` / `--format qa` ghost format references, common-pitfalls flag-name updated. - docs/usermanuals/{en,tr}/data/pii-masking.md L93 — replaced the broken `forgelm ingest --pii-mask --pii-locale de` example with the shipped `forgelm audit --pii-ml --pii-ml-language de` invocation; clarified the regex layer needs no locale hint. - docs/usermanuals/{en,tr}/data/language-detection.md — "See also" cross-ref redirected from the non-existent `--language` ingest flag to `audit --pii-ml-language`. - docs/usermanuals/{en,tr}/data/synthetic-data.md — "See also" cross-ref redirected from the non-existent `--format qa` ingest form to "produces the raw text JSONL that synthetic-data tools consume" framing. Validation - ruff format / check — clean (no Python files touched). - python3 tools/check_bilingual_parity.py --strict — 23/23 OK. - python3 -m pytest --no-cov -q — 1413 passed, 14 skipped. - grep residuals — only `docs/analysis/` (gitignored research tree) carries pre-this-commit references; user-facing tree clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ide + usermanual EN+TR)
Wave 5 / Faz 30 closure-plan Task A bulk delivery via 4 parallel
sub-agent groups. Each agent verified its CLI surface against the
shipped parser (`forgelm <subcommand> --help`) + implementation
modules before authoring; no flag, exit code, or audit-event name
was guessed.
Reference docs (NEW — 11 features × 2 langs = 22 files):
- verify_audit{,.tr}.md (Phase 6 — Article 12 chain validator)
- verify_annex_iv_subcommand{,.tr}.md (Phase 36 — Annex IV bundle integrity)
- verify_gguf_subcommand{,.tr}.md (Phase 36 — GGUF magic + SHA-256 sidecar)
- purge_subcommand{,.tr}.md (Phase 21 — GDPR Art. 17 erasure)
- reverse_pii_subcommand{,.tr}.md (Phase 38 — GDPR Art. 15 access)
- approve_subcommand{,.tr}.md (Phase 9 — Article 14 approve + reject)
- approvals_subcommand{,.tr}.md (Phase 37 — listing counterpart)
- doctor_subcommand{,.tr}.md (Phase 34 — env diagnostics)
- cache_subcommands{,.tr}.md (Phase 35 — air-gap pre-cache)
- safety_eval_subcommand{,.tr}.md (Phase 36 — standalone safety probe)
- library_api_reference{,.tr}.md (Phase 19 — public symbol surface)
Guides (NEW — 5 features × 2 langs = 10 files):
- getting-started{,.tr}.md (Wave 5 onboarding canonical, opens with `forgelm doctor`)
- air_gap_deployment{,.tr}.md (deep deployer cookbook for cache-models / cache-tasks)
- human_approval_gate{,.tr}.md (Article 14 approve/reject deployer cookbook;
cross-links to access_control.md §6's `jq -rs` segregation-of-duties pipeline)
- library_api{,.tr}.md (programmatic ForgeLM use vs CLI)
- performance{,.tr}.md (lazy-torch + safety/judge batching + chunker)
Usermanual pages (NEW — 9 features × 2 langs = 18 files):
- compliance/verify-audit{,.tr}.md
- compliance/annex-iv{,.tr}.md (rewrote pre-existing concept page to
subcommand-focused per task spine)
- compliance/gdpr-erasure{,.tr}.md (covers Art. 15 + 17 sister flows)
- compliance/human-approval-gate{,.tr}.md (deeper deployer companion to
the existing concept-level human-oversight page)
- deployment/verify-gguf{,.tr}.md
- operations/iso-soc2-deployer{,.tr}.md (navigation page → deep
iso_soc2_deployer_guide.md)
- operations/supply-chain{,.tr}.md (navigation page → supply_chain_security.md)
- reference/library-api{,.tr}.md (navigation page → library_api guide)
- reference/performance{,.tr}.md (navigation page → performance guide)
Registry updates (`tools/check_bilingual_parity.py::_PAIRS`):
- +5 guide pairs (air_gap_deployment, getting-started, human_approval_gate,
library_api, performance).
- +11 reference pairs (the 11 new subcommand reference docs +
library_api_reference).
- Net: 23 → 39 pairs. Strict parity green.
Verification
- ruff format / check — clean (no Python files added).
- python3 tools/check_bilingual_parity.py --strict — 39/39 OK.
- python3 -m pytest --no-cov -q — 1413 passed, 14 skipped.
- python3 tools/check_anchor_resolution.py — 36 broken-link baseline
unchanged (zero new broken cross-references introduced by these
50 files; the 36 existing breaks are pre-Wave-5 Faz 30 territory).
Style anchors mirrored: audit_event_catalog.md (reference table-heavy
style), gdpr_erasure.md (deep guide style), human-oversight.md
(usermanual front-matter + sectioned page style). Symbol citations
prefer stable forgelm.<symbol> over line-number anchors per the Wave
4 round-2 / Faz 26 rewrite policy.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… drift guard Wave 5 / Faz 30 Task J: a CI guard that catches the bug class we've chased four times this wave (GH-8 verify-log, GH-11 benchmark, GH-16 --export-bundle, GH-18 --target kserve, GH-20 --pii-locale, F-W4-01 verify-audit --output-dir/--json, F-W4-PS-02 approve --run-id). Tool design (749 LOC, pure stdlib): - Discovers the live parser surface by spawning `python3 -m forgelm.cli --help` + `forgelm <sub> --help` for every shipped subcommand — resilient to internal `_parser.py` reorganisation (Option A in the task spec). - Walks docs/{guides,reference,usermanuals/{en,tr}/} + README.md for fenced bash/shell code blocks containing `forgelm <sub> ...` invocations. - Reports three drift classes: (1) subcommand not in parser surface; (2) flag not in parser surface for that subcommand; (3) flag value not in parser's `choices` list. - Skips `docs/analysis/` (gitignored research) + `docs/marketing/` (gitignored strategy) by default. - False-positive avoidance: skips synopsis lines (containing `[` or `(...|...)` choice indicators), and skips code blocks in "Wrong:"/"Don't:"/"Anti-pattern:"/"Legacy:"/"Yanlış:"-tagged context. Forward-reference framing (`planned`/`roadmap`/`future`/ `not in v0.5.5`/`(planned)`) within ±3 lines of the cited line is also tolerated as a legitimate ghost callout. - Exit codes follow `tools/` contract: 0 advisory / 1 strict. Tests (298 LOC, 15 cases — 3:23 wall-clock since each spawns the real forgelm parser surface): - Real shipping syntax (`verify-audit ./outputs/audit_log.jsonl --require-hmac`) → exit 0. - Each historical drift class regression-pinned (verify-audit --output-dir/--json, benchmark --model X, deploy --target kserve, chat --top-p). - Forward-reference framing + Wrong:-tagged blocks → tolerated. - --quiet suppression of the OK summary. - Parser-surface discovery sanity tests (synthetic empty repo, missing-subcommand short-circuit). CI wiring: - New advisory-mode step in `.github/workflows/ci.yml` runs the tool with `continue-on-error: true` after the bilingual-parity step. Matches the `check_anchor_resolution.py` Faz 26 precedent: gate lands first, baseline cleanup follows, then a later commit flips to `--strict` fail-the-build mode. Baseline drift on the live tree: 40 findings across 20 docs. Top hits cluster in `docs/usermanuals/{en,tr}/reference/cli.md` (legacy flag tables drifted from the audit/verify-audit/chat/deploy/ingest parsers as Wave 2-4 reshaped them) and a handful of phase-13/14 "planned" subcommands (`forgelm trend`, `forgelm compare-runs`) documented as if shipped. Cleanup is the next Wave 5 commit. Validation - ruff format / check — clean. - python3 -m pytest tests/test_check_cli_help_consistency.py --no-cov -q — 15/15 pass in 3:23. - python3 -m pytest --no-cov -q — full suite green (1428 tests now; +15 from this commit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…; flip both gates to --strict
Wave 5 / Faz 30 Tasks J + N follow-up: clean up the baselines the
two new guards started with, then flip both CI steps from advisory
to fail-the-build strict mode.
Two parallel sub-agent runs delivered the cleanup:
CLI drift (40 → 0):
- docs/usermanuals/{en,tr}/reference/cli.md flag tables for audit /
verify-audit / chat / deploy / ingest rewritten to the live parser
surface (Wave 2-4 reshapes had outpaced the cli.md tables).
- docs/usermanuals/{en,tr}/data/deduplication.md, leakage.md,
quality-filter.md, secrets.md — five ghost flags on `audit`
(--dedup-threshold, --dedup-algo, --num-perm, --remove-duplicates,
--output-clean) replaced with pointers at the YAML `audit:` block
in configuration.md (the audit pipeline is config-driven, not
flag-driven).
- docs/usermanuals/{en,tr}/evaluation/trend-tracking.md +
operations/experiment-tracking.md — `forgelm trend` /
`forgelm compare-runs` are Phase 13 Pro CLI scope; added explicit
"> Note: planned for v0.6.0+ Pro CLI tier" callouts (the tool's
forward-reference heuristic recognises the framing).
- docs/usermanuals/{en,tr}/deployment/chat.md + roadmap/
phase-10-post-training.md — `forgelm chat --top-p` / `--safety`
ghosts annotated as v0.6.0+ planned.
- docs/reference/usage{,.tr}.md — stale flag references corrected.
- Final tally: 449 forgelm invocations across docs + README.md all
match the live parser surface.
Anchor drift (36 → 0):
- forgelm/data_audit.py references (Faz 14 split into a package)
→ forgelm/data_audit/<submodule>.py per symbol; affects coding.md,
error-handling.md, logging-observability.md, regex.md, three
roadmap phase docs, plus a few backlog references.
- forgelm/cli.py references (Faz 15 split) → forgelm/cli/ similarly.
- Slug-case mismatches (double-dash collapse): roadmap.md +
-tr.md and three phase docs cited `#v050--document-ingestion-...`;
GFM canonical slug uses single dash, fixed to
`#v050-document-ingestion-...`.
- Turkish-letter slug drift in usermanuals/tr/data/pii-ml.md: the
GFM custom-ID extension `{#dil-secimi}` is not parsed by
check_anchor_resolution; removed the literal `{#...}` annotations
and updated link refs to the Unicode-preserving canonical slugs
(`#dil-seçimi` / `#pre-flight-neden-önemli`).
- documentation.md self-illustrative `[Other doc 1](other.md)`
examples inside fenced code blocks were tripping the link regex;
rephrased as plain prose preserving pedagogical intent.
- usermanuals/{en,tr}/compliance/overview.md `compliance.html`
forward-ref to a public-site page that doesn't exist as a
doc file → reference/compliance_summary.md (the real in-tree
surface).
- Final tally: 0 broken anchor / relative-link references across
263 markdown files under docs/.
CI wiring:
- .github/workflows/ci.yml: two new strict-mode steps in the
validate job after bilingual-parity:
* "Markdown anchor resolution check (strict)" runs
`tools/check_anchor_resolution.py --strict`.
* "CLI / docs help-consistency drift check (strict)" was flipped
from advisory `continue-on-error: true` to `--strict`.
Validation
- python3 tools/check_anchor_resolution.py --strict — 263/263 OK.
- python3 tools/check_cli_help_consistency.py --strict — 449/449 OK.
- python3 tools/check_bilingual_parity.py --strict — 39/39 OK.
- python3 -m pytest --no-cov -q — 1428 passed, 14 skipped.
- ruff format / check — clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rmanual pages Wave 5 / Faz 30 Task B: register the 8 new usermanual pages shipped in commit 2a32842 in the navigation tree. Task C (build_usermanuals.py rebuild) is exercised but the artefacts (`site/js/usermanuals/*.js`) are gitignored — the GitHub Actions site-deploy workflow rebuilds them on every push from `_meta.yaml` + the per-language source markdown. New page entries: - compliance/gdpr-erasure (operator quick-ref for purge + reverse-pii) - compliance/human-approval-gate (deeper deployer companion to the existing concept-level human-oversight page) - compliance/verify-audit (chain validator) - deployment/verify-gguf (GGUF magic + SHA-256 sidecar) - operations/iso-soc2-deployer (navigation → iso_soc2_deployer_guide) - operations/supply-chain (navigation → supply_chain_security.md) - reference/library-api (navigation → library_api guide) - reference/performance (navigation → performance guide) Each entry carries full 6-language title set (en/tr/de/fr/es/zh) for navigation consistency with existing pages, even though DE/FR/ES/ZH source content is deferred per the locale policy (Task L). The build emits only EN + TR bags; missing-language sources fall back to EN with a "translation pending" banner per the existing convention. Local rebuild verification (artefacts NOT committed — gitignored): - python3 tools/build_usermanuals.py → 64 pages × EN+TR translated, no fallbacks emitted, deferred bags (de/fr/es/zh) skipped. - Page count: 56 → 64 (matches the +8 new entries × EN+TR source mirrors landed in commit 2a32842). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…dmap/standards final pass Wave 5 / Faz 30 Tasks D + E + F + G delivered as a single sweep (two parallel sub-agents). Task D — site/* HTML pages (5 of 8 touched; rest didn't need edits): - index.html: hero badge v0.5.0 → v0.5.5; stat tile #4 swapped from "100% / Config-driven" to "1428 / Tests passing on every commit"; capabilities grid +3 cards (Library API, ISO 27001 / SOC 2 alignment, GDPR rights tooling). - features.html: +6 Enterprise/MLOps cards (Library API, doctor, air-gap pre-cache, --workers, supply-chain SBOM/pip-audit/ bandit, safety-eval); new "Compliance & GDPR tooling" card group with Article 14 staging gate, GDPR Art. 15 + 17, verify-annex-iv, verify-gguf, ISO/SOC 2 alignment. - compliance.html: Article 14 card rewritten around the shipped forgelm approve/reject/approvals subcommands; GDPR Art. 15 + 17 cards added; new "Aligned, not certified" 4-pillar grid (audit trail, change control, data lineage, supply chain) cross-linked to docs/guides/iso_soc2_deployer_guide.md. - quickstart.html: install line `pip install forgelm==0.5.5` + pinned extras; +`forgelm doctor` smoke check; Step 4 extended with verify-audit + approvals + approve flow. - privacy.html: install line bumped + new "Data-subject rights you can ship in your own pipeline" section (Art. 15 / Art. 17 + ISO/SOC 2 cross-link). - guide.html / contact.html / terms.html — no version literal or copy needing change; untouched. - site/js/translations.js: 6-language bumps for hero badge + install commands; full key sets in EN+TR for all new home / features / compliance / privacy keys; DE/FR/ES/ZH new keys fall back to EN per i18n.js design. Task E — top-level docs: - README.md: feature list extended for Wave 2-5 deliverables (Library API, GDPR rights tooling, doctor, air-gap pre-cache, approvals, ISO/SOC 2 alignment, supply-chain security); stat block refreshed to ~72 modules / 1428 tests; CI guard list updated for the two new strict gates (anchor + cli-help consistency). - CONTRIBUTING.md: validation gauntlet command at the bottom refreshed to include the new tools/check_*.py guards. - CLAUDE.md: repository structure section updated for the data_audit/ + cli/ package splits, the new tools/check_* guards, and the current skill listing. Task F — roadmap: - docs/roadmap.md + -tr.md: Phase 12.6 closure cycle ✅ Done; v0.5.5 next-up; Phase 13 (Pro CLI) still planned (out-of-v0.5.5 scope). - docs/roadmap/releases.md: v0.5.5 row added (release commit will finalise date + ChangeLog excerpt). - docs/roadmap/risks-and-decisions.md: Wave 5 cycle decisions appended (Path B chosen over Path A; bilingual policy; cli-help-consistency baseline cleanup; anchor --strict CI flip). - docs/roadmap/phase-12-6-closure-cycle.md (NEW): summary listing all 38 fazlar with their wave-merge SHA references. Task G — standards final pass: - docs/standards/README.md: index complete (10 standards files). - architecture.md: data_audit/ + cli/ package splits reflected. - documentation.md: bilingual H3/H4 parity rule formalised; new CI guards (check_anchor_resolution, check_cli_help_consistency) added to the doc-related guard list. - localization.md: EN+TR mandatory + DE/FR/ES/ZH deferred policy formalised; QMS row reflected as "Yes" post-Wave-4 sweep. - release.md: v0.5.5 release sequence + cross-OS matrix + pre-commit + cut-release skill all documented. - testing.md: test count refreshed to ~72 modules / 1428 tests. pyproject.toml version bumped 0.5.1rc1 → 0.5.5 because check_site_claims.py --strict enforces site/code version match (site cites 0.5.5 install command). Faz 33 release workflow runs unchanged on this version. Validation - python3 tools/check_site_claims.py --strict — 5 templates, 16 GPU profiles, version 0.5.5: OK. - python3 tools/check_anchor_resolution.py --strict — 264/264 OK. - python3 tools/check_cli_help_consistency.py --strict — 449/449 OK. - python3 tools/check_bilingual_parity.py --strict — 39/39 OK. - python3 -m pytest --no-cov -q — 1428 passed, 14 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…az 30 complete Wave 5 / Faz 30 Tasks H + I + K + L + M dispositions: - Task H (bilingual parity strict CI): green (39/39 pairs). - Task I (site-claim CI strict): green (5 templates, 16 GPU profiles, version 0.5.5). - Task K (config docs auto-gen final regen): closed without building the speculatively-named `tools/regenerate_config_doc.py` — the description→doc-source-of-truth contract is gated by the three guards we already ship: `check_field_descriptions.py --strict`, `check_bilingual_parity.py --strict`, and `check_cli_help_consistency.py --strict`. `configuration.md` + `-tr.md` are hand-maintained; Wave 5 commits 2a32842 + fbb082d brought the surface current with the live Pydantic schema. Closure plan §10 Faz 30 Task K + acceptance row updated to reflect this honest disposition (no phantom-tool reference). - Task L (locale policy formalisation): landed in Task G's standards/localization.md edit (Wave 5 commit 4610dc6) — EN+TR mandatory, DE/FR/ES/ZH deferred to a future cycle, QMS row flipped from "No (yet)" to "Yes" post-Wave-4 sweep. - Task M (diagram / asset sweep): user-facing tree is image-free (only `docs/analysis/` gitignored research carries image refs); no v0.5.0 → v0.5.5 architectural diagrams require update; site/ assets/ have been v0.5.5-current since Wave 4 stat-block sweep. Final validation gauntlet (post Wave 5 completion): - ruff format / check — 167 files unchanged, all checks pass. - check_bilingual_parity.py --strict — 39/39 OK. - check_anchor_resolution.py --strict — 264/264 OK. - check_cli_help_consistency.py --strict — 449/449 OK. - check_site_claims.py --strict — all green. - check_field_descriptions.py --strict — every Pydantic field has a description=. - forgelm --config config_template.yaml --dry-run — green. - python3 -m pytest --no-cov -q — 1428 passed, 14 skipped. Wave 5 = Faz 30 full sweep is now COMPLETE. Next step: Faz 33 v0.5.5 PyPI release (POST-WAVE — out of this PR's scope). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ve 5 Wave 5 / pre-merge review artifact, matching the Wave 3 + 4 cadence: each wave ships with a self-contained 4-agent review prompt that the maintainer can launch verbatim before merging the wave PR. Prompt covers Wave 5's full diff (122 files, ~9272 insertions across 7 commits e18baa0 → 6448c2d): - §A: 50 new doc-triplet files (Task A) — CLI surface accuracy, audit events, symbol citations, bilingual parity. - §B: tools/check_cli_help_consistency.py — tool correctness, cognitive complexity, the 15 pinned tests, strict CI flip. - §C: anchor checker --strict CI flip + 36→0 baseline cleanup. - §D: site/* v0.5.5 finalisation (5 pages + i18n keys). - §E: README + CONTRIBUTING + CLAUDE.md final pass. - §F: 10 standards files final pass. - §G: roadmap final state + new phase-12-6-closure-cycle.md. - §H: Tier 1 ghost-feature drift residuals (GH-11/016/018/020). - §I: pyproject.toml 0.5.1rc1 → 0.5.5 (brought forward from Faz 33 because check_site_claims.py --strict enforces site/code version match). - §J: 5 strict guards baseline + pytest 1428 + dry-run + closure plan honesty. Severity calibration unchanged from Wave 3 + 4: CRITICAL = data corruption / audit loss / public exit-code break / alignment-vs- certified mislead; HIGH = runtime crash on happy path or missing test for documented contract; MEDIUM/LOW/NIT calibrated per the prompt's own severity table. Deliverable shape per agent: single Markdown report at `docs/analysis/code_reviews/wave5-<agent-name>.md` with verdict + severity counts + per-finding entries (severity tag, ID, file:line, reasoning, suggested fix, regression test/guard). Launch: 4 parallel sub-agents with the same prompt; each picks up its focus list (§B/C/H/I/J for code-correctness, §A.1-3+§D for privacy-security, §A.4+§E/F/G+§J for standards, §B.1-2+§C.1+§J.2 for test-rigour). .gitignore updated to allow-list the new prompt under docs/analysis/code_reviews/ (matches the wave3-followup-review-prompt + wave4-review-prompt + wave4-followup-review-prompt allow-list pattern). Validation - python3 tools/check_anchor_resolution.py --strict — 264/264 OK (the new prompt's internal links all resolve). - python3 tools/check_bilingual_parity.py --strict — 39/39 OK. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each prior wave (1, 2a, 2b, 3, 4) has its own CHANGELOG section under [Unreleased] documenting the Faz that wave closed. Wave 5 was missed during the bulk commits; this commit adds the section above the existing Wave 4 entry, ordered newest-first per the project's Keep-a-Changelog convention. Section covers all 7 Wave 5 commits (e18baa0 through a93ec45): - Faz 30 Task A: 50 new doc-triplet files (11 reference docs + 5 guides + 9 usermanual pages × EN+TR) + parity registry +16 pairs. - Faz 30 Task J: tools/check_cli_help_consistency.py (749 LOC, 15 pinned tests) + advisory CI step + strict flip in fbb082d. - Faz 30 Task N: anchor resolver --strict CI flip + 36→0 baseline cleanup. - Faz 30 Task O: Tier 1 ghost-feature drift residuals (GH-11/016/018/020) cleared. - Faz 30 Tasks B + C: _meta.yaml +8 page entries + JS bundle rebuild (56 → 64 pages). - Faz 30 Tasks D + E + F + G: site v0.5.5 final state + README/CONTRIBUTING/CLAUDE.md + roadmap + 10 standards files final pass. - Faz 30 Tasks H + I + K + L + M: dispositions documented (incl. honest closure of Task K — `tools/regenerate_config_doc.py` was never built; the three strict guards lock the contract instead). Migration notes section calls out: - pyproject.toml version 0.5.1rc1 → 0.5.5 (brought forward from Faz 33 because check_site_claims.py --strict enforces site/code version match). - Two new strict CI gates (anchor resolution + cli-help consistency). - 8 new usermanual pages + 16 new bilingual doc pairs (no user-facing behaviour change). Faz 33 release commit will rename [Unreleased] → [0.5.5] — DATE and possibly fold the Wave 4 + Wave 5 sections into a single release-grouped entry; until then [Unreleased] holds the work that lands in v0.5.5. Validation: anchor strict 264/264 OK, parity strict 39/39 OK, cli-help strict 449/449 OK (the new section's Markdown is clean — no broken anchors, no flag drift in the migration-notes examples). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… model-merging callout Final Wave 5 sweep — closes the last two remaining Faz 30 acceptance items the post-Wave-5 audit surfaced: 1. Phase 14 deferred ghost callout for `forgelm merge-sweep` (GH-15 — Phase 14 pipeline-chains scope) was not yet on `model-merging.md`'s tip block. Both EN + TR mirrors now carry the explicit "planned for v0.6.0+ pipeline-chains scope (Phase 14 roadmap; not in v0.5.5 — see [phase-14-pipeline-chains] (../../../roadmap/phase-14-pipeline-chains.md))" framing, matching the existing Phase 13 callout pattern in `trend-tracking.md` / `experiment-tracking.md` / `chat.md`. 2. Closure-plan §10 Faz 30 Acceptance checklist all 16 items ticked with their wave-merge SHA references. The Faz 30 row in the §2 status table flips from 🔄 in-progress to ✅ Wave 4 partial + Wave 5 full sweep. Note on the Tier 1 grep acceptance item: legitimate residual matches (e.g. `q2_k`/`f16` real parser choices, `fp16`/ `bf16: false` real Pydantic config keys, `FORGELM_CACHE_DIR` air-gap troubleshooting "rejected-as-ghost" callout, historical references in `docs/analysis/`) are documented as expected; user-facing tree is drift-free. Validation - python3 tools/check_anchor_resolution.py --strict — 264/264 OK. - python3 tools/check_bilingual_parity.py --strict — 39/39 OK. - python3 tools/check_cli_help_consistency.py --strict — 449/449 OK. After this commit, every Faz 30 deliverable is closed against the checklist; remaining work is the POST-WAVE Faz 33 v0.5.5 PyPI release. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
There was a problem hiding this comment.
Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 1 medium 1 minor |
| CodeStyle | 98 minor |
🟢 Metrics 162 complexity · 0 duplication
Metric Results Complexity 162 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Code Review
This pull request completes the Phase 30 'Faz 30' full sweep for ForgeLM, finalizing documentation, adding strict CI gates for CLI consistency and anchor resolution, and formalizing the project's compliance and release standards. The review identified several critical issues in the newly introduced tools/check_cli_help_consistency.py guard, specifically regarding multi-line command handling, tokenization of quoted arguments, and overly broad heuristics for identifying synopsis lines. These have been addressed to ensure the tool correctly validates the CLI surface against the documentation.
| yield (inner_idx + 1, lines[inner_idx]) | ||
| idx = close_idx + 1 |
There was a problem hiding this comment.
The current implementation of _iter_code_block_lines yields lines individually, which causes _scan_doc to miss flags and arguments in multi-line commands (those using \ for line continuation). Since _scan_doc only processes lines containing the "forgelm" token, subsequent lines of a multi-line command are ignored. This is a significant gap as many documentation examples use this format.
current_line = ""
start_line_no = 0
for inner_idx in range(block_start + 1, block_end):
line = lines[inner_idx]
if not current_line:
start_line_no = inner_idx + 1
if line.strip().endswith("\\"):
current_line += line.rstrip()[:-1] + " "
else:
current_line += line
yield (start_line_no, current_line)
current_line = ""There was a problem hiding this comment.
Absorbed in 9c67f77. _iter_code_block_lines now re-stitches multi-line continuations into one logical invocation; new test test_multiline_continuation_logical_line_stitching pins the contract by planting a backslash-continued forgelm deploy and asserting the --target kserve choice drift on the continuation line is caught. The fix also surfaced 2 real bugs the prior implementation had masked (gguf-export multi-quant + safety-eval --output → --output-dir); both fixed in the same commit.
| if "[" in line: | ||
| return True |
There was a problem hiding this comment.
The heuristic in _is_synopsis_line that skips any line containing a [ character is too broad. It causes the tool to skip valid command invocations that use shell globbing patterns with brackets (e.g., data/[0-9]*.jsonl), leading to false negatives. A more specific check, such as looking for a space before the bracket (typical in argparse usage strings), would be more accurate.
| if "[" in line: | |
| return True | |
| if " [" in line and "]" in line: |
There was a problem hiding this comment.
Absorbed in 9c67f77. _is_synopsis_line now requires the leading-space + bracket pattern AND a closing bracket on the same line — argparse usage strings always carry the leading space; shell-glob patterns like data/[0-9]*.jsonl put the bracket flush against the path with no leading space, so they pass through to the invocation parser as intended.
| body = body.rstrip("\\").strip() | ||
| cleaned: list[str] = [] | ||
| for raw in body.split(): | ||
| token = raw.strip("()[],|") |
There was a problem hiding this comment.
The strip() call should also include quotation marks (" and '). Since _tokenise_invocation uses a simple split() which is not quote-aware, quoted tokens in documentation (e.g., --target "ollama") will include the quotes in the token, causing them to fail validation against the parser's choice sets.
| token = raw.strip("()[],|") | |
| token = raw.strip("()[],|\"'") |
There was a problem hiding this comment.
Absorbed in 9c67f77. _tokenise_invocation strip set now includes both quote characters so quoted tokens reach validation as the underlying value rather than carrying the quote and failing the choice check.
| else: | ||
| flags.append(tok) | ||
| # Peek the next token to capture a value (if any). | ||
| if idx + 1 < len(tokens) and not tokens[idx + 1].startswith("-"): |
There was a problem hiding this comment.
The logic for capturing flag values assumes that any token starting with - is a new flag. This prevents the tool from correctly identifying negative numbers (e.g., --temperature -1.0) as flag values. Changing the check to look for -- (the prefix for long flags used in this project) would allow numeric values starting with a single dash to be correctly consumed as arguments.
| if idx + 1 < len(tokens) and not tokens[idx + 1].startswith("-"): | |
| if idx + 1 < len(tokens) and not tokens[idx + 1].startswith("--"): |
There was a problem hiding this comment.
Absorbed in 9c67f77. The flag-value capture now requires the long-flag prefix before treating the next token as a new flag. Single-dash numeric tokens are consumed as values; the project ships no short flags so this is the right call. Verified via the live parser surface.
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
docs/usermanuals/en/data/leakage.md (1)
72-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConfig reference is inconsistent with the shown YAML block.
Line 72 tells users to use
audit.leakage_check.remove_cross_split_overlapand “see the configuration block below,” but that key is not present in Lines 76-84. Please either add the key to the YAML example or change the instruction to avoid pointing to a missing config entry.Suggested doc fix
audit: leakage_check: enabled: true threshold: 0 # zero tolerance — fail audit at any leakage near_dup_hamming: 3 # match threshold fields_to_check: ["prompt", "chosen", "response"] + remove_cross_split_overlap: true # drop leaked rows from smaller split and write cleaned output fail_severity: "error" # `error` blocks training, `warn` just logsAs per coding guidelines, "Avoid documentation drift: every README claim must point to real, working code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/usermanuals/en/data/leakage.md` around lines 72 - 84, The doc references audit.leakage_check.remove_cross_split_overlap but the example YAML under the Configuration section lacks that key; either add the remove_cross_split_overlap entry (under audit.leakage_check) to the shown YAML with its expected knobs, or reword the sentence that points to the configuration block to no longer claim “see the configuration block below” and instead point to the relevant section or file where remove_cross_split_overlap is actually defined; update the YAML example keys (audit.leakage_check.enabled, threshold, near_dup_hamming, fields_to_check, fail_severity) accordingly so the text and example are consistent.docs/usermanuals/tr/data/leakage.md (1)
72-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winYAML örneği ile metin yönlendirmesi uyumsuz.
Line 72’de
audit.leakage_check.remove_cross_split_overlapknob’una yönlendirme yapılıyor ve “aşağıdaki konfigürasyon bloğuna bakın” deniyor; ancak Lines 76-84’te bu anahtar yok. Lütfen ya YAML örneğine bu alanı ekleyin ya da metni buna göre düzeltin.Önerilen dokümantasyon düzeltmesi
audit: leakage_check: enabled: true threshold: 0 # sıfır tolerans — herhangi sızıntıda audit fail near_dup_hamming: 3 # eşleşme eşiği fields_to_check: ["prompt", "chosen", "response"] + remove_cross_split_overlap: true # küçük split'ten sızdıran satırları kaldırır, temiz çıktı üretir fail_severity: "error" # `error` eğitimi engeller, `warn` sadece loglarAs per coding guidelines, "Avoid documentation drift: every README claim must point to real, working code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/usermanuals/tr/data/leakage.md` around lines 72 - 84, The doc references the YAML knob audit.leakage_check.remove_cross_split_overlap but the provided YAML block under the Konfigürasyon header does not include that key; either add the remove_cross_split_overlap setting to the example YAML (under audit.leakage_check) with a sensible default and brief inline comment, or change the explanatory sentence on Line 72 to stop pointing to the missing knob and instead reference the actual keys shown (enabled, threshold, near_dup_hamming, fields_to_check, fail_severity); locate the mention by the symbol audit.leakage_check.remove_cross_split_overlap and update the YAML example or the text so the documentation and example remain consistent.docs/roadmap.md (1)
39-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMermaid summary path is now out of sync with the status table.
The table says the next milestone is Phase 12.6 →
v0.5.5, but the graph still jumps from Phase 12.5 toward Phase 14/v0.6.0without showing Phase 12.6. Please add the missing node/edge so readers don’t get two different release paths in one page.As per coding guidelines "Avoid documentation drift: every README claim must point to real, working code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/roadmap.md` around lines 39 - 57, Add a missing Phase 12.6 node and edges to the mermaid graph: create a new node P126 with label "Phase 12.6<br/>Data Curation<br/>v0.5.5" (or similar), add a forward edge from P125 --> P126, add a release marker node V25 labeled "v0.5.5" and connect P126 -.-> V25, and then connect P126 --> P14 (or the appropriate next phase) so the visual flow matches the roadmap table; update any adjacent dashed edges (e.g., remove or adjust the direct P125 -.-> V2/V23 link if it would duplicate the path).docs/usermanuals/tr/operations/experiment-tracking.md (1)
115-124:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPlanned Pro CLI command is still formatted as runnable shell usage.
Since
compare-runsis explicitly marked as future (v0.6.0+ Pro CLI), presenting$ forgelm compare-runs ...in a shell block is misleading. Please mark it as preview/non-runnable output.Suggested fix
-```shell -$ forgelm compare-runs runs/v1.0 runs/v1.1 runs/v1.2 +```text +# preview (planned v0.6.0+ Pro CLI) +forgelm compare-runs runs/v1.0 runs/v1.1 runs/v1.2 v1.0 v1.1 v1.2 hellaswag 0.612 0.617 0.621 truthfulqa 0.480 0.482 0.475 ↓ S5_max 0.041 0.038 0.082 ↑↑ loss 1.43 1.39 1.35</details> As per coding guidelines: "Avoid documentation drift: every README claim must point to real, working code". <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/usermanuals/tr/operations/experiment-tracking.mdaround lines 115 -
124, The code block misleadingly presents the plannedforgelm compare-runs
(planned v0.6.0+ Pro CLI) as an executable shell command; update the fenced
block to a non-runnable preview by changing the language fromshelltotext,
remove the leading$prompt, and add a short inline comment like# preview (planned v0.6.0+ Pro CLI)before the command so readers understand this is mock
output for the futurecompare-runsfeature.</details> </blockquote></details> <details> <summary>docs/usermanuals/en/data/ingestion.md (1)</summary><blockquote> `72-81`: _⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **Resolve contradictory output-format behavior in this section.** Line 72 documents `instructions` as default output, but Lines 78-81 state v0.5.5 has no `--format` and emits raw `{"text": ...}` chunks. Please keep only one behavior (the new raw-chunk behavior) to avoid operator confusion. <details> <summary>Suggested doc fix</summary> ```diff -By default, `forgelm ingest` emits the `instructions` format with synthetic prompts: - -```json -{"prompt": "Summarise the following passage.", "completion": "Section 4.2 of the policy specifies…", "metadata": {"source": "policy.pdf", "chunk": 17}} -``` - `forgelm ingest` emits raw chunks (`{"text": "..."}` JSONL) — there is no `--format` flag in v0.5.5. Operators that want synthetic-prompt or Q&A datasets layer that as a downstream step (see [Synthetic Data](`#/data/synthetic-data`)) against the raw JSONL this command produces:As per coding guidelines "Avoid documentation drift: every README claim must point to real, working code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/usermanuals/en/data/ingestion.md` around lines 72 - 81, Remove the contradictory claim that `forgelm ingest` defaults to the `instructions` format and instead document the actual v0.5.5 behavior: state that `forgelm ingest` emits raw JSONL chunks like {"text": "..."} and that there is no `--format` flag; replace the `instructions` example JSON with a raw-chunk example, keep the note that synthetic-prompt or Q&A datasets are created downstream (see Synthetic Data), and ensure mentions of `--format`, `instructions`, and `forgelm ingest` are updated so the docs only describe the raw-chunk behavior.docs/analysis/code_reviews/closure-plan-202604300906.md (1)
119-131:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResolve conflicting PR
#33status within the same status block.Line 119 marks PR
#33as merged (with commit01e40ba), while Line 131 still saysPR#33pending. Please normalize this to one definitive status.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/analysis/code_reviews/closure-plan-202604300906.md` around lines 119 - 131, The document contains conflicting statuses for PR `#33`: one place lists "✅ Wave 4 (PR `#33`) — closure/wave4-integration → development, 2026-05-06 (merge commit 01e40ba)" while another later block says "PR `#33` pending"; pick the definitive merged state and normalize both occurrences to reflect the merge. Replace the "PR `#33` pending" mention with the merged wording (include branch closure/wave4-integration, the merge date 2026-05-06 and commit 01e40ba) and ensure the Wave 4 summary block and the earlier status line consistently show PR `#33` as merged.
🟡 Minor comments (20)
docs/roadmap/phase-12-6-closure-cycle.md-123-123 (1)
123-123:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale parenthetical — version is already
0.5.5in this PR.Line 123 reads
(currently \0.5.1rc1`)butpyproject.tomlwas bumped to0.5.5` in this same PR. A maintainer following the Faz 33 instructions would see Step 1 as already done while the parenthetical implies otherwise.📝 Suggested fix
-1. `pyproject.toml` `version = "0.5.5"` (currently `0.5.1rc1`) +1. `pyproject.toml` `version = "0.5.5"` ✅ (bumped in Wave 5 PR)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/roadmap/phase-12-6-closure-cycle.md` at line 123, The parenthetical note after the bullet about pyproject.toml is stale; update the text that currently reads "(currently `0.5.1rc1`)" to the correct current version `0.5.5` (or remove the parenthetical entirely) so the line `1. \`pyproject.toml\` \`version = "0.5.5"\`` accurately reflects the repo state; locate the sentence referencing the current version near the `pyproject.toml` line and replace the old version string with `0.5.5`.docs/usermanuals/en/compliance/gdpr-erasure.md-72-72 (1)
72-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResolve duplicate heading text to keep anchors/ToC stable.
### What gets recordedis repeated (also at Line 38), which triggers duplicate-heading lint and creates ambiguous section anchors. Rename one of them (e.g., the Article 15 section).Suggested doc-only fix
-### What gets recorded +### Article 15 audit event recorded🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/usermanuals/en/compliance/gdpr-erasure.md` at line 72, Duplicate heading "What gets recorded" causes lint/anchor conflicts; rename one occurrence (for example, change the Article 15 section heading to "Article 15 — What gets recorded" or "What gets recorded (Article 15)") so anchors/ToC are unique, update any internal links if present, and ensure the other heading text remains unchanged to preserve existing references.docs/usermanuals/en/deployment/verify-gguf.md-147-147 (1)
147-147:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHash explanation is technically incorrect in this pitfall note.
A file that “lost tail bytes after sidecar generation” cannot still match SHA-256; any byte change alters the digest. This sentence should be revised to avoid misleading integrity expectations.
As per coding guidelines: "Avoid documentation drift: every README claim must point to real, working code."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/usermanuals/en/deployment/verify-gguf.md` at line 147, Revise the misleading sentence in the "Skipping the metadata layer in CI by not installing `gguf`." paragraph: remove or reword the claim that a file could "still match SHA-256" after losing tail bytes and instead state the correct integrity behavior (any byte change—including truncation—alters the SHA-256 digest) and clarify that the metadata reader can detect truncation even if magic/sidecar checks might not; update the sentence referencing "lost its tail bytes after the sidecar was generated" accordingly so the doc aligns with actual verification code and avoids implying SHA-256 would remain unchanged.docs/reference/verify_gguf_subcommand.md-141-141 (1)
141-141:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix mismatched
verify-auditreference link target.The label references
verify_audit_subcommand.mdbut the href points toverify_audit.md; one of them is stale. Please align both to the real file so navigation doesn’t break.As per coding guidelines "Avoid documentation drift: every README claim must point to real, working code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/verify_gguf_subcommand.md` at line 141, The link label `verify_audit_subcommand.md` and its href `verify_audit.md` in docs/reference/verify_gguf_subcommand.md are mismatched; open the docs tree to see which actual file exists (either verify_audit_subcommand.md or verify_audit.md) and update the markdown so both the link text and target match the real filename (the link that references the companion verifier for audit_log.jsonl), ensuring the label and href are identical and point to the existing file.docs/guides/library_api.md-49-51 (1)
49-51:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove broken
forgelm.__version__example that contradicts package API.The example on lines 50-51 assumes
forgelm.__version__andforgelm.__api_version__exist, but web search of the published ForgeLM package confirms these attributes are not exported. Users will encounterAttributeErrorwhen running this snippet. Useimportlib.metadata.version('forgelm')instead, or remove the version attribute lines entirely if not applicable to the guide.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/library_api.md` around lines 49 - 51, The example in the docs referencing forgelm.__version__ and forgelm.__api_version__ is broken because those attributes are not exported; remove the two lines that access __version__ and __api_version__ from the snippet, or replace them with a working alternative using importlib.metadata.version('forgelm') to show package version; update the snippet surrounding the forgelm import/inspection (the lines with "print(forgelm.__version__)" and "print(forgelm.__api_version__)") accordingly so the example no longer raises AttributeError.docs/guides/air_gap_deployment-tr.md-19-28 (1)
19-28:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to the fenced block to satisfy markdown lint rules.
Use a language identifier (e.g.,
text) on this fence.Suggested fix
-``` +```text ┌─────────────────────────────────┐ ┌────────────────────────────────┐ │ Bağlı staging host │ │ Air-gap target host │ ... -``` +```🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/air_gap_deployment-tr.md` around lines 19 - 28, The markdown fenced block in docs/guides/air_gap_deployment-tr.md is missing a language tag; update the opening fence for the ASCII diagram to include a language identifier (e.g., change the starting ``` to ```text) so the block becomes ```text ... ``` to satisfy the lint rule for fenced code blocks.site/index.html-372-376 (1)
372-376:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInternal phase number leaks onto the public landing page.
Phase 19is a development milestone identifier with no meaning to end users. All otherfeature-tagelements on the page describe user-visible concepts (forgelm.api,SBOM,pip-audit, etc.). Exposing internal sprint labels on the public homepage erodes the page's polish.🔧 Proposed fix
<div class="feature-tags"> <span class="feature-tag">forgelm.api</span> - <span class="feature-tag">Phase 19</span> + <span class="feature-tag">embed</span> </div>Or remove the tag entirely if a suitable user-facing label isn't available.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/index.html` around lines 372 - 376, The page exposes an internal milestone label ("Phase 19") inside the feature-tags block; remove the <span class="feature-tag">Phase 19</span> or replace it with a user-facing label (e.g., a meaningful capability name) so only user-visible concepts remain; update the markup inside the "feature-tags" container (look for the feature-tag spans in site/index.html) and ensure the rendered tags list contains only customer-facing items or omit the tag entirely.docs/reference/verify_annex_iv_subcommand-tr.md-117-117 (1)
117-117:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame link display/href mismatch as the English mirror.
Line 117 has the identical
verify_audit_subcommand.mdvsverify_audit.mdinconsistency as the English reference doc. Both files need to be fixed together.🔧 Proposed fix (align with whatever correction is applied to the EN doc)
-[`verify_audit_subcommand.md`](verify_audit.md) — `audit_log.jsonl` için kardeş doğrulayıcı. +[`verify_audit.md`](verify_audit.md) — `audit_log.jsonl` için kardeş doğrulayıcı.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/verify_annex_iv_subcommand-tr.md` at line 117, The link display text and href are inconsistent — change the mismatched `verify_audit_subcommand.md` vs `verify_audit.md` so both the link label and the target filename match the corrected English reference; update the Turkish Annex IV subcommand doc's link token (the `verify_audit_subcommand.md`/`verify_audit.md` occurrence) to the same canonical filename chosen in the EN doc and ensure the sibling audit verifier reference is updated in both language files together.docs/guides/air_gap_deployment.md-193-193 (1)
193-193:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGrammatically broken sentence — clause has no predicate.
The sentence starting "Also: gated models require the same
HF_TOKENto be set duringcache-modelsas during" runs into "forgelm doctor --offlinevalidates…" with no connecting predicate, creating a dangling "as during" fragment.🔧 Proposed fix
-Also: gated models require the same `HF_TOKEN` to be set during `cache-models` as during `forgelm doctor --offline` validates the **presence** of the snapshot, not the **license**. +Also: gated models require the same `HF_TOKEN` to be set during `cache-models` as during training. `forgelm doctor --offline` validates the **presence** of the snapshot, not the **license**.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/air_gap_deployment.md` at line 193, The sentence fragment is missing a predicate; update the sentence that begins "Also: gated models require the same `HF_TOKEN` to be set during `cache-models` as during" to include the missing clause and read clearly—e.g., "Also: gated models require the same `HF_TOKEN` to be set during `cache-models` as during `forgelm doctor --offline`, because `forgelm doctor --offline` validates the presence of the snapshot (not the license)." Ensure references to `HF_TOKEN`, `cache-models`, `forgelm doctor --offline`, and HF Hub IDs remain intact and case-sensitivity note about `meta-llama/Llama-Guard-3-8B` vs `meta-llama/llama-guard-3-8b` is preserved.docs/guides/performance.md-184-184 (1)
184-184:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove ghost-feature recommendation:
forgelm audit --output-format parquetdoes not exist.Line 184 recommends
forgelm audit --output-format parquetas a corpus conversion step. The ForgeLM CLI does not support this command combination. Available options are--output-format jsononly. Remove this recommendation or replace it with valid, tested alternatives.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/performance.md` at line 184, The doc currently recommends a nonexistent CLI command `forgelm audit --output-format parquet`; update the guidance in the performance doc so it no longer suggests that ghost feature. Remove or replace the sentence referencing `forgelm audit --output-format parquet` and instead point readers to valid alternatives (e.g., use the actual supported `forgelm audit --output-format json` and then convert JSONL→parquet with a standard tool or script, or advise using parquet inputs directly) while keeping the surrounding advice about `dataloader_num_workers` and `prepare_dataset` intact.docs/roadmap/phase-11-5-backlog.md-10-12 (1)
10-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix link text to match package reality.
Lines 10 and 12 display link text as single files (
data_audit.py,cli.py) but link to package directories (forgelm/data_audit/,forgelm/cli/). Per architecture.md, these are packages, not single files.📝 Proposed fix
- [`forgelm/data_audit.py`](../../forgelm/data_audit/), + [`forgelm/data_audit/`](../../forgelm/data_audit/), [`forgelm/ingestion.py`](../../forgelm/ingestion.py), - [`forgelm/cli.py`](../../forgelm/cli/), + [`forgelm/cli/`](../../forgelm/cli/),Based on learnings: User manual and reference docs should accurately reflect the current code structure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/roadmap/phase-11-5-backlog.md` around lines 10 - 12, Update the markdown link text to reflect packages instead of single files: replace the displayed link texts `forgelm/data_audit.py` and `forgelm/cli.py` with package-style names matching the targets (e.g., `forgelm/data_audit/` and `forgelm/cli/` or `forgelm.data_audit` and `forgelm.cli`) so the link text matches the linked directories; keep the existing hrefs (`../../forgelm/data_audit/` and `../../forgelm/cli/`) unchanged and only modify the visible text for `forgelm/data_audit.py` and `forgelm/cli.py`.docs/standards/testing.md-140-140 (1)
140-140:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale bilingual parity count in the CI guard description.
Line 140 still says
23/23, but this PR’s own validation target is39/39. Please align the number to avoid drift in the testing standard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/standards/testing.md` at line 140, Update the stale bilingual parity count in the testing standard: locate the line containing the command `python3 tools/check_bilingual_parity.py --strict` and replace the `23/23` pair count with the current target `39/39` so the CI guard description matches the repository's validation target.docs/standards/testing.md-198-198 (1)
198-198:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake checklist commands consistently executable when copy-pasted.
Line 198 mixes
python3 ...with bare script paths. Usepython3for all three guard commands (or ensure all scripts are executable with shebang + mode) so local execution is deterministic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/standards/testing.md` at line 198, The checklist mixes an explicit python3 invocation for check_bilingual_parity.py with bare script paths for check_anchor_resolution.py and check_cli_help_consistency.py; make them consistent by either prefixing both check_anchor_resolution.py and check_cli_help_consistency.py with `python3` in the checklist or by ensuring those scripts have a proper shebang and executable bit so they can be run bare—update the checklist line to use `python3 tools/check_bilingual_parity.py --strict`, `python3 tools/check_anchor_resolution.py --strict`, `python3 tools/check_cli_help_consistency.py --strict` (or alternatively add shebangs and set executable mode on the two scripts).docs/usermanuals/tr/operations/supply-chain.md-8-16 (1)
8-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTR mirror should match the corrected supply-chain scan cadence.
Line 8 and Line 16 currently describe
banditas PR-scoped. Please mirror whatever final corrected cadence you keep in the EN page/standards doc so EN/TR stay semantically aligned.Based on learnings: User-facing documentation must be bilingual (English + Turkish) mirrors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/usermanuals/tr/operations/supply-chain.md` around lines 8 - 16, Update the Turkish supply-chain doc so the "bandit" trigger/cadence text matches the corrected English version: adjust the table row that currently reads "Her PR + main push" and any references to PR-scoped bandit to reflect the finalized cadence used in the EN page/standards doc (use the same phrasing and severity policy), and ensure the `bandit`/`tools/check_bandit.py` mention and its result/policy cells mirror the EN doc exactly so TR and EN remain semantically aligned.docs/usermanuals/en/operations/supply-chain.md-8-17 (1)
8-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign bandit trigger cadence with standards docs.
Line 8 and Line 16 state bandit runs on every PR, but the updated testing standard in this PR describes supply-chain checks (
pip-audit+bandit) as nightly + release-tag scoped. Please synchronize the trigger wording to one source of truth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/usermanuals/en/operations/supply-chain.md` around lines 8 - 17, Update the documentation so the bandit trigger wording matches the testing standard: replace the "Every PR + main push" trigger for bandit with the same cadence used for supply-chain checks (nightly + release-tag scoped), and ensure the table entry for the Tool (tools/check_bandit.py) and the Trigger column align with the pip-audit description (tools/check_pip_audit.py) and the release-tag row (tools/generate_sbom.py); keep the Result and Failure policy semantics consistent with the standard (nightly run at 03:00 UTC and release-tag generation) so there is a single source of truth for when bandit runs.site/js/translations.js-1927-1927 (1)
1927-1927:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix localStorage count mismatch in privacy text.
Line 1927 says “Three values are stored in
localStorage” but only two keys are listed. This should be corrected to avoid policy-text inconsistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/js/translations.js` at line 1927, The privacy text under the translation key "privacy.body" claims "Three values are stored in localStorage" but only lists two keys; update the string for "privacy.body" in site/js/translations.js to say "Two values are stored in localStorage" (or add the missing third key if intended) so the count matches the listed <code>forgelm-lang</code> and <code>forgelm-theme</code> entries; ensure you update only the displayed number in the "privacy.body" value.docs/reference/approvals_subcommand-tr.md-26-26 (1)
26-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStaging path format is inconsistent within the same reference page.
Line 26 describes
final_model.staging/, while Line 84’s JSON sample showsfinal_model.staging.<run_id>. Please align to one canonical path format to avoid operator/script drift.As per coding guidelines, "Avoid documentation drift: every README claim must point to real, working code".
Also applies to: 84-84
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/approvals_subcommand-tr.md` at line 26, The staging path is inconsistent: update the reference for the `--output-dir DIR` description and the JSON sample so they use a single canonical pattern (choose either `final_model.staging/` or `final_model.staging.<run_id>`), then make both occurrences match; locate and change the text that mentions `final_model.staging/` and the JSON sample entry `final_model.staging.<run_id>` to the chosen canonical form so docs reflect the real output layout consistently.site/js/translations.js-1445-1445 (1)
1445-1445:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse fully qualified CLI command for
rejectin compliance copy.Line 1445 and Line 1504 show
<code>reject</code>without theforgelmprefix, which makes copy/paste usage ambiguous. Use<code>forgelm reject</code>for parity with the rest of the command surface.Suggested patch
- ... <code>forgelm approve</code> / <code>reject</code> + <code>forgelm approvals</code> ... + ... <code>forgelm approve</code> / <code>forgelm reject</code> + <code>forgelm approvals</code> ...Also applies to: 1504-1504
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/js/translations.js` at line 1445, The translation string for "compliance.art14.body" contains an ambiguous `<code>reject</code>` token; update that token (and the other translation entry that also uses `<code>reject</code>`) to use the fully qualified CLI form `<code>forgelm reject</code>` so copy/paste commands are unambiguous—search for the exact `<code>reject</code>` occurrences in translations.js (including the "compliance.art14.body" entry) and replace them with `<code>forgelm reject</code>`.docs/reference/approvals_subcommand.md-26-26 (1)
26-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStaging path format should be canonicalized in this reference.
Line 26 says
final_model.staging/, but Line 84’s sample usesfinal_model.staging.<run_id>. Please normalize this so CI/operator docs point to one real path convention.As per coding guidelines, "Avoid documentation drift: every README claim must point to real, working code".
Also applies to: 84-84
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/approvals_subcommand.md` at line 26, The docs reference uses two different staging path formats (the flag `--output-dir DIR` description mentions `final_model.staging/` while the sample shows `final_model.staging.<run_id>`); pick the canonical convention (prefer `final_model.staging.<run_id>`), and update the `--output-dir DIR` description and any other mentions (including the `audit_log.jsonl`/sample section) so all references consistently use `final_model.staging.<run_id>` across the document.docs/usermanuals/tr/compliance/gdpr-erasure.md-38-72 (1)
38-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDisambiguate duplicate
### Neyi kaydederheadingsLine 38 and Line 72 reuse the same H3 title. That creates ambiguous anchor names and weakens direct-link navigation. Rename one (e.g., "
reverse-piineyi kaydeder").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/usermanuals/tr/compliance/gdpr-erasure.md` around lines 38 - 72, The H3 "### Neyi kaydeder" is duplicated (once above the audit events list and again below the `forgelm reverse-pii` section), causing ambiguous anchors; rename the second occurrence to a unique heading such as "### `reverse-pii` neyi kaydeder" (or similar) so anchors are distinct and direct links work; update the heading text where you see the `forgelm reverse-pii` section header (`## Madde 15 — `forgelm reverse-pii``) to reference the new H3 title.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ecd91214-a0e6-4ac4-9d3f-b9ce74f4d8b3
📒 Files selected for processing (127)
.github/workflows/ci.yml.gitignoreCHANGELOG.mdCLAUDE.mdCONTRIBUTING.mdREADME.mddocs/analysis/code_reviews/closure-plan-202604300906.mddocs/analysis/code_reviews/wave5-review-prompt.mddocs/guides/air_gap_deployment-tr.mddocs/guides/air_gap_deployment.mddocs/guides/getting-started-tr.mddocs/guides/getting-started.mddocs/guides/human_approval_gate-tr.mddocs/guides/human_approval_gate.mddocs/guides/library_api-tr.mddocs/guides/library_api.mddocs/guides/performance-tr.mddocs/guides/performance.mddocs/qms/roles_responsibilities.mddocs/qms/sop_model_training.mddocs/reference/approvals_subcommand-tr.mddocs/reference/approvals_subcommand.mddocs/reference/approve_subcommand-tr.mddocs/reference/approve_subcommand.mddocs/reference/cache_subcommands-tr.mddocs/reference/cache_subcommands.mddocs/reference/doctor_subcommand-tr.mddocs/reference/doctor_subcommand.mddocs/reference/library_api_reference-tr.mddocs/reference/library_api_reference.mddocs/reference/purge_subcommand-tr.mddocs/reference/purge_subcommand.mddocs/reference/reverse_pii_subcommand-tr.mddocs/reference/reverse_pii_subcommand.mddocs/reference/safety_eval_subcommand-tr.mddocs/reference/safety_eval_subcommand.mddocs/reference/usage-tr.mddocs/reference/usage.mddocs/reference/verify_annex_iv_subcommand-tr.mddocs/reference/verify_annex_iv_subcommand.mddocs/reference/verify_audit-tr.mddocs/reference/verify_audit.mddocs/reference/verify_gguf_subcommand-tr.mddocs/reference/verify_gguf_subcommand.mddocs/roadmap-tr.mddocs/roadmap.mddocs/roadmap/completed-phases.mddocs/roadmap/phase-10-post-training.mddocs/roadmap/phase-11-5-backlog.mddocs/roadmap/phase-11-data-ingestion.mddocs/roadmap/phase-12-5-backlog.mddocs/roadmap/phase-12-6-closure-cycle.mddocs/roadmap/phase-12-data-curation-maturity.mddocs/roadmap/phase-14-pipeline-chains.mddocs/roadmap/releases.mddocs/roadmap/risks-and-decisions.mddocs/standards/architecture.mddocs/standards/code-review.mddocs/standards/coding.mddocs/standards/documentation.mddocs/standards/error-handling.mddocs/standards/localization.mddocs/standards/logging-observability.mddocs/standards/regex.mddocs/standards/release.mddocs/standards/testing.mddocs/usermanuals/_meta.yamldocs/usermanuals/en/compliance/annex-iv.mddocs/usermanuals/en/compliance/gdpr-erasure.mddocs/usermanuals/en/compliance/human-approval-gate.mddocs/usermanuals/en/compliance/overview.mddocs/usermanuals/en/compliance/verify-audit.mddocs/usermanuals/en/data/deduplication.mddocs/usermanuals/en/data/ingestion.mddocs/usermanuals/en/data/language-detection.mddocs/usermanuals/en/data/leakage.mddocs/usermanuals/en/data/pii-masking.mddocs/usermanuals/en/data/quality-filter.mddocs/usermanuals/en/data/secrets.mddocs/usermanuals/en/data/synthetic-data.mddocs/usermanuals/en/deployment/chat.mddocs/usermanuals/en/deployment/deploy-targets.mddocs/usermanuals/en/deployment/model-merging.mddocs/usermanuals/en/deployment/verify-gguf.mddocs/usermanuals/en/evaluation/benchmarks.mddocs/usermanuals/en/evaluation/trend-tracking.mddocs/usermanuals/en/operations/experiment-tracking.mddocs/usermanuals/en/operations/iso-soc2-deployer.mddocs/usermanuals/en/operations/supply-chain.mddocs/usermanuals/en/reference/cli.mddocs/usermanuals/en/reference/library-api.mddocs/usermanuals/en/reference/performance.mddocs/usermanuals/tr/compliance/annex-iv.mddocs/usermanuals/tr/compliance/gdpr-erasure.mddocs/usermanuals/tr/compliance/human-approval-gate.mddocs/usermanuals/tr/compliance/overview.mddocs/usermanuals/tr/compliance/verify-audit.mddocs/usermanuals/tr/data/deduplication.mddocs/usermanuals/tr/data/ingestion.mddocs/usermanuals/tr/data/language-detection.mddocs/usermanuals/tr/data/leakage.mddocs/usermanuals/tr/data/pii-masking.mddocs/usermanuals/tr/data/pii-ml.mddocs/usermanuals/tr/data/quality-filter.mddocs/usermanuals/tr/data/secrets.mddocs/usermanuals/tr/data/synthetic-data.mddocs/usermanuals/tr/deployment/chat.mddocs/usermanuals/tr/deployment/deploy-targets.mddocs/usermanuals/tr/deployment/model-merging.mddocs/usermanuals/tr/deployment/verify-gguf.mddocs/usermanuals/tr/evaluation/benchmarks.mddocs/usermanuals/tr/evaluation/trend-tracking.mddocs/usermanuals/tr/operations/experiment-tracking.mddocs/usermanuals/tr/operations/iso-soc2-deployer.mddocs/usermanuals/tr/operations/supply-chain.mddocs/usermanuals/tr/reference/cli.mddocs/usermanuals/tr/reference/library-api.mddocs/usermanuals/tr/reference/performance.mdpyproject.tomlsite/compliance.htmlsite/features.htmlsite/index.htmlsite/js/translations.jssite/quickstart.htmltests/test_check_cli_help_consistency.pytools/check_bilingual_parity.pytools/check_cli_help_consistency.py
… doc drifts Wave 5 / PR #34 round-1 absorption from gemini-code-assist inline review on closure/wave5-integration. All 4 findings verified valid against the live parser surface; the multi-line stitching fix exposed two further legitimate doc drifts that the prior tool implementation had silently masked. Gemini findings (4): - HIGH (`tools/check_cli_help_consistency.py:509`) — `_iter_code_block_lines` yielded each physical line in isolation, missing flags on the continuation lines of multi-line ``\``-continued shell invocations. Now re-stitches logical commands and reports the start-line as the citation. New tests pin the contract: shell-variable references skip choice validation; multi-line continuations surface drift on flags cited after the backslash. - MEDIUM (`tools/check_cli_help_consistency.py:424`) — `_is_synopsis_line` skipped any line containing `[`, which false-negative-skipped legitimate invocations using shell-glob brackets like ``data/[0-9]*.jsonl``. Tightened to require a leading space before the bracket (the argparse synopsis form) AND a closing `]` on the same line. - MEDIUM (`tools/check_cli_help_consistency.py:398`) — `_tokenise_invocation`'s `strip("()[],|")` set didn't include quotation marks, so `--target "ollama"` would carry the quote into the token and fail validation against the parser's choice set. Strip set now includes `"` and `'`. - MEDIUM (`tools/check_cli_help_consistency.py:463`) — flag-value capture rejected any token starting with `-` as a new flag, breaking on negative numeric values like `--temperature -1.0`. Tightened to require `--` (the long-flag prefix used in this project) so a single `-` numeric value is consumed as a value, not mis-classified as a new flag. Newly-surfaced drift (the multi-line stitching exposed these; both are real bugs the prior implementation silently masked): - `docs/usermanuals/{en,tr}/deployment/gguf-export.md:53` — `forgelm export ... --quant "q4_k_m,q5_k_m,q8_0"` (comma-list passed as one value) is invalid. Parser accepts only a single choice from `{q2_k,q3_k_m,q4_k_m,q5_k_m,q8_0,f16}`. Replaced the "multi-quant export" example with a `for q in ...; do forgelm export ... --quant "${q}"; done` shell loop matching the actual contract. - `docs/usermanuals/{en,tr}/evaluation/safety.md:76` — `forgelm safety-eval ... --output baselines/...json` is invalid; parser uses `--output-dir DIR` (per-prompt results land as files inside the directory, not a single JSON artefact). Replaced with the correct flag. Tool enhancement (`_is_shell_variable_reference`): the choice-set validator now skips values that start with `$` (covering `$VAR`, `${VAR}`, `"$VAR"`, `"${VAR}"` after quote-stripping). Without this, the new multi-quant `for` loop above would itself trip the guard — shell-variable references are a legitimate documentation pattern when the exact value is operator-supplied at runtime. Tests: 15 → 17 in `tests/test_check_cli_help_consistency.py` (+2): - shell-variable reference skips choice validation - multi-line continuation logical-line stitching surfaces drift on continuation-line flags Validation - ruff format / check — clean. - python3 -m pytest tests/test_check_cli_help_consistency.py --no-cov -q — 17/17 pass. - python3 tools/check_cli_help_consistency.py --strict — 421/421 OK (was 449 pre-fix; the count drops because multi-line stitching consolidates physically-separate continuations into one logical invocation, and the quote-stripping fix collapses ``--target "ollama"`` + ``--target ollama`` into one canonical form). - python3 tools/check_anchor_resolution.py --strict — 264/264 OK. - python3 tools/check_bilingual_parity.py --strict — 39/39 OK. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round-2 absorption from CodeRabbit's full review of PR #34. 17 findings classified, 16 absorbed, 1 skipped (CodeRabbit conflated two verify-* result classes; doc shape was already correct). Documentation accuracy (10): - README.md L59 — Library API symbol list updated to actual `forgelm.__all__` exports (`audit_dataset`, `verify_audit_log`, `verify_annex_iv_artifact`, `mask_pii`, `mask_secrets`, …; not the speculative `run_audit`/`gdpr_purge`/`reverse_pii_query` names that never landed). - site/quickstart.html L191 — `forgelm approve` example uses the shipped positional `<run-id>` + `--output-dir` + `--comment` contract (matches Wave 4 round-1 fix; the site copy still had the legacy `--reason` flag form). - site/features.html L377 — `verify-gguf` card claim trimmed: the verifier validates the magic header, parses the metadata block, and re-checks the SHA-256 sidecar; there is no size-manifest validation in the implementation. - usermanuals/{en,tr}/compliance/annex-iv.md exit-code table — non-object root maps to validation failure (exit 1), not I/O error (exit 2). - usermanuals/{en,tr}/deployment/verify-gguf.md exit-code table — non-file path maps to EXIT_CONFIG_ERROR (1) per `_verify_gguf.py`; EXIT_TRAINING_ERROR (2) only fires on OSError reading a real file. - usermanuals/{en,tr}/data/secrets.md L67 vs L124 — resolved the contradiction (L67 says secrets scan cannot be disabled; L124 instructed `--skip-secrets` which doesn't exist on the parser). Replaced the L124 paragraph with the correct data-governance manifest workflow. Also dropped the non-existent `--secrets-strict` references at L93-95. - usermanuals/{en,tr}/evaluation/trend-tracking.md — restructured so the working `jq` flow is shown first as runnable; the speculative `forgelm trend` UX is moved into a `text`-tagged block (skipped by check_cli_help_consistency) with explicit "pseudo-output, NOT runnable today" header. - usermanuals/{en,tr}/operations/experiment-tracking.md — same restructure: working `jq` flow first; speculative `forgelm compare-runs` UX moved to a `text` block. - usermanuals/tr/deployment/chat.md L69 — clarified that the per-turn safety screen ships today via the `safety:` YAML block; the `forgelm chat ... --safety` flag-form preview is NOT runnable today (the prior wording was internally contradictory). Roadmap accuracy (3): - docs/roadmap/phase-10-post-training.md L29 — `--safety` flag example is now framed as planned (NOT runnable today); the shipping form is `safety: enabled: true` in the YAML config. - docs/roadmap/phase-14-pipeline-chains.md L73-74 — target release reslotted from the stale `v0.5.1` to `v0.6.0`, matching the v0.5.5 closure-cycle's deferred-callout policy. - docs/roadmap/releases.md L133 — release-state sentence rephrased to remove the "all 38 fazlar merged" vs "Wave 5 pending" contradiction; now states "first 4 waves merged + Wave 5 in flight". Also: L153 `--data-audit` removal note clarifies that the deprecation cadence (one minor warning window) was honoured (deprecation v0.5.0 → removal v0.5.5 with v0.5.0–v0.5.4 as the warning window). L143 Library API symbol list synced to README's corrected list. GitHub Actions snippet (2 — EN+TR): - docs/guides/human_approval_gate{,-tr}.md — `steps.train.exit_code` is not a valid GHA context field. Snippet now uses the documented `steps.<id>.outcome` form (success vs failure) and defers to the audit log as the source of truth for distinguishing exit 4 (paused) from exit 0 (success). The step now correctly uses `id: train` so `steps.train.*` resolves. Tool hardening (2): - tools/check_cli_help_consistency.py L213-217 — empty `<sub> --help` output now raises RuntimeError instead of silently dropping the subcommand from the parser surface (which would falsely report every doc citing the command as "subcommand not in parser surface"). - tools/check_cli_help_consistency.py L300-304 — `_read_lines` now propagates OSError instead of silently returning `[]`. In strict CI, swallowing the error would hide real drift by excluding the unreadable file from the scan. Skipped (1): - 3194808377 (verify-gguf.md L64 result-shape claim) — CodeRabbit asserted the doc's `valid` + `reason` + `checks` (nested) shape is wrong and the real shape is `valid` + `reason` + `parsed` + `error` + `manifest_hash_*`. Verified against forgelm/cli/subcommands/_verify_gguf.py: VerifyGgufResult has `__slots__ = ("valid", "reason", "checks")` — the doc shape is correct. The `manifest_hash_*` fields belong to VerifyAnnexIVResult; CodeRabbit conflated the two classes. No change. Validation - ruff format / check — clean. - python3 -m pytest tests/test_check_cli_help_consistency.py --no-cov -q — 17/17 pass. - python3 tools/check_cli_help_consistency.py --strict — 421/421 OK (no new drift surfaced). - python3 tools/check_anchor_resolution.py --strict — 264/264 OK. - python3 tools/check_bilingual_parity.py --strict — 39/39 OK. - python3 tools/check_site_claims.py --strict — green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gent review findings - Wave 5 PR #34 round 3 absorption: full sweep of CodeRabbit's 6 OOD + 20 Minor inline review comments PLUS 24 multi-agent review findings (code-correctness / privacy-security / standards / test-rigor docs under docs/analysis/code_reviews/wave5-*.md). HIGH/Major absorptions: - 4 broken `forgelm.cli.subcommands.*` symbol citations across purge/approvals reference docs (en+tr): _run_row_erasure → _run_purge_row_id, _handle_pending → _run_approvals_list_pending, _handle_show → _run_approvals_show (F-W5-PRIV-02). - configuration.md (en+tr): drop `kserve | triton` ghost choices and the entire `## deployment:` ghost YAML block (no `DeploymentConfig` in ForgeConfig; extra="forbid" rejects it at load) — replace with a v0.6.0+ Phase 14 callout (F-W5-S1, F-W5-S2). - ingestion.md (en+tr): drop conflicting "instructions format default" claim; document raw-chunk JSONL with summary-only --output-format {text,json} (CR-OOD ingestion). - closure-plan PR #33 status: line 131 said "PR #33 pending" but line 119 said merged with `01e40ba` (CR-OOD closure-plan). - leakage.md (en+tr): remove `audit.leakage_check.remove_cross_split_overlap` knob reference — knob doesn't exist in code; replace with the documented jq-based removal flow (CR-OOD leakage). - TR + EN experiment-tracking: drop `$ ` prompt prefix in `text`-tagged preview blocks for `forgelm compare-runs` (CR-OOD experiment-tracking). - roadmap mermaid (en+tr): add Phase 12.6 (Closure Cycle) → v0.5.5 node + edge so visual flow matches table (CR-OOD roadmap). Minor absorptions: - phase-12-6-closure-cycle.md L123: stale "(currently 0.5.1rc1)" parenthetical → already-bumped during Wave 5 (F-W5-S4 / CR Minor). - gdpr-erasure.md (en+tr): rename duplicate `### What gets recorded` / `### Neyi kaydeder` H3 to disambiguate Article 17 vs 15 sections (CR Minor #2 + #20). - verify-gguf.md (en+tr) L147: revise misleading "still match SHA-256" hash explanation; clarify magic-header layer doesn't catch truncation independent of sidecar (CR Minor #3). - verify_audit link mismatch in 4 reference docs (en+tr × verify_gguf + verify_annex_iv): label text now matches href (CR Minor #4 + #8). - air_gap_deployment-tr.md L19: add `text` language tag to ASCII diagram fence (CR Minor #6). - site/index.html L374: drop internal "Phase 19" milestone leak; tag reads `embed` (user-facing label) (CR Minor #7). - air_gap_deployment.md L193: fix dangling sentence fragment in HF_TOKEN gated-models tip (CR Minor #9). - performance.md (en+tr) L184: drop ghost `forgelm audit --output-format parquet`; replace with pandas/pyarrow conversion step (CR Minor #10). - phase-11-5-backlog.md L10-12: link text → package paths matching href (forgelm/data_audit.py → forgelm/data_audit/, etc.) (CR Minor #11). - testing.md L140: stale `23/23` → `39/39` bilingual parity count (CR Minor #12). - testing.md L198: `python3` prefix consistency in checklist (CR Minor #13). - supply-chain.md (en+tr): bandit cadence aligned to standards doc (Nightly 03:00 UTC + release tag, not "every PR + main push") (CR Minor #14 + #15). - translations.js L1927: "Three values" → "Two values" (only forgelm-lang + forgelm-theme listed) (CR Minor #16). - approvals_subcommand.md (en+tr) L26: staging path → match L84 JSON sample (`final_model.staging.<run_id>` canonical, run-id-less form honoured as fallback) (CR Minor #17 + #19). - translations.js L1445/L1462/L1504/L1521 (en+tr): `<code>reject</code>` → `<code>forgelm reject</code>` for fully-qualified copy/paste parity (CR Minor #18). - _meta.yaml header: add formal DE/FR/ES/ZH deferred-policy banner per closure-plan Task L (F-W5-S6). - release.md L20 + L359: stale "0.3.1rc1 current" + "no __version__" claims; corrected to 0.5.5 + reflect that __version__ + __api_version__ ARE exported per Library API standard (F-W5-S3). - purge_subcommand.md (en+tr): add WARNING-on-fallback language for --output-dir asymmetry vs reverse-pii (F-W5-PRIV-05). - verify-audit.md (en+tr): replace overstated "escrow" wording with KMS/HSM-based separation (F-W5-PRIV-06). Tool tightening + new tests: - check_cli_help_consistency.py: tighten _FORWARD_REF_MARKERS to whole-phrase forms (drop bare "planned"/"future"/"roadmap" which matched "planned attacks" / "future work" / "roadmap-driven" in unrelated prose); add Turkish equivalents for bilingual docs (F-W5-01). - test_check_cli_help_consistency.py: 3 new regression tests: bare-substring negative (F-W5-01), >±3-line forward-ref window negative (F-W5-TR-01a), and Wrong:-block scope-to-one-block resumption (F-W5-TR-01b). Suite 17 → 20 tests, all green. User-manual orphan fix: - _meta.yaml: register `reference/json-output` page (existed in filesystem since Wave 2 but never declared in nav config — was invisible on the live site). Now declared = on-disk = 65 each. Skipped (with reason): - CR Minor #5 (library_api.md `forgelm.__version__` example): INVALID — verified `python3 -c "import forgelm; print(forgelm.__version__)"` prints 0.5.5 cleanly; both __version__ and __api_version__ ARE exported per Library API standard. CodeRabbit's web-search must have hit pre-Wave-5 PyPI. Doc is correct; code matches. - F-W5-S5 (parity tool autodiscover usermanuals): registry shape refactor (~50 new pairs) is scope creep for round 3; defer to v0.6. - F-W5-S8 (compliance_summary-tr.md): explicitly deferred to v0.6 by the finding itself. - F-W5-S9, F-W5-TR-11/12 (NIT cite/style): non-load-bearing. - F-W5-02/03/04 (cognitive complexity, unterminated fence, resume-after-Wrong unit test): refactor scope; covered by F-W5-TR-01 block-scope tests for the resume case. - F-W5-TR-02 (cleanup commit baseline regression test): the live CI --strict gate IS the guard; doubling up the integration-test layer is defensible but not blocking. - F-W5-TR-03/05/06/07/08/09/10 (extras): non-blocking coverage gaps. Self-review gauntlet (all green): ruff format/check ✅ bilingual_parity --strict 39/39 ✅ anchor_resolution --strict 264/264 ✅ cli_help_consistency --strict 425/425 ✅ (reduced from 449 due to intentional invocation removals: ghost YAML block, parquet ghost, 4 broken symbol-cite paths, etc.) site_claims --strict (artefacts/templates/GPU/version) ✅ field_descriptions --strict ✅ pytest 1433 passed / 14 skipped (+5 new tests vs round 2) ✅ forgelm --config config_template.yaml --dry-run ✅ build_usermanuals.py: en/tr 65/65 OK, JS bundles regenerate ✅ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round 3 absorption summary (commit
|
| Check | Result |
|---|---|
ruff format --check . + ruff check . |
✅ clean |
check_bilingual_parity.py --strict |
✅ 39/39 |
check_anchor_resolution.py --strict |
✅ 264/264 |
check_cli_help_consistency.py --strict |
✅ 425/425 (was 449; reduction is intentional — ghost YAML block + parquet ghost + 4 symbol-cite paths removed) |
check_site_claims.py --strict |
✅ 4/4 |
check_field_descriptions.py --strict forgelm/config.py |
✅ |
pytest --no-cov -q tests/ |
✅ 1433 passed / 14 skipped (+5 vs round 2) |
forgelm --config config_template.yaml --dry-run |
✅ exit 0 |
build_usermanuals.py |
✅ en/tr 65/65 |
CI will re-trigger on this push; ready for re-review.
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
site/js/translations.js (1)
1927-1972:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe privacy summary contradicts the later Google Fonts disclosure.
Each
privacy.bodycopy says the only network call from the site goes to Formspree on form submit, but the same text later explains that Google Fonts is loaded from a third party on page load. That contradiction weakens the privacy notice. Please update all locale variants so the short version matches the actual third-party requests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/js/translations.js` around lines 1927 - 1972, The short "privacy.body" summaries currently state the only network call is to Formspree, but the same "privacy.body" texts later disclose Google Fonts are loaded on page load; update each locale's privacy.body (the Object.assign blocks for T.en, T.tr, T.de, T.fr, T.es, T.zh) so the short version correctly lists both the Google Fonts request on page load and the Formspree request on contact-form submit (e.g. change "The only network call you make from this site goes to Formspree..." to a sentence that mentions Google Fonts are fetched on page load and Formspree is contacted only on form submit), and ensure equivalent phrasing is applied in all translations.
♻️ Duplicate comments (1)
docs/reference/verify_annex_iv_subcommand-tr.md (1)
31-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExit-code table is inconsistent with the current Annex IV contract
malformed JSONandnon-object rootare validation failures (valid=False) and should map to exit1, while exit2should stay for true file I/O problems (not found/unreadable). The current row mixes these and will mislead CI branching.Suggested minimal fix
-| `1` | Gerekli alan eksik / boş VEYA manifest hash uyuşmazlığı — operatör eylemli: artefakt mevcut hâliyle Annex IV uyumlu değil. | -| `2` | Runtime hatası: dosya bulunamadı, okunamadı, bozuk JSON ya da kök bir JSON nesnesi değil. | +| `1` | Gerekli alan eksik / boş, manifest hash uyuşmazlığı, bozuk JSON veya kök nesne değil (validation failure — `valid=False`). | +| `2` | Runtime/I/O hatası: dosya bulunamadı veya okunamadı. |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/verify_annex_iv_subcommand-tr.md` around lines 31 - 34, Update the exit-code table so that JSON validation failures map to exit code 1 and true file I/O problems map to exit code 2: specifically change the row that currently lumps "bozuk JSON" and "kök bir JSON nesnesi değil" with I/O errors so that "malformed JSON" and "non-object root" are described as validation failures (valid=False) and assigned exit `1`, while keeping "dosya bulunamadı, okunamadı" as exit `2`; locate and edit the table entries for exit codes `1` and `2` in the verify_annex_iv_subcommand documentation to reflect this mapping.
🧹 Nitpick comments (5)
docs/analysis/code_reviews/closure-plan-202604300906.md (1)
102-102: ⚡ Quick winClarify Wave 5 state vs. Faz 30 completion state in one canonical sentence.
These lines currently read as both “in progress / PR pending” and “all Faz 30 acceptance criteria met.” Please normalize wording (e.g., “implementation complete, merge pending”) to avoid ambiguous release-readiness interpretation.
Also applies to: 120-121, 133-133, 1371-1386
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/analysis/code_reviews/closure-plan-202604300906.md` at line 102, Replace ambiguous wording that simultaneously indicates both "in progress / PR pending" and "all Faz 30 acceptance criteria met" with one canonical status sentence across occurrences mentioning Wave 5 and Faz 30 (e.g., "Implementation complete; merge pending") so that lines referencing Wave 5, Faz 30 acceptance criteria, PR `#33` and related entries (the table row with **30** and the other spots flagged at 120-121, 133, 1371-1386) consistently state the same release-readiness status; update every matching phrase to the chosen canonical sentence and ensure related notes (like "partial" vs "full sweep") are adjusted to avoid contradictory language.docs/usermanuals/tr/deployment/chat.md (2)
111-111: ⚡ Quick winAlign Turkish phrasing with line 66 for consistency.
Line 111 uses "v0.6.0+ Pro CLI seviyesi için" while line 66 uses the cleaner "v0.6.0+ Pro CLI için". Drop "seviyesi" for consistency and more idiomatic Turkish.
📝 Suggested update
-> Not: `chat-compare` subcommand'ı v0.6.0+ Pro CLI seviyesi için planlanmıştır ([Phase 13 yol haritası](`#/roadmap/phase-13`)). Bugün aynı karşılaştırma her checkpoint'e karşı `forgelm --benchmark-only` ve aşağı akış judge config'i ile koşturulur; aşağıdaki snippet planlanan özel UX'i önizler. +> Not: `chat-compare` subcommand'ı v0.6.0+ Pro CLI için planlanmıştır ([Phase 13 yol haritası](`#/roadmap/phase-13`)). Bugün aynı karşılaştırma her checkpoint'e karşı `forgelm --benchmark-only` ve aşağı akış judge config'i ile koşturulur; aşağıdaki snippet planlanan özel UX'i önizler.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/usermanuals/tr/deployment/chat.md` at line 111, Replace the phrase "v0.6.0+ Pro CLI seviyesi için" with the more concise "v0.6.0+ Pro CLI için" on the sentence that begins "Not: `chat-compare` subcommand'ı…" (the line currently containing "v0.6.0+ Pro CLI seviyesi için") so it matches the wording used at line 66 ("v0.6.0+ Pro CLI için") for consistent and idiomatic Turkish.
14-14: ⚡ Quick winUpdate example version to match release.
The example output shows
ForgeLM 0.5.2, but this documentation is for the v0.5.5 release. Update to0.5.5for consistency with what users will actually see.📝 Suggested update
-ForgeLM 0.5.2 — checkpoints/customer-support ile sohbet +ForgeLM 0.5.5 — checkpoints/customer-support ile sohbet🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/usermanuals/tr/deployment/chat.md` at line 14, Replace the example version string "ForgeLM 0.5.2" in the displayed text "ForgeLM 0.5.2 — checkpoints/customer-support ile sohbet" with "ForgeLM 0.5.5" so the example output matches the v0.5.5 release; locate and update the literal string "ForgeLM 0.5.2" in the chat documentation content and change it to "ForgeLM 0.5.5".docs/guides/air_gap_deployment.md (1)
19-19: 💤 Low valueOptional: Add language hint to fenced code block.
The ASCII diagram at line 19 could specify a language identifier (e.g.,
```textor```ascii) to satisfy linting tools, though the diagram renders correctly as-is.Minor formatting improvement
-``` +```text ┌─────────────────────────────────┐ ┌────────────────────────────────┐ │ Connected staging host │ │ Air-gapped target host │🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/air_gap_deployment.md` at line 19, The fenced code block containing the ASCII diagram (the block that begins with the box characters "┌─────────────────────────────────┐" / "│ Connected staging host │") should include a language hint such as ```text or ```ascii; update the opening triple-backtick to ```text (or ```ascii) so the diagram satisfies linters while preserving rendering.site/index.html (1)
165-166: ⚡ Quick winAvoid hardcoding a volatile CI test count on the homepage.
1428is true only for one release snapshot, so this stat will go stale on the next test-suite change while the marketing page keeps claiming precision. Either source the value from release/build metadata or switch to a stable claim such as “CI green” / “tests on every commit.”Based on learnings: Documentation claims in README and marketing material must point to real code; prevent documentation drift
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/index.html` around lines 165 - 166, Replace the hardcoded “1428” in the homepage stat block (the element with class "stat-value" paired with the label having data-i18n="home.stats.tests") so it cannot drift: either wire it to a build-time variable (e.g., inject a CI/test-count value into the template as {{ci_test_count}} from release/build metadata during your pipeline) or change the copy to a stable claim such as “CI green” or “Tests run on every commit” (update the stat-value content and/or the i18n key accordingly and ensure the template expects the variable if you choose the dynamic route).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/guides/human_approval_gate-tr.md`:
- Around line 228-229: The Turkish guide file human_approval_gate-tr.md
currently links to English reference files approve_subcommand.md and
approvals_subcommand.md; update those link targets to the Turkish mirror
filenames (approve_subcommand-tr.md and approvals_subcommand-tr.md) where TR
equivalents exist (otherwise keep EN and add a note), i.e. change the link
references in the lines that mention `approve`/`reject` and `approvals` so they
point to the `*-tr.md` files to preserve language parity; verify the anchor text
remains the same and commit the updated links.
- Around line 138-143: The text says "üç satır" but the JSONL audit example only
has two lines; update the prose to accurately say "iki satır" or add a third
JSONL line to match "üç satır". Locate the sentence near the JSONL example
referencing "üç satır" and either change that phrase to "iki satır" or append a
third audit event (e.g., another JSONL object similar to the existing
"human_approval.required" / "human_approval.granted" entries) so the description
and the example are consistent.
In `@docs/guides/human_approval_gate.md`:
- Around line 138-143: The docs text says "three rows" but the JSONL example
under the human_approval gate only contains two events
("human_approval.required" and "human_approval.granted"), causing a mismatch;
either add the missing third audit row (e.g., an explicit
"human_approval.denied" or an initial "human_approval.requested" event for the
same run_id "fg-abc123def456") or change the prose to read "two rows" so the
count matches the shown events (update references to events like
human_approval.required and human_approval.granted accordingly).
In `@docs/reference/purge_subcommand.md`:
- Around line 107-111: The failure message in _run_purge_row_id currently echoes
the raw args.row_id into the user-facing error/JSON output; change it to
redact/hash the identifier by calling _hash_target_id(args.row_id) (or rendering
a shortened form like "(hashed: {hash[:8]}…)" ) before composing the msg string
so the JSON error payload ({"success": False, "error": msg}) and any stdout/logs
never contain the plain PII; ensure you update the code paths that build msg in
_run_purge_row_id to use the hashed/redacted value instead of args.row_id.
In `@docs/reference/verify_gguf_subcommand.md`:
- Around line 31-33: Update the exit-code table in
docs/reference/verify_gguf_subcommand.md so that missing or non-regular paths
(e.g., "file not found" or unreadable input path) are described under exit code
`1` (caller/input errors) rather than exit code `2`, and reserve exit code `2`
exclusively for genuine runtime I/O failures on an existing file; also ensure
the TR mirror text for these rows matches this change and that the table entries
for `0`, `1`, and `2` remain consistent with the public contract in
error-handling.md.
In `@docs/roadmap.md`:
- Line 13: Replace the localization leak in the roadmap line by changing the
phrase "38 fazlar across 5 waves" to "38 phases across 5 waves" in the string
"Phase 12.6 — Closure Cycle (38 fazlar across 5 waves)" so the English roadmap
line uses consistent English wording.
In `@docs/usermanuals/en/compliance/gdpr-erasure.md`:
- Around line 90-93: The two sentences conflict: update the doc so
FORGELM_AUDIT_SECRET rotation does NOT change identifier hashes (which are
derived using the per-output-dir salt) and only rotates the chain HMAC key; edit
the lines referencing FORGELM_AUDIT_SECRET, identifier-hash XOR, chain HMAC key,
and per-output-dir salt so the text states that identifier hashes remain stable
across env-var changes (due to per-output-dir salt) and that rotating
FORGELM_AUDIT_SECRET only rotates the chain HMAC key (remove or change any claim
that it re-salts or re-XORs identifier hashes).
In `@docs/usermanuals/en/data/secrets.md`:
- Line 124: Update the broken in-page reference in the sentence that currently
reads “see 'Always on' above”: locate that sentence near the mention of the
manifest key legitimate_secret_content and replace the incorrect title text with
the actual section header present in this document (or convert it to a neutral
reference like “see the Secrets scan behavior section” if the intended header is
missing); ensure the new text exactly matches an existing section title or uses
a clear, consistent label so operators can navigate unambiguously.
In `@docs/usermanuals/en/deployment/verify-gguf.md`:
- Around line 12-14: The docs currently instruct to "fail the release on exit
`1`" after a `forgelm export`, which is incorrect because the CLI tool
`verify-gguf` can return other non-zero codes for different failure modes;
update the three bullets referencing `verify-gguf` (and the CI/CD guidance that
follows `forgelm export`) to say "fail on any non-zero exit" (or equivalently
"treat non-zero exit codes as failure") and add a short parenthetical noting the
public contract uses 0/1/2/3/4 per error-handling.md so operators should gate on
any non-zero exit rather than only `1`.
- Around line 146-148: The warning text in the :::warn block incorrectly
describes how a .sha256 sidecar can be misused for truncated files; update the
admonition that mentions gguf, .sha256 and magic-header to state that a sidecar
from the original full file cannot validate a truncated upload because
truncation changes the SHA-256 digest, and that the real bypass is an attacker
removing or regenerating the sidecar after tampering; reword the paragraph to
recommend installing gguf in CI so the metadata reader can reject
malformed/truncated files independently of sidecars and retain references to
gguf, magic-header and .sha256 for clarity.
In `@docs/usermanuals/tr/data/ingestion.md`:
- Around line 56-58: Replace English UI prose terms with Turkish equivalents in
the sentence that currently reads "Shipped `--strategy` seçenekleri: `sliding`,
`paragraph`, `markdown`." — specifically change "Shipped" to a Turkish word like
"Sunulan" (or "Dağıtılan") and any other English prose occurrences such as
"top-level" to "üst düzey" while leaving CLI flags/keys unchanged (keep
`--strategy`, `sliding`, `paragraph`, `markdown`, `tokens`, `sentence`, and
`parser` exactly as-is); also apply the same substitution to the other
referenced occurrence around line 85 so all non-CLI prose is Turkish and CLI
identifiers remain English.
In `@docs/usermanuals/tr/data/leakage.md`:
- Line 72: Update the Turkish (and verify English) remediation step that
currently references the wrong JSON path and omits the documented CLI flag:
change the text referring to leakage.cross_split_overlap to use the correct path
`.cross_split_overlap.pairs[]` (matching the jq example) and replace the
sentence claiming no automatic removal with guidance that recommends using the
CLI flag `--remove-cross-split-overlap=val|test` as the primary remediation
method, mentioning the manual `jq` extraction/removal flow only as an advanced
fallback.
In `@README.md`:
- Around line 291-295: Replace the hard-coded test counts next to the "tests/"
entry (the string "68 test modules, ~1442 tests") with a stable, non-stale
description—e.g. mention how to run the suite ("pytest tests/") and either link
to the CI badge/latest run or use a conservative bound like "1400+ tests" or
simply "many tests"—so update the README line that contains "tests/" and the
quoted counts accordingly.
In `@tools/check_cli_help_consistency.py`:
- Around line 467-522: The _extract_invocation function's flag-parsing while
loop pushes cognitive complexity over the limit—extract that loop into a new
helper _parse_flags_from_tokens(tokens: list[str], start_idx: int) ->
tuple[list[str], dict[str, str | None], int] which implements the exact
semantics currently present (support "--flag=value", consume a following token
as the value only if it does not start with "--" to allow negative
numbers/short-flag values, record flags and flag_values, and return the final
index consumed). Replace the loop in _extract_invocation with a call to
_parse_flags_from_tokens(tokens, 2) and use its returned flags/flag_values,
converting flags to tuple as before; add unit tests for _parse_flags_from_tokens
to cover value forms and edge cases so behavior is unchanged.
- Around line 525-579: The function _iter_code_block_lines has high cognitive
complexity and an unused source: Path parameter; extract the inner multi-line
stitching logic into a new helper _stitch_block_lines(lines, block_start,
block_end) that yields the same (line_no, line) tuples for a block (preserving
the current behavior for trailing continuations and unterminated continuations),
remove the unused source parameter from _iter_code_block_lines signature and
update callers (e.g., _scan_doc) to stop passing source, and keep the outer
fence scanning/skip logic in _iter_code_block_lines while delegating all
per-block line re-stitching to _stitch_block_lines to reduce complexity and
eliminate the SonarCloud warning.
---
Outside diff comments:
In `@site/js/translations.js`:
- Around line 1927-1972: The short "privacy.body" summaries currently state the
only network call is to Formspree, but the same "privacy.body" texts later
disclose Google Fonts are loaded on page load; update each locale's privacy.body
(the Object.assign blocks for T.en, T.tr, T.de, T.fr, T.es, T.zh) so the short
version correctly lists both the Google Fonts request on page load and the
Formspree request on contact-form submit (e.g. change "The only network call you
make from this site goes to Formspree..." to a sentence that mentions Google
Fonts are fetched on page load and Formspree is contacted only on form submit),
and ensure equivalent phrasing is applied in all translations.
---
Duplicate comments:
In `@docs/reference/verify_annex_iv_subcommand-tr.md`:
- Around line 31-34: Update the exit-code table so that JSON validation failures
map to exit code 1 and true file I/O problems map to exit code 2: specifically
change the row that currently lumps "bozuk JSON" and "kök bir JSON nesnesi
değil" with I/O errors so that "malformed JSON" and "non-object root" are
described as validation failures (valid=False) and assigned exit `1`, while
keeping "dosya bulunamadı, okunamadı" as exit `2`; locate and edit the table
entries for exit codes `1` and `2` in the verify_annex_iv_subcommand
documentation to reflect this mapping.
---
Nitpick comments:
In `@docs/analysis/code_reviews/closure-plan-202604300906.md`:
- Line 102: Replace ambiguous wording that simultaneously indicates both "in
progress / PR pending" and "all Faz 30 acceptance criteria met" with one
canonical status sentence across occurrences mentioning Wave 5 and Faz 30 (e.g.,
"Implementation complete; merge pending") so that lines referencing Wave 5, Faz
30 acceptance criteria, PR `#33` and related entries (the table row with **30**
and the other spots flagged at 120-121, 133, 1371-1386) consistently state the
same release-readiness status; update every matching phrase to the chosen
canonical sentence and ensure related notes (like "partial" vs "full sweep") are
adjusted to avoid contradictory language.
In `@docs/guides/air_gap_deployment.md`:
- Line 19: The fenced code block containing the ASCII diagram (the block that
begins with the box characters "┌─────────────────────────────────┐" / "│
Connected staging host │") should include a language hint such as ```text or
```ascii; update the opening triple-backtick to ```text (or ```ascii) so the
diagram satisfies linters while preserving rendering.
In `@docs/usermanuals/tr/deployment/chat.md`:
- Line 111: Replace the phrase "v0.6.0+ Pro CLI seviyesi için" with the more
concise "v0.6.0+ Pro CLI için" on the sentence that begins "Not: `chat-compare`
subcommand'ı…" (the line currently containing "v0.6.0+ Pro CLI seviyesi için")
so it matches the wording used at line 66 ("v0.6.0+ Pro CLI için") for
consistent and idiomatic Turkish.
- Line 14: Replace the example version string "ForgeLM 0.5.2" in the displayed
text "ForgeLM 0.5.2 — checkpoints/customer-support ile sohbet" with "ForgeLM
0.5.5" so the example output matches the v0.5.5 release; locate and update the
literal string "ForgeLM 0.5.2" in the chat documentation content and change it
to "ForgeLM 0.5.5".
In `@site/index.html`:
- Around line 165-166: Replace the hardcoded “1428” in the homepage stat block
(the element with class "stat-value" paired with the label having
data-i18n="home.stats.tests") so it cannot drift: either wire it to a build-time
variable (e.g., inject a CI/test-count value into the template as
{{ci_test_count}} from release/build metadata during your pipeline) or change
the copy to a stable claim such as “CI green” or “Tests run on every commit”
(update the stat-value content and/or the i18n key accordingly and ensure the
template expects the variable if you choose the dynamic route).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9456d674-2c15-44fb-85dc-62d31794e0b4
📒 Files selected for processing (59)
README.mddocs/analysis/code_reviews/closure-plan-202604300906.mddocs/guides/air_gap_deployment-tr.mddocs/guides/air_gap_deployment.mddocs/guides/human_approval_gate-tr.mddocs/guides/human_approval_gate.mddocs/guides/performance-tr.mddocs/guides/performance.mddocs/reference/approvals_subcommand-tr.mddocs/reference/approvals_subcommand.mddocs/reference/purge_subcommand-tr.mddocs/reference/purge_subcommand.mddocs/reference/verify_annex_iv_subcommand-tr.mddocs/reference/verify_annex_iv_subcommand.mddocs/reference/verify_gguf_subcommand-tr.mddocs/reference/verify_gguf_subcommand.mddocs/roadmap-tr.mddocs/roadmap.mddocs/roadmap/phase-10-post-training.mddocs/roadmap/phase-11-5-backlog.mddocs/roadmap/phase-12-6-closure-cycle.mddocs/roadmap/phase-14-pipeline-chains.mddocs/roadmap/releases.mddocs/standards/release.mddocs/standards/testing.mddocs/usermanuals/_meta.yamldocs/usermanuals/en/compliance/annex-iv.mddocs/usermanuals/en/compliance/gdpr-erasure.mddocs/usermanuals/en/compliance/verify-audit.mddocs/usermanuals/en/data/ingestion.mddocs/usermanuals/en/data/leakage.mddocs/usermanuals/en/data/secrets.mddocs/usermanuals/en/deployment/gguf-export.mddocs/usermanuals/en/deployment/verify-gguf.mddocs/usermanuals/en/evaluation/safety.mddocs/usermanuals/en/evaluation/trend-tracking.mddocs/usermanuals/en/operations/experiment-tracking.mddocs/usermanuals/en/operations/supply-chain.mddocs/usermanuals/en/reference/configuration.mddocs/usermanuals/tr/compliance/annex-iv.mddocs/usermanuals/tr/compliance/gdpr-erasure.mddocs/usermanuals/tr/compliance/verify-audit.mddocs/usermanuals/tr/data/ingestion.mddocs/usermanuals/tr/data/leakage.mddocs/usermanuals/tr/data/secrets.mddocs/usermanuals/tr/deployment/chat.mddocs/usermanuals/tr/deployment/gguf-export.mddocs/usermanuals/tr/deployment/verify-gguf.mddocs/usermanuals/tr/evaluation/safety.mddocs/usermanuals/tr/evaluation/trend-tracking.mddocs/usermanuals/tr/operations/experiment-tracking.mddocs/usermanuals/tr/operations/supply-chain.mddocs/usermanuals/tr/reference/configuration.mdsite/features.htmlsite/index.htmlsite/js/translations.jssite/quickstart.htmltests/test_check_cli_help_consistency.pytools/check_cli_help_consistency.py
✅ Files skipped from review due to trivial changes (2)
- docs/reference/approvals_subcommand.md
- docs/usermanuals/en/data/leakage.md
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_check_cli_help_consistency.py
| - [`../reference/approve_subcommand.md`](../reference/approve_subcommand.md) — `approve` / `reject` için flag-başına, event-başına referans. | ||
| - [`../reference/approvals_subcommand.md`](../reference/approvals_subcommand.md) — `approvals` için flag-başına, event-başına referans. |
There was a problem hiding this comment.
TR rehberde referans linkleri TR sayfalara yönelmeli.
Bu satırlarda TR rehber, EN reference dosyalarına gidiyor. TR karşılıkları mevcutsa *-tr.md linkleri kullanılmalı; aksi halde dil-paritesi bozuluyor.
Önerilen link düzeltmesi
-- [`../reference/approve_subcommand.md`](../reference/approve_subcommand.md) — `approve` / `reject` için flag-başına, event-başına referans.
-- [`../reference/approvals_subcommand.md`](../reference/approvals_subcommand.md) — `approvals` için flag-başına, event-başına referans.
+- [`../reference/approve_subcommand-tr.md`](../reference/approve_subcommand-tr.md) — `approve` / `reject` için flag-başına, event-başına referans.
+- [`../reference/approvals_subcommand-tr.md`](../reference/approvals_subcommand-tr.md) — `approvals` için flag-başına, event-başına referans.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [`../reference/approve_subcommand.md`](../reference/approve_subcommand.md) — `approve` / `reject` için flag-başına, event-başına referans. | |
| - [`../reference/approvals_subcommand.md`](../reference/approvals_subcommand.md) — `approvals` için flag-başına, event-başına referans. | |
| - [`../reference/approve_subcommand-tr.md`](../reference/approve_subcommand-tr.md) — `approve` / `reject` için flag-başına, event-başına referans. | |
| - [`../reference/approvals_subcommand-tr.md`](../reference/approvals_subcommand-tr.md) — `approvals` için flag-başına, event-başına referans. |
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 229-229: docs/guides/human_approval_gate-tr.md#L229
Expected: 80; Actual: 135
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/guides/human_approval_gate-tr.md` around lines 228 - 229, The Turkish
guide file human_approval_gate-tr.md currently links to English reference files
approve_subcommand.md and approvals_subcommand.md; update those link targets to
the Turkish mirror filenames (approve_subcommand-tr.md and
approvals_subcommand-tr.md) where TR equivalents exist (otherwise keep EN and
add a note), i.e. change the link references in the lines that mention
`approve`/`reject` and `approvals` so they point to the `*-tr.md` files to
preserve language parity; verify the anchor text remains the same and commit the
updated links.
| --output-clean data/clean/ | ||
| ✓ val'den 47 split-arası satır kaldırıldı (train tarafı korundu) | ||
| ``` | ||
| 3. **Kaldırma** küçük split'ten sızdıran satırları manuel olarak (audit JSON zarfının `leakage.cross_split_overlap` dizisi her duplicate satır id'sini adlandırır). `jq` ile süzüp çıkarın, sonra `forgelm audit`'i yeniden koşturarak zincirin temiz geçtiğini doğrulayın. v0.5.5'te otomatik kaldırma knob'u yoktur — yol haritasında bulunsa da gelene kadar açık `jq` adımı silmeyi denetlenebilir tutar. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify --remove-cross-split-overlap flag exists in CLI
forgelm audit --help | grep -i "remove.*overlap" || echo "Flag not found"
# Check EN version for consistency
cat docs/usermanuals/en/data/leakage.md | grep -A5 "remove-cross-split-overlap"
# Verify JSON structure in audit implementation
rg -n "cross_split_overlap" --type=py -C3Repository: cemililik/ForgeLM
Length of output: 6672
🏁 Script executed:
#!/bin/bash
# Read the Turkish documentation file to see lines 63 and 72
echo "=== Lines around line 63-72 of Turkish doc ==="
sed -n '55,75p' docs/usermanuals/tr/data/leakage.md
echo -e "\n=== Full file to check content ==="
wc -l docs/usermanuals/tr/data/leakage.mdRepository: cemililik/ForgeLM
Length of output: 1301
🏁 Script executed:
#!/bin/bash
# Search for any flag or function related to removal/overlap in the codebase
echo "=== Search for --remove or removal flags ==="
rg -i "(--remove|remove.*overlap)" --type=py --type=md -C2
echo -e "\n=== Search for version 0.5.5 or version-specific features ==="
rg -i "0\.5\.5|v0\.5\.5" --type=py --type=md
echo -e "\n=== Check CLI audit subcommand for removal options ==="
head -100 forgelm/cli/subcommands/_audit.py | grep -A5 -B5 "argparse\|add_argument"Repository: cemililik/ForgeLM
Length of output: 39183
🏁 Script executed:
#!/bin/bash
# Verify JSON structure more carefully across the codebase
echo "=== Check _types.py definition ==="
grep -n "cross_split_overlap" forgelm/data_audit/_types.py
echo -e "\n=== Check how it's populated in orchestrator ==="
grep -n "cross_split_overlap" forgelm/data_audit/_orchestrator.py | head -20Repository: cemililik/ForgeLM
Length of output: 358
Correct JSON path and add missing flag guidance in remediation steps.
Two issues in the Turkish remediation instructions that also appear in the English version:
-
Incorrect JSON path: Line 72 refers to
leakage.cross_split_overlap, but the working example on line 63 shows.cross_split_overlap.pairs[]. The jq query on line 63 is correct; line 72's text needs to match it. Verify both EN and TR versions and update to.cross_split_overlap.pairs[]. -
Missing automated remediation flag: The
--remove-cross-split-overlap=val|testflag is already documented in the CLI reference (both languages) and provides automated removal. However, line 72 states "v0.5.5'te otomatik kaldırma knob'u yoktur" (no automatic removal knob in v0.5.5) and only describes manualjq-based removal. This contradicts the documented flag. Rewrite line 72 to recommend the--remove-cross-split-overlapflag as the primary remediation path, with the manual jq approach as an optional fallback for advanced users.
🧰 Tools
🪛 LanguageTool
[misspelling] ~72-~72: Söz ve sayı arasında defis yoqtur: "v-0"
Context: ...ak zincirin temiz geçtiğini doğrulayın. v0.5.5'te otomatik kaldırma knob'u yoktur ...
(NUMBER_BEFORE_DEFIS_MISSING)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/usermanuals/tr/data/leakage.md` at line 72, Update the Turkish (and
verify English) remediation step that currently references the wrong JSON path
and omits the documented CLI flag: change the text referring to
leakage.cross_split_overlap to use the correct path
`.cross_split_overlap.pairs[]` (matching the jq example) and replace the
sentence claiming no automatic removal with guidance that recommends using the
CLI flag `--remove-cross-split-overlap=val|test` as the primary remediation
method, mentioning the manual `jq` extraction/removal flow only as an advanced
fallback.
| def _extract_invocation(source: Path, line_no: int, line: str) -> Invocation | None: | ||
| """Extract a structured ``forgelm`` invocation from one shell-block line.""" | ||
| payload = _strip_prompt(line) | ||
| if not payload.startswith("forgelm"): | ||
| return None | ||
| if _is_synopsis_line(payload): | ||
| return None | ||
| tokens = _tokenise_invocation(payload) | ||
| if len(tokens) < 2: | ||
| return None | ||
| if tokens[0] != "forgelm": | ||
| return None | ||
| sub = tokens[1] | ||
| # If the second token is itself a flag (e.g. ``forgelm | ||
| # --version``), it is a top-level form, not a subcommand | ||
| # invocation. We pass it through with subcommand="" so the | ||
| # caller can flag top-level drift separately if it ever wants | ||
| # to — but today's contract focuses on subcommand drift, so we | ||
| # ignore these. | ||
| if sub.startswith("-"): | ||
| return None | ||
| flags: list[str] = [] | ||
| flag_values: dict[str, str | None] = {} | ||
| idx = 2 | ||
| while idx < len(tokens): | ||
| tok = tokens[idx] | ||
| if tok.startswith("--"): | ||
| # ``--flag=value`` form. | ||
| if "=" in tok: | ||
| flag, _, value = tok.partition("=") | ||
| flags.append(flag) | ||
| flag_values[flag] = value | ||
| else: | ||
| flags.append(tok) | ||
| # Peek the next token to capture a value (if any). | ||
| # Only treat the next token as a NEW flag when it | ||
| # starts with ``--`` (the long-flag prefix used in | ||
| # this project). A single ``-`` prefix could be a | ||
| # short flag (rare in this project; shipped tools | ||
| # don't ship short forms) OR a negative numeric | ||
| # value like ``--temperature -1.0``; we'd rather | ||
| # consume the value than mis-classify it as a flag. | ||
| if idx + 1 < len(tokens) and not tokens[idx + 1].startswith("--"): | ||
| flag_values[tok] = tokens[idx + 1] | ||
| idx += 1 | ||
| else: | ||
| flag_values[tok] = None | ||
| idx += 1 | ||
| return Invocation( | ||
| source=source, | ||
| line=line_no, | ||
| raw=payload, | ||
| subcommand=sub, | ||
| flags=tuple(flags), | ||
| flag_values=flag_values, | ||
| ) |
There was a problem hiding this comment.
SonarCloud pipeline failure: _extract_invocation Cognitive Complexity 18 exceeds the 15 limit.
The flag-parsing while loop (lines 491–514) is the primary contributor. Extracting it to a _parse_flags_from_tokens helper drops _extract_invocation's CC to ~9 and gives the parsing logic its own testable surface.
♻️ Proposed refactor — extract _parse_flags_from_tokens
+def _parse_flags_from_tokens(
+ tokens: list[str],
+) -> tuple[list[str], dict[str, str | None]]:
+ """Parse ``--flag`` tokens and their values from a post-subcommand token slice."""
+ flags: list[str] = []
+ flag_values: dict[str, str | None] = {}
+ idx = 0
+ while idx < len(tokens):
+ tok = tokens[idx]
+ if tok.startswith("--"):
+ if "=" in tok:
+ flag, _, value = tok.partition("=")
+ flags.append(flag)
+ flag_values[flag] = value
+ else:
+ flags.append(tok)
+ if idx + 1 < len(tokens) and not tokens[idx + 1].startswith("--"):
+ flag_values[tok] = tokens[idx + 1]
+ idx += 1
+ else:
+ flag_values[tok] = None
+ idx += 1
+ return flags, flag_values
+
+
def _extract_invocation(source: Path, line_no: int, line: str) -> Invocation | None:
"""Extract a structured ``forgelm`` invocation from one shell-block line."""
payload = _strip_prompt(line)
if not payload.startswith("forgelm"):
return None
if _is_synopsis_line(payload):
return None
tokens = _tokenise_invocation(payload)
if len(tokens) < 2:
return None
if tokens[0] != "forgelm":
return None
sub = tokens[1]
if sub.startswith("-"):
return None
- flags: list[str] = []
- flag_values: dict[str, str | None] = {}
- idx = 2
- while idx < len(tokens):
- tok = tokens[idx]
- if tok.startswith("--"):
- # ``--flag=value`` form.
- if "=" in tok:
- flag, _, value = tok.partition("=")
- flags.append(flag)
- flag_values[flag] = value
- else:
- flags.append(tok)
- if idx + 1 < len(tokens) and not tokens[idx + 1].startswith("--"):
- flag_values[tok] = tokens[idx + 1]
- idx += 1
- else:
- flag_values[tok] = None
- idx += 1
+ flags, flag_values = _parse_flags_from_tokens(tokens[2:])
return Invocation(
source=source,
line=line_no,
raw=payload,
subcommand=sub,
flags=tuple(flags),
flag_values=flag_values,
)🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 467-467: Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/check_cli_help_consistency.py` around lines 467 - 522, The
_extract_invocation function's flag-parsing while loop pushes cognitive
complexity over the limit—extract that loop into a new helper
_parse_flags_from_tokens(tokens: list[str], start_idx: int) -> tuple[list[str],
dict[str, str | None], int] which implements the exact semantics currently
present (support "--flag=value", consume a following token as the value only if
it does not start with "--" to allow negative numbers/short-flag values, record
flags and flag_values, and return the final index consumed). Replace the loop in
_extract_invocation with a call to _parse_flags_from_tokens(tokens, 2) and use
its returned flags/flag_values, converting flags to tuple as before; add unit
tests for _parse_flags_from_tokens to cover value forms and edge cases so
behavior is unchanged.
| def _iter_code_block_lines(source: Path, lines: Sequence[str]) -> Iterable[tuple[int, str]]: | ||
| """Yield ``(line_no, line)`` tuples for lines inside qualifying code blocks. | ||
|
|
||
| Qualifying = lang tag in :data:`_BASH_LANG_TAGS` AND the block | ||
| is not flagged as forward-reference / anti-pattern. | ||
|
|
||
| Multi-line shell continuations (``\\`` at end of line) are | ||
| re-stitched into a single logical line so callers see the | ||
| full ``forgelm cmd --flag1 ... --flag2 ...`` invocation rather | ||
| than only the first physical line. The reported ``line_no`` | ||
| is the 1-indexed start line of the logical command — that's | ||
| where a reviewer expects the file-line citation to land. | ||
| """ | ||
| idx = 0 | ||
| while idx < len(lines): | ||
| match = _FENCE_RE.match(lines[idx]) | ||
| if not match: | ||
| idx += 1 | ||
| continue | ||
| lang = match.group(2).lower() | ||
| # Find the closing fence. | ||
| close_idx = idx + 1 | ||
| while close_idx < len(lines) and not _FENCE_CLOSE_RE.match(lines[close_idx]): | ||
| close_idx += 1 | ||
| if close_idx >= len(lines): | ||
| # Unterminated fence — bail out gracefully. | ||
| return | ||
| block_start, block_end = idx, close_idx | ||
| if _should_skip_block(lines, block_start, block_end, lang): | ||
| idx = close_idx + 1 | ||
| continue | ||
| # Yield interior lines, re-stitching ``\``-continued multi-line | ||
| # commands so each logical command surfaces as one (line_no, | ||
| # line) pair rather than N lines where the first carries the | ||
| # command name and the rest carry the flags in isolation. | ||
| current = "" | ||
| start_line_no = 0 | ||
| for inner_idx in range(block_start + 1, block_end): | ||
| raw = lines[inner_idx] | ||
| stripped = raw.rstrip() | ||
| if not current: | ||
| start_line_no = inner_idx + 1 | ||
| if stripped.endswith("\\"): | ||
| # Strip the trailing backslash + preserve one space | ||
| # so adjacent tokens don't fuse together. | ||
| current += stripped[:-1] + " " | ||
| else: | ||
| current += raw | ||
| yield (start_line_no, current) | ||
| current = "" | ||
| if current: | ||
| # Unterminated continuation at block end — yield what we | ||
| # have so the line is not silently dropped. | ||
| yield (start_line_no, current) | ||
| idx = close_idx + 1 |
There was a problem hiding this comment.
SonarCloud pipeline failure: _iter_code_block_lines Cognitive Complexity 21 exceeds the 15 limit; source parameter is also unused (SonarCloud warning).
Two issues in this function:
-
CC=21: The double
while(outer block scanner + inner fence-close finder) combined with the stitchingforloop and its nestedif/elsebranches drive the complexity up. Extracting the stitching logic to_stitch_block_linesdrops the outer function's CC to ~10. -
Unused
source: Pathparameter: The parameter is declared and passed by_scan_doc(line 620) but never referenced inside the function body, confirmed by SonarCloud. Removing it eliminates the warning and slightly reduces the function's cognitive surface; if source-aware error messages are ever needed (e.g., logging the file path on unterminated fence), it can be added back then.
♻️ Proposed refactor — extract _stitch_block_lines, drop unused source
+def _stitch_block_lines(
+ lines: Sequence[str], block_start: int, block_end: int
+) -> Iterable[tuple[int, str]]:
+ """Yield stitched ``(line_no, line)`` pairs for the interior of a code block.
+
+ Multi-line shell continuations (``\\`` at end of line) are re-stitched
+ into a single logical line; the reported ``line_no`` is the 1-indexed
+ start of the first physical line of each logical command.
+ """
+ current = ""
+ start_line_no = 0
+ for inner_idx in range(block_start + 1, block_end):
+ raw = lines[inner_idx]
+ stripped = raw.rstrip()
+ if not current:
+ start_line_no = inner_idx + 1
+ if stripped.endswith("\\"):
+ current += stripped[:-1] + " "
+ else:
+ current += raw
+ yield (start_line_no, current)
+ current = ""
+ if current:
+ yield (start_line_no, current)
+
+
-def _iter_code_block_lines(source: Path, lines: Sequence[str]) -> Iterable[tuple[int, str]]:
+def _iter_code_block_lines(lines: Sequence[str]) -> Iterable[tuple[int, str]]:
"""Yield ``(line_no, line)`` tuples for lines inside qualifying code blocks.
...
"""
idx = 0
while idx < len(lines):
match = _FENCE_RE.match(lines[idx])
if not match:
idx += 1
continue
lang = match.group(2).lower()
close_idx = idx + 1
while close_idx < len(lines) and not _FENCE_CLOSE_RE.match(lines[close_idx]):
close_idx += 1
if close_idx >= len(lines):
return
block_start, block_end = idx, close_idx
if _should_skip_block(lines, block_start, block_end, lang):
idx = close_idx + 1
continue
- current = ""
- start_line_no = 0
- for inner_idx in range(block_start + 1, block_end):
- raw = lines[inner_idx]
- stripped = raw.rstrip()
- if not current:
- start_line_no = inner_idx + 1
- if stripped.endswith("\\"):
- current += stripped[:-1] + " "
- else:
- current += raw
- yield (start_line_no, current)
- current = ""
- if current:
- yield (start_line_no, current)
+ yield from _stitch_block_lines(lines, block_start, block_end)
idx = close_idx + 1Update the call site in _scan_doc:
- for line_no, line in _iter_code_block_lines(source, lines):
+ for line_no, line in _iter_code_block_lines(lines):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _iter_code_block_lines(source: Path, lines: Sequence[str]) -> Iterable[tuple[int, str]]: | |
| """Yield ``(line_no, line)`` tuples for lines inside qualifying code blocks. | |
| Qualifying = lang tag in :data:`_BASH_LANG_TAGS` AND the block | |
| is not flagged as forward-reference / anti-pattern. | |
| Multi-line shell continuations (``\\`` at end of line) are | |
| re-stitched into a single logical line so callers see the | |
| full ``forgelm cmd --flag1 ... --flag2 ...`` invocation rather | |
| than only the first physical line. The reported ``line_no`` | |
| is the 1-indexed start line of the logical command — that's | |
| where a reviewer expects the file-line citation to land. | |
| """ | |
| idx = 0 | |
| while idx < len(lines): | |
| match = _FENCE_RE.match(lines[idx]) | |
| if not match: | |
| idx += 1 | |
| continue | |
| lang = match.group(2).lower() | |
| # Find the closing fence. | |
| close_idx = idx + 1 | |
| while close_idx < len(lines) and not _FENCE_CLOSE_RE.match(lines[close_idx]): | |
| close_idx += 1 | |
| if close_idx >= len(lines): | |
| # Unterminated fence — bail out gracefully. | |
| return | |
| block_start, block_end = idx, close_idx | |
| if _should_skip_block(lines, block_start, block_end, lang): | |
| idx = close_idx + 1 | |
| continue | |
| # Yield interior lines, re-stitching ``\``-continued multi-line | |
| # commands so each logical command surfaces as one (line_no, | |
| # line) pair rather than N lines where the first carries the | |
| # command name and the rest carry the flags in isolation. | |
| current = "" | |
| start_line_no = 0 | |
| for inner_idx in range(block_start + 1, block_end): | |
| raw = lines[inner_idx] | |
| stripped = raw.rstrip() | |
| if not current: | |
| start_line_no = inner_idx + 1 | |
| if stripped.endswith("\\"): | |
| # Strip the trailing backslash + preserve one space | |
| # so adjacent tokens don't fuse together. | |
| current += stripped[:-1] + " " | |
| else: | |
| current += raw | |
| yield (start_line_no, current) | |
| current = "" | |
| if current: | |
| # Unterminated continuation at block end — yield what we | |
| # have so the line is not silently dropped. | |
| yield (start_line_no, current) | |
| idx = close_idx + 1 | |
| def _stitch_block_lines( | |
| lines: Sequence[str], block_start: int, block_end: int | |
| ) -> Iterable[tuple[int, str]]: | |
| """Yield stitched ``(line_no, line)`` pairs for the interior of a code block. | |
| Multi-line shell continuations (``\\`` at end of line) are re-stitched | |
| into a single logical line; the reported ``line_no`` is the 1-indexed | |
| start of the first physical line of each logical command. | |
| """ | |
| current = "" | |
| start_line_no = 0 | |
| for inner_idx in range(block_start + 1, block_end): | |
| raw = lines[inner_idx] | |
| stripped = raw.rstrip() | |
| if not current: | |
| start_line_no = inner_idx + 1 | |
| if stripped.endswith("\\"): | |
| current += stripped[:-1] + " " | |
| else: | |
| current += raw | |
| yield (start_line_no, current) | |
| current = "" | |
| if current: | |
| yield (start_line_no, current) | |
| def _iter_code_block_lines(lines: Sequence[str]) -> Iterable[tuple[int, str]]: | |
| """Yield ``(line_no, line)`` tuples for lines inside qualifying code blocks. | |
| Qualifying = lang tag in :data:`_BASH_LANG_TAGS` AND the block | |
| is not flagged as forward-reference / anti-pattern. | |
| Multi-line shell continuations (``\\`` at end of line) are | |
| re-stitched into a single logical line so callers see the | |
| full ``forgelm cmd --flag1 ... --flag2 ...`` invocation rather | |
| than only the first physical line. The reported ``line_no`` | |
| is the 1-indexed start line of the logical command — that's | |
| where a reviewer expects the file-line citation to land. | |
| """ | |
| idx = 0 | |
| while idx < len(lines): | |
| match = _FENCE_RE.match(lines[idx]) | |
| if not match: | |
| idx += 1 | |
| continue | |
| lang = match.group(2).lower() | |
| # Find the closing fence. | |
| close_idx = idx + 1 | |
| while close_idx < len(lines) and not _FENCE_CLOSE_RE.match(lines[close_idx]): | |
| close_idx += 1 | |
| if close_idx >= len(lines): | |
| # Unterminated fence — bail out gracefully. | |
| return | |
| block_start, block_end = idx, close_idx | |
| if _should_skip_block(lines, block_start, block_end, lang): | |
| idx = close_idx + 1 | |
| continue | |
| # Yield interior lines, re-stitching ``\``-continued multi-line | |
| # commands so each logical command surfaces as one (line_no, | |
| # line) pair rather than N lines where the first carries the | |
| # command name and the rest carry the flags in isolation. | |
| yield from _stitch_block_lines(lines, block_start, block_end) | |
| idx = close_idx + 1 |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 525-525: Remove the unused function parameter "source".
[failure] 525-525: Refactor this function to reduce its Cognitive Complexity from 21 to the 15 allowed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/check_cli_help_consistency.py` around lines 525 - 579, The function
_iter_code_block_lines has high cognitive complexity and an unused source: Path
parameter; extract the inner multi-line stitching logic into a new helper
_stitch_block_lines(lines, block_start, block_end) that yields the same
(line_no, line) tuples for a block (preserving the current behavior for trailing
continuations and unterminated continuations), remove the unused source
parameter from _iter_code_block_lines signature and update callers (e.g.,
_scan_doc) to stop passing source, and keep the outer fence scanning/skip logic
in _iter_code_block_lines while delegating all per-block line re-stitching to
_stitch_block_lines to reduce complexity and eliminate the SonarCloud warning.
…ck findings Wave 5 PR #34 round 4 absorption: CodeRabbit re-scanned the round-3 absorption commit (dcbdcf5) and surfaced 15 new inline + 1 OOD + 1 duplicate + 4 nitpick findings. 25 files changed, +65/-44 LOC. HIGH/Major absorptions: - forgelm/cli/subcommands/_purge.py L465-502 (`_validate_match_count_or_fail`): redact raw `args.row_id` from BOTH the audit-event `error_message` field AND the operator-facing stdout/JSON error payload. The function now takes `target_id_hash` and emits `<id_hash:{first12}…>` so neither the persisted audit log nor the CI consumer's stdout leaks a potentially-PII identifier. Audit event already used hashed `target_id`; now `error_message` is consistent (CR-purge-PII). - docs/reference/verify_gguf_subcommand.md (en+tr) L31-33 + verify_annex_iv_subcommand.md (en+tr) L31-34: split exit code 1 (caller/input + integrity-mismatch — non-file path, malformed JSON, non-object root, magic mismatch, hash mismatch) from exit code 2 (genuine I/O failure on existing file). Aligns reference docs with the canonical _verify_gguf.py + _verify_annex_iv.py emit sites; the usermanual versions were already updated in round 2 (CR exit-code). - docs/usermanuals/en/compliance/gdpr-erasure.md (en+tr) L90-93: fix rotation contradiction. FORGELM_AUDIT_SECRET rotation rotates ONLY the chain HMAC key; identifier hashes derive from per-output-dir salt and are stable across env-var rotations. Previous text claimed it rotated "BOTH the identifier-hash XOR and the chain HMAC key" contradicting the next bullet (CR-gdpr-rotation). - docs/usermanuals/en/deployment/verify-gguf.md (en+tr) L12-14: "fail on exit 1" → "fail on any non-zero exit code" with reference to error-handling.md 0/1/2/3/4 contract (CR verify-gguf-exit). - docs/usermanuals/en/deployment/verify-gguf.md (en+tr) L146-148: rewrite hash-explanation pitfall — round 3 had it wrong in the opposite direction. Truncation DOES change SHA-256, so a sidecar from the original CANNOT validate a truncated copy. Real bypass is attacker who removes/regenerates the .sha256 sidecar after tampering. Installing `gguf` is defence-in-depth, not sidecar replacement (CR-verify-gguf-hash). Minor absorptions: - docs/guides/human_approval_gate.md (en+tr) L138 + L228-229: add third audit row (`compliance.artifacts_exported`) to match "three rows / üç satır" prose; switch TR `approve` / `approvals` reference links to the `*-tr.md` mirrors (CR-3-rows + CR-tr-mirror). - docs/roadmap.md L13: "38 fazlar" → "38 phases" (drop TR token from EN roadmap line) (CR-tr-leak). - docs/usermanuals/en/data/secrets.md L124 + tr L124: drop broken `see "Always on" above` reference to nonexistent section; replace with explicit always-on description + link to `Audit-only mode` section that explains scan-mode semantics (CR-secrets-anchor). - docs/usermanuals/tr/data/ingestion.md L56 + L85: translate English prose `Shipped` → `Sunulan`, `top-level` → `üst düzey`. CLI flag identifiers (`--strategy`, `sliding`, `paragraph`, `markdown`, `--recursive`) kept exactly as-is per project i18n policy (CR-tr-prose). - docs/usermanuals/{en,tr}/data/leakage.md L72: fix wrong JSON path — `leakage.cross_split_overlap` → `cross_split_overlap.pairs[...]` (matches actual `_data_audit/_summary.py:368-371` shape). Skipped bot's recommendation to cite `--remove-cross-split-overlap=val|test` CLI flag — VERIFIED that flag does NOT exist (`forgelm audit --help | grep remove` returns nothing); bot is hallucinating. The jq-based manual-removal flow remains the documented v0.5.5 method (CR-leakage-path). - README.md L291: drop hardcoded `68 test modules, ~1442 tests` — replace with stable `Test suite — run with pytest tests/ (count grows over time; CI gates on green-on-every-commit)` so the README doesn't drift after every PR (CR-readme-stat). - site/index.html L165: hardcoded `1428` test count → stable `CI ✓` badge with same i18n label, eliminating drift (CR-site-stat). - docs/analysis/code_reviews/closure-plan-202604300906.md L102: ambiguous status → canonical `Implementation complete; merge pending` with PR #33 merge `01e40ba` cited and PR #34 still open on `closure/wave5-integration` (CR-closure-canonical). - docs/guides/air_gap_deployment.md L19: add `text` lang tag to ASCII diagram fence (TR was already done in round 3) (CR-en-fence). - docs/usermanuals/{en,tr}/deployment/chat.md L14: example version string `ForgeLM 0.5.2` → `0.5.5` (CR-version-example). - docs/usermanuals/tr/deployment/chat.md L111: `Pro CLI seviyesi için` → `Pro CLI için` for consistent Turkish wording matching L66 (CR-tr-wording). Outside-diff absorption: - site/js/translations.js privacy.body (all 6 locales en/tr/de/fr/ es/zh): short version now correctly discloses BOTH network calls — Google Fonts on every page load, Formspree only on contact-form submit. Previous text said "the only network call goes to Formspree" while the same body's "Third-party assets" section listed Google Fonts; rewrite reconciles the contradiction without changing the detailed disclosure below (CR-OOD-fonts). Skipped (with reason): - CR `_extract_invocation` refactor (extract `_parse_flags_from_tokens`): cognitive-complexity refactor — the helper ships with 20 pinned tests (round 3 added 3); refactor is mechanical but adds risk for zero user-visible change. Defer to v0.6 maintenance window. - CR `_iter_code_block_lines` refactor (extract `_stitch_block_lines`, drop unused `source: Path` param): same scope-creep argument; the function is 100% covered by the 20 pinned tests and the unused parameter is a one-line cleanup, not a correctness issue. Self-review gauntlet (all green): ruff format + check ✅ clean bilingual_parity --strict 39/39 ✅ anchor_resolution --strict 264/264 ✅ cli_help_consistency --strict 425/425 ✅ site_claims --strict ✅ field_descriptions --strict ✅ pytest 1433 passed / 14 skipped (test_gdpr_erasure assertions updated to reflect new redacted-error-payload contract) ✅ forgelm --config config_template.yaml --dry-run ✅ build_usermanuals.py: en/tr 65/65 OK ✅ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round 4 absorption summary (commit
|
| Check | Result |
|---|---|
ruff format + ruff check |
✅ clean |
check_bilingual_parity --strict |
✅ 39/39 |
check_anchor_resolution --strict |
✅ 264/264 |
check_cli_help_consistency --strict |
✅ 425/425 |
check_site_claims --strict |
✅ |
check_field_descriptions --strict |
✅ |
pytest --no-cov -q |
✅ 1433 passed / 14 skipped (gdpr test updated to assert redacted contract) |
forgelm --config config_template.yaml --dry-run |
✅ |
build_usermanuals.py |
✅ en/tr 65/65 |
CI re-running on this push.
… + 2 MEDIUM + 2 LOW + 2 NIT) Wave 5 PR #34 round 5 absorption: post-rounds-1-4 multi-agent review (`docs/analysis/code_reviews/wave5-pre-merge-{code-correctness, privacy-security,standards,test-rigor}.md`) surfaced 2 HIGH + 2 MEDIUM + 2 LOW + 2 NIT findings. All absorbed. 8 files changed, +149/-18 LOC, suite 1433 → 1436 (+3 new TR forward-ref tests). HIGH absorptions (merge blockers cleared): - privacy-security F-W5R5-01: `_verify_gguf.py` + `_verify_annex_iv.py` exit-code drift between docs and code. Round 4 doc rewrite said exit 1 for "file not found" / "malformed JSON" / "non-object root" (caller-input errors per error-handling.md), but the dispatcher routed all three through `EXIT_TRAINING_ERROR` (= 2). Option A applied: code moved to match docs. - `_verify_gguf.py:202-215`: `os.path.isfile == False` → `EXIT_CONFIG_ERROR` (was `EXIT_TRAINING_ERROR`). - `_verify_annex_iv.py:196-215`: same change for file-not-found AND `JSONDecodeError` paths. Genuine I/O failures (`OSError` mid-read on existing file) remain at `EXIT_TRAINING_ERROR`. - Both functions' error-arms now carry inline comments tying the code to the documented contract. - `verify_annex_iv_artifact` docstring updated to reflect the new exit-code routing for `JSONDecodeError`. - Tests updated: `test_file_not_found_exits_runtime_error` → `test_file_not_found_exits_config_error` (en+tr verify suite); `test_malformed_json_exits_runtime_error` → `test_malformed_json_exits_config_error`. Each test now asserts `ei.value.code == 1` with a comment explaining the round-5 contract clarification. - standards F-W5R5-01: `docs/usermanuals/tr/reference/exit-codes.md` was missing the entire `### Jenkins` H3 subsection (the parity gate registry-blind drift the first review flagged). EN page shipped with `### GitHub Actions` + `### GitLab CI` + `### Jenkins` (3 H3); TR mirror only had the first two. Spine count now 10/10. Translated the `groovy` snippet's comment + error message to Turkish (`onay için beklet`, `Eğitim ${status} çıkış kodu ile başarısız oldu`) per localization.md prose-translation policy. MEDIUM absorptions: - standards F-W5R5-02: closure-plan L1376 stale `449/449` → `425/425 (down from 449 in Wave 5 round-2 baseline due to ghost-YAML-block + parquet-ghost + 4 broken symbol-cite removals in round 3, see commit `dcbdcf5`)`. The drop was deliberate + documented in round 3's commit message; the closure-plan acceptance checkbox was just not refreshed alongside. - standards F-W5R5-03: closure-plan L120 + L133 narrative sync with L102. Round 4 flipped L102 to `🟡 Implementation complete; merge pending` but L120 retained `🔄 ... in progress` and L133 retained `Wave 5 (in progress, ...)`. All three lines now use the `🟡 PR #34 open, implementation complete + merge pending` framing consistently. LOW absorptions: - code-correctness F-W5R5-01 / test-rigor F-W5R5-01: pin row-id redaction on the multi-match refusal path. `test_multi_match_one_mode_refuses` previously asserted only `"matched" in payload["error"].lower()` which would silently pass under a regression that re-introduced raw `args.row_id` on the multi-match leg. Test now seeds the corpus with `id="alice@example.com"` (PII-shaped, exercising the realistic threat model) and asserts the same five-claim contract the no-match leg uses: `"alice@example.com" not in payload["error"]`, `"<id_hash:" in payload["error"]`, `error_class == "MultiMatchRefused"`, `"alice@example.com" not in failed.error_message`, `"<id_hash:" in failed.error_message`. - test-rigor F-W5R5-02: 3 new TR forward-ref marker fixture tests (`test_planlanan_marker_skips_block`, `test_yol_haritasi_marker_skips_block`, `test_ileride_marker_skips_block`). Round 3's `_FORWARD_REF_MARKERS` tightening explicitly added Turkish equivalents (`planlanan`, `yol haritası`, `yol haritasında`, `ileride`) but every fixture in `TestForwardReferenceWhitelist` used English markers only — a silent drop of a TR marker would surface only at live `--strict` CI gate runtime ("16 invocations now flagged"), not at unit-test runtime. The new tests pin the markers per-fixture so the failure surface is "1 marker dropped" instead. NIT absorptions: - test-rigor F-W5R5-03: `test_wrong_block_does_not_taint_subsequent_block` now asserts `"1 drift finding" in out` (via the tool's summary line) so a regression that broke the anti-pattern whitelist entirely (every block scanned, "2 drift finding" reported) is caught at unit-test runtime instead of only at integration via shipped-doc behaviour. (The first count-based attempt asserted `out.count("--top-p") == 1` but the tool's diagnostic format prints the flag twice per finding, so the cleaner invariant is the summary's finding count.) - privacy-security F-W5R5-02: `_validate_match_count_or_fail` docstring now pins the 12-char hex prefix choice (48 bits = ~2.8 × 10¹⁴ collision space; do not crop below 12 / grow above 16 — symmetric with dry-run preview's 16-char rendering) so a future contributor doesn't drift the format in either direction. Skipped (with reason): - v0.6 deferred items remain deferred (F-W5-S5 parity-tool autodiscover refactor, F-W5-S8 compliance_summary-tr.md). Not introduced or worsened in round 5. - code-correctness F-W5R5-02 (hash-prefix length asymmetry between refusal `[:12]` and dry-run `[:16]`): explicitly documented as intentional — refusal site is brief privacy-preserving id; dry-run is operator-confidence ack. Now pinned in the round 5 docstring above. - code-correctness F-W5R5-03 (review-prompt §G mis-attributing round-1 test additions): documentation accuracy NIT in a frozen internal artefact; not committed to the repo. Self-review gauntlet (all green): ruff format + check ✅ clean bilingual_parity --strict 39/39 ✅ anchor_resolution --strict 264/264 ✅ cli_help_consistency --strict 425/425 ✅ (suite 20 → 23 tests) site_claims --strict ✅ field_descriptions --strict ✅ pytest --no-cov -q tests/ ✅ 1436 passed / 14 skipped (+3 new TR forward-ref tests; Wrong-block + multi-match assertions tightened; verify-gguf/annex-iv test names updated to reflect new exit-code contract) forgelm --config config_template.yaml --dry-run ✅ build_usermanuals.py: en/tr 65/65 OK ✅ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round 5 absorption summary (commit
|
| Check | Result |
|---|---|
ruff format + ruff check |
✅ clean |
check_bilingual_parity --strict |
✅ 39/39 |
check_anchor_resolution --strict |
✅ 264/264 |
check_cli_help_consistency --strict |
✅ 425/425 |
check_site_claims --strict |
✅ |
check_field_descriptions --strict |
✅ |
pytest --no-cov -q |
✅ 1436 passed / 14 skipped (+3 new TR forward-ref tests; multi-match + Wrong-block assertions tightened; verify-gguf/annex-iv test names updated) |
forgelm --config config_template.yaml --dry-run |
✅ |
build_usermanuals.py |
✅ en/tr 65/65 |
CI re-running on this push.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
forgelm/cli/subcommands/_verify_gguf.py (1)
9-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winModule docstring exit-code summary is stale after the round-5 alignment.
The dispatcher now maps a missing/non-regular path to
EXIT_CONFIG_ERROR(1) and reservesEXIT_TRAINING_ERROR(2) for I/O failures on an existing file (lines 202-213, 216-224). The reference doc, TR mirror, and TR usermanual were updated to match, but this module docstring still says "2 — runtime error (file not found, unreadable)" and omits the input-validation cases from the exit-1 list. Please reconcile so a future reader of the source agrees withdocs/reference/verify_gguf_subcommand.md.📝 Proposed fix
Exit codes: - 0 — magic OK, metadata parses, SHA-256 matches sidecar (when present). -- 1 — magic mismatch, metadata corrupted, OR SHA-256 mismatch. -- 2 — runtime error (file not found, unreadable). +- 1 — caller / input error: missing path, not a regular file, magic + mismatch, metadata corruption, malformed sidecar, or SHA-256 + mismatch. Artifact is not safe to serve. +- 2 — runtime I/O failure on an existing file (read error / permission + denied mid-read). """🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@forgelm/cli/subcommands/_verify_gguf.py` around lines 9 - 14, The module docstring's exit-code summary is outdated; update the paragraph that lists exit codes so it matches the dispatcher behavior by naming EXIT_CONFIG_ERROR (1) for magic mismatch, corrupted metadata, SHA-256 mismatch, and input-validation failures such as missing or non-regular path, and naming EXIT_TRAINING_ERROR (2) for runtime I/O failures on an existing file (e.g., unreadable after opening); ensure the 0 case still states successful verification (magic OK, metadata parses, SHA-256 matches sidecar) and explicitly reference the constants EXIT_CONFIG_ERROR and EXIT_TRAINING_ERROR so readers can correlate the docstring with the dispatcher logic.forgelm/cli/subcommands/_verify_annex_iv.py (1)
197-225:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove
isfile()pre-check and handle path-validation errors inside the try block.
os.path.isfile()returnsFalsefor permission-denied cases, causing existing-but-unreadable files to exit withEXIT_CONFIG_ERRORinstead ofEXIT_TRAINING_ERROR. This breaks the documented exit-code contract.Suggested fix
- if not os.path.isfile(path): - # Caller-input error: the path is missing or not a regular - # file. Per docs/standards/error-handling.md and the public - # exit-code contract in docs/reference/verify_annex_iv_subcommand.md, - # this is exit 1 (config / caller error). - _output_error_and_exit( - output_format, - f"Annex IV artifact not found: {path!r}.", - EXIT_CONFIG_ERROR, - ) try: result = verify_annex_iv_artifact(path) + except (FileNotFoundError, IsADirectoryError, NotADirectoryError): + # Caller-input error: the path is missing or not a regular file. + _output_error_and_exit( + output_format, + f"Annex IV artifact not found or not a regular file: {path!r}.", + EXIT_CONFIG_ERROR, + ) except json.JSONDecodeError as exc: # Validation failure: the artifact is reachable but not parseable # as JSON, OR the parsed root is not a JSON object. Both are # operator-actionable input errors per the public contract. _output_error_and_exit( output_format, f"Annex IV artifact at {path!r} is not valid JSON: {exc.msg} (line {exc.lineno}).", EXIT_CONFIG_ERROR, ) except OSError as exc: # Real I/O failure on an existing file (read error / permission # denied). Covers PermissionError and other runtime failures. _output_error_and_exit( output_format, f"Could not read Annex IV artifact {path!r}: {exc}.", EXIT_TRAINING_ERROR,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@forgelm/cli/subcommands/_verify_annex_iv.py` around lines 197 - 225, Remove the os.path.isfile(path) pre-check and move path validation into the try/except around verify_annex_iv_artifact(path); call verify_annex_iv_artifact(path) inside a try, catch FileNotFoundError and json.JSONDecodeError and call _output_error_and_exit(output_format, ... , EXIT_CONFIG_ERROR) for those, and catch OSError (including PermissionError/read errors) and call _output_error_and_exit(output_format, ... , EXIT_TRAINING_ERROR); ensure error messages reference path and include the original exception text; keep uses of verify_annex_iv_artifact, _output_error_and_exit, EXIT_CONFIG_ERROR, EXIT_TRAINING_ERROR, path and output_format to locate the changes.
♻️ Duplicate comments (3)
docs/usermanuals/en/deployment/chat.md (1)
66-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
--safetysection still mixes planned and current behavior.Line 68 reintroduces
--safety onas if it’s currently usable, but Line 66 says the flag is planned-only. Please make YAMLsafety:the only current instruction and keep--safetystrictly as preview wording.Based on learnings: "Every README and user-facing claim must point to real code or documented features; avoid documentation drift" and "User-facing documentation must be bilingual (English + Turkish mirrors); code, CLI output, logs, and config keys must be English only".Suggested wording fix
-With `--safety on`, every prompt and response is screened by Llama Guard: +With `safety: enabled: true` in YAML, every prompt and response is screened by Llama Guard:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/usermanuals/en/deployment/chat.md` around lines 66 - 69, The doc currently mixes the planned CLI flag --safety with the existing YAML safety: block; update the wording so the YAML safety: block is presented as the only current/usable mechanism and mark --safety strictly as a planned/preview feature (e.g., "planned for v0.6.0+ (preview)"). Specifically, remove or rephrase the sentence that reads "With `--safety on`, every prompt..." to instead show the example using the YAML `safety:` block and add a single clarifying preview note that `--safety` is not yet available and is planned for future releases; ensure all user-facing text remains English.docs/guides/human_approval_gate-tr.md (1)
229-230:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate cross-references to point to Turkish mirrors.
The Turkish guide currently links to English reference files. Update these to point to the Turkish equivalents for language parity.
🔗 Proposed fix
-- [`../reference/approve_subcommand.md`](../reference/approve_subcommand.md) — `approve` / `reject` için flag-başına, event-başına referans. -- [`../reference/approvals_subcommand.md`](../reference/approvals_subcommand.md) — `approvals` için flag-başına, event-başına referans. +- [`../reference/approve_subcommand-tr.md`](../reference/approve_subcommand-tr.md) — `approve` / `reject` için flag-başına, event-başına referans. +- [`../reference/approvals_subcommand-tr.md`](../reference/approvals_subcommand-tr.md) — `approvals` için flag-başına, event-başına referans.As per coding guidelines: "User-facing documentation must be bilingual (English + Turkish mirrors)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/human_approval_gate-tr.md` around lines 229 - 230, Replace the English reference links in the human_approval_gate-tr.md snippet so they point to the Turkish mirror files: change ../reference/approve_subcommand.md to ../reference/tr/approve_subcommand.md and ../reference/approvals_subcommand.md to ../reference/tr/approvals_subcommand.md (update the two lines containing the link texts referencing `approve_subcommand.md` and `approvals_subcommand.md` accordingly).site/js/translations.js (1)
340-341:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the real package entry point here instead of
forgelm.api.These strings now tell users to import a
forgelm.apisurface, but the public API documented elsewhere in this PR is the root package (from forgelm import .../forgelm.__all__). Shipping a non-existent import path in the site copy will send readers straight intoImportErrors.🔎 Verification script
#!/bin/bash set -euo pipefail echo "== root package exports ==" sed -n '1,220p' forgelm/__init__.py echo echo "== api module candidates ==" fd -a '^(api\.py|api)$' forgelm echo echo "== docs/site references ==" rg -n -C2 '\bforgelm\.api\b|__all__|__api_version__|ForgeTrainer|audit_dataset|verify_audit_log' README.md site/js/translations.js forgelmBased on learnings: "Every README and user-facing claim must point to real code or documented features; avoid documentation drift."
Also applies to: 422-423, 504-505, 586-587, 668-669, 750-751, 849-850, 952-953
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/js/translations.js` around lines 340 - 341, The site copy references a non-existent import path "forgelm.api"; update the translation strings (e.g. "home.cap.api.title"/"home.cap.api.body" and the other occurrences you noted) to instruct users to import from the root package (use "from forgelm import ..." or mention "forgelm" as the package) so the examples match the real public API (forgelm.__all__); search for "forgelm.api" in site/js/translations.js and replace with the correct root-package phrasing in each key to avoid ImportError.
🧹 Nitpick comments (1)
tests/test_gdpr_erasure.py (1)
450-467: ⚡ Quick winAssert the full redaction token, not just its prefix.
These checks only prove that some
<id_hash:marker exists. A regression to the wrong prefix length, non-hex characters, or mismatched payload/audit hashes would still pass. Since this PR is pinning the 12-char redaction contract, it would be safer to assert the exact<id_hash:[0-9a-f]{12}…>shape — ideally the same rendered token in both the JSON payload and thedata.erasure_failedevent.Also applies to: 510-521
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_gdpr_erasure.py` around lines 450 - 467, The test currently only checks for the presence of "<id_hash:"; update the assertions to validate the full redaction token shape and that the same token appears in both the JSON payload and the audit event: extract the redaction token from payload["error"] (matching the exact pattern "<id_hash:[0-9a-f]{12}…>" — 12 lowercase hex chars then an ellipsis or consistent suffix) and assert it matches a strict regex, then assert that the same extracted token equals the token found in failed.get("error_message", ""); apply the same stronger assertions to the other block around lines 510-521 as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/analysis/code_reviews/closure-plan-202604300906.md`:
- Around line 1374-1376: Update the checklist entry that currently reads
"`tools/check_cli_help_consistency.py --strict` exit 0 — 425/425 OK on HEAD" to
reflect the current HEAD result of 449/449 OK; locate the line mentioning
tools/check_cli_help_consistency.py in the closure-plan document (the checked
list item around the other tools) and change the numeric tally from 425/425 to
449/449 so the evidence matches the PR summary.
- Around line 119-120: Update the Wave 5 bullet to reflect that Round 5 has been
absorbed: change the phrase "4 absorption round'u absorbe edildi (rounds 1-4:
...)" to indicate "5 absorption rounds absorbed (rounds 1-5: ...)" and add the
Round 5 commit reference `e7cde64` and date (2026-05-06) into the parenthetical
so the `closure/wave5-integration` bullet accurately reflects the current PR
history and absorption status.
In `@docs/guides/air_gap_deployment.md`:
- Line 220: Update the example pip commands to use copy/paste-safe quoted extras
and the canonical extra name: replace unquoted `forgelm[<extras>]` with a
quoted, concrete example such as `forgelm["distributed"]` (or
`forgelm['distributed']`) in both the `pip download` and `pip install --no-index
--find-links` lines, and remove or avoid presenting non-canonical extras like
`deepspeed`; ensure both commands in the doc use the same quoted canonical extra
(`distributed`) so shells won't expand brackets and readers have a clear,
copyable example.
In `@docs/guides/human_approval_gate.md`:
- Around line 138-143: Fix the parenthetical text to accurately describe the
three JSONL rows shown: the first row is the trainer's human_approval.required
event, the second is human_approval.granted, and the third is
compliance.artifacts_exported (not an "approval requested" preamble); update the
sentence around human_approval.required / human_approval.granted /
compliance.artifacts_exported to reflect that ordering and relationship so it
matches the example.
In `@docs/reference/verify_annex_iv_subcommand-tr.md`:
- Around line 105-112: The example under "Hata: bozuk JSON" shows an exit code
of 2 but the codebase maps json.JSONDecodeError to EXIT_CONFIG_ERROR (value 1)
in the dispatcher; update the example output to show exit 1 so it matches the
exit-code table and the implementation (change the shown "$ echo $?" value from
2 to 1 in both the Turkish and English files for the verify-annex-iv subcommand
docs).
In `@docs/reference/verify_annex_iv_subcommand.md`:
- Around line 105-112: Update the malformed-JSON example so its exit code
matches the current error contract: change the shown exit status from 2 to 1 in
the example block that runs "forgelm verify-annex-iv compliance/annex_iv.json"
and the trailing "$ echo $?" line; ensure the narrative and code snippet reflect
that malformed JSON is a caller/input error with exit code 1 (per the exit codes
defined in error-handling.md).
In `@docs/usermanuals/en/data/secrets.md`:
- Around line 93-95: Documentation references a non-existent field
secret_findings_review_notes in the audit output; update the docs to remove that
reference and point readers to the actual AuditReport structure (use AuditReport
and its secrets_summary dict) or, if intended, implement and expose per-match
prose context by adding a secret_findings_review_notes field to the AuditReport
API and populating it during audit generation so excluded matches have
one-row-per-match review notes; ensure the docs and the AuditReport
type/signature remain consistent.
In `@docs/usermanuals/tr/deployment/verify-gguf.md`:
- Line 159: Update the Turkish usermanual link in verify-gguf.md to point to the
Turkish reference mirror instead of the English page: replace the target
../../../reference/verify_gguf_subcommand.md with
../../../reference/verify_gguf_subcommand-tr.md (the surrounding link text can
remain the same or be adjusted to indicate the TR variant); locate this in the
verify-gguf.md file where the reference link is defined.
In `@README.md`:
- Line 59: Update the README entry for the "forgelm purge" CLI to clarify that
audit-log entries are not redacted in place but preserved via appended
compensating events: change the wording around the existing line describing
"forgelm purge --subject-id <id>" to state that it "redacts data + adapters in
place, audit-log integrity preserved via appended compensating erasure events"
(or similar). Reference the implementation in compliance.py and _purge.py and
the "forgelm purge" command when making the edit so the README aligns with the
actual behavior of appending events rather than modifying existing audit-log
entries.
---
Outside diff comments:
In `@forgelm/cli/subcommands/_verify_annex_iv.py`:
- Around line 197-225: Remove the os.path.isfile(path) pre-check and move path
validation into the try/except around verify_annex_iv_artifact(path); call
verify_annex_iv_artifact(path) inside a try, catch FileNotFoundError and
json.JSONDecodeError and call _output_error_and_exit(output_format, ... ,
EXIT_CONFIG_ERROR) for those, and catch OSError (including PermissionError/read
errors) and call _output_error_and_exit(output_format, ... ,
EXIT_TRAINING_ERROR); ensure error messages reference path and include the
original exception text; keep uses of verify_annex_iv_artifact,
_output_error_and_exit, EXIT_CONFIG_ERROR, EXIT_TRAINING_ERROR, path and
output_format to locate the changes.
In `@forgelm/cli/subcommands/_verify_gguf.py`:
- Around line 9-14: The module docstring's exit-code summary is outdated; update
the paragraph that lists exit codes so it matches the dispatcher behavior by
naming EXIT_CONFIG_ERROR (1) for magic mismatch, corrupted metadata, SHA-256
mismatch, and input-validation failures such as missing or non-regular path, and
naming EXIT_TRAINING_ERROR (2) for runtime I/O failures on an existing file
(e.g., unreadable after opening); ensure the 0 case still states successful
verification (magic OK, metadata parses, SHA-256 matches sidecar) and explicitly
reference the constants EXIT_CONFIG_ERROR and EXIT_TRAINING_ERROR so readers can
correlate the docstring with the dispatcher logic.
---
Duplicate comments:
In `@docs/guides/human_approval_gate-tr.md`:
- Around line 229-230: Replace the English reference links in the
human_approval_gate-tr.md snippet so they point to the Turkish mirror files:
change ../reference/approve_subcommand.md to
../reference/tr/approve_subcommand.md and ../reference/approvals_subcommand.md
to ../reference/tr/approvals_subcommand.md (update the two lines containing the
link texts referencing `approve_subcommand.md` and `approvals_subcommand.md`
accordingly).
In `@docs/usermanuals/en/deployment/chat.md`:
- Around line 66-69: The doc currently mixes the planned CLI flag --safety with
the existing YAML safety: block; update the wording so the YAML safety: block is
presented as the only current/usable mechanism and mark --safety strictly as a
planned/preview feature (e.g., "planned for v0.6.0+ (preview)"). Specifically,
remove or rephrase the sentence that reads "With `--safety on`, every prompt..."
to instead show the example using the YAML `safety:` block and add a single
clarifying preview note that `--safety` is not yet available and is planned for
future releases; ensure all user-facing text remains English.
In `@site/js/translations.js`:
- Around line 340-341: The site copy references a non-existent import path
"forgelm.api"; update the translation strings (e.g.
"home.cap.api.title"/"home.cap.api.body" and the other occurrences you noted) to
instruct users to import from the root package (use "from forgelm import ..." or
mention "forgelm" as the package) so the examples match the real public API
(forgelm.__all__); search for "forgelm.api" in site/js/translations.js and
replace with the correct root-package phrasing in each key to avoid ImportError.
---
Nitpick comments:
In `@tests/test_gdpr_erasure.py`:
- Around line 450-467: The test currently only checks for the presence of
"<id_hash:"; update the assertions to validate the full redaction token shape
and that the same token appears in both the JSON payload and the audit event:
extract the redaction token from payload["error"] (matching the exact pattern
"<id_hash:[0-9a-f]{12}…>" — 12 lowercase hex chars then an ellipsis or
consistent suffix) and assert it matches a strict regex, then assert that the
same extracted token equals the token found in failed.get("error_message", "");
apply the same stronger assertions to the other block around lines 510-521 as
well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aec48a81-59ff-44f8-8aae-9d5aabdf243c
📒 Files selected for processing (30)
README.mddocs/analysis/code_reviews/closure-plan-202604300906.mddocs/guides/air_gap_deployment.mddocs/guides/human_approval_gate-tr.mddocs/guides/human_approval_gate.mddocs/reference/verify_annex_iv_subcommand-tr.mddocs/reference/verify_annex_iv_subcommand.mddocs/reference/verify_gguf_subcommand-tr.mddocs/reference/verify_gguf_subcommand.mddocs/roadmap.mddocs/usermanuals/en/compliance/gdpr-erasure.mddocs/usermanuals/en/data/leakage.mddocs/usermanuals/en/data/secrets.mddocs/usermanuals/en/deployment/chat.mddocs/usermanuals/en/deployment/verify-gguf.mddocs/usermanuals/tr/compliance/gdpr-erasure.mddocs/usermanuals/tr/data/ingestion.mddocs/usermanuals/tr/data/leakage.mddocs/usermanuals/tr/data/secrets.mddocs/usermanuals/tr/deployment/chat.mddocs/usermanuals/tr/deployment/verify-gguf.mddocs/usermanuals/tr/reference/exit-codes.mdforgelm/cli/subcommands/_purge.pyforgelm/cli/subcommands/_verify_annex_iv.pyforgelm/cli/subcommands/_verify_gguf.pysite/index.htmlsite/js/translations.jstests/test_check_cli_help_consistency.pytests/test_gdpr_erasure.pytests/test_verification_toolbelt.py
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/roadmap.md
| - ✅ Wave 4 (PR #33) — `closure/wave4-integration` → development, 2026-05-06 (merge commit `01e40ba`); ISO 27001 / SOC 2 alignment dalı + QMS bilingual mirror sweep + Faz 30 partial (Tier 1 ghost-feature drift + stat blocks). | ||
| - 🟡 Wave 5 (PR #34 open, implementation complete + merge pending) — `closure/wave5-integration` → development; Faz 30 full sweep tamamlandı (Tier 1 ghost-feature drift items, sample-audit user-facing doc triplet completion, `tools/check_cli_help_consistency.py` addition, full-suite anchor-checker `--strict` CI wire-up, site/* finalization, README/CONTRIBUTING/CLAUDE/roadmap/standards final pass). 4 absorption round'u absorbe edildi (rounds 1-4: gemini + CodeRabbit + multi-agent review). |
There was a problem hiding this comment.
Refresh Wave 5 status to include Round 5 absorption.
These bullets still say rounds 1–4 are absorbed, but the current PR history already includes Round 5 (e7cde64, 2026-05-06). Keeping the plan one round behind makes the closure status look stale right before merge.
Also applies to: 133-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/analysis/code_reviews/closure-plan-202604300906.md` around lines 119 -
120, Update the Wave 5 bullet to reflect that Round 5 has been absorbed: change
the phrase "4 absorption round'u absorbe edildi (rounds 1-4: ...)" to indicate
"5 absorption rounds absorbed (rounds 1-5: ...)" and add the Round 5 commit
reference `e7cde64` and date (2026-05-06) into the parenthetical so the
`closure/wave5-integration` bullet accurately reflects the current PR history
and absorption status.
| - [x] `tools/check_bilingual_parity.py --strict` exit 0 — 39/39 OK on HEAD. | ||
| - [x] `tools/check_site_claims.py --strict` exit 0 — 5 templates / 16 GPU profiles / version 0.5.5 OK. | ||
| - [x] `tools/check_cli_help_consistency.py --strict` exit 0 — 425/425 OK on HEAD (down from 449 in Wave 5 round-2 baseline due to ghost-YAML-block + parquet-ghost + 4 broken symbol-cite removals in round 3, see commit `dcbdcf5`); tool itself yeni (Wave 5 commit `c7bedc9`); strict CI flip in `fbb082d`. |
There was a problem hiding this comment.
Update the CLI-help strict count to the current HEAD result.
This acceptance item still records 425/425 OK, while the current PR summary reports 449/449 OK on HEAD. That leaves the closure evidence internally inconsistent even if the guard is green.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/analysis/code_reviews/closure-plan-202604300906.md` around lines 1374 -
1376, Update the checklist entry that currently reads
"`tools/check_cli_help_consistency.py --strict` exit 0 — 425/425 OK on HEAD" to
reflect the current HEAD result of 449/449 OK; locate the line mentioning
tools/check_cli_help_consistency.py in the closure-plan document (the checked
list item around the other tools) and change the numeric tally from 425/425 to
449/449 so the evidence matches the PR summary.
|
|
||
| ### "Do I need to bundle the Python packages too?" | ||
|
|
||
| Often, yes. The cache subcommands handle HF artefacts; Python wheels (`forgelm` + extras) are a separate problem. Use `pip download forgelm[<extras>] -d ./airgap-bundle/wheels` on the staging host and `pip install --no-index --find-links ./airgap-bundle/wheels forgelm[<extras>]` on the target. |
There was a problem hiding this comment.
Quote pip extras and show canonical extra names in the offline wheel example.
The command on Line 220 is easy to mis-copy: unquoted [] can be shell-expanded, and <extras> invites non-canonical values. Please use copy/paste-safe quoted examples and include the canonical distributed extra explicitly.
Proposed doc fix
-Often, yes. The cache subcommands handle HF artefacts; Python wheels (`forgelm` + extras) are a separate problem. Use `pip download forgelm[<extras>] -d ./airgap-bundle/wheels` on the staging host and `pip install --no-index --find-links ./airgap-bundle/wheels forgelm[<extras>]` on the target.
+Often, yes. The cache subcommands handle HF artefacts; Python wheels (`forgelm` + extras) are a separate problem. Use `pip download 'forgelm[eval]' -d ./airgap-bundle/wheels` (or `pip download 'forgelm[distributed]' -d ./airgap-bundle/wheels`) on the staging host and `pip install --no-index --find-links ./airgap-bundle/wheels 'forgelm[eval]'` (or `'forgelm[distributed]'`) on the target.Based on learnings: use the canonical pip extra name distributed in docs/examples and avoid presenting deepspeed as an install extra.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Often, yes. The cache subcommands handle HF artefacts; Python wheels (`forgelm` + extras) are a separate problem. Use `pip download forgelm[<extras>] -d ./airgap-bundle/wheels` on the staging host and `pip install --no-index --find-links ./airgap-bundle/wheels forgelm[<extras>]` on the target. | |
| Often, yes. The cache subcommands handle HF artefacts; Python wheels (`forgelm` + extras) are a separate problem. Use `pip download 'forgelm[eval]' -d ./airgap-bundle/wheels` (or `pip download 'forgelm[distributed]' -d ./airgap-bundle/wheels`) on the staging host and `pip install --no-index --find-links ./airgap-bundle/wheels 'forgelm[eval]'` (or `'forgelm[distributed]'`) on the target. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/guides/air_gap_deployment.md` at line 220, Update the example pip
commands to use copy/paste-safe quoted extras and the canonical extra name:
replace unquoted `forgelm[<extras>]` with a quoted, concrete example such as
`forgelm["distributed"]` (or `forgelm['distributed']`) in both the `pip
download` and `pip install --no-index --find-links` lines, and remove or avoid
presenting non-canonical extras like `deepspeed`; ensure both commands in the
doc use the same quoted canonical extra (`distributed`) so shells won't expand
brackets and readers have a clear, copyable example.
| After approval, three rows describe the gate's full lifecycle (the third is the genesis-bound "approval requested" preamble that pairs with the trainer's `human_approval.required` event for end-to-end correlation): | ||
|
|
||
| ```jsonl | ||
| {"event":"human_approval.required","run_id":"fg-abc123def456","operator":"gha:Acme/pipelines:training:run-42","staging_path":"outputs/run42/final_model.staging.fg-abc123def456","metrics":{...}} | ||
| {"event":"human_approval.granted","run_id":"fg-abc123def456","operator":"alice@acme.example","approver":"alice@acme.example","comment":"Reviewed eval report; S5 max 0.04 acceptable. Ticket #4711.","promote_strategy":"rename"} | ||
| {"event":"compliance.artifacts_exported","run_id":"fg-abc123def456","operator":"alice@acme.example","artifact_kind":"final_model","source":"final_model.staging.fg-abc123def456","destination":"final_model"} |
There was a problem hiding this comment.
Clarify the parenthetical description of the third audit row.
Line 138 says "the third is the genesis-bound 'approval requested' preamble that pairs with the trainer's human_approval.required event," but the actual third row shown in the example is compliance.artifacts_exported, not an "approval requested" event. The human_approval.required is already the first row. This mismatch could confuse readers trying to understand the lifecycle or auditors verifying the chain.
📝 Suggested fix
-After approval, three rows describe the gate's full lifecycle (the third is the genesis-bound "approval requested" preamble that pairs with the trainer's `human_approval.required` event for end-to-end correlation):
+After approval, three rows describe the gate's full lifecycle:Or, if the description is intentional, reword to match the example:
-After approval, three rows describe the gate's full lifecycle (the third is the genesis-bound "approval requested" preamble that pairs with the trainer's `human_approval.required` event for end-to-end correlation):
+After approval, three rows describe the gate's full lifecycle (the first is the trainer's pause event, the second is the reviewer's decision, and the third is the model-promotion artifact record):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/guides/human_approval_gate.md` around lines 138 - 143, Fix the
parenthetical text to accurately describe the three JSONL rows shown: the first
row is the trainer's human_approval.required event, the second is
human_approval.granted, and the third is compliance.artifacts_exported (not an
"approval requested" preamble); update the sentence around
human_approval.required / human_approval.granted / compliance.artifacts_exported
to reflect that ordering and relationship so it matches the example.
| ### Hata: bozuk JSON | ||
|
|
||
| ```shell | ||
| $ forgelm verify-annex-iv compliance/annex_iv.json | ||
| ERROR: Annex IV artifact at 'compliance/annex_iv.json' is not valid JSON: Expecting value (line 1). | ||
| $ echo $? | ||
| 2 | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Check the English mirror doc for the same broken-JSON example
fd "verify_annex_iv_subcommand.md" --type f -E "*-tr.md" --exec grep -n -A 6 "not valid JSON" {}
# 2. Check the implementation for what is raised on JSONDecodeError
fd "_verify_annex_iv.py" --exec grep -n -B 2 -A 4 "JSONDecodeError\|json\.loads\|json\.load\|EXIT_CONFIG_ERROR\|EXIT_RUNTIME_ERROR" {}Repository: cemililik/ForgeLM
Length of output: 2546
🏁 Script executed:
fd "_exit_codes.py" --type f --exec grep -n "EXIT_CONFIG_ERROR\|EXIT_SUCCESS\|EXIT_TRAINING_ERROR" {}Repository: cemililik/ForgeLM
Length of output: 299
Broken-JSON example shows exit 2 but the exit-code table assigns exit 1 to "bozuk JSON" — change line 111 to match implementation.
Line 32 of the exit-code table correctly lists bozuk JSON (malformed JSON content) as exit 1. The implementation confirms this: JSONDecodeError is explicitly caught and mapped to EXIT_CONFIG_ERROR (= 1), as stated in the docstring: "json.JSONDecodeError for parse failures (dispatcher → EXIT_CONFIG_ERROR since malformed JSON is a caller-input error)". The example at line 111 showing exit 2 contradicts both the exit-code table and the implementation.
🐛 Fix
$ echo $?
-2
+1Note: This bug exists in both the English (docs/reference/verify_annex_iv_subcommand.md lines 109–112) and Turkish versions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/reference/verify_annex_iv_subcommand-tr.md` around lines 105 - 112, The
example under "Hata: bozuk JSON" shows an exit code of 2 but the codebase maps
json.JSONDecodeError to EXIT_CONFIG_ERROR (value 1) in the dispatcher; update
the example output to show exit 1 so it matches the exit-code table and the
implementation (change the shown "$ echo $?" value from 2 to 1 in both the
Turkish and English files for the verify-annex-iv subcommand docs).
| ### Failure: malformed JSON | ||
|
|
||
| ```shell | ||
| $ forgelm verify-annex-iv compliance/annex_iv.json | ||
| ERROR: Annex IV artifact at 'compliance/annex_iv.json' is not valid JSON: Expecting value (line 1). | ||
| $ echo $? | ||
| 2 | ||
| ``` |
There was a problem hiding this comment.
Fix the malformed-JSON example’s exit code.
This example still shows $? == 2, but the contract in this doc now classifies malformed JSON as a caller/input error under exit code 1. Leaving the example at 2 will mislead operators who copy the snippet into CI checks.
Suggested doc fix
$ echo $?
-2
+1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/reference/verify_annex_iv_subcommand.md` around lines 105 - 112, Update
the malformed-JSON example so its exit code matches the current error contract:
change the shown exit status from 2 to 1 in the example block that runs "forgelm
verify-annex-iv compliance/annex_iv.json" and the trailing "$ echo $?" line;
ensure the narrative and code snippet reflect that malformed JSON is a
caller/input error with exit code 1 (per the exit codes defined in
error-handling.md).
| - **Test/example exclusion list** — common dummy values (`AKIAIOSFODNN7EXAMPLE`, `xxx`, `your_key_here`) bypass detection. | ||
|
|
||
| For a high-stakes audit (e.g. legal disclosure scan), use `--secrets-strict` to disable these guards and accept higher false-positive rate. | ||
| For a high-stakes audit (e.g. legal disclosure scan), the test-exclusion list is intentional — review the scan output's `secret_findings_review_notes` (one row per excluded match, with the prose context) so a human can confirm none of the dummies are real secrets in disguise. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=py -C2 '\bsecret_findings_review_notes\b'
rg -nP -C2 '\bsecret_findings_review_notes\b' --type=mdRepository: cemililik/ForgeLM
Length of output: 1301
🏁 Script executed:
#!/bin/bash
# Search ALL file types for the field (not just .py)
echo "=== Searching all files for secret_findings_review_notes ==="
rg -n '\bsecret_findings_review_notes\b' --type-list | head -5
rg -n '\bsecret_findings_review_notes\b' 2>/dev/null | head -20
# Look for output structure definitions
echo ""
echo "=== Searching for output/result structures ==="
rg -n 'class.*Output|class.*Result|def.*output|excluded.*match' --type=py -A 3 | head -30
# Search for "findings" or "review" related patterns
echo ""
echo "=== Searching for related field names ==="
rg -n 'review_notes|findings_review|excluded.*notes' --type=py | head -20
# Look at the codebase structure to understand main modules
echo ""
echo "=== Repository structure ==="
git ls-files | grep -E '\.(py|json|yaml)$' | grep -v __pycache__ | head -20Repository: cemililik/ForgeLM
Length of output: 3531
🏁 Script executed:
#!/bin/bash
# Find secrets-related code
echo "=== Looking for secrets module/scanning code ==="
find . -type f -name "*secret*" ! -path "./.git/*" | head -20
echo ""
echo "=== Search for scanning/audit output code ==="
rg -n 'excluded.*match|excluded_matches|scan.*output' --type=py -B 2 -A 2 | head -40
echo ""
echo "=== Look for dataclasses/types defining output structure ==="
rg -n '@dataclass|class.*Finding|class.*Scan' --type=py -A 5 | head -50
echo ""
echo "=== Search documentation file itself for context ==="
head -150 docs/usermanuals/en/data/secrets.md | tail -80Repository: cemililik/ForgeLM
Length of output: 9655
🏁 Script executed:
#!/bin/bash
# Read the secrets module
echo "=== Contents of forgelm/data_audit/_secrets.py ==="
wc -l forgelm/data_audit/_secrets.py
head -200 forgelm/data_audit/_secrets.py
echo ""
echo "=== Search for output/result definitions in _secrets.py ==="
rg -n 'def.*return|class.*Result|@dataclass' forgelm/data_audit/_secrets.py -A 5
echo ""
echo "=== Look for any review, notes, or excluded related fields ==="
rg -n 'review|notes|excluded|finding' forgelm/data_audit/_secrets.py -iRepository: cemililik/ForgeLM
Length of output: 5800
🏁 Script executed:
#!/bin/bash
# Search for audit report/output generation
echo "=== Audit subcommand implementation ==="
rg -n 'def audit|class.*Audit|secret.*finding' forgelm/cli/subcommands/ --type=py | head -20
echo ""
echo "=== Look for JSON output/reporting ==="
rg -n 'json\.dumps|\.to_dict|to_json|report' forgelm/data_audit/ --type=py | head -20
echo ""
echo "=== Check what gets printed/emitted from audit ==="
rg -n 'def audit|return.*{' forgelm/cli/subcommands/_audit.py -A 3 | head -40Repository: cemililik/ForgeLM
Length of output: 2849
🏁 Script executed:
#!/bin/bash
# Look at AuditReport class definition
echo "=== AuditReport class definition ==="
rg -n 'class AuditReport' forgelm/data_audit/ --type=py -A 20
echo ""
echo "=== Check _summary.py for all report fields used ==="
rg -n 'report\.' forgelm/data_audit/_summary.py | head -30
echo ""
echo "=== Search entire data_audit package for excluded or review ==="
rg -n 'excluded|review' forgelm/data_audit/ --type=py -iRepository: cemililik/ForgeLM
Length of output: 4028
🏁 Script executed:
#!/bin/bash
# Get the full AuditReport class definition
echo "=== Full AuditReport class ==="
sed -n '107,150p' forgelm/data_audit/_types.py
echo ""
echo "=== Check if secrets_findings field exists ==="
rg -n 'secrets_findings|findings_review|findings.*notes' forgelm/data_audit/_types.pyRepository: cemililik/ForgeLM
Length of output: 2010
Remove the reference to secret_findings_review_notes field—it does not exist in the audit output.
The AuditReport dataclass contains only secrets_summary (a dict of type counts), not a secret_findings_review_notes field with per-match prose context. The documentation instructs users to "review the scan output's secret_findings_review_notes" for excluded matches, but this field is never populated or exposed by the audit API. If high-stakes audits require detailed review of excluded matches, the feature needs to be implemented first.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/usermanuals/en/data/secrets.md` around lines 93 - 95, Documentation
references a non-existent field secret_findings_review_notes in the audit
output; update the docs to remove that reference and point readers to the actual
AuditReport structure (use AuditReport and its secrets_summary dict) or, if
intended, implement and expose per-match prose context by adding a
secret_findings_review_notes field to the AuditReport API and populating it
during audit generation so excluded matches have one-row-per-match review notes;
ensure the docs and the AuditReport type/signature remain consistent.
| - [GGUF Export](#/deployment/gguf-export) — bu doğrulayıcının tükettiği sidecar'ı yazan üretim tarafı. | ||
| - [Audit Log Doğrulama](#/compliance/verify-audit) — uyumluluk yüzeyindeki kardeş doğrulayıcı. | ||
| - [Annex IV Doğrulama](#/compliance/annex-iv) — teknik dokümantasyon artifact'ı için kardeş doğrulayıcı. | ||
| - [`verify_gguf_subcommand.md`](../../../reference/verify_gguf_subcommand.md) — tam bayrak tablosu ve kütüphane-sembol atıfları içeren referans doc. |
There was a problem hiding this comment.
Link the TR usermanual to the TR reference doc.
The EN reference target should be swapped for its Turkish mirror so a TR reader doesn't get bounced to the English page. The TR variant exists per this PR (docs/reference/verify_gguf_subcommand-tr.md).
📝 Proposed fix
-- [`verify_gguf_subcommand.md`](../../../reference/verify_gguf_subcommand.md) — tam bayrak tablosu ve kütüphane-sembol atıfları içeren referans doc.
+- [`verify_gguf_subcommand-tr.md`](../../../reference/verify_gguf_subcommand-tr.md) — tam bayrak tablosu ve kütüphane-sembol atıfları içeren referans doc.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [`verify_gguf_subcommand.md`](../../../reference/verify_gguf_subcommand.md) — tam bayrak tablosu ve kütüphane-sembol atıfları içeren referans doc. | |
| - [`verify_gguf_subcommand-tr.md`](../../../reference/verify_gguf_subcommand-tr.md) — tam bayrak tablosu ve kütüphane-sembol atıfları içeren referans doc. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/usermanuals/tr/deployment/verify-gguf.md` at line 159, Update the
Turkish usermanual link in verify-gguf.md to point to the Turkish reference
mirror instead of the English page: replace the target
../../../reference/verify_gguf_subcommand.md with
../../../reference/verify_gguf_subcommand-tr.md (the surrounding link text can
remain the same or be adjusted to indicate the TR variant); locate this in the
verify-gguf.md file where the reference link is defined.
| - **EU AI Act Compliance**: Auto-generated audit trails, data provenance (SHA-256), training manifests | ||
| - **ISO 27001 / SOC 2 Type II Alignment**: Software cannot be ISO/SOC 2 *certified* — only organisations can — but ForgeLM produces the audit-trail, change-management, data-lineage, and supply-chain evidence the deployer's auditor explicitly asks for. CycloneDX 1.5 SBOM per release, `pip-audit` nightly, `bandit` CI, append-only HMAC-chained audit log (HMAC integrity is active **only when** `FORGELM_AUDIT_SECRET` is set + KMS-managed + rotated between output-dir lifecycles — see [`docs/qms/access_control.md`](docs/qms/access_control.md) §3.4), Article 14 staging gate, Article 15/17 GDPR tooling. See [`docs/guides/iso_soc2_deployer_guide.md`](docs/guides/iso_soc2_deployer_guide.md) for the deployer audit cookbook + 93-control coverage map. | ||
| - **Library API**: `from forgelm import ForgeTrainer, audit_dataset, verify_audit_log, verify_annex_iv_artifact, verify_gguf, mask_pii, mask_secrets, ...` — every CLI surface has a stable Python entry point under `forgelm.__all__`, version-pinned via `forgelm.__api_version__` (decoupled from `__version__`). Run `python -c "import forgelm; print(sorted(forgelm.__all__))"` for the full surface. | ||
| - **GDPR Tooling**: `forgelm purge --subject-id <id>` (Article 17 right-to-erasure: redacts data + audit-log + adapters in place, hash chain preserved via compensating entries) and `forgelm reverse-pii --query <fragment>` (Article 15 right-of-access: locates PII matches across artefacts without re-loading raw data). |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== purge/audit implementation =="
while IFS= read -r f; do
echo "-- $f --"
set +e
rg -n -C3 'audit_log|append|compensat|redact|subject_id|purge' "$f"
status=$?
set -e
if [ "$status" -gt 1 ]; then
exit "$status"
fi
done < <(fd -a '(_purge\.py|compliance\.py|test_gdpr_erasure\.py)$' .)
echo
echo "== docs that describe purge semantics =="
rg -n -C2 'forgelm purge|append-only audit log|compensating' README.md docs site/js/translations.jsRepository: cemililik/ForgeLM
Length of output: 170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Looking for purge/audit related files =="
fd -a '(purge|compliance|gdpr|erasure)' . --type f | head -20
echo
echo "== Checking README.md directly =="
cat -n README.md | sed -n '55,65p'
echo
echo "== Searching for audit-log patterns in codebase =="
rg -n 'audit.?log|append.?only|compensat' . --type-list | head -1
rg -n 'audit.?log' . | head -20Repository: cemililik/ForgeLM
Length of output: 4212
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== _purge.py implementation =="
wc -l forgelm/cli/subcommands/_purge.py
head -100 forgelm/cli/subcommands/_purge.py
echo
echo "== compliance.py audit-log definition =="
wc -l forgelm/compliance.py
grep -n -A5 -B5 'audit.*log\|append' forgelm/compliance.py | head -50Repository: cemililik/ForgeLM
Length of output: 7063
Clarify that audit-log entries are preserved via appended events, not redacted in place.
Line 59 says forgelm purge "redacts data + audit-log + adapters in place, hash chain preserved via compensating entries". The implementation confirms that audit logs are append-only (Article 17(3)(b)) and the purge command appends six new erasure events (data.erasure_requested, data.erasure_completed, etc.) rather than modifying existing audit entries. The current wording misleadingly groups audit-log with data and adapters as if all three are redacted in place, when in fact the audit-log grows with compensating entries instead.
Reword to: "redacts data + adapters in place, audit-log integrity preserved via appended compensating erasure events" or similar to align with the append-only contract seen in compliance.py and _purge.py.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 59, Update the README entry for the "forgelm purge" CLI to
clarify that audit-log entries are not redacted in place but preserved via
appended compensating events: change the wording around the existing line
describing "forgelm purge --subject-id <id>" to state that it "redacts data +
adapters in place, audit-log integrity preserved via appended compensating
erasure events" (or similar). Reference the implementation in compliance.py and
_purge.py and the "forgelm purge" command when making the edit so the README
aligns with the actual behavior of appending events rather than modifying
existing audit-log entries.
…+ 9 inline + 1 NIT) CodeRabbit re-scanned the round-5 commit (e7cde64) and surfaced 2 OOD + 9 inline + 1 NIT findings. 10 absorbed, 1 skipped with verification. 16 files changed, +108/-59 LOC. HIGH/Major absorptions: - _verify_annex_iv.py L197-225 + _verify_gguf.py L195-218 OOD Major: refactor `os.path.isfile()` pre-check → in-try exception dispatch so permission-denied on an existing file routes to exit 2 (`EXIT_TRAINING_ERROR`) instead of exit 1. Previous `os.path.isfile` returns False for both "path doesn't exist" AND "path exists but user can't stat" (PermissionError); the public exit-code contract per error-handling.md needs the latter as exit 2. Now catches `(FileNotFoundError, IsADirectoryError)` → exit 1 (caller-input error) and `OSError` → exit 2 (genuine I/O failure on a reachable path). Order matters because `FileNotFoundError` is a subclass of `OSError`. Both files share the same shape and inline rationale comments. The `verify_gguf` and `verify_annex_iv` test names remain unchanged — the existing assertions on `ei.value.code == 1` for missing-path / malformed-JSON still pass; the fix is a classifier improvement. - _verify_gguf.py module docstring (OOD Minor): exit-code summary reconciled with round-5's dispatcher behaviour. Now reads: `0 → EXIT_SUCCESS; 1 → EXIT_CONFIG_ERROR (caller/input + integrity); 2 → EXIT_TRAINING_ERROR (genuine I/O on reachable path)` with explicit constant references so a future code reader can correlate the docstring with the dispatcher logic. - README.md L59 Major: clarified that `forgelm purge` does NOT redact audit-log entries in place. Article 17(3)(b) preservation: the audit log is append-only, and the purge command appends six compensating events (`data.erasure_requested`, `_completed`, etc.) rather than modifying existing audit entries. Previous wording "redacts data + audit-log + adapters in place" misleadingly grouped audit-log with data + adapters as if all three are redacted in place. Minor absorptions: - closure-plan L120 + L133: refresh Wave 5 status to include round 5 absorption — `5 absorption rounds absorbed (rounds 1-5; round 5 commit e7cde64, 2026-05-06)`. - air_gap_deployment.md (en+tr) L220: pip extras quoted + canonical extra names (`'forgelm[eval]'` / `'forgelm[distributed]'` instead of unquoted `forgelm[<extras>]`). Quote-rationale prose added so readers don't strip the quotes thinking they're cosmetic. - human_approval_gate.md (en+tr) L138: parenthetical fixed — previous round-4 wording said "the third is the genesis-bound 'approval requested' preamble" which contradicted the actual third row shown in the example (`compliance.artifacts_exported`). Now: first = trainer pause; second = reviewer terminal decision (granted/rejected); third = post-promotion artefact record. Same reword in TR mirror. - verify_annex_iv_subcommand.md (en+tr) L111: malformed-JSON example exit code 2 → 1 to match round-5 contract change. - secrets.md (en+tr) L93-95: drop reference to non-existent `secret_findings_review_notes` field (`AuditReport` exposes only `secrets_summary` dict). Replaced with concrete pointer to `AuditReport.secrets_summary` + per-row `--output-format json` (the canonical prose-review surface today). Note added that a dedicated review-notes envelope is on the v0.6+ roadmap. - TR usermanual verify-gguf.md L159: link target → `verify_gguf_subcommand-tr.md` (TR readers no longer get bounced to the EN reference page). - human_approval_gate-tr.md L229-230 (re-fix): TR cross-references → `*-tr.md` mirrors. Earlier round attempted this fix but the edit didn't persist; verified now via `grep -n 'approve_subcommand' docs/guides/human_approval_gate-tr.md` returning the `-tr.md` variants only. - chat.md (en) L68: `--safety on` mixed-behavior reword to make YAML `safety:` block the only currently-runnable mechanism and treat the `--safety` flag as v0.6.0+ preview output. (TR mirror was already correctly framed in earlier rounds — verified.) - site/js/translations.js: `forgelm.api` (a non-existent submodule; `python -c "import forgelm.api"` raises `ModuleNotFoundError`) replaced with `from forgelm import …` in 8 translation strings (en/tr/de/fr/es/zh × `home.cap.api.body` + en/tr × `features.ent.api.body`). Each updated string now mentions `forgelm.__all__` as the canonical surface registry. NIT absorption: - test_gdpr_erasure.py L450-467 + L510-521: strengthened the redaction-token assertion to a regex (`<id_hash:[0-9a-f]{12}…>`, exactly 12 lowercase hex chars + U+2026 ellipsis) and pinned that the SAME token appears in the JSON payload AND the audit-event `error_message` (cross-tool correlation invariant per the `_purge.py` contract). A regression that cropped the prefix to a different length, or computed a different hash for the audit emit, would now fail at unit-test time instead of slipping past a substring-only check. Skipped (with verification): - closure-plan L1376 stale `425/425` claim (CR Minor): bot says "current PR summary reports 449/449" but that is a hallucination. Verified live at HEAD `69ab304`: ``` $ python3 tools/check_cli_help_consistency.py --strict OK: 425 forgelm invocation(s) across docs, README.md all match the live parser surface. ``` The 425/425 figure in closure-plan is correct; the round-3 commit message explicitly documented the deliberate drop from 449 → 425 due to ghost-removal cleanup. SKIP. Self-review gauntlet (all green at HEAD): ruff format + check ✅ clean bilingual_parity --strict 39/39 ✅ anchor_resolution --strict 264/264 ✅ cli_help_consistency --strict 425/425 ✅ site_claims --strict ✅ field_descriptions --strict ✅ pytest --no-cov -q tests/ ✅ 1436 passed / 14 skipped (token regex assertion strengthens both the no-match leg and the multi-match leg of the redaction contract; verify-gguf/ annex-iv tests still pass with the in-try refactor) forgelm --config config_template.yaml --dry-run ✅ build_usermanuals.py: en/tr 65/65 OK ✅ User-confirmed (Turkish mid-round): EN/TR parity verified for the 4 docs/usermanuals files touched in round 6 (en+tr × secrets.md ghost-field; chat.md `--safety` framing — EN brought into line with TR's already-correct earlier-round wording; TR verify-gguf.md cross-locale reference link — EN already correct, TR newly fixed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round 6 absorption summary (commit
|
| File | EN | TR |
|---|---|---|
secrets.md ghost-field replacement |
✅ done | ✅ done (mirror translation) |
chat.md --safety framing |
✅ done in round 6 | ✅ already correct from earlier rounds |
verify-gguf.md cross-locale link |
✅ already correct (EN→EN) | ✅ done in round 6 (TR→TR) |
7-check gauntlet (all green at HEAD 7975afd)
| Check | Result |
|---|---|
ruff format + ruff check |
✅ clean |
check_bilingual_parity --strict |
✅ 39/39 |
check_anchor_resolution --strict |
✅ 264/264 |
check_cli_help_consistency --strict |
✅ 425/425 |
check_site_claims --strict |
✅ |
check_field_descriptions --strict |
✅ |
pytest --no-cov -q |
✅ 1436 passed / 14 skipped |
forgelm --config config_template.yaml --dry-run |
✅ |
build_usermanuals.py |
✅ en/tr 65/65 |
CI re-running on this push.
|
…UM + 34 LOW + 25 NIT findings PR #29 master review (8 parallel agents A1-A8 + consolidator A9) identified 102 deduped findings across release-ritual / standards / docs / config / tests / tools / closure-honesty surfaces. This commit absorbs ~80 of them in-PR; the remaining 19 are documented as v0.6.x deferrals in `docs/roadmap/risks-and-decisions.md` with the canonical Removal Contract (PR-merge + CHANGELOG-ref + CI-guard) gating their eventual removal. ## CRITICAL (1) — release ritual half-state * **F-PR29-A6-01** — `CHANGELOG.md` `[Unreleased]` → `[0.5.5] — 2026-05-06` rename + fresh `[Unreleased]` skeleton + compare-link footer. PyPI publish with the prior state would have shipped a v0.5.5 wheel whose CHANGELOG carried zero entries under `[0.5.5]`. ≅ A8-04. ## HIGH (12) — pre-merge contract drift * **F-PR29-A1-01** — `forgelm/data.py` lazy-import: `datasets` + `transformers` moved to function-body imports + `TYPE_CHECKING` block. `tests/test_lazy_imports.py` corpus extended (+6 modules: data, benchmark, safety, inference, synthetic, judge — all clean today). * **F-PR29-A1-02 / C-4** — 10 notebooks bumped to `forgelm==0.5.5` (was 9× `0.5.0` + 1× `>=0.5.2`); new `tools/check_notebook_pins.py` guard for future drift. * **F-PR29-A3-01** — `docs/usermanuals/{en,tr}/reference/exit-codes.md` trimmed to actual 5-code public contract (0/1/2/3/4); removed fictional `5 = Cost ceiling`, `130 = Ctrl+C`, `exit_status.txt` sidecar, `cost_tracking.halt_threshold_usd` references. * **F-PR29-A4-01 / A6-08** — `testing.md` `|| true` discipline codified: scanner + severity-tiering helper carve-out (pip-audit / bandit) is the only sanctioned shape; bare `|| true` still banned. * **F-PR29-A4-02** — `tests/test_smoke.py::test_cli_help_runs` rewritten to actually invoke `forgelm --help` and assert on `usage:` + `--config`. * **F-PR29-A5-01 / A5-04 / A5-08 / A5-09** — `safety_compliance-tr.md` rebuilt as full structural mirror of EN (38 H1-H4 entries vs prior 6); registered in `tools/check_bilingual_parity.py::_PAIRS` (40/40 now); `localization.md` demotes prior "bilingualised today" claim to "structurally bilingual; content translation pending v0.6.0". * **F-PR29-A6-02** — `tools/check_module_size.py` ships (warn ≥ 1000 LOC / fail ≥ 1500 LOC; 5 grandfathered modules listed). New modules cannot drift past the ceiling; existing splits routed to v0.6.x. * **F-PR29-A6-03** — *user action required*: PR #29 body checkboxes (Summary / Type / Testing / Checklist) — only the PR author can fill these; flagged here so the merger does not skip. * **F-PR29-A6-04** — `architecture.md` codifies the `_optional.py` silent-import-fallback carve-out: `_HAS_X = False` is sanctioned ONLY when (i) every read site gates on the boolean, (ii) the missing-extra path is behaviourally-equivalent slower, (iii) isolated to one module, (iv) tests monkeypatch. * **F-PR29-A6-05** — `forgelm/ingestion.py:518` `NotImplementedError` for `_chunk_semantic` now carries Phase 13+ deferral reason + issue TODO marker (was bare "planned for follow-up"). * **F-PR29-A7-01** — `docs/standards/release.md` drops the false "pip-audit runs in every release matrix" claim; pip-audit gates daily on `main` via `nightly.yml`, the publish pipeline emits SBOMs only. Adjacent `forgelm/cli.py:1424-1428` deprecation worked-example fixed to `forgelm/cli/_dispatch.py:165-172` (Phase 15 split). * **F-PR29-A7-02** — `docs/standards/code-review.md` drops gitleaks from the required-CI-check list; gitleaks lives in `.pre-commit-config.yaml` only as a local hook. * **F-PR29-A7-03** — 3× SKILL.md fixed: `--cov-fail-under=25` → 40 (matches `pyproject.toml` floor); add-trainer-feature SKILL stops pointing at the removed `forgelm/cli.py` (Phase 15 split renamed it to `forgelm/cli/_parser.py`); cut-release SKILL no longer claims `publish.yml` creates GitHub Releases (it doesn't). * **F-PR29-A8-01** — Closure-plan v-stamp ladder bumped to v2.8 + Wave 5 row promoted from `🟡 PR #34 open` to `✅ PR #34 merged 8f9f951`; ≅ A8-09 / A8-14. * **F-PR29-A8-02 / C-2** — Carry-over C-2 (build_usermanuals incremental cache + ProcessPoolExecutor) explicitly surfaced as **Open at HEAD despite CHANGELOG implying Closed**; routed to v0.6.x with a risks-and-decisions row that the Removal Contract gates. ## MEDIUM / LOW / NIT (89) — high-signal cluster summary * **Standards posture** — `coding.md` NOSONAR-discipline section (rule code + rationale required); `error-handling.md` named `except KeyboardInterrupt:` carve-out for CLI dispatch; `logging-observability.md` print() carve-out widened to `forgelm/cli/` + `forgelm/chat.py`; `documentation.md` cross-link convention codified; `localization.md` Türkçe template-title carve-out + DE/FR/ES/ZH deferred-tier section (with new `tools/check_site_chrome_parity.py` advisory guard). * **Library API surface** — `forgelm/__init__.py` underscore-rebound the `TYPE_CHECKING` / `annotations` / `config` import names so `dir(forgelm)` no longer leaks them; `compute_minhash` promoted to top-level export per design doc Experimental tier; design-doc §5.1 Status callout clarifies 3-segment `__api_version__`. * **Config docs** — `RetentionConfig` (5 fields + GDPR Article 17 deprecation) + `SafetyConfig.batch_size` + `JudgeConfig.{judge_api_base, batch_size}` all documented in EN+TR config reference; `config_template.yaml` RiskTier comments now list all 5 literals (was 3); commented `cost_tracking:` block flagged as v0.6.x planned. * **Code correctness** — `_get_version` triplication collapsed (`forgelm._version` is now the canonical source; `cli/_logging.py` + `compliance.py` delegate); `_dispatch.py` deprecation-audit emission `logger.debug` → `logger.warning` per error-handling.md best-effort hygiene; `_purge.py` row_id_hash None guard; `_verify_audit.py` exit-code constants honest (option errors → 1); `_ingest.py` lazy import of `forgelm.ingestion` (no longer pulled by every `import forgelm.cli`). * **Tests / CI hygiene** — `tests/__init__.py` package marker added (fixes `from tests._helpers...` import semantics); `tests/conftest.py` fixture body uses qualified path; `tests/_helpers/__init__.py` re-exports `minimal_config` (was zero-byte, CLAUDE.md anti-pattern); `tests/test_check_module_size.py` (25 tests, NEW); `tests/test_check_pip_audit.py` (8 tests, NEW); 6 lazy-import regression tests added (data, benchmark, safety, inference, synthetic, judge); `test_compliance.py` + `test_supply_chain_security.py` assertions updated for the verify-audit + pip-audit policy changes above. * **Tooling** — `tools/check_pip_audit.py` UNKNOWN severity flipped from silent advisory to fail-closed (pip-audit JSON omits OSV severity for almost every real vuln; failing closed forces explicit triage); `tools/check_site_claims.py` regex tightened (`\s` → `[ \t]`, satisfies regex.md Rule 5/8 in addition to existing Rule 4 NOSONAR); `tools/check_bandit.py` + `check_pip_audit.py` docstrings reference the new `|| true` carve-out; `tools/generate_sbom.py` NOSONAR/nosec dual-style normalised; `tests/bench_simhash.py` → `tools/bench_simhash.py` (`git mv`, history preserved); 9 of 10 notebook pins bumped to v0.5.5 (1 outlier `>=0.5.2` aligned). * **Workflows** — `.github/workflows/{ci,nightly,publish}.yml` severity policy comments standardised; nightly latest-deps trade-off documented; 3.11-only coverage upload rationale commented; `publish.yml` got a `--ignore=tests/test_cost_estimation.py` companion existence guard; `.markdownlint.json` deleted (no consumer in CI / pre-commit, was ghost config); `.pre-commit-config.yaml` got 5 advisory local hooks for the doc / parity guards. * **pyproject.toml** — `torch>=2.3.0` floor (was 2.0.0; `FSDPModule` added in 2.3); `[tool.bandit] skips=["B101"]` removed (was redundant with `exclude_dirs=["tests"]`, silently allowed in-product asserts); `[tool.coverage.run] omit=["forgelm/templates/__init__.py"]` (no-code namespace package); `[tool.pytest.ini_options] markers = [...]` declared (5 markers; tagging itself deferred to v0.6.x); extras Phase-marker comments standardised to functional descriptions; `nightly.yml` test-min-deps `torch==2.1.0` → `2.3.0`. * **Honesty sweep** — CHANGELOG stat blocks desensitised (literal test counts dropped in favour of `pytest --collect-only -q`); CLAUDE.md + CONTRIBUTING.md + testing.md test-count claims softened to ephemeral phrasing; CONTRIBUTING.md adds `[dev]` extras / `--cov-fail-under=40` note for first-time contributors; `phase-12-6-closure-cycle.md` Wave 5 status promoted to ✅; `roadmap.md` `38 fazlar` clarified as `37 content + 1 release tag`, Phase 12.6 status correctly shows `complete on development, ready for PR #29 → main`; mermaid arrow direction note added. ## Deferred to v0.6.x (19 items, all in risks-and-decisions.md) Architecture (5): A1-05 ForgeTrainer split, A1-06/07/08 four 1000-LOC ceiling violations, A6-02 module-size-guard rollout. Tests (4): A4-03 factory adoption sweep, A4-04 phase/faz rename sweep, A4-05 runtime_smoke disposition, A8-10 batching determinism test. Security (3): A6-07 PEM regex line-walker, **A6-12 secret-mask centralisation (real bug — operator HF_TOKEN can leak in doctor JSON)**, A8-07 Sonar UI Mark-as-Safe pass. Config (2): A6-14 allow_anonymous_operator → YAML, A2-04 cost_tracking config block implementation. Tooling (3): A7-10 Ruff D-rules pilot, A8-02 / C-2 build_usermanuals perf optimization (Open at HEAD despite CHANGELOG implying Closed — surfaced explicitly), A6-15-followup bare-NOSONAR rule-code sweep. Doc / process (4): A6-18 usermanuals parity registry (F-W5-S5), A8-16 finding-ID convention rolling enforcement, A5-anchor-slugifier GFM divergence, A2-13 design-doc forgelm/api.py references. Each row carries a Removal Contract: PR-merge + CHANGELOG-ref + CI-guard (where applicable) before the row leaves the table. ## Verification | Check | Result | |------------------------------------------------|--------| | `ruff format --check .` | ✅ 173 files clean | | `ruff check .` | ✅ all checks passed | | `check_bilingual_parity --strict` | ✅ 40/40 | | `check_anchor_resolution --strict` | ✅ 264/264 | | `check_cli_help_consistency --strict` | ✅ 423/423 | | `check_site_claims --strict` | ✅ artefacts/templates/GPU/version | | `check_field_descriptions --strict` | ✅ every Pydantic field | | `check_module_size` | ✅ 5 grandfathered, 0 fail | | `check_notebook_pins --strict` | ✅ 10/10 pin v0.5.5 | | `forgelm --config config_template.yaml --dry-run` | ✅ exit 0 | | `pytest tests/` | ✅ 1479 passed / 14 skipped / 0 failed (10:31 min) | ## Files 81 changed (+2186 / −328): 12 source (forgelm/), 11 tests, 25 docs, 10 notebooks, 8 tools (3 NEW), 3 SKILL.md, 3 workflow YAML, root configs (CHANGELOG / CLAUDE / CONTRIBUTING / pyproject / config_template / pre-commit), `.markdownlint.json` deleted, `tests/bench_simhash.py` → `tools/bench_simhash.py`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>



Summary
verify-audit/verify-annex-iv/verify-gguf/purge/reverse-pii/approve/approvals/doctor/cache-models+cache-tasks/safety-eval+ library-API + performance + ISO/SOC 2 deployer + supply-chain.tools/check_anchor_resolution.py --strict(264/264 OK after 36→0 baseline cleanup) +tools/check_cli_help_consistency.py --strict(449/449 OK after 40→0 baseline; new 749-LOC tool with 15 pinned tests).forgelm benchmark --model), Feat/site multilang user guide #16 (--export-bundle), release(v0.5.0): Phase 11 + 11.5 + 12 + 12.5 consolidated release #18 (kserve/triton), Closure/wave1 faz9 32 #20 (ingest flag drift) all resolved EN+TR.phase-12-6-closure-cycle.mdsummary.pyproject.tomlversion bump0.5.1rc1→0.5.5(brought forward from Faz 33 becausecheck_site_claims.py --strictenforces site/code version match).10 commits, 127 files changed, +10051 / −671 LOC.
Faz 30 Acceptance — all 16 items closed
2a32842_meta.yaml+8 page entries2834d62build_usermanuals.pyrebuildcheck_bilingual_parity.py --strictcheck_site_claims.py --strictcheck_cli_help_consistency.py --strictcheck_anchor_resolution.py --strict4610dc64610dc6+ newphase-12-6-closure-cycle.md4610dc6(5 pages)localization.md[Unreleased]Wave 5 sectione9f9b8fe18baa0)fbb082d+cbaeebf(model-merging)2a32842Validation gauntlet
All 5 strict CI guards green on HEAD
cbaeebf:python3 tools/check_bilingual_parity.py --strict— 39/39 OKpython3 tools/check_anchor_resolution.py --strict— 264/264 OKpython3 tools/check_cli_help_consistency.py --strict— 449/449 OKpython3 tools/check_site_claims.py --strict— green (5 templates / 16 GPU profiles / version 0.5.5)python3 tools/check_field_descriptions.py --strict forgelm/config.py— greenPlus:
ruff format /check— clean (167 files unchanged)python3 -m pytest --no-cov -q— 1428 passed / 14 skippedforgelm --config config_template.yaml --dry-run— greenTest plan
python3 -m pytest --no-cov -qpasses 1428.forgelm --config config_template.yaml --dry-runexits 0._subcommand.md, runforgelm <cmd> --help, confirm flag list matches.gh pr viewshows green CI before merge.docs/analysis/code_reviews/wave5-review-prompt.md(matches Wave 3/4 cadence).Next step (POST-WAVE)
Faz 33 — v0.5.5 PyPI release. Once this merges to
development:development: rename CHANGELOG[Unreleased]→[0.5.5] — YYYY-MM-DD, finalisedocs/roadmap/releases.mdv0.5.5 row.cut-releaseskill tagsv0.5.5.publish.yml→ cross-OS test matrix → PyPI publish.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
forgelm doctor(health checks),forgelm approve/reject(human approval gate),forgelm cache-*(air-gap support),forgelm purge/reverse-pii(GDPR rights),forgelm verify-*(audit, Annex IV, GGUF), andforgelm safety-eval.Documentation
Tests & Validation