Repository navigation
docs(post-merge): refresh roadmap + README + CHANGELOG to match merge… - #17
Conversation
…d-on-main state PR #13 (Phase 12 — v0.5.2) is merged on `main`. This sweep aligns the narrative-doc state with that fact and consolidates the post-merge review-cycle fixes under a single follow-up section. roadmap.md / roadmap-tr.md - Phase 11.5 + 12 status descriptions: "merged to development" → "merged to main (PR #11/#12 / #13)" with the actual merge date. - "Latest release on PyPI" line now correctly says v0.4.5 (PyPI state) instead of v0.5.0. Adds explicit clarification that v0.5.0 / v0.5.1 / v0.5.2 are merged to main but not yet tagged + published. - Phase-count matches v0.5.3 plan: "3 phases (12.5, 13, 14) planned". - TR mirror status icons updated to "🟡 Birleşti" matching the EN "🟡 Merged" — were previously prematurely marked "✅ Tamam". - Mermaid diagram in roadmap-tr.md gains the missing P125 node and bumps merged-but-not-published phases to a lighter green. - New cross-link to issue #14 (webhook SSRF DNS-rebinding TOCTOU) surfaces in both the v0.5.2 row and the v0.5.3 "Next" line as the tracked v0.5.3 hardening follow-up. roadmap/phase-14-pipeline-chains.md / roadmap/releases.md - "starts after Phase 12 lands" → concrete release event (`v0.5.2` PyPI tag is published). - v0.5.1 + v0.5.2 release-page status both moved off "Merged on development" to "Merged on main … remaining release-engineering steps" with the issue #14 hardening reference on v0.5.2. README.md - Always-on secrets-scan list updated: was 7 families with the wrong "private-key headers" wording; now lists all nine families per `forgelm.data_audit.SECRET_TYPES` (full BEGIN…END for OpenSSH / PGP). CHANGELOG.md - New "Fixed — post-PR-#13 review-cycle batches (rounds 8-12)" block in [Unreleased] consolidating the post-merge inline-comment batches: audit log hardening, deployer-instructions Markdown injection, quality-filter denominator, numpy fast-path bits guard, sliding-overlap clamp, batch-tokenizer narrow-except, webhook redirect/secret-fallback safety, trainer governance failure visibility, compliance-manifest exception narrowing, strict messages-format validation, wizard ASCII regex flag, GGUF converter case-insensitive validation, markdown-chunker complexity refactor, bidirectional MinHash extraction, streaming length digest, and the doc/test-count drift sweep. Pointers to issue #14 for the remaining DNS-rebinding follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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. ✨ 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 GuideDocumentation-only sweep to align roadmap, release docs, README, and changelog with Phase 12 (v0.5.2) being merged on main, clarify actual PyPI state, and consolidate post-PR #13 review-cycle fixes with explicit links to issue #14. Flow diagram for updated phase and release roadmap (EN/TR)graph LR
%% Phases
P10["Phase 10<br/>Post-Training Completion"]
P105["Phase 10.5<br/>Quickstart Layer"]
P11["Phase 11<br/>Document Ingestion<br/>& Data Audit"]
P115["Phase 11.5<br/>Ingestion / Audit Polish"]
P12["Phase 12<br/>Data Curation Maturity"]
P125["Phase 12.5<br/>Data Curation Follow-up"]
P14["Phase 14<br/>Pipeline Chains"]
P13["Phase 13<br/>Pro CLI + Dashboard"]
%% Releases
V0_4_0["v0.4.0"]
V0_4_5["v0.4.5 (latest PyPI)"]
V0_5_0["v0.5.0 pending"]
V0_5_1["v0.5.1 pending"]
V0_5_2["v0.5.2 pending"]
V0_5_3["v0.5.3 planned"]
V0_6_0_pro["v0.6.0-pro planned"]
%% Phase dependencies
P10 --> P105
P105 --> P11
P11 --> P115
P115 --> P12
P12 --> P125
P12 --> P14
P14 --> P13
%% Phase → release mapping
P10 -.-> V0_4_0
P105 -.-> V0_4_5
P11 -.-> V0_5_0
P115 -.-> V0_5_1
P12 -.-> V0_5_2
P14 -.-> V0_5_3
P13 -.-> V0_6_0_pro
%% Styling: shipped vs merged-on-main vs planned
style P10 fill:#003300,stroke:#00ff88
style P105 fill:#003300,stroke:#00ff88
style P11 fill:#003300,stroke:#00ff88
%% Tier 11.5 / 12: merged on main but not yet published
style P115 fill:#004400,stroke:#88ff88
style P12 fill:#004400,stroke:#88ff88
%% Phase 12.5 / 14: planned
style P125 fill:#002244,stroke:#00aaff
style P14 fill:#002244,stroke:#00aaff
%% Phase 13: future pro
style P13 fill:#442200,stroke:#ffaa00
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In the README “secrets/credential scan” sentence, the claim of “nine families per
forgelm.data_audit.SECRET_TYPES” is a bit opaque given the grouped wording (e.g., OpenAI/Google, OpenSSH/PGP); consider either enumerating all nine explicitly or rephrasing so the family count and the examples line up more obviously for readers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the README “secrets/credential scan” sentence, the claim of “nine families per `forgelm.data_audit.SECRET_TYPES`” is a bit opaque given the grouped wording (e.g., OpenAI/Google, OpenSSH/PGP); consider either enumerating all nine explicitly or rephrasing so the family count and the examples line up more obviously for readers.
## Individual Comments
### Comment 1
<location path="CHANGELOG.md" line_range="35" />
<code_context>
+ actually evaluated (text-bearing dict rows) instead of `total_samples`.
+ A corpus that's 50 % null but 100 % clean on the rest now reads `1.0`
+ instead of `0.5`.
+- **Numpy-fast-path bits guard** (`forgelm/data_audit.py::compute_simhash`) —
+ the `_compute_simhash_numpy` dispatch now also gates on `bits <= 64`;
+ without it, `np.uint64` would silently truncate digests wider than 64
</code_context>
<issue_to_address>
**nitpick (typo):** Consider correcting the library name capitalization from "Numpy" to "NumPy".
Using the official capitalization (“NumPy”) here will keep the changelog consistent with the project’s name.
```suggestion
- **NumPy-fast-path bits guard** (`forgelm/data_audit.py::compute_simhash`) —
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…ration
- CHANGELOG.md: "Numpy" → "NumPy" so the changelog line uses the
library's official capitalization (the immediately following
identifier `_compute_simhash_numpy` is a Python module name and is
correctly lowercase).
- README.md: split the slash-grouped pairs in the secrets-scan sentence
("OpenAI / Google API keys", "OpenSSH/PGP private-key blocks") into
individual entries so the spoken count of nine families matches the
nine items the reader sees, and add an em-dash framing to make the
enumeration parse cleanly. Source list is unchanged — still
forgelm.data_audit.SECRET_TYPES.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bring in the merged-on-main work from PRs #15 and #16 (multi-language user guide, site refactor, sonar/codacy/gemini fixes) so PR #17 can re-base cleanly. The two textual conflicts are kept on the development side — they ARE the substance of PR #17: - CHANGELOG.md: keep "NumPy" (not "Numpy") — addresses the latest reviewer nit on the v0.5.2 changelog block. - README.md: keep the enumerated nine-family secrets-scan sentence (single items for OpenAI / Google API keys and OpenSSH / PGP private keys) so the spoken count and the listed examples agree, addressing the same review pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the project's documentation and changelog to reflect a series of hardening and bug-fix batches. Key changes include audit log HMAC security improvements, Markdown injection sanitization, refined data quality calculations, and narrowed exception handling in the ingestion and trainer modules. The roadmaps and README were also updated to align with the current project state and upcoming release plans. I have no feedback to provide.
|
…gent review findings - Wave 5 PR #34 round 3 absorption: full sweep of CodeRabbit's 6 OOD + 20 Minor inline review comments PLUS 24 multi-agent review findings (code-correctness / privacy-security / standards / test-rigor docs under docs/analysis/code_reviews/wave5-*.md). HIGH/Major absorptions: - 4 broken `forgelm.cli.subcommands.*` symbol citations across purge/approvals reference docs (en+tr): _run_row_erasure → _run_purge_row_id, _handle_pending → _run_approvals_list_pending, _handle_show → _run_approvals_show (F-W5-PRIV-02). - configuration.md (en+tr): drop `kserve | triton` ghost choices and the entire `## deployment:` ghost YAML block (no `DeploymentConfig` in ForgeConfig; extra="forbid" rejects it at load) — replace with a v0.6.0+ Phase 14 callout (F-W5-S1, F-W5-S2). - ingestion.md (en+tr): drop conflicting "instructions format default" claim; document raw-chunk JSONL with summary-only --output-format {text,json} (CR-OOD ingestion). - closure-plan PR #33 status: line 131 said "PR #33 pending" but line 119 said merged with `01e40ba` (CR-OOD closure-plan). - leakage.md (en+tr): remove `audit.leakage_check.remove_cross_split_overlap` knob reference — knob doesn't exist in code; replace with the documented jq-based removal flow (CR-OOD leakage). - TR + EN experiment-tracking: drop `$ ` prompt prefix in `text`-tagged preview blocks for `forgelm compare-runs` (CR-OOD experiment-tracking). - roadmap mermaid (en+tr): add Phase 12.6 (Closure Cycle) → v0.5.5 node + edge so visual flow matches table (CR-OOD roadmap). Minor absorptions: - phase-12-6-closure-cycle.md L123: stale "(currently 0.5.1rc1)" parenthetical → already-bumped during Wave 5 (F-W5-S4 / CR Minor). - gdpr-erasure.md (en+tr): rename duplicate `### What gets recorded` / `### Neyi kaydeder` H3 to disambiguate Article 17 vs 15 sections (CR Minor #2 + #20). - verify-gguf.md (en+tr) L147: revise misleading "still match SHA-256" hash explanation; clarify magic-header layer doesn't catch truncation independent of sidecar (CR Minor #3). - verify_audit link mismatch in 4 reference docs (en+tr × verify_gguf + verify_annex_iv): label text now matches href (CR Minor #4 + #8). - air_gap_deployment-tr.md L19: add `text` language tag to ASCII diagram fence (CR Minor #6). - site/index.html L374: drop internal "Phase 19" milestone leak; tag reads `embed` (user-facing label) (CR Minor #7). - air_gap_deployment.md L193: fix dangling sentence fragment in HF_TOKEN gated-models tip (CR Minor #9). - performance.md (en+tr) L184: drop ghost `forgelm audit --output-format parquet`; replace with pandas/pyarrow conversion step (CR Minor #10). - phase-11-5-backlog.md L10-12: link text → package paths matching href (forgelm/data_audit.py → forgelm/data_audit/, etc.) (CR Minor #11). - testing.md L140: stale `23/23` → `39/39` bilingual parity count (CR Minor #12). - testing.md L198: `python3` prefix consistency in checklist (CR Minor #13). - supply-chain.md (en+tr): bandit cadence aligned to standards doc (Nightly 03:00 UTC + release tag, not "every PR + main push") (CR Minor #14 + #15). - translations.js L1927: "Three values" → "Two values" (only forgelm-lang + forgelm-theme listed) (CR Minor #16). - approvals_subcommand.md (en+tr) L26: staging path → match L84 JSON sample (`final_model.staging.<run_id>` canonical, run-id-less form honoured as fallback) (CR Minor #17 + #19). - translations.js L1445/L1462/L1504/L1521 (en+tr): `<code>reject</code>` → `<code>forgelm reject</code>` for fully-qualified copy/paste parity (CR Minor #18). - _meta.yaml header: add formal DE/FR/ES/ZH deferred-policy banner per closure-plan Task L (F-W5-S6). - release.md L20 + L359: stale "0.3.1rc1 current" + "no __version__" claims; corrected to 0.5.5 + reflect that __version__ + __api_version__ ARE exported per Library API standard (F-W5-S3). - purge_subcommand.md (en+tr): add WARNING-on-fallback language for --output-dir asymmetry vs reverse-pii (F-W5-PRIV-05). - verify-audit.md (en+tr): replace overstated "escrow" wording with KMS/HSM-based separation (F-W5-PRIV-06). Tool tightening + new tests: - check_cli_help_consistency.py: tighten _FORWARD_REF_MARKERS to whole-phrase forms (drop bare "planned"/"future"/"roadmap" which matched "planned attacks" / "future work" / "roadmap-driven" in unrelated prose); add Turkish equivalents for bilingual docs (F-W5-01). - test_check_cli_help_consistency.py: 3 new regression tests: bare-substring negative (F-W5-01), >±3-line forward-ref window negative (F-W5-TR-01a), and Wrong:-block scope-to-one-block resumption (F-W5-TR-01b). Suite 17 → 20 tests, all green. User-manual orphan fix: - _meta.yaml: register `reference/json-output` page (existed in filesystem since Wave 2 but never declared in nav config — was invisible on the live site). Now declared = on-disk = 65 each. Skipped (with reason): - CR Minor #5 (library_api.md `forgelm.__version__` example): INVALID — verified `python3 -c "import forgelm; print(forgelm.__version__)"` prints 0.5.5 cleanly; both __version__ and __api_version__ ARE exported per Library API standard. CodeRabbit's web-search must have hit pre-Wave-5 PyPI. Doc is correct; code matches. - F-W5-S5 (parity tool autodiscover usermanuals): registry shape refactor (~50 new pairs) is scope creep for round 3; defer to v0.6. - F-W5-S8 (compliance_summary-tr.md): explicitly deferred to v0.6 by the finding itself. - F-W5-S9, F-W5-TR-11/12 (NIT cite/style): non-load-bearing. - F-W5-02/03/04 (cognitive complexity, unterminated fence, resume-after-Wrong unit test): refactor scope; covered by F-W5-TR-01 block-scope tests for the resume case. - F-W5-TR-02 (cleanup commit baseline regression test): the live CI --strict gate IS the guard; doubling up the integration-test layer is defensible but not blocking. - F-W5-TR-03/05/06/07/08/09/10 (extras): non-blocking coverage gaps. Self-review gauntlet (all green): ruff format/check ✅ bilingual_parity --strict 39/39 ✅ anchor_resolution --strict 264/264 ✅ cli_help_consistency --strict 425/425 ✅ (reduced from 449 due to intentional invocation removals: ghost YAML block, parquet ghost, 4 broken symbol-cite paths, etc.) site_claims --strict (artefacts/templates/GPU/version) ✅ field_descriptions --strict ✅ pytest 1433 passed / 14 skipped (+5 new tests vs round 2) ✅ forgelm --config config_template.yaml --dry-run ✅ build_usermanuals.py: en/tr 65/65 OK, JS bundles regenerate ✅ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>



…d-on-main state
PR #13 (Phase 12 — v0.5.2) is merged on
main. This sweep aligns the narrative-doc state with that fact and consolidates the post-merge review-cycle fixes under a single follow-up section.roadmap.md / roadmap-tr.md
roadmap/phase-14-pipeline-chains.md / roadmap/releases.md
v0.5.2PyPI tag is published).README.md
forgelm.data_audit.SECRET_TYPES(full BEGIN…END for OpenSSH / PGP).CHANGELOG.md
Summary
Changes
Type
Testing
pytest tests/passesruff check .passesruff format --check .passesforgelm --config config_template.yaml --dry-runworksChecklist
pip install forgelm[...])config_template.yaml) updated if new config fields addedSummary by Sourcery
Align documentation and changelog with Phase 12 (v0.5.2) being merged to main and consolidate post-PR-13 review fixes into the unreleased changelog.
Enhancements:
Documentation: