Repository navigation
feat(export): write the SHA-256 sidecar verify-gguf relies on, and document the converter requirement - #998
Merged
Merged
Conversation
…mentation rely on `forgelm export` computed the digest of the GGUF but never wrote `<output>.sha256`; the only writer in the package was the checkpoint-tarball helper. `forgelm verify-gguf`, the user manual and the GGUF reference all describe the sidecar as something the exporter produces, so the sidecar check had nothing to compare against unless the operator created the file by hand. The exporter now writes `<output>.sha256` in `sha256sum` binary-mode format (`<hex> *<name>`) through a temporary name and a rename. A sidecar that cannot be written fails the export (exit 2, `error_kind="runtime"`) rather than leaving a GGUF the documentation says is covered; the digest is still reported. For a K-quant request the file written is the intermediate `*.f16.gguf`, so the sidecar belongs to that file. A `model_integrity.json` that cannot be updated is now logged as a warning instead of at debug level. The sidecar detects corruption and accidental change; it is not tamper evidence, and the pages now say so. The same change rewrites the parts of the GGUF pages that described things the code does not do: the invented manifest and `verify-gguf` output (vocabulary and tokenizer checks that do not exist), K-quants shown as one-step, "always merges LoRA", and a clean `verify-gguf` exit presented as more than it proves. English and Turkish. Refs #83, #84, #85 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…the export extra forgelm export runs llama.cpp's convert_hf_to_gguf.py with ForgeLM's own Python. The installation page, the export extra's comment in pyproject.toml, the module docstring, the usage and quickstart guides and the post-training notebook said it was llama-cpp-python's "bundled" script, and the error hint advised `pip install 'llama-cpp-python>=0.2.90'`. No released wheel puts the script where ForgeLM searches (0.2.90 installs it under bin/, 0.3.x does not ship it), so a plain install ends with "convert_hf_to_gguf.py not found inside the llama-cpp-python installation" (exit 2) unless FORGELM_GGUF_CONVERTER points at the script in a llama.cpp checkout. The upstream script imports sibling modules (conversion, gguf), which is why the checkout's copy has to be used; the upstream repository and the requirements path were checked. The pages now say so, with the two commands that make it work; the error message names FORGELM_GGUF_CONVERTER and a llama.cpp checkout instead of a reinstall, and a test pins that. Searching the locations real wheels use, or bundling the script, are code changes and stay open. Refs #113, #83 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…est entry is best effort Review findings on the export page: the K-quant intermediate is the output name with its .gguf suffix replaced by .f16.gguf (model.gguf becomes model.f16.gguf), not <output>.f16.gguf appended; and the exported_artifacts entry is best effort, because _update_integrity_manifest logs a warning and the export succeeds when model_integrity.json is not valid JSON or cannot be written. English and Turkish. Refs #84 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
|
Sorry @cemililik, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 12 hours and 24 minutes by commenting |
Reviewer's GuideThe PR makes GGUF export produce the SHA-256 sidecar expected by verification, handling atomic writes, K-quant intermediates, and failures; it also rewrites English and Turkish guidance to reflect actual verification/export behavior and documents the required llama.cpp converter setup. Sequence diagram for GGUF export with SHA-256 sidecarsequenceDiagram
participant User
participant Export as forgelm_export
participant Converter as convert_hf_to_gguf.py
participant Filesystem
participant Manifest as model_integrity.json
User->>Export: export_model()
Export->>Converter: convert_hf_to_gguf.py
Converter->>Filesystem: write GGUF
Export->>Export: _sha256_file()
Export->>Filesystem: _write_sha256_sidecar()
Filesystem-->>Export: atomic os.replace()
opt K-quant request
Export-->>User: intermediate .f16.gguf and .f16.gguf.sha256
Export-->>User: followup_command for llama-quantize
end
opt integrity manifest exists
Export->>Manifest: update exported_artifacts
Manifest-->>Export: warning on update failure
end
Export-->>User: ExportResult success
Sequence diagram for GGUF verification with optional checkssequenceDiagram
participant User
participant Verify as forgelm_verify_gguf
participant GGUF
participant Sidecar
participant Metadata as gguf_package
User->>Verify: verify-gguf path
Verify->>GGUF: check GGUF magic
opt gguf package installed
Verify->>Metadata: parse metadata
Metadata-->>Verify: metadata result
end
opt path.sha256 exists
Verify->>Sidecar: read digest
Verify->>GGUF: recompute SHA-256
Sidecar-->>Verify: expected digest
Verify-->>User: sidecar match or mismatch
end
Verify-->>User: result proves only checks that ran
Flow diagram for GGUF export converter setupflowchart TD
A["Install forgelm[export]"] --> B[Clone llama.cpp checkout]
B --> C[Set FORGELM_GGUF_CONVERTER]
C --> D[forgelm export]
D --> E[convert_hf_to_gguf.py]
E --> F[GGUF output]
D --> G{K-quant requested?}
G -->|No| H[Write output.sha256]
G -->|Yes| I[Write intermediate .f16.gguf.sha256]
I --> J[Run llama-quantize manually]
J --> K[Create quantized-file.sha256]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Warning Review limit reachedEnable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Next included review available in 21 minutes. View limit details
Comment |
This was referenced Oct 18, 2026
# Conflicts: # CHANGELOG.md
10 tasks done
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
forgelm exportnow writes the SHA-256 sidecar thatforgelm verify-ggufand the documentation already assumed, and the GGUF documentation describes what really happens. Two earlier pull requests combined into one (#982 and #995, which will be closed as superseded); the bot findings on #982 are folded in.Related issues
Refs #84, #85, #86 (docs), #83, #113. Making
verify-ggufstricter than "magic header + optional sidecar", and searching the locations real llama-cpp-python wheels use for the converter, are separate changes and stay open. The site strings that mention the sidecar are in a later pull request.Changes
Sidecar (was #982)
forgelm/export.py: after the GGUF is written and hashed,<output>.sha256is written insha256sumformat (<digest> *<name>) through a temporary file andos.replace. A write failure returnssuccess=False(runtime error, exit 2) naming the GGUF path. For a K-quant request the file written is the intermediate.f16.ggufand the sidecar belongs to it. Themodel_integrity.jsonupdate failure is now aWARNING, notDEBUG.docs/usermanuals/{en,tr}/deployment/{gguf-export,verify-gguf}.mdanddocs/reference/verify_gguf_subcommand{,-tr}.md: real output, the K-quant two-step flow, the realexported_artifactsJSON, whatverify-ggufchecks and what it cannot, and the--adapterpitfall. Review findings applied: the intermediate file is named by replacing.ggufwith.f16.gguf; the manifest entry is best effort.Converter script (was #995)
pip install 'forgelm[export]'does not provideconvert_hf_to_gguf.py: no released llama-cpp-python wheel puts it where ForgeLM searches (0.2.90 installs it underbin/, 0.3.x does not ship it). The installation page, CLI reference, usage and quickstart guides, the notebook,pyproject.tomlcomment andexport.pydocstring now say to pointFORGELM_GGUF_CONVERTERat the script in a llama.cpp checkout; the upstream repository location, requirements path and imports were checked.FORGELM_GGUF_CONVERTERand a llama.cpp checkout instead of advising a reinstall.Type
Testing
tests/test_export.py: sidecar format and content,sha256sum -candverify-ggufaccept it, atomic write and cleanup, K-quant sidecar, failure path, plus the new converter hint test (fails against the old message).CLAUDE.mdgauntlet from a clean worktree of this branch: exit 0.Checklist
[Unreleased]🤖 Generated with Claude Code
Summary by Sourcery
Add reliable SHA-256 sidecars to GGUF export and align converter requirements, verification behavior, and user documentation with the implemented workflow.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: