[cemililik]

Summary

This PR consolidates the first 8 phases of the v0.5.5 closure plan (closure-plan-202604300906.md) into a single foundation bundle. All 8 phases were implemented in parallel git worktrees and merged here for review.

Closes 8 Critical findings + 30+ Major findings from master-review-opus-202604300906.md. 1322 lines added/modified across 55 files.

Phase breakdown

Faz	Scope	Critical/Major closures
1	Site & doc honesty + count drift sweep	F-business-001/002/003 (3 Critical), F-business-004/007/014, M-DOC-001/010/011
2	CI gates & standards drift cleanup	F-test-001 (Critical cov-fail-under), F-test-006/010/017, F-business-011
3	Operator identity + audit forensic completeness	F-compliance-102 (Critical), F-compliance-114/117/120/121; new audit_event_catalog.md (+ TR)
4 v2	Lazy torch + safety/judge batching + paragraph chunker + lang_sample slice	F-performance-101/102/103/106
5	Notebook PyPI pin (==0.5.0) + nightly grep guard	F-business-005, M-DOC-005
6	forgelm verify-audit subcommand + library function	F-compliance-103 (Critical)
7	safe_post HTTP discipline (Theme ε) — new forgelm/_http.py	M-201, M-202
8	Webhook lifecycle vocabulary — notify_reverted + notify_awaiting_approval	F-compliance-104

Detailed task lists, file:line evidence, and trade-off notes for each phase are in the squashed commit message.

Acceptance gates

• pytest tests/ --ignore=tests/test_cost_estimation.py → 881 passed, 13 skipped, 0 failed (pre-existing F-code-015 cluster test_cost_estimation.py import error → Faz 12 fixture consolidation)
• ruff format --check forgelm/ tests/ tools/ → 81 files clean
• ruff check forgelm/ tests/ tools/ → all checks passed
• forgelm --config config_template.yaml --dry-run → valid
• Coverage gate enforced (pyproject [tool.pytest.ini_options].addopts = "--cov-fail-under=40")
• No conflict markers; manual conflict resolution in 4 places (safety.py audit_logger+batch_size, test_compliance.py Faz 3 + Faz 6 test classes, audit_event_catalog.md/-tr.md Faz 3 vocabulary + Faz 8 webhook section, test_webhook.py Faz 7 SafePost + Faz 8 LifecycleVocabulary)
• Patch path migration: all 32 webhook tests use @patch("forgelm._http.requests.post") (the new central HTTP boundary)
• Slack token literal broken into fragments per regex.md Rule 7 so GitHub secret scanning passes

Test plan

• Code review: each phase as a logical unit (commit message has phase boundaries)
• CI green on closure/foundation-faz1-8 branch
• Manual smoke: forgelm verify-audit <some_audit_log.jsonl> exits 0 on a valid log
• Manual smoke: a webhook receiver sees event=training.reverted (not training.failure) when an evaluation gate rejects a run
• Manual smoke: setting FORGELM_OPERATOR=alice produces operator=alice in audit log; unsetting it on a distroless container raises ConfigError (the intended behavior)

Deferred (within plan, not dropped)

These are documented in the closure plan, with explicit phase ownership:

• F-performance-105 (build_usermanuals.py incremental cache + ProcessPoolExecutor) → Faz 30 final doc sweep
• safety_compliance-tr.md full structural mirror → Faz 30
• docs/usermanuals/** generated drift (byod-domain-expert, math-reasoning, "18 GPU profiles", annex_iv.json) → Faz 30 user-manual rebuild
• docs/reference/usage*.md "18 GPU" mention → Faz 30
• F-compliance-101 Article 14 path-b real staging directory + forgelm approve subcommand → Faz 9
• post_training_workflow.ipynb forgelm>=0.5.2 pin → Faz 33 release PR (bump to ==0.5.5)
• Pre-existing tests/test_cost_estimation.py:7 collection error (F-code-015 / F-test-005 cluster) → Faz 12 fixture consolidation

Companion documents

• 📋 closure-plan-202604300906.md — full 33-phase plan
• 🔬 master-review-opus-202604300906.md — source review (175 findings)
• 🏗 split-design-data_audit-cli-202604300906.md — Faz 14-15 split tasarımı

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • CLI and programmatic audit-log verification; new webhook notifications for revert and awaiting-approval; batched generation across safety/judge/training.

• Configuration

  • Batch-size default enforced (8); operator identity now pinned via env var; CI coverage gate moved into project config and workflows refined.

• Documentation

  • Major additions/updates to compliance guides, audit-event catalog, roadmap, standards, and translated materials.

• Improvements

  • Hardened audit durability and recovery; centralized outbound HTTP/webhook safety with masking and SSRF protections.

• Tests

  • Expanded test coverage for compliance, HTTP safety, webhooks, lazy imports, batching, and audit verification.

[qodo-code-review]

ⓘ 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.

[sourcery-ai]

Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e7d78600-96dd-4d5d-a7d3-42b93b2e3989

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR consolidates Phase 11-12.5 work for v0.5.0 release, introducing HTTP safety controls via a new module, audit log verification capability, hardened operator attribution, lazy ML imports, batch processing for evaluation, and pinned PyPI notebook installations alongside comprehensive documentation updates.

Changes

Cohort / File(s)	Summary
CI/Workflow Configuration ​.github/workflows/ci.yml, ​.github/workflows/nightly.yml, ​.github/workflows/usermanuals-validate.yml	Adds workflow-wide FORGELM_OPERATOR=ci-smoke; adjusts CI fail-fast and coverage reporting; nightly adds notebook install-string enforcement; usermanuals validate runs on main/development pushes for docs paths.
HTTP Safety & SSRF Protection forgelm/_http.py, forgelm/judge.py, forgelm/synthetic.py, forgelm/webhook.py	Adds safe_post and HttpSafetyError, centralizing HTTPS-default, SSRF/private-IP blocking, timeout floor, no-redirects, TLS/CA precedence; rewires judge/synthetic/webhook to use safe_post, updates webhook payloads and re-exports safety helpers.
Audit Log Verification forgelm/cli.py, forgelm/compliance.py	Adds forgelm verify-audit CLI and verify_audit_log() with VerifyResult; validates SHA-256 hash chain, optional per-line HMAC, cross-checks manifest sidecar, and maps results to specific exit codes.
Operator Attribution & Durability forgelm/compliance.py, tests/conftest.py	Removes "unknown" fallback; requires FORGELM_OPERATOR or derives user@host (with opt-in anonymous behavior); audit writes now fsync after flush; tests pin operator env for deterministic runs.
Lazy ML Imports & Trainer forgelm/model.py, forgelm/trainer.py	Defers heavy ML imports (torch, peft, transformers) to call sites; stringifies _build_lora_config return annotation; trainer imports deferred and revert/audit notifier wiring refactored.
Batch Processing & Generation forgelm/judge.py, forgelm/safety.py, forgelm/config.py	Introduces batch_size (default 8) in safety/judge paths, batched generation with left padding and per-batch OOM fallback to per-prompt retries; adds SafetyEvalThresholds dataclass and SafetyConfig.batch_size config field.
Data Processing Optimizations forgelm/data_audit.py, forgelm/ingestion.py	Truncates language-sample payloads to 512 chars to bound memory; precomputes paragraph token counts in batch for chunking; minor comments/clarifications.
Webhook Enhancements forgelm/webhook.py	Moves posting to safe_post, adds _mask_and_truncate_reason, new notify methods notify_reverted and notify_awaiting_approval, includes model_path in payloads, and documents re-exports in __all__.
Notebooks & Quickstarts notebooks/*.ipynb, site/quickstart.html	Pins ForgeLM installs to forgelm[qlora]==0.5.0 across notebooks; updates quickstart template names (domain-expert, grpo-math).
Documentation: Audit & Compliance docs/reference/audit_event_catalog.md, docs/reference/audit_event_catalog-tr.md, docs/guides/safety_compliance.md, docs/guides/safety_compliance-tr.md	Adds canonical audit event catalog, audit-log integrity guides, CLI/programmatic verification docs, HMAC/genesis-manifest behavior, and webhook mapping/security guidance.
Documentation: Standards & Release docs/standards/*.md, CHANGELOG.md, README.md, CONTRIBUTING.md	Adds deprecation policy, updates testing coverage floor and counts, documents release consolidation for v0.5.0, updates README counts and quickstart templates, and replaces static project-structure ASCII with architecture link.
Site/UI & Translations site/index.html, site/features.html, site/compliance.html, site/js/translations.js	Updates GPU profile count from 18→16, changes annex/artifact filenames, quickstart template labels, and translation strings across locales.
Tests: Compliance & Verification tests/test_compliance.py, tests/*	Adds extensive tests for operator resolution, fsync durability, classifier-load audit events, HF dataset fingerprinting, end-to-end audit verification (hash-chain, truncation, HMAC, manifest), and CLI exit codes; many test updates to assert safe_post usage and lazy-import behavior (new tests added: lazy import subprocess checks, judge/synthetic/webhook safety tests, GRPO trust_remote_code assertions).
Build / Config / Ignore pyproject.toml, .gitignore, tools/build_usermanuals.py	Adds pytest addopts to enable coverage and enforce 40% fail-under; updates .gitignore for docs/analysis and .claude/worktrees/; adjusts usermanuals build print marker.

Sequence Diagram(s)

sequenceDiagram
    participant User as User/CLI
    participant Verify as Verify-CLI<br/>(forgelm verify-audit)
    participant Core as verify_audit_log<br/>(compliance)
    participant File as Filesystem<br/>(audit_log.jsonl / manifest)
    participant Hash as HashChain<br/>(SHA-256 validation)
    participant HMAC as HMAC<br/>(optional per-line check)

    User->>Verify: forgelm verify-audit <path> [--require-hmac]
    Verify->>Verify: Resolve HMAC secret from env
    alt require-hmac & secret missing
        Verify-->>User: exit(2)
    end
    Verify->>Core: verify_audit_log(path, hmac_secret?, require_hmac)
    Core->>File: open/read audit_log.jsonl (stream lines)
    Core->>Hash: compute/compare genesis & prev_hash per line
    alt hash mismatch
        Core-->>Verify: VerifyResult(valid=False, first_invalid_index=N, reason=...)
        Verify-->>User: exit(1)
    end
    alt hmac provided/required
        Core->>HMAC: compute per-line HMAC and compare
        alt hmac mismatch
            Core-->>Verify: VerifyResult(valid=False, first_invalid_index=N, reason="HMAC mismatch")
            Verify-->>User: exit(1)
        end
    end
    Core-->>Verify: VerifyResult(valid=True, entries_count=N)
    Verify-->>User: exit(0)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Phase 12 — Data Curation Maturity (v0.5.2) #13: Directly overlaps on audit verification, HMAC/genesis sidecar behavior, and HTTP safety consolidation — strong code-level similarity.
• feat: Phase 10 post-training completion (v0.4.0) #7: Related webhook/compliance/judge HTTP refactor and SSRF-guarding patterns that this PR extends.
• release(v0.5.0): Phase 11 + 11.5 + 12 + 12.5 consolidated release #18: Touches data-audit and CLI audit surfaces; closely related to the data-audit and verification work here.

Poem

🐰 A cautious hop through guarded posts,

hashes braided, HMACs like ghosts,
Notebooks pinned, lazy imports play,
Batches hum and tests hold sway.
Audit bells ring for v0.5's day.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 78.47% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: consolidating the first 8 phases of a closure plan into a foundation bundle, which is the core purpose of this PR.
Description check	✅ Passed	The PR description is comprehensive with a clear summary, phase breakdown, acceptance gates, and test plan. It follows the template structure with detailed context about what was changed and why.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch closure/foundation-faz1-8

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codacy-production]

Not up to standards ⛔

🔴 Issues 1 minor

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category	Results
BestPractice	1 minor

View in Codacy

🟢 Metrics 155 complexity · 6 duplication

Metric	Results
Complexity	155
Duplication	6

View in Codacy

_{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.}

[gemini-code-assist]

Code Review

This pull request introduces significant enhancements to the ForgeLM audit and compliance infrastructure, including a new verify-audit CLI command for integrity checking, improved operator identity attribution, and hardened outbound HTTP communication via a centralized safe_post utility. Additionally, it consolidates several roadmap phases into the v0.5.0 release and includes various documentation updates. I have identified two actionable improvements: the redundancy in the webhook event documentation and the memory scalability concerns in the audit log verification function.

[coderabbitai]

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

tests/conftest.py (1)

34-35: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle empty FORGELM_OPERATOR as missing in the test fixture

Line 34 preserves an empty environment value (""), which can still break operator resolution. Use a truthy fallback so empty values become "test-operator".

Suggested patch

-    monkeypatch.setenv("FORGELM_OPERATOR", os.environ.get("FORGELM_OPERATOR", "test-operator"))
+    monkeypatch.setenv("FORGELM_OPERATOR", os.environ.get("FORGELM_OPERATOR") or "test-operator")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/conftest.py` around lines 34 - 35, The test fixture currently preserves
an empty FORGELM_OPERATOR value; change the monkeypatch call to use a truthy
fallback so empty strings become "test-operator": read the environment with
os.environ.get("FORGELM_OPERATOR") or use "test-operator" and pass that into
monkeypatch.setenv("FORGELM_OPERATOR", value). Update the code around the
monkeypatch.setenv call to use this truthy fallback (referencing
monkeypatch.setenv and the FORGELM_OPERATOR env var).

forgelm/trainer.py (1)

1156-1169: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pass self.audit into run_safety_evaluation so safety-gate failures are audit-chained.

At Line 1156, the safety evaluation call omits audit_logger, so classifier/load-path failures in the safety module may not be recorded into the run’s append-only audit trail.

Suggested fix

         return run_safety_evaluation(
             model=self.trainer.model,
             tokenizer=self.tokenizer,
             classifier_path=safety_cfg.classifier,
             test_prompts_path=safety_cfg.test_prompts,
             max_safety_regression=safety_cfg.max_safety_regression,
             output_dir=output_dir,
             scoring=getattr(safety_cfg, "scoring", "binary"),
             min_safety_score=getattr(safety_cfg, "min_safety_score", None),
             min_classifier_confidence=getattr(safety_cfg, "min_classifier_confidence", 0.7),
             track_categories=getattr(safety_cfg, "track_categories", False),
             severity_thresholds=getattr(safety_cfg, "severity_thresholds", None),
             batch_size=getattr(safety_cfg, "batch_size", 8),
+            audit_logger=self.audit,
         )

As per coding guidelines: "**/*.py: Every decision gate must emit a structured event to an append-only audit log; never edit or delete entries".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/trainer.py` around lines 1156 - 1169, The call to
run_safety_evaluation is missing the audit logger so safety-gate failures won't
be append-only audited; update the invocation in trainer.py to pass
audit_logger=self.audit (i.e., add audit_logger=self.audit to the argument list
of run_safety_evaluation), and if run_safety_evaluation's signature does not yet
accept audit_logger, modify its function signature and any downstream callers to
accept and forward an audit_logger parameter so all classifier/load-path
failures are emitted to the append-only audit log.

docs/roadmap-tr.md (1)

18-31: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Sync the roadmap graph with the newly published v0.5.0 status.

The text now marks v0.5.0 as released, but the Mermaid timeline still shows it as pending, so the same page reports two different states.

Suggested doc fix

-    P11 -.-> V2[v0.5.0 pending]
+    P11 -.-> V2[v0.5.0]

As per coding guidelines: "**/*.md: Every README claim must point to real code; avoid documentation drift between marketing claims and actual implementation".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/roadmap-tr.md` around lines 18 - 31, The roadmap text shows v0.5.0 as
released but the embedded Mermaid timeline still marks it pending, causing a
doc-state mismatch; locate the Mermaid timeline block that references "v0.5.0"
(search for the timeline containing the labels `v0.5.0` / `v0.5.1` etc. and the
string "Mermaid" or "timeline") and update its node/status for `v0.5.0` to
"released"/completed (match the prose "Yayınlandı: v0.5.0"), then run a quick
scan in the same document for other mentions of `v0.5.0` (e.g., headings like
"PyPI'deki son sürüm") to ensure all occurrences use the released state so the
Mermaid chart and text remain consistent.

🧹 Nitpick comments (3)

CLAUDE.md (1)

60-60: ⚡ Quick win

Avoid hard-coded test-module counts in guidance docs

The explicit 47 test modules value is likely to drift again; prefer stable wording (or a generated value) to keep this file evergreen.

Suggested edit

-├── tests/                   # pytest, 47 test modules
+├── tests/                   # pytest test suite (see `tests/` for current module set)

As per coding guidelines, "Every README claim must point to real code; avoid documentation drift between marketing claims and actual implementation."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` at line 60, Replace the hard-coded test count in the docs line
"tests/                   # pytest, 47 test modules" with stable wording or a
dynamic/generated value; specifically edit the CLAUDE.md entry for the tests
directory (the line containing "pytest, 47 test modules") to remove the literal
"47 test modules" and replace it with a non-drifting phrase like "pytest test
modules" or insert a build-time/generated count token so the README never
asserts a fixed number.

forgelm/webhook.py (1)

9-10: 💤 Low value

Intentional re-export for backwards compatibility.

The _is_private_destination as _is_private_destination import is a deliberate re-export so existing tests/consumers can continue importing from forgelm.webhook. The static analysis F401 warning is a false positive for this re-export pattern.

To silence the linter while keeping the intent clear, consider adding a noqa comment:

-from ._http import _is_private_destination as _is_private_destination  # re-export for back-compat
+from ._http import _is_private_destination as _is_private_destination  # noqa: F401 — re-export for back-compat

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/webhook.py` around lines 9 - 10, The re-export of
_is_private_destination is intentional but triggers F401; update the import line
that re-exports _is_private_destination (the "_is_private_destination as
_is_private_destination" import) to silence the linter by appending a noqa
(e.g., "# noqa: F401") so the symbol remains exported for back-compat while
suppressing the unused-import warning; ensure the safe_post and HttpSafetyError
imports remain unchanged.

forgelm/judge.py (1)

163-230: 💤 Low value

Batched generation with correct CUDA OOM resilience.

The implementation correctly:

• Sets padding_side="left" for decoder-only generation (Line 185)
• Restores original padding side in finally block (Line 228)
• Falls back to per-prompt generation on CUDA OOM (Lines 205-217)
• Clears CUDA cache before retry (Lines 212-215)

Optional: Consider extracting the OOM retry logic to reduce cognitive complexity.

SonarCloud flags complexity 19 > 15, which is justified by the nested error handling. If desired, the OOM-retry loop could be extracted to a helper, but the current structure is readable and the resilience behavior is correct.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/judge.py` around lines 163 - 230, Refactor the nested
batched-generation error handling in _generate_responses_batched by extracting
the batch-level generation + OOM/retry logic into a new helper (e.g.,
_generate_batch_with_oom_retry) that takes (model, tokenizer, batch,
batch_start, max_new_tokens) and returns List[str]; move the try/except
torch.cuda.OutOfMemoryError and generic Exception handling (including
torch.cuda.empty_cache() and per-prompt fallbacks calling _generate_response)
into that helper and have _generate_responses_batched simply iterate batches,
call the helper, and extend responses; keep tokenizer.padding_side management
(original_padding_side restore) and device moves in _generate_responses_batched
to avoid changing behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/reference/audit_event_catalog-tr.md`:
- Around line 90-91: Insert a single blank line between the end of the table row
containing "Satır başına HMAC-SHA-256" and the "## Webhook olayları" heading so
the table is separated from the section heading; update the content around the
table end and the "## Webhook olayları" line to ensure a blank line exists
immediately before the heading.

In `@docs/reference/audit_event_catalog.md`:
- Around line 71-73: There are duplicate "## Webhook events" headings causing
TOC/navigation confusion; fix by either renaming the first occurrence to a
clarifying intro heading (e.g., "## Webhook events (intro)" or "## Webhook
events — scope and note") or by removing the brief paragraph under the first "##
Webhook events" and keeping only the detailed section that starts at the later
"## Webhook events"; update the heading text or remove the redundant paragraph
so the document contains a single, clearly titled "Webhook events" section.

In `@docs/roadmap.md`:
- Around line 16-19: The roadmap shows v0.5.0 as released in the text but the
Mermaid diagram still labels v0.5.0 as "pending"; open the Mermaid graph block
(the node for v0.5.0 around the Mermaid definition at/near line 50) and update
that node's label/state to match the text (e.g., change "v0.5.0 pending" to
"v0.5.0 — Released" or the ✅ icon), and apply the same change to the other
inconsistent nodes referenced (lines 27-32) so the diagram and the "Released:
v0.5.0" text are consistent.

In `@docs/standards/logging-observability.md`:
- Around line 173-177: Update the retention guidance reference in
logging-observability.md: replace the incorrect filename
"<output_dir>/compliance/audit.jsonl" with the canonical
"<output_dir>/compliance/audit_log.jsonl" so it matches the rest of the docs and
downstream scripts; confirm the string change appears in the paragraph under
"Retention guidance" and run a quick grep across the repo to ensure all other
references use "audit_log.jsonl" for consistency.
- Around line 141-154: The document uses two different vocabularies for the same
webhook lifecycle events (the table lists
training.start/training.success/training.failure while Rule 4 and audit mappings
use training.started/training.succeeded/failed); pick and apply one canonical
set (preferably the dot-separated forms used in the table or update the table to
the past-tense forms) and make these changes consistently across the doc: update
Rule 4's example strings, the Notifier method names, the paired audit event
mappings (e.g., ensure training.start ↔ training.started or both use the same
token), and any gating flags (`webhook.notify_on_*`) so all references use the
identical event identifiers.

In `@forgelm/config.py`:
- Around line 255-256: Add validation to the SafetyConfig dataclass so
batch_size has ge=1 (e.g., batch_size: int = Field(8, ge=1) or equivalent) so
invalid 0/negative values fail at parse time; after adding this, remove the
defensive resets that silently coerce values (the checks that do "if batch_size
<= 0: batch_size = 1") from the code paths in safety.py and judge.py (the places
that read SafetyConfig.batch_size and reassign batch_size) so the config parse
enforces correctness instead of runtime correction.

In `@notebooks/dpo_alignment.ipynb`:
- Line 21: Update the pinned ForgeLM package version in the notebook pip install
line so it matches pyproject.toml: replace occurrences of
"forgelm[qlora]==0.5.0" with "forgelm[qlora]==0.5.1rc1" in the notebooks (e.g.,
the pip install command found in notebooks dpo_alignment.ipynb,
galore_memory_optimization.ipynb, grpo_reasoning.ipynb,
kto_binary_feedback.ipynb, multi_dataset.ipynb, quickstart_sft.ipynb,
safety_evaluation.ipynb, and synthetic_data_training.ipynb) to ensure the
notebooks use the same ForgeLM version as the project.

---

Outside diff comments:
In `@docs/roadmap-tr.md`:
- Around line 18-31: The roadmap text shows v0.5.0 as released but the embedded
Mermaid timeline still marks it pending, causing a doc-state mismatch; locate
the Mermaid timeline block that references "v0.5.0" (search for the timeline
containing the labels `v0.5.0` / `v0.5.1` etc. and the string "Mermaid" or
"timeline") and update its node/status for `v0.5.0` to "released"/completed
(match the prose "Yayınlandı: v0.5.0"), then run a quick scan in the same
document for other mentions of `v0.5.0` (e.g., headings like "PyPI'deki son
sürüm") to ensure all occurrences use the released state so the Mermaid chart
and text remain consistent.

In `@forgelm/trainer.py`:
- Around line 1156-1169: The call to run_safety_evaluation is missing the audit
logger so safety-gate failures won't be append-only audited; update the
invocation in trainer.py to pass audit_logger=self.audit (i.e., add
audit_logger=self.audit to the argument list of run_safety_evaluation), and if
run_safety_evaluation's signature does not yet accept audit_logger, modify its
function signature and any downstream callers to accept and forward an
audit_logger parameter so all classifier/load-path failures are emitted to the
append-only audit log.

In `@tests/conftest.py`:
- Around line 34-35: The test fixture currently preserves an empty
FORGELM_OPERATOR value; change the monkeypatch call to use a truthy fallback so
empty strings become "test-operator": read the environment with
os.environ.get("FORGELM_OPERATOR") or use "test-operator" and pass that into
monkeypatch.setenv("FORGELM_OPERATOR", value). Update the code around the
monkeypatch.setenv call to use this truthy fallback (referencing
monkeypatch.setenv and the FORGELM_OPERATOR env var).

---

Nitpick comments:
In `@CLAUDE.md`:
- Line 60: Replace the hard-coded test count in the docs line "tests/           
# pytest, 47 test modules" with stable wording or a dynamic/generated value;
specifically edit the CLAUDE.md entry for the tests directory (the line
containing "pytest, 47 test modules") to remove the literal "47 test modules"
and replace it with a non-drifting phrase like "pytest test modules" or insert a
build-time/generated count token so the README never asserts a fixed number.

In `@forgelm/judge.py`:
- Around line 163-230: Refactor the nested batched-generation error handling in
_generate_responses_batched by extracting the batch-level generation + OOM/retry
logic into a new helper (e.g., _generate_batch_with_oom_retry) that takes
(model, tokenizer, batch, batch_start, max_new_tokens) and returns List[str];
move the try/except torch.cuda.OutOfMemoryError and generic Exception handling
(including torch.cuda.empty_cache() and per-prompt fallbacks calling
_generate_response) into that helper and have _generate_responses_batched simply
iterate batches, call the helper, and extend responses; keep
tokenizer.padding_side management (original_padding_side restore) and device
moves in _generate_responses_batched to avoid changing behavior.

In `@forgelm/webhook.py`:
- Around line 9-10: The re-export of _is_private_destination is intentional but
triggers F401; update the import line that re-exports _is_private_destination
(the "_is_private_destination as _is_private_destination" import) to silence the
linter by appending a noqa (e.g., "# noqa: F401") so the symbol remains exported
for back-compat while suppressing the unused-import warning; ensure the
safe_post and HttpSafetyError imports remain unchanged.

🪄 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: 630ccd5a-b6ab-4d91-aeb8-816db72d9ff7

📥 Commits

Reviewing files that changed from the base of the PR and between 6b515ed and 885395d.

📒 Files selected for processing (55)

• .github/workflows/ci.yml
• .github/workflows/nightly.yml
• .github/workflows/usermanuals-validate.yml
• .gitignore
• CHANGELOG.md
• CLAUDE.md
• CONTRIBUTING.md
• README.md
• docs/guides/safety_compliance-tr.md
• docs/guides/safety_compliance.md
• docs/qms/sop_data_management.md
• docs/reference/audit_event_catalog-tr.md
• docs/reference/audit_event_catalog.md
• docs/roadmap-tr.md
• docs/roadmap.md
• docs/roadmap/releases.md
• docs/roadmap/risks-and-decisions.md
• docs/standards/logging-observability.md
• docs/standards/release.md
• docs/standards/testing.md
• forgelm/_http.py
• forgelm/cli.py
• forgelm/compliance.py
• forgelm/config.py
• forgelm/data_audit.py
• forgelm/ingestion.py
• forgelm/judge.py
• forgelm/model.py
• forgelm/safety.py
• forgelm/synthetic.py
• forgelm/templates/domain-expert/README.md
• forgelm/trainer.py
• forgelm/webhook.py
• notebooks/dpo_alignment.ipynb
• notebooks/galore_memory_optimization.ipynb
• notebooks/grpo_reasoning.ipynb
• notebooks/kto_binary_feedback.ipynb
• notebooks/multi_dataset.ipynb
• notebooks/quickstart_sft.ipynb
• notebooks/safety_evaluation.ipynb
• notebooks/synthetic_data_training.ipynb
• pyproject.toml
• site/compliance.html
• site/features.html
• site/index.html
• site/js/translations.js
• site/quickstart.html
• tests/conftest.py
• tests/test_compliance.py
• tests/test_grpo_reward.py
• tests/test_judge_functions.py
• tests/test_lazy_imports.py
• tests/test_synthetic.py
• tests/test_webhook.py
• tools/build_usermanuals.py

[coderabbitai]

Actionable comments posted: 18

🧹 Nitpick comments (1)

tests/test_safety_advanced.py (1)

54-79: ⚡ Quick win

Add a negative config case for evaluation.safety.batch_size.

This bundle moved batch_size correctness to parse time, but the changed tests only exercise valid configs. A small load_config(...) case for batch_size=0 (and ideally -1) raising ConfigError would lock in the new contract and catch any regression back to silent coercion.

🧪 Suggested test shape

+    def test_invalid_batch_size_fails_config_load(self, tmp_path):
+        import yaml
+        from forgelm.config import ConfigError, load_config
+
+        data = {
+            "model": {"name_or_path": "org/model"},
+            "lora": {},
+            "training": {},
+            "data": {"dataset_name_or_path": "org/dataset"},
+            "evaluation": {"safety": {"enabled": True, "batch_size": 0}},
+        }
+        cfg_path = str(tmp_path / "config.yaml")
+        with open(cfg_path, "w") as f:
+            yaml.dump(data, f)
+
+        with pytest.raises(ConfigError, match="batch_size"):
+            load_config(cfg_path)

Based on learnings, “Reliability before features: every new capability ships with tests, docs, and CI coverage; 'I'll add tests later' means the PR is not ready.”

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_safety_advanced.py` around lines 54 - 79, Add a negative test that
ensures load_config raises ConfigError when evaluation.safety.batch_size is
invalid: create a YAML config similar to test_full_config_yaml but set
evaluation.safety.batch_size to 0 (and a second case -1), write it to tmp_path,
and assert with pytest.raises(ConfigError) when calling load_config(cfg_path);
place the new test (e.g., test_safety_batch_size_invalid) alongside
test_full_config_yaml and reference load_config and ConfigError in the test.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/analysis/code_reviews/closure-plan-202604300906.md`:
- Line 1464: The table cell contains a literal regex example
(r'!"#$%&\'()*+,-./:;<=>?@[\]^_`{|}~') with unescaped pipe characters, causing
markdownlint MD056 and column misalignment; fix by either moving the regex
example out of the table into a fenced code block or escape each pipe character
in the table cell (replace '|' with '\|') so the row renders correctly and the
MD056 warning is resolved.
- Around line 1327-1328: Summary: The table rows for "Cross-OS scope" and
"Pre-commit policy" have incorrect phase numbers; update them to match the
definitions above. Locate the table row entries for "Cross-OS scope" and
"Pre-commit policy" and change their phase values from "Faz 30" and "Faz 31" to
"Faz 31" and "Faz 32" respectively so they align with the previously defined
phases. Ensure the labels "Cross-OS scope" and "Pre-commit policy" remain
unchanged to preserve lookup consistency.
- Around line 227-228: Phase 5 currently pins several notebooks to
'forgelm[qlora]==0.5.5' (and `forgelm==0.5.5`) which would break installs before
the release in Phase 33; revert the Phase 5 changes so the affected notebook
entries (`dpo_alignment`, `safety_evaluation`, `grpo_reasoning`,
`kto_binary_feedback`, `galore_memory_optimization`, `multi_dataset`,
`synthetic_data_training`) continue to use the existing git+https dependency
spec (`git+https://github.com/cemililik/ForgeLM.git`) and leave the PyPI version
bump for `quickstart_sft.ipynb` and `data_curation.ipynb` to Phase 33 where
`0.5.5` is published, ensuring Phase 5 only contains the git+https guard and not
the hard `==0.5.5` pins.
- Around line 1093-1124: The release gate as written is invalid because
publish.yml cannot declare needs: cross-os-tests across different workflow files
and because tests install the editable source (pip install -e .) instead of the
packaged artifacts users will install; update the CI by either moving the
cross-os-tests job into the same workflow that contains the publish job (ensure
the job name cross-os-tests is used as the dependency) or replace the
cross-workflow dependency with a workflow_run trigger in publish.yml that waits
for the release-prep workflow to finish, and change the Test step in
release-prep.yml’s cross-os-tests job to build the distribution (wheel/sdist)
and install that artifact (build via python -m build or similar, then pip
install the generated .whl/.tar.gz) before running pytest so you validate the
actual packaged install rather than pip install -e ..

In `@docs/analysis/code_reviews/foundation-pr-review-20250504-opus.md`:
- Around line 13-17: Update the report wording wherever it claims "except
Exception" catches KeyboardInterrupt/SystemExit—specifically revise findings
F-001, F-002, F-003, F-007, and F-008 and the text at the listed line ranges—to
state that "except Exception" will swallow unexpected runtime errors (e.g.,
ValueError, TypeError) but does NOT catch KeyboardInterrupt or SystemExit;
reserve the interrupt/exit claim only for bare except: or except BaseException:
handlers and adjust the rationale and severity language accordingly so the
justification no longer depends on the incorrect inheritance claim.

In `@docs/analysis/code_reviews/split-design-data_audit-cli-202604300906.md`:
- Around line 499-506: The markdown table header declares five columns but rows
C-1..C-6 only have four cells, breaking rendering; fix by either adding the
missing fifth cell (e.g., populate the "Risk" column for each row) for rows
labeled "C-1" through "C-6" or by updating the header to match four columns
(remove one header column and its separator) so the column count is consistent
across the header and rows; adjust the rows referenced by the symbols "C-1",
"C-2", "C-3", "C-4", "C-5", and "C-6" accordingly.

In `@docs/reference/audit_event_catalog.md`:
- Around line 98-104: Update the audit-event mirror for the failure row in
docs/reference/audit_event_catalog.md so it uses the actual audit event name
emitted by the system (change the mirror from `pipeline.failed` to
`training.failed`) and ensure the corresponding Turkish bilingual doc
audit_event_catalog-tr.md is updated in the same way so both English and Turkish
versions stay in sync; verify the row for `training.failure` still lists the
same gated-by and payload fields after the change.

In `@forgelm/_http.py`:
- Around line 117-133: _mask_secrets_in_text currently only replaces raw header
values, but exception strings can include the full request URL (which may embed
the bearer token); update _mask_secrets_in_text to also redact any URL-like
substrings that contain secret header values or token-like patterns: after
replacing header values, scan the text for URL patterns (e.g., "http://" or
"https://") and if a URL substring contains any of the values for sensitive
headers (Authorization, X-API-Key, proxy auth) or matches common token patterns,
replace that entire URL substring with "[REDACTED_URL]"; apply the same change
to the duplicate masking logic referenced around the second block (the one at
lines 223-229) so both places redact webhook URLs as well as header values.

In `@forgelm/compliance.py`:
- Around line 61-103: The code currently gates anonymous audit mode by reading
the FORGELM_ALLOW_ANONYMOUS_OPERATOR env var inside the operator resolution path
(see operator_env, FORGELM_ALLOW_ANONYMOUS_OPERATOR, and the ConfigError raise),
which allows ambient env to change Article 12 behavior; instead remove env
sniffing and require the opt-in to come from validated configuration or an
explicit constructor parameter (e.g. add or read Config.allow_anonymous_operator
or an allow_anonymous_operator arg passed to the class initializer), then use
that config/arg to decide whether to raise ConfigError or set self.operator to
"anonymous@{hostname}"; update any callers/tests to pass the new flag and remove
reliance on the environment variable.
- Around line 544-560: The enrichment currently only catches (ImportError,
OSError, ValueError) causing unexpected HF Hub errors from HfApi.dataset_info to
propagate; update _fingerprint_hf_revision so the import of huggingface_hub is
handled separately (catch ImportError) and the call to
HfApi().dataset_info(dataset_path) is wrapped in its own try/except Exception
that logs and skips on any error—this ensures compute_dataset_fingerprint()
remains best-effort while still surfacing import failures distinctly.

In `@forgelm/judge.py`:
- Around line 213-240: Add an explicit API-boundary check at the start of
_generate_responses_batched to ensure batch_size is a positive integer (>0):
before touching the tokenizer or entering the loop, validate
isinstance(batch_size, int) and batch_size > 0, and raise a clear ValueError
like "batch_size must be a positive integer" if the check fails; apply the same
early validation to the other batched-generation helper that follows the same
pattern so invalid batch_size values are rejected with a descriptive error
rather than failing deep inside range().

In `@forgelm/safety.py`:
- Around line 86-98: The current _generate_one_safety_response function swallows
all exceptions and returns an empty string; narrow the handler to only expected
generation/ CUDA OOM failures and let other exceptions propagate: catch
torch.cuda.OutOfMemoryError (and optionally RuntimeError instances whose message
contains "out of memory"), log the full exception and call
torch.cuda.empty_cache(), then return ""; for any other Exception re-raise it so
the safety run fails loudly. Apply the same change to the other fallback handler
using the same pattern (the handler around lines 142-151) so only
OOM/generation-specific errors are handled and all unexpected errors surface.
- Around line 464-473: The code currently swallows failures from
audit_logger.log_event (in the classifier load failure path) by logging a
warning and proceeding to raise only the original RuntimeError, which loses the
required audit entry; change the except block around audit_logger.log_event so
that any exception from audit emission is not downgraded—re-raise it (or raise a
new exception that chains the audit exception to preserve both contexts) instead
of calling logger.warning, ensuring the audit failure surfaces and the overall
operation fails; refer to audit_logger.log_event, the
"audit.classifier_load_failed" event, the except Exception as audit_exc handler,
and the final raise RuntimeError(str(e)) from e when implementing this change.

In `@forgelm/trainer.py`:
- Around line 886-888: The notifier call in _run_training_pipeline is publishing
an "awaiting approval" event with model_path=final_path which points at the
already-finalized final_model/ artifacts; change the call to either supply the
real staging path (e.g., pass staging_path or a newly computed
staging_model_path instead of final_path) or alter the event name/payload in
notifier.notify_awaiting_approval (e.g., rename to
notify_approval_of_finalized_model or add a
lifecycle_state="finalized"/source="final" field) so the webhook accurately
reflects that the artifacts are already finalized; update references to run_name
and model_path accordingly.
- Around line 534-538: The revert path must emit the structured audit event
_EVT_REVERT_TRIGGERED on every code path that leads to a revert; update
_revert_model (the function which currently calls
self.notifier.notify_reverted(run_name=self.run_name, reason=...)) to first emit
the structured audit event (including run_name, reason, and source/gate like
"NaN/Inf" or threshold) via the existing audit/event API used by
benchmark/safety/judge, then call notify_reverted; ensure
execute_evaluation_checks continues to call _revert_model so NaN/Inf and
threshold failures now produce both the webhook and the matching
_EVT_REVERT_TRIGGERED audit record.

In `@forgelm/webhook.py`:
- Around line 101-112: The call to safe_post in forgelm/webhook.py currently
hard-codes allow_insecure_http=True; change this to require an explicit,
validated configuration flag (e.g., webhook_allow_plaintext or
allow_insecure_http) instead of enabling plaintext by default. Update the
safe_post invocation in the function that posts webhooks so allow_insecure_http
is set from the validated config flag (default False) and reject/log/raise if an
http:// URL is used without the opt-in; add/ensure the validation lives in your
config parsing layer so the flag cannot be toggled via env-sniffing and is
explicitly opted-in via YAML. Ensure references to url, payload, timeout,
ca_bundle, and allow_private remain unchanged except for driving
allow_insecure_http from the validated config.

In `@tests/test_compliance.py`:
- Around line 500-518: The helper _build_log currently mutates os.environ to set
FORGELM_AUDIT_SECRET and then pops it at the end, but if AuditLogger(...) or
logger.log_event(...) raises the env var can leak; update _build_log to ensure
the env var is always restored by wrapping the env set and logger
creation/events in a try/finally (or use pytest's
monkeypatch.setenv/monkeypatch.delenv) so that FORGELM_AUDIT_SECRET is removed
or restored in the finally block regardless of exceptions; reference _build_log,
AuditLogger, and logger.log_event when applying the change.

In `@tests/test_webhook.py`:
- Line 308: The trailing comment on the bearer_token assignment (bearer_token =
"sk-" + "supersecret123") combines noqa and NOSONAR, which prevents Sonar from
recognizing the suppression; split them into two separate comments so linters
parse both—leave "# noqa: S105" on the assignment and add a separate "# NOSONAR
test fixture, fragment-built" comment (or vice‑versa) adjacent to the same line.

---

Nitpick comments:
In `@tests/test_safety_advanced.py`:
- Around line 54-79: Add a negative test that ensures load_config raises
ConfigError when evaluation.safety.batch_size is invalid: create a YAML config
similar to test_full_config_yaml but set evaluation.safety.batch_size to 0 (and
a second case -1), write it to tmp_path, and assert with
pytest.raises(ConfigError) when calling load_config(cfg_path); place the new
test (e.g., test_safety_batch_size_invalid) alongside test_full_config_yaml and
reference load_config and ConfigError in the test.

🪄 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: 29a5a015-eb46-47dd-8e61-b36a6a75e1b3

📥 Commits

Reviewing files that changed from the base of the PR and between 885395d and 2dabd83.

📒 Files selected for processing (27)

• .gitignore
• CHANGELOG.md
• CLAUDE.md
• docs/analysis/code_reviews/closure-plan-202604300906.md
• docs/analysis/code_reviews/foundation-pr-review-20250504-opus.md
• docs/analysis/code_reviews/foundation-pr-review-prompt.md
• docs/analysis/code_reviews/master-review-opus-202604300906.md
• docs/analysis/code_reviews/split-design-data_audit-cli-202604300906.md
• docs/reference/audit_event_catalog-tr.md
• docs/reference/audit_event_catalog.md
• docs/roadmap-tr.md
• docs/roadmap.md
• docs/standards/logging-observability.md
• forgelm/_http.py
• forgelm/compliance.py
• forgelm/config.py
• forgelm/data_audit.py
• forgelm/judge.py
• forgelm/safety.py
• forgelm/trainer.py
• forgelm/webhook.py
• tests/conftest.py
• tests/test_compliance.py
• tests/test_judge_functions.py
• tests/test_safety_advanced.py
• tests/test_synthetic.py
• tests/test_webhook.py

✅ Files skipped from review due to trivial changes (2)

• CLAUDE.md
• CHANGELOG.md

🚧 Files skipped from review as they are similar to previous changes (5)

• .gitignore
• tests/test_synthetic.py
• forgelm/data_audit.py
• docs/roadmap-tr.md
• docs/roadmap.md

[coderabbitai]

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

forgelm/compliance.py (1)

152-173: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

The 4 KiB fallback can hash a truncated last record.

If the final JSONL entry is larger than the tail window, Lines 167-172 reread from the same seek_start and hash lines[-1], which is only the suffix of that entry. That seeds _prev_hash from partial JSON and breaks the chain on the next append. Walk backwards until you find the newline before the last record instead of rehashing a fixed tail slice.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/compliance.py` around lines 152 - 173, The current 4 KiB fallback can
hash a truncated last record; change the fallback (the branch that re-reads from
seek_start and then calls self._decode_lines(tail) and hashes lines[-1]) to
instead scan the read tail backwards for the last newline and extract the
complete final line before hashing. Concretely: after re-reading tail, find the
index of the last '\n' (or last newline byte) in tail, slice tail from that
newline+1 to the end to obtain the full final record, decode that slice with
self._decode_lines (or decode appropriately), and then compute the
hashlib.sha256 of that full record before assigning _prev_hash; if no newline is
found, treat as the entire tail as the final record (or fall back to "genesis")
to avoid seeding _prev_hash with a truncated JSONL entry.

♻️ Duplicate comments (4)

forgelm/safety.py (2)

464-473: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't downgrade audit.classifier_load_failed to best-effort.

If classifier loading fails and audit_logger.log_event(...) also fails, this path only warns and returns the classifier error. That loses the required audit record exactly when the safety gate is failing; re-raise the audit failure (or chain both exceptions) instead of swallowing it.

Based on learnings, "Every decision gate must emit a structured audit log event; never edit or delete entries from the append-only audit log".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/safety.py` around lines 464 - 473, The current exception handling
when classifier loading fails swallows any exception from audit_logger.log_event
(in the audit_logger.log_event call for "audit.classifier_load_failed") and only
logs a warning, then raises the original RuntimeError; instead, ensure the audit
failure is not lost by propagating it or chaining it with the classifier error:
in the exception block around audit_logger.log_event, raise the audit exception
(or raise a combined exception) so the function does not silently drop the audit
failure—replace the logger.warning(...) swallow with re-raising audit_exc (or
raise a new Exception that includes both the original classifier error and
audit_exc) so that failures in audit_logger.log_event are surfaced to callers
along with the classifier_load failure.

---

90-98: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't turn unexpected generation bugs into empty samples.

Both fallback paths still swallow arbitrary exceptions and synthesize "", so tokenizer/model regressions get counted as bad examples instead of failing the safety pass loudly. Keep the OOM fallback, but let non-OOM generation errors propagate.

As per coding guidelines, "No silent failures: do not use bare except clauses, do not use except Exception: pass, do not use || true in CI scripts, and do not log-and-swallow errors except for explicitly-non-fatal paths (webhooks, cleanup tasks)".

Also applies to: 142-151

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/safety.py` around lines 90 - 98, The current try/except in the
generation block swallows all exceptions and returns an empty string; change it
to only catch and handle out-of-memory errors (e.g., torch.cuda.OutOfMemoryError
or RuntimeError with "out of memory") and re-raise any other exceptions so
non-OOM failures propagate; update the logic in the generation wrapper (the try
around tokenizer/model.generate and the logger.warning) to detect OOM, log and
return "" for OOM only, and re-raise otherwise; apply the same change to the
second occurrence referenced (the similar try/except around generation at lines
142-151) so only explicit OOMs are swallowed and all other errors bubble up.

forgelm/compliance.py (1)

93-103: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Move the anonymous-operator opt-in into validated config.

FORGELM_ALLOW_ANONYMOUS_OPERATOR still changes audit semantics outside the YAML/constructor path, so two runs with the same config can produce different Article 12 records. This should come from validated config (or an explicit initializer argument), not ambient process state.

Based on learnings, "Behaviour must be determined by validated YAML config files; do not use environment variables for behaviour control (only for secrets)".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/compliance.py` around lines 93 - 103, Replace the ambient env-var
check for FORGELM_ALLOW_ANONYMOUS_OPERATOR inside the compliance code with a
validated config or explicit initializer argument: add an
allow_anonymous_operator boolean to your validated config (or to the Compliance
constructor), validate/parse it before calling the code that currently contains
the os.getenv(...) branch, then in the block that references getpass.getuser(),
use that validated allow_anonymous_operator flag (not os.getenv) to decide
whether to raise ConfigError or set self.operator = f"anonymous@{hostname}";
update any callsites that construct Compliance to pass the new flag or to read
it from the parsed YAML config.

forgelm/webhook.py (1)

101-111: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Don't bypass the HTTPS policy for every webhook by default.

Hard-coding allow_insecure_http=True turns safe_post's scheme guard into a warning-only path, so any http:// webhook will send run metadata unencrypted. Drive this from a validated config flag and keep the default as False.

Based on learnings, "Behaviour must be determined by validated YAML config files; do not use environment variables for behaviour control (only for secrets)" and "Enterprise features (compliance export, human approval, safety evaluation) must be opt-in; new users must not be burdened with these features".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/webhook.py` around lines 101 - 111, The call to safe_post in
webhook.py currently hardcodes allow_insecure_http=True which bypasses HTTPS by
default; change this to read a validated config flag (e.g.,
webhook_config.allow_insecure_http or app_config.allow_insecure_http) with a
default of False, validate that flag during YAML config loading, and pass that
boolean into the safe_post call instead of the literal True (update any callers
or wrapper that construct the safe_post parameters if needed) so insecure HTTP
remains opt-in and disabled for new users.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/analysis/code_reviews/closure-plan-202604300906.md`:
- Around line 1175-1177: The acceptance text still references release-prep.yml
even though the workflows were consolidated into publish.yml; update the Faz 31
Acceptance bullet so it references publish.yml (e.g., "git tag -a v0.5.5-test -m
'test' triggers publish.yml") or, if you intend to keep the old split design,
restore the release-prep.yml workflow files and adjust the task description to
match that split; ensure the text consistently uses the chosen workflow name
(publish.yml or release-prep.yml) throughout.
- Around line 1135-1147: The current "Install packaged wheel (NOT editable)"
step uses POSIX shell syntax and restricts extras to --find-links dist, causing
failures on windows-latest and preventing resolution of transitive extras
dependencies; fix by splitting the extras install into a separate step
conditional on Linux (use if: runner.os == 'Linux') or set shell: bash for that
step, and change the extras install to use the full package index (remove
--no-index/--find-links or add an appropriate --extra-index-url or pre-built
wheels that include deps) so pip can resolve qlora/ingestion/eval dependencies
while keeping the core python -m pip install dist/*.whl behavior unchanged.

In `@docs/analysis/code_reviews/foundation-pr-review-20250504-opus.md`:
- Around line 70-77: F-006 in the review doc is stale: the tests were changed to
patch the correct symbol (forgelm._http.requests.post) so update the F-006 entry
to either mark it closed or re-scope it to any remaining mismatched patch sites,
and edit the description to state that tests now patch
forgelm._http.requests.post (or mock forgelm._http.safe_post) rather than global
requests.post; apply the same correction to the other duplicate occurrences of
this text elsewhere in the document so the report is consistent.
- Around line 6-9: The front matter contains a timestamp inconsistency: the
field "Review tarihi: 2025-05-04" predates the referenced commit "269637b" /
branch "closure-plan-202604300906" yet the file also claims "Verified against
current code at commit 269637b"; either update the "Review tarihi" value to a
date at or after the commit (e.g., 2026-04-30 or the actual verification date)
or remove the "Verified against current code at commit 269637b" claim so the
document has a single, auditable provenance; edit the lines containing "Review
tarihi" and "Verified against current code at commit 269637b" accordingly.

In `@docs/analysis/code_reviews/split-design-data_audit-cli-202604300906.md`:
- Line 145: Add language identifiers to all fenced code blocks to satisfy
markdownlint MD040: replace triple-backtick fences like ``` with fenced blocks
that specify a language (e.g., ```text). Update each unlabeled fenced block in
this document (the ones noted in the review and any other occurrences) so every
opening ``` includes a language tag such as text.

In `@forgelm/compliance.py`:
- Around line 1065-1097: When a manifest file exists at manifest_path but cannot
be read or lacks required pinned fields, fail verification instead of warning
and returning success; in the except block for open/json.load return a
VerifyResult with valid=False, entries_count=entries_count,
first_invalid_index=1 and a reason indicating the manifest is unreadable, and
after loading check that manifest contains first_entry_sha256 and run_id and if
either is missing return a similar VerifyResult (use the same fields and a
reason like "manifest missing required pinned fields"); keep using the existing
symbols pinned, pinned_run, first_line_hash, first_run_id, entries_count,
VerifyResult and logger to construct the error responses so upstream callers see
the manifest-present-but-invalid outcome.

In `@forgelm/safety.py`:
- Around line 484-499: The public API should validate the batch_size parameter
to avoid range()/loop failures and silent no-op behavior; add an upfront check
in the function (before any batching logic and before thresholds handling) that
raises ValueError if batch_size < 1, referencing the batch_size parameter and
returning/raising a clear message (e.g., "batch_size must be >= 1"); update the
function that returns SafetyResult (which uses SafetyEvalThresholds and
audit_logger) to perform this validation immediately after parameter parsing to
prevent downstream errors.

In `@forgelm/trainer.py`:
- Around line 558-561: Update the revert notification message to be
artifact-neutral: in the call to self.notifier.notify_reverted (used with
run_name=self.run_name and reason=...), replace the hardcoded suffix "Adapters
discarded." with a neutral phrase like "Artifacts discarded." so the webhook is
correct whether save_final_model produced adapters, a merged model, or other
outputs; keep the surrounding code and parameters (self.run_name, reason
formatting) unchanged.

---

Outside diff comments:
In `@forgelm/compliance.py`:
- Around line 152-173: The current 4 KiB fallback can hash a truncated last
record; change the fallback (the branch that re-reads from seek_start and then
calls self._decode_lines(tail) and hashes lines[-1]) to instead scan the read
tail backwards for the last newline and extract the complete final line before
hashing. Concretely: after re-reading tail, find the index of the last '\n' (or
last newline byte) in tail, slice tail from that newline+1 to the end to obtain
the full final record, decode that slice with self._decode_lines (or decode
appropriately), and then compute the hashlib.sha256 of that full record before
assigning _prev_hash; if no newline is found, treat as the entire tail as the
final record (or fall back to "genesis") to avoid seeding _prev_hash with a
truncated JSONL entry.

---

Duplicate comments:
In `@forgelm/compliance.py`:
- Around line 93-103: Replace the ambient env-var check for
FORGELM_ALLOW_ANONYMOUS_OPERATOR inside the compliance code with a validated
config or explicit initializer argument: add an allow_anonymous_operator boolean
to your validated config (or to the Compliance constructor), validate/parse it
before calling the code that currently contains the os.getenv(...) branch, then
in the block that references getpass.getuser(), use that validated
allow_anonymous_operator flag (not os.getenv) to decide whether to raise
ConfigError or set self.operator = f"anonymous@{hostname}"; update any callsites
that construct Compliance to pass the new flag or to read it from the parsed
YAML config.

In `@forgelm/safety.py`:
- Around line 464-473: The current exception handling when classifier loading
fails swallows any exception from audit_logger.log_event (in the
audit_logger.log_event call for "audit.classifier_load_failed") and only logs a
warning, then raises the original RuntimeError; instead, ensure the audit
failure is not lost by propagating it or chaining it with the classifier error:
in the exception block around audit_logger.log_event, raise the audit exception
(or raise a combined exception) so the function does not silently drop the audit
failure—replace the logger.warning(...) swallow with re-raising audit_exc (or
raise a new Exception that includes both the original classifier error and
audit_exc) so that failures in audit_logger.log_event are surfaced to callers
along with the classifier_load failure.
- Around line 90-98: The current try/except in the generation block swallows all
exceptions and returns an empty string; change it to only catch and handle
out-of-memory errors (e.g., torch.cuda.OutOfMemoryError or RuntimeError with
"out of memory") and re-raise any other exceptions so non-OOM failures
propagate; update the logic in the generation wrapper (the try around
tokenizer/model.generate and the logger.warning) to detect OOM, log and return
"" for OOM only, and re-raise otherwise; apply the same change to the second
occurrence referenced (the similar try/except around generation at lines
142-151) so only explicit OOMs are swallowed and all other errors bubble up.

In `@forgelm/webhook.py`:
- Around line 101-111: The call to safe_post in webhook.py currently hardcodes
allow_insecure_http=True which bypasses HTTPS by default; change this to read a
validated config flag (e.g., webhook_config.allow_insecure_http or
app_config.allow_insecure_http) with a default of False, validate that flag
during YAML config loading, and pass that boolean into the safe_post call
instead of the literal True (update any callers or wrapper that construct the
safe_post parameters if needed) so insecure HTTP remains opt-in and disabled for
new users.

🪄 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: e0db1268-04ab-4620-9ba6-5ff2a3c91529

📥 Commits

Reviewing files that changed from the base of the PR and between 885395d and 9480ca0.

📒 Files selected for processing (28)

• .gitignore
• CHANGELOG.md
• CLAUDE.md
• docs/analysis/code_reviews/closure-plan-202604300906.md
• docs/analysis/code_reviews/foundation-pr-review-20250504-opus.md
• docs/analysis/code_reviews/foundation-pr-review-prompt.md
• docs/analysis/code_reviews/master-review-opus-202604300906.md
• docs/analysis/code_reviews/split-design-data_audit-cli-202604300906.md
• docs/reference/audit_event_catalog-tr.md
• docs/reference/audit_event_catalog.md
• docs/roadmap-tr.md
• docs/roadmap.md
• docs/standards/logging-observability.md
• forgelm/_http.py
• forgelm/compliance.py
• forgelm/config.py
• forgelm/data_audit.py
• forgelm/judge.py
• forgelm/safety.py
• forgelm/trainer.py
• forgelm/webhook.py
• tests/conftest.py
• tests/test_compliance.py
• tests/test_judge_functions.py
• tests/test_safety_advanced.py
• tests/test_synthetic.py
• tests/test_trainer.py
• tests/test_webhook.py

✅ Files skipped from review due to trivial changes (6)

• tests/test_trainer.py
• .gitignore
• docs/analysis/code_reviews/master-review-opus-202604300906.md
• CLAUDE.md
• docs/roadmap.md
• docs/roadmap-tr.md

🚧 Files skipped from review as they are similar to previous changes (4)

• tests/conftest.py
• tests/test_synthetic.py
• docs/standards/logging-observability.md
• tests/test_compliance.py

[cemililik]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

forgelm/judge.py (1)

287-300: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate batch_size at the public API boundary.

run_judge_evaluation(batch_size=0) currently fails from the batching loop, and a negative value can skip generation entirely and degrade into a misleading "No valid judge scores" path. Since this is a public library API, reject non-positive values up front with a clear ValueError.

Suggested patch

 def run_judge_evaluation(
     model: Any,
     tokenizer: Any,
     eval_dataset_path: str,
@@
     batch_size: int = 8,
 ) -> JudgeResult:
     """Evaluate fine-tuned model outputs using an LLM judge.
@@
     Returns:
         JudgeResult with scores and pass/fail status.
     """
+    if not isinstance(batch_size, int) or batch_size < 1:
+        raise ValueError(f"batch_size must be a positive integer, got {batch_size!r}")
+
     if not os.path.isfile(eval_dataset_path):
         logger.error("Judge eval dataset not found: %s", eval_dataset_path)
         return JudgeResult(passed=False, failure_reason=f"Eval dataset not found: {eval_dataset_path}")

Based on learnings, "Only add/require runtime validation when entering true public library-API boundaries."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/judge.py` around lines 287 - 300, The public function
run_judge_evaluation must validate its batch_size parameter to reject
non-positive values; add an upfront check at the start of run_judge_evaluation
(before any batching/loop logic) that ensures isinstance(batch_size, int) and
batch_size > 0 and raise a ValueError with a clear message like "batch_size must
be a positive integer" if the check fails so invalid inputs (0 or negative) are
rejected at the API boundary rather than causing downstream errors in the
batching loop.

forgelm/trainer.py (1)

1214-1223: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pass JudgeConfig.batch_size through to run_judge_evaluation().

This call site still relies on the callee default 8, so the YAML-configured JudgeConfig.batch_size is ignored and operators cannot tune judge-eval memory/latency from config. Wire the validated config value through here, using getattr(..., 8) only if this PR needs backward compatibility with older config objects.

Suggested patch

         return run_judge_evaluation(
             model=self.trainer.model,
             tokenizer=self.tokenizer,
             eval_dataset_path=judge_cfg.eval_dataset,
             judge_model=judge_cfg.judge_model,
             judge_api_key=api_key,
             min_score=judge_cfg.min_score,
             output_dir=output_dir,
             api_base=getattr(judge_cfg, "judge_api_base", None),
+            batch_size=getattr(judge_cfg, "batch_size", 8),
         )

As per coding guidelines, "`**/*.py`: Behavior must be determined by validated YAML config; no env-var sniffing for behavior (only for secrets), no hardcoded feature flags`."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/trainer.py` around lines 1214 - 1223, The call to
run_judge_evaluation is not passing the configured batch size so the YAML
JudgeConfig.batch_size is ignored; update the call site (where
run_judge_evaluation(...) is invoked with model=self.trainer.model,
tokenizer=self.tokenizer, eval_dataset_path=judge_cfg.eval_dataset, etc.) to
pass the validated batch size as batch_size=judge_cfg.batch_size (or
batch_size=getattr(judge_cfg, "batch_size", 8) only if you must preserve
backward compatibility with older config objects).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/analysis/code_reviews/split-design-data_audit-cli-202604300906.md`:
- Around line 290-325: The new test omits asserting the package-level Presidio
sentinels; update tests/test_data_audit_import_stability.py so the package
surface check includes both _HAS_PRESIDIO and _PresidioAnalyzer (add them to
PUBLIC or PRIVATE_TEST_TOUCHED as appropriate) and ensure
test_public_api_resolves asserts hasattr(m, "_HAS_PRESIDIO") and hasattr(m,
"_PresidioAnalyzer"); also keep the existing import assertion in
test_patchable_optional_sentinels_resolve for from forgelm.data_audit._optional
import _HAS_PRESIDIO to ensure both package-level aliases and the _optional
module symbols are covered.

In `@docs/reference/audit_event_catalog-tr.md`:
- Line 16: Update the operator description to include the conditional anonymous
fallback: document that AuditLogger can emit the value from FORGELM_OPERATOR or
the default `<getpass.getuser()>@<hostname>`, and when the environment flag
FORGELM_ALLOW_ANONYMOUS_OPERATOR=1 is set (see AuditLogger behavior in
compliance-related code), it may instead emit `anonymous@<hostname>`; mention
all three possible sources (FORGELM_OPERATOR, getpass-based default,
anonymous@<hostname>) and note the conditional nature tied to
FORGELM_ALLOW_ANONYMOUS_OPERATOR and AuditLogger.

In `@docs/reference/audit_event_catalog.md`:
- Around line 36-55: The documentation lists payload keys that don't match the
emitters in forgelm/trainer.py (events human_approval.required uses model_path,
compliance.governance_exported uses path, compliance.artifacts_exported uses
directory), so update the emitters to match the documented schema: change
human_approval.required to emit payload keys 'gate' (plus reason/metrics),
change compliance.governance_exported to emit 'output_path' (and dataset_count
if available), and change compliance.artifacts_exported to emit 'output_dir' and
'files' (or include files list), then run/update any unit/integration tests and
verifiers; finally mirror the same authoritative schema into
audit_event_catalog.md and audit_event_catalog-tr.md so docs and code stay in
sync.

In `@forgelm/compliance.py`:
- Around line 1142-1153: The verifier currently assumes parsed JSON `entry` is a
mapping and calls `entry.get(...)`, which raises AttributeError for JSON
scalars/arrays; update the verification logic (in the function handling the loop
where `entry = json.loads(line)` and the subsequent `prev_hash =
entry.get("prev_hash")`) to first check `isinstance(entry, dict)` and if not
return a VerifyResult with valid=False, entries_count=entries_count,
first_invalid_index=idx and a descriptive reason like "line {idx} is not a JSON
object" (also ensure the same pattern is used before accessing any other dict
methods or keys, and keep use of `expected_prev` in the surrounding prev_hash
comparison).

In `@forgelm/judge.py`:
- Around line 95-97: The except block catching HttpSafetyError in
forgelm/judge.py should not swallow the error; change the handler for
HttpSafetyError (the except HttpSafetyError as e around the API judge request)
to either re-raise the exception or raise/return a distinct failure type that
run_judge_evaluation() can treat as an immediate hard failure instead of
returning {"score": None}; update the control flow so HttpSafetyError surfaces
as a configuration/HTTP-safety failure (refer to the HttpSafetyError symbol and
run_judge_evaluation function) rather than being logged-and-swallowed.

In `@forgelm/safety.py`:
- Around line 428-431: The docstring that currently starts with the non-English
word "Kondenses the five Phase 9 knobs (`scoring`, `min_safety_score`,
`min_classifier_confidence`, `track_categories`, `severity_thresholds`) into one
parameter..." should be rewritten in English; replace "Kondenses" with
"Condenses" and ensure the entire docstring text is fully English and clearly
explains that these five Phase 9 knobs are consolidated into a single parameter
(keep the same parameter names in backticks), updating any surrounding phrasing
to be English-only.

---

Outside diff comments:
In `@forgelm/judge.py`:
- Around line 287-300: The public function run_judge_evaluation must validate
its batch_size parameter to reject non-positive values; add an upfront check at
the start of run_judge_evaluation (before any batching/loop logic) that ensures
isinstance(batch_size, int) and batch_size > 0 and raise a ValueError with a
clear message like "batch_size must be a positive integer" if the check fails so
invalid inputs (0 or negative) are rejected at the API boundary rather than
causing downstream errors in the batching loop.

In `@forgelm/trainer.py`:
- Around line 1214-1223: The call to run_judge_evaluation is not passing the
configured batch size so the YAML JudgeConfig.batch_size is ignored; update the
call site (where run_judge_evaluation(...) is invoked with
model=self.trainer.model, tokenizer=self.tokenizer,
eval_dataset_path=judge_cfg.eval_dataset, etc.) to pass the validated batch size
as batch_size=judge_cfg.batch_size (or batch_size=getattr(judge_cfg,
"batch_size", 8) only if you must preserve backward compatibility with older
config objects).

🪄 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: 2f53c634-7de4-4ae2-b750-e0a094b62b26

📥 Commits

Reviewing files that changed from the base of the PR and between 885395d and 39d99c4.

📒 Files selected for processing (27)

• .gitignore
• CHANGELOG.md
• CLAUDE.md
• docs/analysis/code_reviews/closure-plan-202604300906.md
• docs/analysis/code_reviews/foundation-pr-review-prompt.md
• docs/analysis/code_reviews/master-review-opus-202604300906.md
• docs/analysis/code_reviews/split-design-data_audit-cli-202604300906.md
• docs/reference/audit_event_catalog-tr.md
• docs/reference/audit_event_catalog.md
• docs/roadmap-tr.md
• docs/roadmap.md
• docs/standards/logging-observability.md
• forgelm/_http.py
• forgelm/compliance.py
• forgelm/config.py
• forgelm/data_audit.py
• forgelm/judge.py
• forgelm/safety.py
• forgelm/trainer.py
• forgelm/webhook.py
• tests/conftest.py
• tests/test_compliance.py
• tests/test_judge_functions.py
• tests/test_safety_advanced.py
• tests/test_synthetic.py
• tests/test_trainer.py
• tests/test_webhook.py

✅ Files skipped from review due to trivial changes (4)

• CLAUDE.md
• docs/analysis/code_reviews/master-review-opus-202604300906.md
• tests/test_synthetic.py
• docs/roadmap-tr.md

🚧 Files skipped from review as they are similar to previous changes (10)

• .gitignore
• forgelm/config.py
• tests/conftest.py
• CHANGELOG.md
• tests/test_compliance.py
• forgelm/data_audit.py
• forgelm/webhook.py
• docs/roadmap.md
• docs/standards/logging-observability.md
• tests/test_judge_functions.py

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Pin the package-level Presidio sentinels in the import-stability test.

Section 1.e still promises _HAS_PRESIDIO and _PresidioAnalyzer at forgelm.data_audit.<name>, but this proposed regression test only asserts _optional resolution. If a future split drops those package-level aliases, the acceptance test stays green while the documented compatibility contract breaks. Add both names to the asserted surface here, or drop the package-level guarantee earlier in the design.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/analysis/code_reviews/split-design-data_audit-cli-202604300906.md`
around lines 290 - 325, The new test omits asserting the package-level Presidio
sentinels; update tests/test_data_audit_import_stability.py so the package
surface check includes both _HAS_PRESIDIO and _PresidioAnalyzer (add them to
PUBLIC or PRIVATE_TEST_TOUCHED as appropriate) and ensure
test_public_api_resolves asserts hasattr(m, "_HAS_PRESIDIO") and hasattr(m,
"_PresidioAnalyzer"); also keep the existing import assertion in
test_patchable_optional_sentinels_resolve for from forgelm.data_audit._optional
import _HAS_PRESIDIO to ensure both package-level aliases and the _optional
module symbols are covered.

[coderabbitai]

🧹 Nitpick comments (1)

forgelm/judge.py (1)

367-373: 💤 Low value

Consider extracting duplicate log format string.

SonarCloud flagged the literal "JUDGE EVALUATION FAILED: %s" appearing 3 times (lines 372, 464, 479). A module-level constant like _LOG_JUDGE_FAILED = "JUDGE EVALUATION FAILED: %s" would reduce drift risk.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/judge.py` around lines 367 - 373, The literal log format "JUDGE
EVALUATION FAILED: %s" is duplicated in multiple places (seen near the
HttpSafetyError handling block in the except HttpSafetyError as e: clause that
calls logger.error and returns JudgeResult); extract this string into a
module-level constant (e.g. _LOG_JUDGE_FAILED = "JUDGE EVALUATION FAILED: %s")
and replace the inline literals in all occurrences (the logger.error calls
around the HttpSafetyError handler and the other two places flagged by
SonarCloud) to use that constant instead, leaving the existing logger.error
usage and JudgeResult return logic unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@forgelm/judge.py`:
- Around line 367-373: The literal log format "JUDGE EVALUATION FAILED: %s" is
duplicated in multiple places (seen near the HttpSafetyError handling block in
the except HttpSafetyError as e: clause that calls logger.error and returns
JudgeResult); extract this string into a module-level constant (e.g.
_LOG_JUDGE_FAILED = "JUDGE EVALUATION FAILED: %s") and replace the inline
literals in all occurrences (the logger.error calls around the HttpSafetyError
handler and the other two places flagged by SonarCloud) to use that constant
instead, leaving the existing logger.error usage and JudgeResult return logic
unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0a5f09d9-1516-4061-b81b-5ff703021600

📥 Commits

Reviewing files that changed from the base of the PR and between 39d99c4 and 327e82f.

📒 Files selected for processing (9)

• docs/analysis/code_reviews/closure-plan-202604300906.md
• docs/reference/audit_event_catalog-tr.md
• docs/reference/audit_event_catalog.md
• forgelm/compliance.py
• forgelm/config.py
• forgelm/judge.py
• forgelm/safety.py
• forgelm/trainer.py
• tests/test_judge_functions.py

[sonarqubecloud]

Quality Gate failed

Failed conditions
6 Security Hotspots

See analysis details on SonarQube Cloud