Repository navigation
Development - #10
Merged
Merged
Development#10
Conversation
PR #9 landed Phase 10.5 on main (merge commit 03999d0), but the v0.4.5 git tag and PyPI upload have not happened yet — `git tag -l v0.4.5` returns empty. The roadmap docs claimed 'Tagged but not yet on PyPI' which over-reported the release state. - docs/roadmap.md: latest-release block + doc map line clarified. - docs/roadmap-tr.md: parallel Turkish wording. - docs/roadmap/phase-12-quickstart.md: DONE banner now says 'merged to main as v0.4.5 via PR #9 (...); tag + PyPI publish are the remaining release-engineering steps' instead of 'shipped in v0.4.5'. No content/code changes — wording-only, gets the docs back in sync with what git actually contains.
…reward, CI) CHANGELOG.md, docs/roadmap/releases.md, docs/guides/quickstart.md were written before two later changes landed: - code-assistant fallback model bumped to Qwen2.5-Coder-1.5B-Instruct (was generic SmolLM2-1.7B-Instruct). - medical-qa-tr fallback model bumped to Qwen2.5-1.5B-Instruct (was English-only SmolLM2; new fallback is multilingual). - bundled GRPO baseline reward (forgelm/grpo_rewards.py + _math_reward_fn) replaced the old 'no reward → ValueError' wiring. - Test/CI surface grew with test_quickstart_hardening, test_grpo_math_ reward, test_grpo_format_reward, test_wizard_byod, test_cli_quickstart_ wiring, test_packaging, plus a wheel-install-smoke nightly job. Also corrected example counts in the release notes and CHANGELOG to match the actual JSONL line counts (58/59/49/40 — same numbers used in LICENSES.md), and added a per-template fallback table to the quickstart guide so the auto-downsize behavior is no longer described with a stale single-example sentence. releases.md status banner softened from 'Released' to 'Merged to main via PR #9; tag + PyPI publish pending' so it matches roadmap.md and phase-12-quickstart.md, since 'git tag -l v0.4.5' is empty.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughDocumentation updates reflecting v0.4.5 release status, template-specific GPU fallback guidance, expanded test coverage documentation, and GRPO trainer configuration defaults. Changes document merged code awaiting PyPI publication and clarify quickstart behavior across multiple templates. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
✨ 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 |
Up to standards ✅🟢 Issues
|
|
Reviewer's GuideDocs-only PR that updates changelog and roadmap/quickstart documentation to reflect finalized template configurations (models, example counts, GRPO reward behavior), adds detailed notes on new GRPO baseline reward + test/CI coverage, and clarifies release status and GPU fallback behavior across templates. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![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 updates the project's documentation and changelogs to reflect the merging of the Quickstart Layer (v0.4.5) into the main branch. Key updates include a detailed model fallback table for different VRAM capacities, information on GRPO baseline rewards, and descriptions of the expanded test suite and CI workflows. A review comment identified a discrepancy in the documentation regarding CLI flag propagation, specifically noting that the --output-format flag should only be documented as propagating to the training subprocess, not the chat subprocess, to align with the actual implementation.
| ### Features: | ||
| 1. [x] **`forgelm/quickstart.py`** — Template registry (`@dataclass(frozen=True) Template`), `auto_select_model()` GPU-aware downsizing (≥10 GB VRAM → primary model; otherwise fallback ≤2B), `run_quickstart()` end-to-end orchestrator that copies the bundled seed dataset, substitutes `model.name_or_path` and `data.dataset_name_or_path`, and writes a `configs/<template>-YYYYMMDDHHMMSS.yaml` the existing trainer accepts unchanged. | ||
| 2. [x] **`forgelm quickstart <template>` CLI subcommand** — `--list` (text + JSON via `--output-format json`), `--model` / `--dataset` overrides, `--dry-run` (generate config but skip training), `--no-chat` (skip post-training chat REPL), `--output` (custom YAML path). On a successful train, subprocess-invokes `forgelm chat <output_dir>` for an immediate sanity loop. | ||
| 2. [x] **`forgelm quickstart <template>` CLI subcommand** — `--list` (text + JSON via `--output-format json`), `--model` / `--dataset` overrides, `--dry-run` (generate config but skip training), `--no-chat` (skip post-training chat REPL), `--output` (custom YAML path). On a successful train, subprocess-invokes `forgelm chat <output_dir>` for an immediate sanity loop. Top-level flags (`--output-format`, `--quiet`, `--log-level`, `--offline`) propagate to the train + chat subprocesses. |
There was a problem hiding this comment.
The documentation states that the --output-format flag propagates to both the train and chat subprocesses. However, according to the implementation in forgelm/cli.py (lines 802-808), this flag is explicitly excluded from the chat subprocess to avoid interfering with the interactive terminal output. Please update the documentation to reflect that --output-format only propagates to the training subprocess.
Suggested change
| 2. [x] **`forgelm quickstart <template>` CLI subcommand** — `--list` (text + JSON via `--output-format json`), `--model` / `--dataset` overrides, `--dry-run` (generate config but skip training), `--no-chat` (skip post-training chat REPL), `--output` (custom YAML path). On a successful train, subprocess-invokes `forgelm chat <output_dir>` for an immediate sanity loop. Top-level flags (`--output-format`, `--quiet`, `--log-level`, `--offline`) propagate to the train + chat subprocesses. | |
| 2. [x] **`forgelm quickstart <template>` CLI subcommand** — `--list` (text + JSON via `--output-format json`), `--model` / `--dataset` overrides, `--dry-run` (generate config but skip training), `--no-chat` (skip post-training chat REPL), `--output` (custom YAML path). On a successful train, subprocess-invokes `forgelm chat <output_dir>` for an immediate sanity loop. Top-level flags (`--quiet`, `--log-level`, `--offline`) propagate to the train + chat subprocesses; `--output-format` propagates to training only. |
Merged
cemililik
added a commit
that referenced
this pull request
May 6, 2026
…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>
6 tasks done
This was referenced May 9, 2026
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 by Sourcery
Update quickstart and GRPO documentation, template descriptions, and roadmap notes to reflect the merged v0.4.5 release and its behavior, including GRPO rewards and template-specific fallbacks.
Enhancements:
CI:
Documentation:
Summary by CodeRabbit
Documentation