Repository navigation
release: development cycle — PR #50 bug-fix batch + agent skills + housekeeping - #51
Merged
Merged
Conversation
…I, audit policy 8 findings from the comprehensive code review fixed in one bundled commit (per the "fix scope policy" — confirmed bugs ship together, no leftovers). P1 (production blockers) - forgelm/cli/_no_train_modes.py: --benchmark-only now uses inference.load_model() with PEFT auto-detection (adapter_config.json → base_model_name_or_path + adapter). Previously wrapped a fresh untrained LoRA on top of the supplied checkpoint via the training loader, silently evaluating the wrong weights. - forgelm/data.py: _ensure_validation_split() now skips the split for datasets with <2 rows instead of crashing on train_test_split. - tests/test_compliance.py: test_hub_dataset now mocks the HF Hub helpers; the test no longer requires network connectivity. P2 (compliance / privacy / config-validation) - forgelm/cli/subcommands/_approve.py: _resolve_approver_identity() mirrors AuditLogger's anonymous-operator policy — refuses to silently return "anonymous" without FORGELM_ALLOW_ANONYMOUS_OPERATOR=1 opt-in (Article 12 record-keeping consistency). - forgelm/judge.py, forgelm/safety.py: judge_results.json and safety_results.json no longer persist raw prompt/response snippets. In-memory JudgeResult / SafetyResult details are unchanged so webhooks and trainer logs still have full context. - forgelm/judge.py: _parse_judge_json() logs only the response length and a fixed reason string on parse failure, not the raw model output. - forgelm/config.py: 12 TrainingConfig numeric fields now carry ge/gt/le constraints (max_steps, num_train_epochs, batch sizes, learning_rate, warmup_ratio, weight_decay, eval/save/limit steps, early_stopping_patience, GRPO generations & completion length). P3 (resilience) - forgelm/cli/_dispatch.py: --data-audit deprecation breadcrumb now catches Exception (was OSError-only); ConfigError from AuditLogger in a regulated env should not abort the audit run. Skipped (with rationale in the review prompt): - P2-4 (FORGELM_GGUF_CONVERTER): already validated to .py + warning. - P3-2 (FORGELM_AUDIT_TEST_WORKER_RAISES): documented test hook, fail-closed on exact split-name match. - P3-3 (ingestion.py 2600+ lines): behavior-neutral split deferred to its own PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… PEFT tokenizer, tests Addresses the three blocking review findings on PR #50 plus the two non-blocking ones, and lands regression tests for every new behaviour the previous commit introduced. Blocking (3) - P1-2 downstream: forgelm/trainer.py now downshifts eval_strategy to "no" and disables load_best_model_at_end / metric_for_best_model / greater_is_better when no validation split exists. Previously the builder always emitted eval_strategy="steps" while _build_trainer passed eval_dataset=None — HF Trainer raises ValueError on that combination. Test: tests/test_trainer.py::TestTrainingArgsValidationGuard (validation-present keeps "steps"; absent or empty downshifts to "no"). - P2-2 reason redaction: forgelm/judge.py and forgelm/safety.py now strip prompt + response + (for judge) the natural-language reason field from on-disk artifacts unless the opt-in flag is set. New Pydantic fields JudgeConfig.include_eval_samples and SafetyConfig.include_eval_samples default to False (privacy by default for GDPR / EU AI Act Art. 10). Trainer + run_*_evaluation thread the flag through. In-memory JudgeResult / SafetyResult details unchanged. Tests: TestJudgeResultRedaction + TestSafetyResultRedaction (default redacts, opt-in keeps all fields). - Tests for the previous batch's behaviour changes: - P1-1 benchmark loader: tests/test_no_train_modes.py covers PEFT detection (adapter_config.json present → load_model(base, adapter=path); absent → load_model(path)) and asserts get_model_and_tokenizer is NOT called. - P1-2 data: tests/test_data_edge_cases.py::TestEnsureValidationSplit asserts single-row / empty datasets skip the split with a warning. - P2-1 approver identity: tests/test_human_approval_gate.py:: TestApproverIdentityPolicy covers FORGELM_OPERATOR priority, getpass fallback, EXIT_CONFIG_ERROR without opt-in, and anonymous@<hostname> with opt-in. - P2-3 warning log sanitisation: TestJudgeResultRedaction's test_parse_judge_json_warning_strips_raw_text asserts the JSON-parse warning + reason no longer echo the raw model output. Non-blocking (2) - P2 PEFT tokenizer caveat: forgelm/inference.py now prefers the adapter directory's tokenizer when adapter/tokenizer_config.json exists — fine-tuning may add special tokens or a custom chat template that the base tokenizer would silently drop. Falls back to base path otherwise. Tests: test_tokenizer_loaded_from_adapter_when_tokenizer_config_present and test_tokenizer_falls_back_to_base_when_adapter_has_no_tokenizer_config in tests/test_inference.py. - P3 stale docstring: forgelm/cli/subcommands/_approve.py's _resolve_approver_identity docstring now describes the opt-in / exit policy instead of the obsolete "anonymous if both fail" line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…umentation sync - Introduced `cut-release` skill for managing ForgeLM releases, including version bumping, changelog finalization, tagging, and publishing. - Added `review-pr` skill to facilitate thorough code reviews for pull requests, ensuring adherence to coding standards and best practices. - Created `sync-bilingual-docs` skill to maintain structural alignment between English and Turkish documentation, ensuring consistency across bilingual resources. - Added `AGENTS.md` to provide guidance for AI agents working on ForgeLM, outlining project principles, skills, and repository structure.
…uard, hoist redact constants Addresses the four bot-review findings on PR #50 plus the two cosmetic tradeoffs left from the previous review-response round. Bot findings (gemini-code-assist + sourcery-ai) - forgelm/cli/_no_train_modes.py:38 (gemini): explicit encoding="utf-8" on adapter_config.json read. - forgelm/cli/_no_train_modes.py:40 (gemini + sourcery overlap): when adapter_config.json is missing base_model_name_or_path or is malformed JSON, exit with EXIT_CONFIG_ERROR and a clear log message instead of silently falling back to model_path (which would crash deep inside PeftModel.from_pretrained with a confusing "config.json not found"). Two new regression tests cover this (corrupt JSON + missing field). - forgelm/cli/subcommands/_approve.py:96 (gemini): wrap socket.gethostname() in try/except OSError; restricted containers / sandboxes can fail hostname resolution and the whole point of FORGELM_ALLOW_ANONYMOUS_OPERATOR=1 is to keep running there. Falls back to "unknown-host" narrowly. - forgelm/judge.py + forgelm/safety.py (sourcery): hoist the redaction field set to module-level frozenset constants (_PII_REDACT_FIELDS). Documents the GDPR / EU AI Act Art. 10 motivation in-line and avoids reallocating the set on every save call. Cosmetic tradeoffs deferred from PR #50 review-response round - forgelm/cli/_no_train_modes.py: flatten `import json as _json` / `import os as _os` to the module-level `json` / `os` imports. The underscore-prefix variants added isolation noise without isolation benefit; behaviour is identical. - forgelm/cli/_dispatch.py: narrow `except Exception` to `(OSError, ConfigError)` on the --data-audit deprecation breadcrumb. Keeps the best-effort contract without silently swallowing unrelated runtime errors. Skipped (with rationale in the PR comment) - sourcery's TrainingConfig 0/None disable semantics: HF's TrainingArguments uses None default (never 0) for save_total_limit; ge=1 is consistent with the schema's int typing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…mells Three SonarQube findings introduced by the review-response rounds — each fixed by extracting helpers, no behaviour change. - forgelm/cli/_no_train_modes.py: extract _resolve_benchmark_load_target() helper that owns PEFT detection + adapter_config.json read + base_model_name_or_path validation. _run_benchmark_only now has a single load_model() call, dropping cognitive complexity from 19 to under the 15 limit. (S3776, Critical) - forgelm/inference.py: extract _pick_tokenizer_source() helper for the "prefer adapter tokenizer when tokenizer_config.json is present" decision. load_model cognitive complexity 17 -> under 15. (S3776, Critical) - forgelm/safety.py: bundle the three Phase 9 category-telemetry arguments (track_categories, category_dist, severity_dist) into a new _CategoryTelemetry dataclass. _save_safety_results parameter count drops from 14 to 12 (the 13-parameter limit) without forcing callers to flatten on the call site. (S107, Major) Existing test_safety_advanced.py::TestSafetyResultRedaction tests updated to use the new dataclass; tests/test_no_train_modes.py PEFT detection tests still pass against the refactored shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix: review-batch P1-P3 — 8 code review findings (1 bundled commit)
…st stabiliser Two cleanup items left over from the PR #50 merge that should land before the next phase opens: 1) tools/check_no_analysis_refs.py: register the agent-agnostic mirror files (AGENTS.md and .agents/skills/sync-bilingual-docs/ SKILL.md) in _EXEMPT. They are functional path filters / agent- guidance documents that legitimately name the gitignored docs/marketing/ + docs/analysis/ directories, mirroring the existing CLAUDE.md and .claude/skills/sync-bilingual-docs/SKILL.md exemptions. Without this, the local 11-guard self-review gauntlet fails on development after PR #50 introduced these files in commit 8d678fa. 2) tests/test_inference.py::test_unsloth_backend_raises_without_package: refactored to call _load_unsloth directly with a sys.modules['unsloth'] = None monkeypatch, instead of going through load_model() with nested MagicMock stubs for torch / transformers. The previous pattern was order-sensitive: after the tokenizer-from-adapter tests populated sys.modules with stubs and torn them down, a follow-up real `import transformers` would hit ValueError on torch.__spec__ or AttributeError on torch.__version__ via the safetensors transitive import. Direct call exercises the same failure surface (`from unsloth import ...` raises ImportError) without the dispatcher's import zinciri. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 1 high |
| CodeStyle | 14 minor |
🟢 Metrics 107 complexity · 19 duplication
Metric Results Complexity 107 Duplication 19
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.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds AGENTS.md and multiple skill docs; implements DNS-rebinding-safe HTTP pinning and Host/SNI preservation; introduces opt-in flags to avoid persisting raw safety/judge samples (default: redact); tightens training config validation; adjusts inference/CLI flows; and adds wide test coverage aligning to these behaviors. ChangesAgent workflow standards, documentation, and PII redaction for evaluations
Sequence Diagram(s)sequenceDiagram
participant Caller
participant _resolve_safe_destination
participant Session as requests.Session
participant HostAdapter as HostHeaderSSLAdapter
Caller->>_resolve_safe_destination: resolve(hostname)
_resolve_safe_destination->>Caller: public_ip_literal
Caller->>Session: send request to http(s)://<public_ip_literal>/... with Host header=original hostname
Session->>HostAdapter: use SNI/Host for TLS (HTTPS)
Session->>Caller: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces AGENTS.md and a comprehensive set of skill definitions to standardize AI agent interactions with the repository. Significant functional updates include the implementation of PII redaction for safety and judge evaluation results (defaulting to privacy-first), stricter operator identity resolution for audit logs, and enhanced robustness for benchmark-only runs and small datasets. Review feedback correctly identified a violation of the English-only comment standard in forgelm/judge.py, where a Turkish term was used.
|
|
||
| # GDPR / EU AI Act Art. 10 — fields stripped from on-disk judge_results.json | ||
| # unless the operator opts in via JudgeConfig.include_eval_samples=True. | ||
| # ``reason`` is included because the judge's natural-language gerekçesi may |
There was a problem hiding this comment.
The comment contains the Turkish word "gerekçesi" (reasoning/justification). According to the project's engineering standards (as stated in AGENTS.md), code comments must be in English only to maintain codebase consistency.
Suggested change
| # ``reason`` is included because the judge's natural-language gerekçesi may | |
| # ``reason`` is included because the judge's natural-language reasoning may |
References
- Single-language comment drift — mixing Turkish + Spanish + English in code comments. English only in code. (link)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_trainer.py (1)
158-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the torch-availability skip guard to this new test class.
TestTrainingArgsValidationGuardexercises_get_common_training_kwargs(), which importstorchat line 285 of forgelm/trainer.py and uses it throughout (lines 318–322). Unlike other torch-dependent test classes in this file, this one is not guarded, so it will fail in no-torch environments.Suggested patch
+@pytest.mark.skipif(not torch_available, reason="torch not installed") class TestTrainingArgsValidationGuard:🤖 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_trainer.py` around lines 158 - 213, The new TestTrainingArgsValidationGuard class exercises ForgeTrainer._get_common_training_kwargs which imports and uses torch, so add a torch-availability skip guard to the class (e.g., apply pytest.importorskip("torch") at top of the class file or decorate the class with pytest.mark.skipif(importlib.util.find_spec("torch") is None, reason="requires torch")); ensure the guard is placed so the entire TestTrainingArgsValidationGuard class is skipped when torch is not installed to match other torch-dependent test classes in this file.
🤖 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 `@AGENTS.md`:
- Around line 115-116: Remove the gitignored docs/marketing/* link(s) in
AGENTS.md (the "Reinforced in [...]" markdown link) and replace them with
public, versioned document links such as docs/roadmap.md or
docs/product_strategy.md; for any internal-only guidance keep the text but do
not linkify it (mark it plain text like "internal-only guidance" or similar) and
apply the same replacement to the other occurrences of docs/marketing/* in this
file.
In `@forgelm/cli/_no_train_modes.py`:
- Around line 47-49: adapter_meta may not be a mapping (json.load can return a
list) so calling adapter_meta.get() will raise AttributeError; update the code
before accessing base_path to validate adapter_meta is a dict/Mapping (e.g.,
isinstance(adapter_meta, dict) or isinstance(adapter_meta, Mapping)), and if it
isn't, call logger.error with a clear message and exit with EXIT_CONFIG_ERROR
(same behavior as the existing error path), then safely read base_path from
adapter_meta.get("base_model_name_or_path").
In `@forgelm/judge.py`:
- Around line 45-49: The code comment above the _PII_REDACT_FIELDS definition
contains a non-English word ("gerekçesi"); update the comment to use
English-only wording (e.g., replace "gerekçesi" with "reasoning") so it complies
with the repository's English-only comment rule and still explains that "reason"
may include verbatim PII from eval prompts/responses (refer to
_PII_REDACT_FIELDS and JudgeConfig.include_eval_samples).
---
Outside diff comments:
In `@tests/test_trainer.py`:
- Around line 158-213: The new TestTrainingArgsValidationGuard class exercises
ForgeTrainer._get_common_training_kwargs which imports and uses torch, so add a
torch-availability skip guard to the class (e.g., apply
pytest.importorskip("torch") at top of the class file or decorate the class with
pytest.mark.skipif(importlib.util.find_spec("torch") is None, reason="requires
torch")); ensure the guard is placed so the entire
TestTrainingArgsValidationGuard class is skipped when torch is not installed to
match other torch-dependent test classes in this file.
🪄 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: a76bc69d-8177-44f9-9b00-041ff4b48002
📒 Files selected for processing (25)
.agents/skills/add-config-field/SKILL.md.agents/skills/add-test/SKILL.md.agents/skills/add-trainer-feature/SKILL.md.agents/skills/cut-release/SKILL.md.agents/skills/review-pr/SKILL.md.agents/skills/sync-bilingual-docs/SKILL.mdAGENTS.mdforgelm/cli/_dispatch.pyforgelm/cli/_no_train_modes.pyforgelm/cli/subcommands/_approve.pyforgelm/config.pyforgelm/data.pyforgelm/inference.pyforgelm/judge.pyforgelm/safety.pyforgelm/trainer.pytests/test_compliance.pytests/test_data_edge_cases.pytests/test_human_approval_gate.pytests/test_inference.pytests/test_judge_functions.pytests/test_no_train_modes.pytests/test_safety_advanced.pytests/test_trainer.pytools/check_no_analysis_refs.py
Comment on lines
+115
to
+116
| Reinforced in [docs/marketing/strategy/05-yapmayacaklarimiz.md](docs/marketing/strategy/05-yapmayacaklarimiz.md). Do not propose or implement: | ||
|
|
Contributor
There was a problem hiding this comment.
Remove references to gitignored docs/marketing/* from this public guide.
These links point to local-only paths and conflict with the project’s own “don’t reference working-memory directories from public docs” rule. Replace them with public, versioned docs (for example docs/roadmap.md / docs/product_strategy.md) and keep the “internal-only” guidance non-linkified.
Based on learnings: "Never reference docs/analysis/ or docs/marketing/ contents from production code, public docs, CHANGELOG entries, commit messages, or PR descriptions."
Also applies to: 199-201
🤖 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 `@AGENTS.md` around lines 115 - 116, Remove the gitignored docs/marketing/*
link(s) in AGENTS.md (the "Reinforced in [...]" markdown link) and replace them
with public, versioned document links such as docs/roadmap.md or
docs/product_strategy.md; for any internal-only guidance keep the text but do
not linkify it (mark it plain text like "internal-only guidance" or similar) and
apply the same replacement to the other occurrences of docs/marketing/* in this
file.
Comment on lines
+47
to
+49
| base_path = adapter_meta.get("base_model_name_or_path") | ||
| if not base_path: | ||
| logger.error( |
Contributor
There was a problem hiding this comment.
Handle non-object adapter metadata before accessing .get()
Line 47 assumes json.load() returns a mapping. If adapter_config.json is valid JSON but not an object (e.g., []), this raises AttributeError and bypasses the intended EXIT_CONFIG_ERROR path.
💡 Suggested fix
try:
with open(adapter_cfg_path, encoding="utf-8") as f:
adapter_meta = json.load(f)
except (OSError, json.JSONDecodeError) as e:
logger.error("Failed to read PEFT adapter_config.json at %s: %s", adapter_cfg_path, e)
sys.exit(EXIT_CONFIG_ERROR)
+ if not isinstance(adapter_meta, dict):
+ logger.error(
+ "PEFT checkpoint at %s has invalid adapter_config.json root type (%s); expected a JSON object.",
+ model_path,
+ type(adapter_meta).__name__,
+ )
+ sys.exit(EXIT_CONFIG_ERROR)
+
base_path = adapter_meta.get("base_model_name_or_path")
- if not base_path:
+ if not isinstance(base_path, str) or not base_path.strip():
logger.error(
"PEFT checkpoint at %s is missing 'base_model_name_or_path' in adapter_config.json — "
"cannot reconstruct the base model + adapter combination.",
model_path,
)
sys.exit(EXIT_CONFIG_ERROR)
- return base_path, model_path
+ return base_path.strip(), model_path🤖 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/_no_train_modes.py` around lines 47 - 49, adapter_meta may not be
a mapping (json.load can return a list) so calling adapter_meta.get() will raise
AttributeError; update the code before accessing base_path to validate
adapter_meta is a dict/Mapping (e.g., isinstance(adapter_meta, dict) or
isinstance(adapter_meta, Mapping)), and if it isn't, call logger.error with a
clear message and exit with EXIT_CONFIG_ERROR (same behavior as the existing
error path), then safely read base_path from
adapter_meta.get("base_model_name_or_path").
Comment on lines
+45
to
+49
| # GDPR / EU AI Act Art. 10 — fields stripped from on-disk judge_results.json | ||
| # unless the operator opts in via JudgeConfig.include_eval_samples=True. | ||
| # ``reason`` is included because the judge's natural-language gerekçesi may | ||
| # quote PII from the eval prompts/responses verbatim. | ||
| _PII_REDACT_FIELDS: frozenset[str] = frozenset({"prompt", "response", "reason"}) |
Contributor
There was a problem hiding this comment.
Use English-only wording in this code comment.
Line 47 includes gerekçesi, which breaks the repository’s English-only comment rule. Replace it with an English term (e.g., “reasoning”).
As per coding guidelines, **/*.{py,js,ts,tsx}: "Keep code comments in English only; do not mix Turkish, Spanish, or other languages in 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 `@forgelm/judge.py` around lines 45 - 49, The code comment above the
_PII_REDACT_FIELDS definition contains a non-English word ("gerekçesi"); update
the comment to use English-only wording (e.g., replace "gerekçesi" with
"reasoning") so it complies with the repository's English-only comment rule and
still explains that "reason" may include verbatim PII from eval
prompts/responses (refer to _PII_REDACT_FIELDS and
JudgeConfig.include_eval_samples).
Pre-fix flow:
1. _is_private_destination(host) calls socket.getaddrinfo → public IP
2. requests.post(host_url, ...) → connect-time DNS lookup
3. If the attacker's DNS server flipped the answer between steps 1
and 2 (TTL=0 + rebinding), the connect lands on 127.0.0.1 /
169.254.169.254 / RFC1918 — payload + bearer token leak.
Hardening:
- New helper `_resolve_safe_destination(host)` resolves exactly once
and returns either a public IP literal or a block reason.
- safe_post / safe_get rebuild the outbound URL with that IP literal
so requests never re-resolves.
- Original hostname is propagated via the `Host` header (mandatory
for virtual-hosted endpoints like Slack / Teams / Discord) and via
SNI through requests_toolbelt's HostHeaderSSLAdapter, mounted on a
per-call Session for the https:// scheme.
- allow_private=True callers keep the legacy requests.post flow so
in-cluster / split-horizon DNS still works for operator-blessed
internal destinations.
- IPv6 IP literals are bracketed per RFC 3986 ([2001:db8::1]:443).
Threat model: attacker needs control of the DNS server for the
operator-configured webhook domain (or a downstream resolver they can
poison). allow_private_destinations=false default already forced the
operator to opt in to private destinations, so DNS rebinding was the
last remaining bypass; this fix closes it for the default-secure path.
Dependency: requests-toolbelt>=1.0.0,<2.0.0 added as a hard dep.
Small (~30KB), same ecosystem as requests, security-critical so an
opt-in extra would weaken the default-secure posture.
Tests:
- tests/test_http_dns_rebinding.py (new, 13 tests):
- _resolve_safe_destination helper coverage (6 unit tests:
public hostname, mixed-result blocking, DNS failure, empty
host, public IP literal, private IP literal).
- DNS rebinding behavioural test asserting getaddrinfo is called
exactly once and the URL is rebuilt with the public IP.
- Host header preservation with auth header round-trip.
- HostHeaderSSLAdapter mounted on https:// scheme.
- HTTP path does not need the SSL adapter.
- allow_private=True bypass exercises the legacy path.
- IPv6 literal bracketed in the rebuilt URL.
- safe_get mirrors safe_post for the same hardening.
- Updated existing webhook / judge / synthetic tests for the
Session.post mock pattern (16 occurrences across 3 files).
Two tests (test_url_env_resolution, test_custom_api_base,
test_api_base_reaches_http_call) reframed to assert on the
`Host` header rather than the URL because the URL now carries
the IP literal.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…4 tests Addresses all actionable findings from PR #52's review bots (gemini, sourcery) and the SonarQube quality-gate fail (1 hotspot + duplication + maintainability). Real bugs (gemini "high" + sourcery "security") - forgelm/_http.py safe_post + safe_get: the auto-set ``Host`` header now uses ``parsed.netloc.rsplit("@", 1)[-1]`` instead of ``parsed.hostname``. Pre-fix the header silently dropped the port (``Host: example.com`` for ``https://example.com:8443/``) and the bracket-wrap for IPv6 literals, violating RFC 7230 § 5.4 — virtual- hosted endpoints that switch on the authority-form would have routed wrong. Userinfo is stripped (``user:pass@host`` → ``host``) so credentials never echo into the request header. - forgelm/_http.py safe_post + safe_get exception handlers: log masking now operates on the outbound ``request_headers`` (which includes the auto-set ``Host`` and any caller secrets) rather than the raw ``headers`` parameter (which may be ``None`` or stale). Pre-fix, if the caller passed ``headers=None``, a secret added via the auto-Host path would not have been masked from the warning log on transport failure. Refactor for maintainability + Sonar duplication - Extracted ``_is_blocked_ip(ip)`` shared helper that owns the RFC1918 / loopback / link-local / IETF-reserved / multicast policy. Both ``_is_private_destination`` (legacy yes/no) and ``_resolve_safe_destination`` (DNS-rebinding-safe resolver) now delegate, so the policy cannot drift between call sites. Closes Sonar's 4.9% new-code duplication finding on this PR. Test gaps (sourcery) - ``TestRfc7230HostHeader`` (4 new tests): non-standard port stays in Host, IPv6 literal stays bracketed, userinfo stripped, explicit caller-supplied Host header takes precedence over the auto-set value. - ``TestNoPublicIpResolvedBranch`` (1 new test): covers the ``"no public IP resolved"`` branch of ``_resolve_safe_destination`` by mocking ``getaddrinfo`` to return only malformed IP strings. - ``TestSafeGetPinning.test_safe_get_allow_private_bypasses_pinning``: mirrors the existing safe_post allow_private bypass test for the read-side helper. Sonar S5332 false-positive - ``tests/test_http_dns_rebinding.py::test_http_session_does_not_mount_ssl_adapter``: the ``http://`` literal is a fixture for the SSL-adapter dispatch branch — no outbound call, no insecure egress. ``# NOSONAR python:S5332`` with justification on the two flagged lines. Skipped (with rationale) - sourcery overall #2 ("distinguish IP-literal-private vs mixed-result reasons"): minor message-specificity, deferred to a separate PR to keep this review-response focused. - sourcery overall #3 ("per-process Session cache to amortise mount cost"): premature optimisation — webhook / judge calls are low- frequency, the per-call ``Session()`` is microsecond-cheap, and a shared session would complicate the SNI-per-host model. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eDict Issue #14 review feedback (codex): the previous round used a plain ``dict(headers or {})`` to materialise the outbound header set, then ``setdefault("Host", ...)`` to fall back to the auto-derived value. Python's ``dict`` is case-sensitive, so a caller passing ``headers={"host": "operator-supplied.example.com"}`` (lowercase) would trigger: request_headers = {"host": "operator-supplied.example.com"} request_headers.setdefault("Host", "<auto>") # → {"host": "operator-supplied.example.com", "Host": "<auto>"} Two Host entries on the wire — invalid per RFC 7230 § 5.4 and a silent break of the "caller override wins" contract this PR's ``test_explicit_host_header_not_overridden`` tried to lock down. Fix: switch to ``requests.structures.CaseInsensitiveDict``, which collapses every casing of the same header to one logical slot. ``"Host" not in request_headers`` (replacing the ``setdefault``) is also case-insensitive, so an explicit caller-supplied ``host`` / ``HOST`` / ``Host`` all suppress the auto-Host injection correctly. Applied to both ``safe_post`` and ``safe_get`` (the identical pattern). ``_mask_secrets_in_text`` annotation widened to ``Optional[MutableMapping[str, str]]`` so the new outbound type flows through the masking helper without a mypy false-positive. Test: new ``test_caller_host_header_with_different_casing_does_not_duplicate`` in ``TestRfc7230HostHeader`` passes a lowercase ``host`` header and asserts (a) exactly one Host entry on the wire (case-insensitive count), and (b) the caller's value wins. 20 / 20 DNS-rebinding tests + 170 / 170 HTTP-related tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
security: close DNS-rebinding TOCTOU in SSRF guard (closes #14)
Closes the four blocking findings + one P3 from the PR #51 review. Blocking #1 — full suite kırmızı (26 fail in offline/sandbox CI): - forgelm/_http.py::safe_post + safe_get: moved cheap local-policy checks (timeout floor, method allowlist) BEFORE the SSRF DNS resolve. A misconfigured caller (timeout=0, sub-floor value, unsupported method) is now rejected without a DNS round-trip — and unit tests for those branches no longer accidentally depend on ``socket.getaddrinfo`` reaching the network. - tests/test_webhook.py, tests/test_judge_functions.py, tests/test_synthetic.py: module-level autouse fixture ``_stub_ssrf_resolver`` makes ``forgelm._http._resolve_safe_destination`` hermetic. IP literals still flow through the real ``_is_blocked_ip`` policy (so the SSRF-block parametric tests keep their teeth) and ``localhost`` is canonically treated as loopback; every other hostname resolves to the public sentinel ``8.8.8.8``. Dedicated resolver coverage in tests/test_http_dns_rebinding.py is untouched because it lives in its own module and patches the same seam at finer granularity. Blocking #2 — AGENTS.md skill links pointed at .Codex/skills/, a directory that does not exist. Rewrote all 8 links to .agents/skills/ so the agent-mirror tema's primary use path actually works. Blocking #3 — tools/check_no_analysis_refs.py added .agents/**/*.md to ``_PUBLIC_GLOBS``. Previously the .agents/ tree was never scanned, so the ``_EXEMPT`` entry added in commit fcbb926 was silently ineffective and the guard's green result was misleading. The exempt entry is still required (and now actually exercised) because .agents/skills/sync-bilingual-docs/SKILL.md legitimately names the gitignored docs/marketing/ and docs/analysis/ directories. P3 — forgelm/cli/_no_train_modes.py::_resolve_benchmark_load_target return type: PEP 604 ``tuple[str, str | None]`` → ``Tuple[str, Optional[str]]`` to match the project's existing typing style (consistent with every other typed helper in this module). Verification - ruff format + check: clean (192 / 192 files) - 11 / 11 self-review gauntlet guards: green (forgelm --dry-run, bilingual parity 46 pairs, anchor resolution 271 md, CLI help consistency 447 invocations, wizard defaults, no-analysis-refs with new glob, no unguarded sys.modules pop, site version). - HTTP-touching modules (webhook + judge + synthetic + dns_rebinding): 96 / 96 green. - Full pytest tests/ (--ignore=test_phase12_review_fixes.py, the documented perf-flake that the previous review marked non-blocking): **1779 passed, 13 skipped, 0 failed in 954.74s (~16 min).** Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
Review response — 188fc8a (pushed to development, auto-included in this PR)All four blocking findings + the P3 fixed. Full suite is green now.
Verification
Onaylanmış noktalar
Nitpick (blocking değil, follow-up'a uygun)
Trunk-merge gate açık. |
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
AGENTS.md (1)
115-116:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove gitignored working-memory path references from this public doc.
Line 115 and Line 201 still reference
docs/marketing/*, and the same section also namesdocs/analysis/*. This keeps a prohibited public-doc citation pattern alive and creates documentation-policy drift.Suggested minimal rewrite
-Reinforced in [docs/marketing/strategy/05-yapmayacaklarimiz.md](docs/marketing/strategy/05-yapmayacaklarimiz.md). Do not propose or implement: +Reinforced in internal-only strategy guidance. Do not propose or implement: @@ -- The `docs/marketing/` directory is gitignored (internal strategy). Content there is real; treat it as a source of truth for direction but don't reference it in public-facing code or docs. -- The `docs/analysis/` directory is gitignored research / audit working memory (PR-cycle review notes, external-repo comparisons, drafts). **Never reference its contents from production code, public docs, CHANGELOG entries, commit messages, or PR descriptions.** Decisions distilled from those notes live in `docs/standards/`, `docs/roadmap/`, the CHANGELOG, and inline code comments — those are the citations reviewers see. -- The roadmap ([docs/roadmap.md](docs/roadmap.md)) is what ships. The marketing strategy roadmap ([docs/marketing/marketing_strategy_roadmap.md](docs/marketing/marketing_strategy_roadmap.md)) is what gets announced. Don't conflate the two. +- Internal strategy and working-memory directories are gitignored and must not be cited from public docs or production artifacts. +- Use public sources for citations: `docs/standards/`, `docs/roadmap/`, `CHANGELOG.md`, and code comments. +- The roadmap ([docs/roadmap.md](docs/roadmap.md)) is the public shipping plan. Keep internal planning artifacts out of public references.As per coding guidelines: “Never reference docs/analysis/ … from … public docs” and “Never reference docs/marketing/ in public-facing code or docs”.
Also applies to: 199-201
🤖 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 `@AGENTS.md` around lines 115 - 116, Remove all gitignored working-memory path references in AGENTS.md: search for the literal strings "docs/marketing/" and "docs/analysis/" (they appear near the referenced lines, e.g., the occurrences that mention docs/marketing/* and docs/analysis/*) and either delete those path mentions or replace them with appropriate public-safe placeholders (e.g., "internal/marketing" or "internal/analysis" or a generic "internal docs") so the public document no longer references private working-memory paths; ensure any cross-reference text and surrounding sentences remain grammatically correct after the replacement.
🤖 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 `@AGENTS.md`:
- Line 47: The fenced tree code block containing the "ForgeLM/" listing in
AGENTS.md is missing a language tag; update that fenced block (the
triple-backtick before the tree output) to use a language tag such as "text" so
the fence becomes ```text, ensuring markdownlint rule MD040 is satisfied.
In `@tools/check_no_analysis_refs.py`:
- Around line 112-114: The PR adds an exemption entry "AGENTS.md":
frozenset({"docs/marketing/", "docs/analysis/"}) to the _EXEMPT set which
undermines the guard by allowing prohibited docs/marketing/ and docs/analysis/
references to bypass checks; remove the "AGENTS.md" exemption (and any duplicate
exemptions at the other occurrences around the same block) so .agents/**/*.md
stays in scan scope, and instead update the AGENTS.md (and mirrored skill docs)
to remove direct path references to docs/marketing/ and docs/analysis/ so the
policy guard can enforce the rule.
---
Duplicate comments:
In `@AGENTS.md`:
- Around line 115-116: Remove all gitignored working-memory path references in
AGENTS.md: search for the literal strings "docs/marketing/" and "docs/analysis/"
(they appear near the referenced lines, e.g., the occurrences that mention
docs/marketing/* and docs/analysis/*) and either delete those path mentions or
replace them with appropriate public-safe placeholders (e.g.,
"internal/marketing" or "internal/analysis" or a generic "internal docs") so the
public document no longer references private working-memory paths; ensure any
cross-reference text and surrounding sentences remain grammatically correct
after the replacement.
🪄 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: 36f2a214-68f1-4749-9b1e-9eddf7563d83
📒 Files selected for processing (10)
AGENTS.mdCHANGELOG.mdforgelm/_http.pyforgelm/cli/_no_train_modes.pypyproject.tomltests/test_http_dns_rebinding.pytests/test_judge_functions.pytests/test_synthetic.pytests/test_webhook.pytools/check_no_analysis_refs.py
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- forgelm/cli/_no_train_modes.py
Comment on lines
+112
to
+114
| # AGENTS.md is the agent-agnostic mirror of CLAUDE.md (Codex / Cursor / | ||
| # generic AI-agent surface); it restates the same gitignored-dir policy. | ||
| "AGENTS.md": frozenset({"docs/marketing/", "docs/analysis/"}), |
Contributor
There was a problem hiding this comment.
These exemptions undermine the guard’s policy objective.
Adding AGENTS.md (and mirrored skill docs) to _EXEMPT lets prohibited docs/marketing/ / docs/analysis/ references pass in public policy docs instead of fixing the source text. The safer pattern is: keep .agents/**/*.md in scan scope, remove these exemptions, and rewrite those docs to avoid direct path references.
As per coding guidelines: “Never reference docs/analysis/ … in public docs” and “Never reference docs/marketing/ in public-facing code or docs”.
Also applies to: 122-124
🤖 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_no_analysis_refs.py` around lines 112 - 114, The PR adds an
exemption entry "AGENTS.md": frozenset({"docs/marketing/", "docs/analysis/"}) to
the _EXEMPT set which undermines the guard by allowing prohibited
docs/marketing/ and docs/analysis/ references to bypass checks; remove the
"AGENTS.md" exemption (and any duplicate exemptions at the other occurrences
around the same block) so .agents/**/*.md stays in scan scope, and instead
update the AGENTS.md (and mirrored skill docs) to remove direct path references
to docs/marketing/ and docs/analysis/ so the policy guard can enforce the rule.
Reviewer (codex) flagged the dilsiz triple-backtick at line 47 of
AGENTS.md. markdownlint MD040 ("fenced code block without language")
applies to opening fences; the fence at line 98 is the closing fence
of the same block and stays bare. Same fix mirrored to CLAUDE.md so
the two agent-guidance trees stay structurally identical.
Skipped (reviewer's other two findings — same theme):
* "Remove the AGENTS.md entry from _EXEMPT and rewrite the
docs/marketing/ / docs/analysis/ references to placeholders."
Rationale: AGENTS.md is the agent-agnostic mirror of CLAUDE.md
(see memory entry ``project-agents-md-mirror``). The 5 path
references in AGENTS.md (lines 115, 169, 199-201) are byte-for-byte
copies of the same references in CLAUDE.md, which has been on the
_EXEMPT list for years. These are policy-statement documents —
they cite the gitignored directories by their real names so the
agent knows not to write into them. Replacing the names with
placeholders ("internal/marketing", etc.) hides the actual
directory name from the very agent the rule is trying to instruct,
defeating the policy. Applying the change only to AGENTS.md would
break mirror parity with CLAUDE.md; applying it to both is a
whole-design pivot that is out of scope for PR #51.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
Review response — c83e0adFix applied
Skipped (with rationale)
AGENTS.md, CLAUDE.md'nin agent-agnostic mirror'ı — 5 path referansının (lines 115, 169, 199-201) birebir CLAUDE.md karşılığı var (grep doğruladı, aynı satır numaraları). CLAUDE.md yıllardır İki doküman da policy-statement niteliğinde — gitignored dizinleri gerçek adlarıyla zikretmek bilinçli: agent'a "buraya yazma" derken hangi path'in kastedildiğini saklamak (örn. "internal/marketing") agent'a yararlı bilgiyi vermemek olur. Policy guard'ın amacı production code / public docs'ta bu path'leri citation olarak kullanmayı engellemek; agent guidance dokümanı bu kuralın kendisini ifade etmek için path'leri zikretmek zorunda. Sadece AGENTS.md'yi değiştirmek mirror parity'yi kırardı (sonraki review döngüsünde "AGENTS.md drift'le" gelir). CLAUDE.md + AGENTS.md ikisini birden placeholder'a çevirmek ise whole-design pivotu — PR #51 scope'u dışı. Verification
|
|
cemililik
added a commit
that referenced
this pull request
May 15, 2026
Two unrelated work items landed together because they were spotted in the same review pass. ────────────────────────────────────────────────────────────────── 1. Nightly CI: requests-toolbelt missing on --no-deps install ────────────────────────────────────────────────────────────────── Nightly compatibility workflow (.github/workflows/nightly.yml) ran on ``main`` HEAD ccb7d84 (PR #51 merge) and failed: E ModuleNotFoundError: No module named 'requests_toolbelt' Across three jobs: - Test with minimum supported versions (3.10) - Test with latest dependencies (3.11) - Test with latest dependencies (3.12) Root cause: the v0.6.0 SSRF DNS-rebinding hardening (issue #14, PR #52) added ``requests-toolbelt>=1.0.0,<2.0.0`` as a hard runtime dependency in pyproject.toml. Both failing nightly jobs install ForgeLM via ``pip install -e . --no-deps`` (after a manual pip install of the dependency list) — so the manual list MUST mirror the pyproject dependency declarations, but ``requests-toolbelt`` was never added. The 2026-05-15 nightly is the first run with this test surface on main, hence the first failure. Fix: - ``test-latest-deps`` matrix: add ``requests-toolbelt`` to the unconstrained latest-deps pin line. Added a comment block that spells out the contract (this list MUST mirror pyproject because the next step is --no-deps) so a future dependency addition doesn't recreate this gap. - ``test-min-deps``: pin ``requests-toolbelt==1.0.0`` (the lower bound from pyproject's ``>=1.0.0,<2.0.0`` range) and added the same contract comment. - The fix lands on development and propagates to main via PR #54. ────────────────────────────────────────────────────────────────── 2. PR #54 review findings — 3 valid (fix) + 1 skipped (reasoned) ────────────────────────────────────────────────────────────────── SKIPPED — docs/reference/configuration-tr.md L313 "en az 1 zorunlu" Reviewer wanted "minimum 2 stages" but the schema enforces ``min_length=1`` (verified in ``forgelm/config.py:1351``). The v0.7.0 cut CHANGELOG explicitly unified the wording around "at least 1 stage" with the note that single-stage ergonomics are better served by omitting the ``pipeline:`` block, but the schema accepts a 1-element pipeline so an operator can iterate up to a multi-stage chain without re-shaping the YAML. Reviewer appears to be reading a prior unmerged variant; current code + docs are internally consistent. FIXED — docs/roadmap{,-tr}.md "Latest release on PyPI" / "PyPI'deki son sürüm" still said v0.5.7 v0.6.0 shipped 2026-05-11; both EN and TR roadmaps still pointed "Latest release on PyPI" at v0.5.7, contradicting the same page's ``Current state: ... v0.6.0`` line just below. Rewrote both paragraphs to lead with v0.6.0 (Phase 15 Ingestion Pipeline Reliability, 2026-05-11) and the full ingest-time-guard enumeration, with a roadmap link to ``completed-phases.md``. FIXED — forgelm/cli/_dispatch.py L277 stale "v0.7.0" comment The dispatch comment for the ``--data-audit`` deprecation said "slated for removal in v0.7.0" but the matching DeprecationWarning message + audit-log ``version`` field were already updated to v0.8.0 in the v0.7.0 cut cycle (per the one-minor warning window policy). Updated the inline comment to match, with a reference to ``docs/standards/release.md#deprecation-cadence``. FIXED — tests/test_pipeline_cli.py:140 IndexError defensive guard (Sonar python:S1244 / S6395 Critical) ``test_empty_input_model_normalised_to_none`` accessed ``configs_seen[0]`` without first asserting the list was non-empty. If a future dispatch-order change short-circuits the run before the fake trainer is instantiated, the test would fail with an opaque ``IndexError: list index out of range`` instead of a "no stage ran" message. Added an ``assert configs_seen, "..."`` guard with a descriptive failure message before the index access. Verification: - ruff format + check clean - 196 / 196 targeted tests pass - All 11 self-review guards green Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bringing
development(7 commits ahead ofmain, 25 files, +2141/-43) into the trunk. Three independent themes that all landed in one cycle and have already been merged + reviewed insidedevelopment:.agents/tree +AGENTS.mdproviding agent-agnostic guidance (Codex / Cursor / generic AI surfaces), mirroring the existing.claude/skills/+CLAUDE.mdcontent._EXEMPTentries for the new agent docs and a stabilisation refactor for thetest_unsloth_backend_raises_without_packageorder-sensitive flake.Bug fixes from the review batch
--benchmark-onlyevaluated fresh-init LoRA, not the trained checkpoint_no_train_modes.py::_run_benchmark_onlynow routes throughinference.load_model()with PEFT auto-detection + fail-loud on malformedadapter_config.json_ensure_validation_splitcrashed on <2-row datasetsdata.pyguard returns the dataset unchanged with a warningeval_strategy="steps"witheval_dataset=Nonewhen validation absenttrainer.py::_get_common_training_kwargsdownshifts toeval_strategy="no"+ disablesload_best_model_at_endtests/test_compliance.pymocks_fingerprint_hf_metadata+_fingerprint_hf_revision_approve.py::_resolve_approver_identityexits withEXIT_CONFIG_ERRORunlessFORGELM_ALLOW_ANONYMOUS_OPERATOR=1judge_results.json/safety_results.jsonleaked raw prompts, responses, and judge reasoningJudgeConfig.include_eval_samples/SafetyConfig.include_eval_samples_parse_judge_jsonlogs only length and a fixed reason stringTrainingConfigfields had no boundsge/gt/leconstraints added--data-auditbreadcrumb caught onlyOSError(OSError, ConfigError)so anonymous-operator failures don't abort the deprecation loginference.load_modelprefersadapter/tokenizer_config.jsonwhen presentWhat's now in the public tree
.agents/skills/{add-config-field, add-test, add-trainer-feature, cut-release, review-pr, sync-bilingual-docs}/SKILL.md(6 skills)AGENTS.md(CLAUDE.md mirror for non-Claude AI surfaces)_PII_REDACT_FIELDSmodule-level constants injudge.py+safety.py_CategoryTelemetrydataclass bundling Phase 9 telemetry args (Sonar S107 fix)_resolve_benchmark_load_target+_pick_tokenizer_sourcehelpers (Sonar S3776 fixes)Verification
ruff format+ruff check: clean (107 / 168 files)forgelm --config config_template.yaml --dry-run_EXEMPTentries)sys.modulespoppytest tests/test_inference.py: 18/18 green (flake fixed in fcbb926)test_no_train_modes.py,test_data_edge_cases.py,test_trainer.py,test_human_approval_gate.py,test_judge_functions.py,test_safety_advanced.py,test_inference.pyOut of scope (deferred to separate PRs)
requests-toolbeltas a new dep and SNI/cert validation testing acrossrequestsversions.B105-style "potential hardcoded secret" false-positive on theUsing tokenizer from adapter directory: %slog line — flagged as false positive; no source-side suppression added per owner decision.Test plan
forgelm --benchmark-only <peft-checkpoint>against a real PEFT adapterforgelm --benchmark-only <merged-model>against a merged full-weight modelforgelm approve <run_id>underFORGELM_OPERATOR=ci-botand again with neither env varinclude_eval_samples=false(default) producesjudge_results.jsonwith noprompt/response/reasonfields🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Documentation
Tests