Repository navigation
Wave 1 post-merge audit closure (2 commits) - #22
Merged
Merged
Conversation
Wave 1 PR (closure/wave1-faz9-32) merged into main. Plan tracking state synchronised with reality: - 12 phases (Faz 9, 10, 11, 12, 13, 14, 15, 25, 27, 29, 31, 32) flipped from "✅ Wave 1 PR" to "✅ DONE" — code shipped, in main. - 18 phases (Faz 16, 17, 18, 19, 20, 21, 22, 23, 24, 26, 28, 30, 34, 35, 36, 37, 38) flipped from "🔄 Wave 1 IN PROGRESS" to "⏳ PENDING" — Wave 2+ scope. - Faz 33 (release) stays "⏳ POST-WAVE". - Status legend rewritten so the three states are unambiguous and carry the merge date as a fixed reference point. - §16 versioning entry v2.3 added with the count of phases moved. No content phase change; tracking-only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wave 1 audit raporundaki açık iş'ler kapatıldı. Plan §11 Code 5/5 + Faz 7 trust_remote_code disiplini + Faz 10 risk taxonomy artık tamamen temiz. CODE — Faz 7 acceptance gate (5 from_pretrained sites): - synthetic.py:260,261 — explicit trust_remote_code=False on local teacher model load (synthetic-data path). - judge.py:166,167 — explicit trust_remote_code=False on local judge model (auto-revert gate consults this). - export.py:151,152 — explicit trust_remote_code=False on the GGUF export merge step. model.py / inference.py / merging.py / fit_check.py already pin via ``model_kwargs["trust_remote_code"]`` or explicit kwargs; the audit's literal grep was a false positive there. CODE — Faz 10 risk taxonomy expansion: - config.py: ``RiskAssessmentConfig.risk_category`` and ``ComplianceMetadataConfig.risk_classification`` Literals widened from the 3-tier shipped set (high-risk/limited-risk/minimal-risk) to the 5-tier EU AI Act set (unknown/minimal-risk/limited-risk/high-risk/ unacceptable). Default stays "minimal-risk" so existing configs validate unchanged. ``unacceptable`` covers Article 5 prohibited practices, ``unknown`` is the explicit unclassified placeholder. - test_config_literal_sweep.py: TestRiskClassificationLiteral updated to cover all 5 values + default-stable test; new TestRiskCategoryLiteral mirror. +9 tests (1001 → 1010). CODE — Faz 27 wider except Exception cluster (~22 sites): The original Faz 27 commit narrowed safety/trainer/judge/compliance/ _streaming, but ~22 ``except Exception as e:`` sites in deploy / benchmark / synthetic / export / chat / ingestion / merging / quickstart / utils / fit_check / inference / model + 2 cli dispatchers were left without BLE001 carve-out justification. Each site now carries a context-aware comment explaining the broad failure surface it covers (subprocess + filesystem + torch + tokenizer + network + HF Hub + lm-eval + pypdf-internal + ...) and why a narrow class is infeasible. Two sites (benchmark.py JSON write + quickstart.py audit event write + export.py model_integrity) narrowed to (OSError, TypeError, ValueError) instead of BLE001 since their surface is genuinely small. DOCS — drift fixes: - docs/roadmap.md:7: header "Tür" → "Type" (Faz 1 quick-win that the original carry-over registry missed). - closure-plan-202604300906.md Faz 25 acceptance: noted the e57aaf8 site-picker revert to 6 languages so the plan no longer contradicts ``site/js/i18n.js:27``. - closure-plan Faz 27 acceptance: added the wider-grep gate (``except Exception as `` form) so the next audit cannot find the same hole. - closure-plan Faz 15: acknowledgement note that forgelm/cli/_parser.py 662 lines is deliberate and exceeds the 600-line cohesion ceiling on purpose (single-source argparse registrar dosyası). - closure-plan §16 versioning: v2.4 entry summarising this round. Tests: 1001 → 1010 passed (+9 risk-Literal tests). ruff format/check clean. Coverage 67.93% (unchanged within rounding). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuidePost-merge audit follow-up that tightens safety/configuration guarantees (explicit trust_remote_code defaults, expanded EU AI Act risk literals, documented broad exception handling) and synchronizes the closure plan/status docs with what actually shipped in Wave 1. Updated class diagram for EU AI Act risk configuration modelsclassDiagram
class BaseModel
class RiskAssessmentConfig {
+str intended_use
+List~str~ foreseeable_misuse
+Literal~unknown|minimal-risk|limited-risk|high-risk|unacceptable~ risk_category = "minimal-risk"
+List~str~ mitigation_measures
+bool vulnerable_groups_considered
}
class ComplianceMetadataConfig {
+str provider_name
+str deployment_region
+str data_controller
+str data_processor
+str intended_purpose
+str known_limitations
+str system_version
+Literal~unknown|minimal-risk|limited-risk|high-risk|unacceptable~ risk_classification = "minimal-risk"
}
BaseModel <|-- RiskAssessmentConfig
BaseModel <|-- ComplianceMetadataConfig
%% Change focus
%% - Both risk_category and risk_classification now share the same 5-tier EU AI Act Literal set
%% - Defaults remain "minimal-risk" for backward-compatible config validation
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 12 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Review Summary by QodoWave 1 post-merge audit closure — trust_remote_code pins, risk taxonomy expansion, exception handling narrowing
WalkthroughsDescription• Close Wave 1 post-merge audit findings with explicit trust_remote_code=False pins on 6 from_pretrained sites (synthetic.py, judge.py, export.py) • Expand risk taxonomy Literals from 3 to 5 EU AI Act tiers (unknown, minimal-risk, limited-risk, high-risk, unacceptable) with backward-compatible defaults • Narrow ~22 broad except Exception as e: catches to context-aware # noqa: BLE001 with justification comments across 10+ modules • Update closure-plan v2.3/v2.4 status tracking: 12 Wave 1 phases marked DONE, 17 phases deferred to Wave 2+ • Fix roadmap.md header "Tür" → "Type" and add 9 new risk-taxonomy tests Diagramflowchart LR
A["Wave 1 PR merged<br/>into main"] -->|"Audit findings<br/>post-merge"| B["Code closures<br/>3 commits"]
B -->|"Faz 7: trust_remote_code"| C["synthetic.py<br/>judge.py<br/>export.py"]
B -->|"Faz 10: risk taxonomy"| D["RiskAssessmentConfig<br/>ComplianceMetadataConfig<br/>3→5 Literal values"]
B -->|"Faz 27: exception handling"| E["~22 sites<br/>BLE001 + justification"]
B -->|"Faz 1: doc fix"| F["roadmap.md<br/>header correction"]
B -->|"Plan sync"| G["closure-plan v2.3/v2.4<br/>status update"]
D -->|"backward compatible"| H["Default unchanged<br/>existing configs valid"]
E -->|"context-aware"| I["benchmark, chat, deploy<br/>export, fit_check, etc."]
File Changes1. forgelm/benchmark.py
|
Code Review by Qodo
1.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The EU AI Act risk tier literals are now duplicated between
RiskAssessmentConfig.risk_categoryandComplianceMetadataConfig.risk_classification; consider centralising the allowed values in a single constant or enum so future taxonomy updates cannot drift despite the lockstep intent. - Several of the new
# noqa: BLE001comments are quite long and repetitive; you might consider extracting a shared error-handling guideline (or referencingerror-handling.md) and tightening the inline justifications to keep the call sites more readable while still satisfying the audit requirements.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The EU AI Act risk tier literals are now duplicated between `RiskAssessmentConfig.risk_category` and `ComplianceMetadataConfig.risk_classification`; consider centralising the allowed values in a single constant or enum so future taxonomy updates cannot drift despite the lockstep intent.
- Several of the new `# noqa: BLE001` comments are quite long and repetitive; you might consider extracting a shared error-handling guideline (or referencing `error-handling.md`) and tightening the inline justifications to keep the call sites more readable while still satisfying the audit requirements.
## Individual Comments
### Comment 1
<location path="forgelm/config.py" line_range="326-329" />
<code_context>
+ # transparency-only and unrestricted tiers; ``unknown`` is the explicit
+ # placeholder for systems that have not yet been classified. The default
+ # stays ``"minimal-risk"`` so existing configs validate unchanged.
+ risk_category: Literal["unknown", "minimal-risk", "limited-risk", "high-risk", "unacceptable"] = "minimal-risk"
mitigation_measures: List[str] = []
vulnerable_groups_considered: bool = False
</code_context>
<issue_to_address>
**suggestion:** The EU AI Act risk Literal is duplicated and could be centralized as a shared type alias
`risk_category` and `risk_classification` currently duplicate the same `Literal[...]` definition. To keep them synchronized, define a shared alias (e.g. `RiskTier = Literal[...]`) in a common module and reference that in both places to prevent future drift.
Suggested implementation:
```python
# EU AI Act risk taxonomy. ``unacceptable`` covers Article 5 prohibited
# practices; ``high-risk`` covers Article 6 systems requiring full Annex
# IV documentation; ``limited-risk`` and ``minimal-risk`` cover the
# transparency-only and unrestricted tiers; ``unknown`` is the explicit
# placeholder for systems that have not yet been classified. The default
# stays ``"minimal-risk"`` so existing configs validate unchanged.
risk_category: RiskTier = "minimal-risk"
```
```python
# See ``RiskAssessmentConfig.risk_category`` for the taxonomy rationale;
# the risk tiers are kept in lockstep via the shared ``RiskTier`` alias so
# the canonical EU AI Act taxonomy remains a single source of truth.
risk_classification: RiskTier = (
"minimal-risk"
```
To fully implement the shared alias and avoid duplication across the codebase:
1. Define the `RiskTier` alias in a shared/common module, for example in a new or existing types module:
- e.g. in `forgelm/types.py`:
- `from typing import Literal`
- `RiskTier = Literal["unknown", "minimal-risk", "limited-risk", "high-risk", "unacceptable"]`
2. Import and use this alias in `forgelm/config.py`:
- Add: `from forgelm.types import RiskTier`
- Ensure any other places that currently use the duplicated `Literal[...]` for EU AI Act tiers are updated to use `RiskTier` instead.
3. If you prefer to keep the alias in `forgelm/config.py` itself, define `RiskTier` at module scope (not inside a class) right after the typing imports, and remove any other duplicated `Literal[...]` definitions in this file.
</issue_to_address>
### Comment 2
<location path="tests/test_config_literal_sweep.py" line_range="85" />
<code_context>
+ assert c.risk_classification == "minimal-risk"
+
+
+class TestRiskCategoryLiteral:
+ """``RiskAssessmentConfig.risk_category`` mirrors ``risk_classification``.
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test that `RiskAssessmentConfig.risk_category` keeps the expected default value, mirroring the `risk_classification` default test.
To fully validate that these fields stay in sync as described in the docstring, please add an analogous default-value test for `RiskAssessmentConfig.risk_category`, e.g.:
```python
def test_minimal_risk_default(self):
from forgelm.config import RiskAssessmentConfig
r = RiskAssessmentConfig()
assert r.risk_category == "minimal-risk"
```
This will surface any future divergence between the two defaults.
</issue_to_address>
### Comment 3
<location path="docs/analysis/code_reviews/closure-plan-202604300906.md" line_range="861" />
<code_context>
**Faz 25 Acceptance:**
-- Site picker sadece EN+TR.
+- ~~Site picker sadece EN+TR.~~ **OVERTURNED (commit `e57aaf8`):** Picker 6 dile geri çekildi — orijinal trim "wrong premise" gerekçesiyle revert edildi. `site/js/i18n.js:27` `SUPPORTED = ['en', 'tr', 'de', 'fr', 'es', 'zh']`. Yeni policy: site-level translations (i18n.js) 6 dil, user-manual content EN+TR-only + sparse-emit (deferred languages için manual EN-fallback). `docs/standards/localization.md:8-13` ve commit `e57aaf8` mesajı authoritative kaynaklar.
- `tools/check_site_claims.py --strict` exit 0; bir testte `compliance.py`'da artefact filename değiştir → CI fail.
- Tüm site sayfaları doğru `og:locale` + `hreflang`.
</code_context>
<issue_to_address>
**suggestion (typo):** Cümlenin son kısmı Türkçe açısından eksik; "authoritative kaynaklar" ifadesi daha düzgün bağlanabilir.
Son bölümdeki "`docs/standards/localization.md:8-13` ve commit `e57aaf8` mesajı authoritative kaynaklar." ifadesini "authoritative kaynaklar olarak alınmalı" ya da "authoritative kaynaklardır" şeklinde tamamlayarak daha akıcı hale getirebilirsiniz.
```suggestion
- ~~Site picker sadece EN+TR.~~ **OVERTURNED (commit `e57aaf8`):** Picker 6 dile geri çekildi — orijinal trim "wrong premise" gerekçesiyle revert edildi. `site/js/i18n.js:27` `SUPPORTED = ['en', 'tr', 'de', 'fr', 'es', 'zh']`. Yeni policy: site-level translations (i18n.js) 6 dil, user-manual content EN+TR-only + sparse-emit (deferred languages için manual EN-fallback). `docs/standards/localization.md:8-13` ve commit `e57aaf8` mesajı authoritative kaynaklardır.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![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 implements a post-merge audit closure for Wave 1, updating the project's closure plan and roadmap to reflect completed phases. Key technical improvements include hardening security by explicitly setting trust_remote_code=False during model loading and refining error handling across numerous modules by narrowing exception types or adding justified suppressions for broad catches. Additionally, the EU AI Act risk classification taxonomy was expanded in the configuration schema with accompanying tests. Feedback was provided to improve consistency in exception handling within the export module by including TypeError and removing redundant exception types.
Comment on lines
+212
to
+214
| except (OSError, ValueError, json.JSONDecodeError) as e: | ||
| # OSError: filesystem write / read failure on integrity_path. | ||
| # ValueError / JSONDecodeError: corrupt or partial existing manifest. |
There was a problem hiding this comment.
For consistency with the other two narrowed exception sites (benchmark.py:96 and quickstart.py:369), TypeError should be caught here as well. json.dump can raise TypeError if the object is not serializable. Also, json.JSONDecodeError is a subclass of ValueError, so it is redundant when ValueError is already caught.
Suggested change
| except (OSError, ValueError, json.JSONDecodeError) as e: | |
| # OSError: filesystem write / read failure on integrity_path. | |
| # ValueError / JSONDecodeError: corrupt or partial existing manifest. | |
| except (OSError, TypeError, ValueError) as e: | |
| # OSError: filesystem write / read failure on integrity_path. | |
| # TypeError / ValueError: serialization failure or corrupt manifest. |
Collaborator
Author
There was a problem hiding this comment.
Fixed in 7abf9a3 — except (OSError, ValueError, json.JSONDecodeError) → except (OSError, TypeError, ValueError). Comment updated to note that JSONDecodeError subclasses ValueError so it's covered by the broader catch.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
forgelm/synthetic.py (1)
63-69:⚠️ Potential issue | 🟠 MajorRe-raise ImportError for missing optional dependencies instead of swallowing with per-prompt soft-fail.
_generate_onecatchesException, so ImportError from lazy-loaded local teacher dependencies (torch, transformers) gets downgraded into repeated prompt failures instead of failing fast with an install action. This violates the requirement inforgelm/**/*.pycoding guidelines: "Optional dependencies must raise ImportError with install hints when missing instead of silent import fallbacks."The call chain is:
_generate_one(line 63) →_call_teacher(line 167) →_call_local_teacher(line 223) →_load_local_teacher(line 254, lazy imports at lines 256–257). When torch or transformers cannot be imported, the ImportError propagates back and hits the broadexcept Exception as eat line 65, causing it to be treated as a soft-fail with structured error capture rather than propagating the setup-time error.Add
except ImportError: raisebefore the broad Exception handler (precedent:forgelm/ingestion.pyuses this same pattern). Consider adding an install hint message when re-raising, following the pattern inforgelm/model.pyfor optional backends.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forgelm/synthetic.py` around lines 63 - 69, The broad except in _generate_one is swallowing ImportError from lazy local-teacher imports (propagated from _call_teacher → _call_local_teacher → _load_local_teacher), so add an explicit except ImportError that re-raises (optionally augmenting with an install hint like the pattern in forgelm/model.py) immediately before the generic except Exception handler in _generate_one; this ensures missing optional dependencies (torch/transformers) fail fast with a clear Install hint rather than being recorded as per-prompt soft failures.
🧹 Nitpick comments (1)
forgelm/config.py (1)
355-360: ⚡ Quick winDeduplicate the EU AI Act tier set.
These two
Literal[...]definitions are still hand-copied. A shared type alias or constant would keeprisk_categoryandrisk_classificationin lockstep without relying on comments/tests to prevent drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forgelm/config.py` around lines 355 - 360, Replace the duplicated Literal[...] sets with a single shared type alias/constant (e.g., EU_AI_Tier or AIActTier) and use that alias for both RiskAssessmentConfig.risk_category and the risk_classification field so they stay in lockstep; update the two places that currently declare Literal["unknown","minimal-risk","limited-risk","high-risk","unacceptable"] to reference the new alias instead of repeating the literal list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@forgelm/config.py`:
- Around line 320-326: The new "unacceptable" value for risk_category is
validated but not propagated to runtime gates or the interactive wizard; update
all places that currently check for risk_category == "high-risk" in
forgelm/config.py to also include "unacceptable" (e.g., change boolean checks or
conditionals that treat only "high-risk" specially to treat both "high-risk" and
"unacceptable"), and update the interactive prompt in forgelm/wizard.py (around
the existing prompt handling at lines ~220-224) to present "unacceptable" as an
option and to handle its selection with the same (or stricter) gating logic as
high-risk so the new tier is reachable and enforced at runtime.
---
Outside diff comments:
In `@forgelm/synthetic.py`:
- Around line 63-69: The broad except in _generate_one is swallowing ImportError
from lazy local-teacher imports (propagated from _call_teacher →
_call_local_teacher → _load_local_teacher), so add an explicit except
ImportError that re-raises (optionally augmenting with an install hint like the
pattern in forgelm/model.py) immediately before the generic except Exception
handler in _generate_one; this ensures missing optional dependencies
(torch/transformers) fail fast with a clear Install hint rather than being
recorded as per-prompt soft failures.
---
Nitpick comments:
In `@forgelm/config.py`:
- Around line 355-360: Replace the duplicated Literal[...] sets with a single
shared type alias/constant (e.g., EU_AI_Tier or AIActTier) and use that alias
for both RiskAssessmentConfig.risk_category and the risk_classification field so
they stay in lockstep; update the two places that currently declare
Literal["unknown","minimal-risk","limited-risk","high-risk","unacceptable"] to
reference the new alias instead of repeating the literal list.
🪄 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: f0160cd2-8ec6-469d-8a93-d38f07f79f44
📒 Files selected for processing (19)
docs/analysis/code_reviews/closure-plan-202604300906.mddocs/roadmap.mdforgelm/benchmark.pyforgelm/chat.pyforgelm/cli/_training.pyforgelm/cli/subcommands/_chat.pyforgelm/config.pyforgelm/deploy.pyforgelm/export.pyforgelm/fit_check.pyforgelm/inference.pyforgelm/ingestion.pyforgelm/judge.pyforgelm/merging.pyforgelm/model.pyforgelm/quickstart.pyforgelm/synthetic.pyforgelm/utils.pytests/test_config_literal_sweep.py
CodeRabbit flagged the most important one: the new ``unacceptable``
risk tier was reachable via Pydantic but not propagated to the runtime
compliance gate or the wizard prompt. Closed alongside five smaller
review-bot suggestions.
CRITICAL — runtime-gate propagation (CodeRabbit):
- config.py: ``_warn_high_risk_compliance`` now matches a shared
``_STRICT_RISK_TIERS = frozenset({"high-risk", "unacceptable"})`` set
instead of a literal ``"high-risk"`` string, so ``unacceptable`` gets
at least the same auto_revert nudge. A dedicated second warning
fires when the classification is ``unacceptable`` to flag EU AI Act
Article 5 prohibited-practices status.
- wizard.py: interactive risk-classification prompt extended from the
3-tier (minimal/limited/high) menu to the full 5-tier RiskTier set
so the wizard never offers a value the schema would reject and
never hides a value the schema accepts.
- test_eu_ai_act.py: ``test_high_risk_warnings`` updated for the new
warning text; new ``test_unacceptable_risk_warnings`` covers the
Article 5 dedicated notice; new ``test_minimal_risk_does_not_warn``
pins the negative case so a future taxonomy edit cannot silently
start nagging minimal-risk operators.
DRY (Sourcery):
- config.py: introduced ``RiskTier = Literal[...]`` type alias and
rewrote ``RiskAssessmentConfig.risk_category`` and
``ComplianceMetadataConfig.risk_classification`` to reference it.
Also exposed ``_STRICT_RISK_TIERS`` as the single source of truth
for the high-risk/unacceptable gate.
Test coverage (Sourcery):
- test_config_literal_sweep.py: added
``TestRiskCategoryLiteral.test_minimal_risk_default`` mirroring the
existing ``TestRiskClassificationLiteral.test_minimal_risk_default``
so the two fields' defaults can never silently diverge.
Narrow exception class (Gemini):
- export.py:212-219: ``except (OSError, ValueError, json.JSONDecodeError)``
→ ``except (OSError, TypeError, ValueError)``. ``JSONDecodeError`` is
a ``ValueError`` subclass so the previous form was redundant; adding
``TypeError`` matches the json.dump unserialisable-object surface and
keeps this site consistent with the other two narrow-class sites
(benchmark.py:96, quickstart.py:369).
English-only (Qodo):
- export.py / judge.py / synthetic.py / test_config_literal_sweep.py:
``Faz N`` → ``Phase N`` in code comments + the test module docstring.
CLAUDE.md mandates English in code artefacts; the closure plan stays
Turkish (it's a project-internal tracking doc, not source).
Style (Sourcery):
- closure-plan §11 Faz 25 acceptance: Turkish grammar nit —
"authoritative kaynaklar." → "authoritative kaynaklardır." for a
cleaner sentence ending.
Tests: 1010 → 1013 passed (+3 new strict-gate scenario tests).
ruff format/check clean. Coverage 67.95% (up 0.02 from 67.93).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
This was referenced May 5, 2026
7 of 9 tasks
cemililik
added a commit
that referenced
this pull request
Jun 15, 2026
…view) The escaping-path audit emit wrote raw resolved paths + run-id into the append-only chain, bypassing the mandated secret->PII->length-bound sanitiser used on the sibling OSError branches. Route it through _sanitise_audit_error_message. CodeRabbit #22. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Wave 1 PR (#21) merged into
development; this follow-up closes the audit gaps a separate review agent flagged onf3511b4(the merged HEAD). Two commits, fully covered by the existing test suite + ruff gate.Scope is closure-only. No new features, no behaviour changes that an end user would notice. The audit raporu hangi açıkları kapatıyor:
305e1a4— code closuresfrom_pretrainedçağrısına explicittrust_remote_code=Falseeklendi (synthetic.py:260,261,judge.py:166,167,export.py:151,152). Davranışsal etki sıfır (HF default zaten False); plan §11 Code 5/5 disiplini için explicit pin.RiskAssessmentConfig.risk_categoryveComplianceMetadataConfig.risk_classificationLiteral'ları 3 değer'den 5-tier EU AI Act setine genişletildi (unknown,minimal-risk,limited-risk,high-risk,unacceptable). Default"minimal-risk"korundu → mevcut config'ler validate olmaya devam eder. +9 Pydantic test.1445419)safety/trainer/judge/compliance/_streamingmodüllerini kapatmıştı; geri kalan ~22except Exception as e:site context-aware# noqa: BLE001+ justification ile kapatıldı (deploy/benchmark/synthetic/export/chat/ingestion/merging/quickstart/utils/fit_check/inference/model+ 2 cli dispatcher). 3 site (benchmark JSON write, quickstart audit write, export integrity update) gerçekten dar olduğu için narrow class'a çevrildi.docs/roadmap.md:7headerTür→Type.f3511b4— closure-plan v2.3 (status sync)Wave 1 merge'i sonrası plan tracking gerçek durumu yansıtmadı. Tablodaki status'lar güncellendi:
✅ Wave 1 PR→✅ DONE🔄 Wave 1 IN PROGRESS→⏳ PENDING(Wave 2+ kapsamı)Plus closure-plan v2.4 entry'si (
305e1a4'in plan-side tarafı): Faz 25 acceptance'a "site picker reverted to 6 languages (commite57aaf8)" notu, Faz 15'e_parser.py662-line acknowledgement, Faz 27 acceptance gate genişletme, §16 versiyon notu.Test plan
ruff format . && ruff check .cleanpytest tests/ -q→ 1010 passed, 13 skipped (önceden 1001 → +9 risk-taxonomy testi)grep -rn "except Exception as " forgelm/ | grep -v "# noqa: BLE001" | grep -v "# pragma: no cover"→ boşgrep -rn "from_pretrained" forgelm/→ kalan tüm site'lar yatrust_remote_code=Falseexplicit yamodel_kwargs["trust_remote_code"]üzerinden geçiriyor (model.py:195, inference.py:87) yatrust_remote_codeparametresini almayan API (PeftModel, FastLanguageModel)risk_classification="minimal-risk"mevcut config'ler kırılmadıRisk
trust_remote_code=FalseHF default ile aynı. Risk Literal genişletme backward-compatible (default unchanged + var olan 3 değer hâlâ valid).Closes (audit findings)
except Exception as e:cluster (22+ sites)Tür→Typeheader drift_parser.pyline-count overrun acknowledgement🤖 Generated with Claude Code
Summary by Sourcery
Align risk classification config with EU AI Act tiers, harden safety and error-handling gates, and sync the closure plan documentation with the actual Wave 1 post-merge state.
Bug Fixes:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
New Features
unknownandunacceptable).Security
Bug Fixes
Tests
Documentation