Skip to content

fix(cli): keep the CLI from sending Hugging Face's anonymous usage ping - #985

Closed
cemililik wants to merge 1 commit into
developmentfrom
fix/cli-disables-hub-telemetry
Closed

cemililik wants to merge 1 commit into
developmentfrom
fix/cli-disables-hub-telemetry

Conversation

@cemililik

@cemililik cemililik commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The privacy page says ForgeLM "does not collect telemetry, does not send usage data anywhere". A default online forgelm run did send one: TRL's trainers call huggingface_hub's send_telemetry when they are built, and huggingface_hub sends a small anonymous usage ping to huggingface.co unless HF_HUB_DISABLE_TELEMETRY is true or offline mode is on. This makes the claim true for the CLI instead of softening it.

Related issues

Fixes #95 for the CLI. The site's privacy strings need no change for the CLI path; using ForgeLM as a library is covered in the Telemetry row below.

Changes

  • forgelm/cli/_config_load.py: _disable_hub_telemetry() does os.environ.setdefault("HF_HUB_DISABLE_TELEMETRY", "1").
  • forgelm/cli/_dispatch.py: main() calls it first, before anything imports huggingface_hub (which reads the variable once, at import).
  • docs/usermanuals/{en,tr}/operations/air-gap.md: the Telemetry row now says what the CLI sets and that library use does not.

Trade-offs

  • setdefault leaves an operator's own value alone, so HF_HUB_DISABLE_TELEMETRY=0 opts back in. This is an environment default for a dependency, not environment-driven ForgeLM behaviour, so it does not break the "config-driven" principle, but it is worth a look.
  • Using ForgeLM as a library does not go through main() and is not covered; the air-gap page says so.
  • Whether huggingface_hub makes other background requests beyond the telemetry ping was not examined.

Type

  • Bug fix

Testing

  • TestHubTelemetryDefault in tests/test_cli.py: the variable is set to 1 by main(), an existing value is kept, and importing forgelm.cli does not import huggingface_hub (a later top-level import would silently defeat the default).
  • Checked against the installed TRL 1.7.1 and huggingface_hub: both read the variable.
  • The full CLAUDE.md gauntlet from a clean worktree of this branch: exit 0.

Checklist

  • English and Turkish mirrors updated together
  • CHANGELOG entry under [Unreleased]
  • Tests added

🤖 Generated with Claude Code

Summary by Sourcery

Disable Hugging Face anonymous telemetry by default for CLI invocations to keep ForgeLM’s CLI privacy behavior consistent with its documentation.

Bug Fixes:

  • Prevent the forgelm CLI from sending Hugging Face’s anonymous usage telemetry by default while preserving explicit operator opt-in values.

Documentation:

  • Document the CLI’s Hugging Face telemetry behavior and its distinction from library usage in the English and Turkish air-gap guides.

Tests:

  • Add coverage for the telemetry default, operator overrides, initialization ordering, and Hugging Face Hub integration.

Chores:

  • Record the telemetry privacy fix in the unreleased changelog.

TRL's trainers call huggingface_hub's `send_telemetry` when they are built, and huggingface_hub sends a small anonymous
usage ping to huggingface.co unless `HF_HUB_DISABLE_TELEMETRY` is true or offline mode is on. The privacy page says
ForgeLM "does not collect telemetry, does not send usage data anywhere", and the air-gap page said only that ForgeLM
has no telemetry service of its own. A default online training run therefore contradicted the page.

`main()` now sets `HF_HUB_DISABLE_TELEMETRY=1` before anything imports huggingface_hub, which reads it once at import;
importing `forgelm.cli` does not import it (a test pins that, so a later top-level import cannot silently defeat the
default). `setdefault` leaves an operator's own value alone, so `HF_HUB_DISABLE_TELEMETRY=0` opts back in. Using ForgeLM
as a library does not go through `main()` and is not covered; the air-gap page (English and Turkish) says so.

Checked against the installed TRL 1.7.1 and huggingface_hub: both read the variable. Whether huggingface_hub makes other
background requests beyond the telemetry ping was not examined.

Refs #95

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 18, 2026

Copy link
Copy Markdown

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 13 hours and 33 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Next included review available in 35 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 3 included reviews currently available. Your 48 included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: a7e4c9ea-98ee-4b00-9757-990f57e3dcf4
📥 Commits

Reviewing files that changed from the base of the PR and between 56b4f0d and 77363e4.

📒 Files selected for processing: 6
  • CHANGELOG.md
  • docs/usermanuals/en/operations/air-gap.md
  • docs/usermanuals/tr/operations/air-gap.md
  • forgelm/cli/_config_load.py
  • forgelm/cli/_dispatch.py
  • tests/test_cli.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Oct 18, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR prevents the forgelm CLI from triggering Hugging Face’s anonymous telemetry ping by setting HF_HUB_DISABLE_TELEMETRY=1 at the earliest dispatch point, while preserving explicit operator values, documenting that library usage is out of scope, and testing import order and dependency behavior.

Sequence diagram for CLI telemetry suppression

sequenceDiagram
    participant Operator
    participant CLI as forgelm CLI
    participant Dispatch as main()
    participant Hub as huggingface_hub
    participant HF as huggingface.co

    Operator->>CLI: invoke forgelm
    CLI->>Dispatch: main()
    Dispatch->>Dispatch: _disable_hub_telemetry()
    Note over Dispatch: os.environ.setdefault(HF_HUB_DISABLE_TELEMETRY, 1)
    Dispatch->>Hub: import after environment setup
    Hub->>Hub: send_telemetry()
    alt HF_HUB_DISABLE_TELEMETRY=1
        Hub-->>HF: no anonymous usage ping
    else operator set HF_HUB_DISABLE_TELEMETRY=0
        Hub->>HF: anonymous usage ping
    end
Loading

Flow diagram for CLI and library telemetry behavior

flowchart TD
    Start[ForgeLM usage] --> Mode{CLI or library?}
    Mode -->|CLI| Main["main()"]
    Main --> Default["_disable_hub_telemetry()"]
    Default --> Env[setdefault HF_HUB_DISABLE_TELEMETRY=1]
    Env --> Trainers[TRL trainers and huggingface_hub]
    Trainers --> Suppressed[Anonymous usage ping suppressed]
    Mode -->|Library| Library[Library entry point]
    Library --> Hub[TRL trainers and huggingface_hub]
    Hub --> Caller[Caller configures telemetry behavior]
Loading

File-Level Changes

Change Details Files
Adds a CLI startup default that disables Hugging Face Hub telemetry before dependency import.
  • Introduces an environment-variable helper using setdefault, preserving explicit operator opt-in or opt-out values.
  • Invokes the helper at the beginning of CLI dispatch before the main execution path and potential huggingface_hub imports.
forgelm/cli/_config_load.py
forgelm/cli/_dispatch.py
Documents the scope and behavior of telemetry suppression.
  • Updates the English and Turkish air-gap telemetry rows to describe the CLI default, operator override, and library-use limitation.
  • Adds an unreleased changelog entry explaining the privacy fix and opt-in behavior.
docs/usermanuals/en/operations/air-gap.md
docs/usermanuals/tr/operations/air-gap.md
CHANGELOG.md
Adds regression coverage for initialization order and environment-variable semantics.
  • Verifies the default is set, existing values are preserved, and dispatch establishes the setting before command execution.
  • Uses fresh subprocesses to ensure importing the CLI does not preload huggingface_hub and that the dependency observes the configured value.
tests/test_cli.py

Assessment against linked issues

Issue Objective Addressed Explanation
#95 Prevent Hugging Face telemetry from being sent during ForgeLM training runs by default, including when ForgeLM is used through its library API. ❌ The change disables telemetry only when the CLI calls main(). The PR explicitly leaves library use uncovered, so training through that entry point can still send the ping.
#95 Make the privacy claims accurate in the privacy page and the English and Turkish air-gap manuals. ❌ The English and Turkish manuals now disclose the CLI behavior and its library-use limitation, but the privacy-page strings in all six locales are unchanged and still make an unqualified no-telemetry claim.
#95 Add a regression test for the telemetry behavior and document the fix in the release notes. ✅

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@cemililik

Copy link
Copy Markdown
Collaborator Author

Superseded by #999: the wave-0 fixes are combined by theme into five pull requests to cut the review load. This branch's commits are included there unchanged apart from the CHANGELOG.md merge; the branch stays on the remote.

@cemililik cemililik closed this Oct 18, 2026
cemililik added a commit that referenced this pull request Oct 18, 2026
…n.enforce, transformers CVE floor, FSDP strategies, purge output dir (#999)

## Summary

Six code fixes combined into one pull request, replacing #984, #985,
#987, #989, #990 and #992 (to be closed as superseded). Each fixes a
case where the code failed, or a documented option did nothing, without
saying so. Each was mutation-checked on its own; together they pass the
full gauntlet.

## Related issues

Refs #103, #95, #92, #98, #109, #104, #102. Not closed by this change:
the top-of-range problems named in #109 (`warmup_ratio` on transformers
5.15+, the string `fsdp` form removed in 5.20), the FSDP2 migration
(#104), and reading `training.output_dir` for the purge memorisation
check (#102).

## Changes

| # | Fix | Where |
|---|---|---|
| #984 | `training.rope_scaling` crashed every model load on
transformers 5 (`KeyError: 'rope_type'`, or `rope_theta` lost). The
requested keys are merged **over** the model's `rope_parameters` and the
finished config is loaded; `sliding_window_attention` travels in the
same config object. | `forgelm/model.py` |
| #985 | The CLI sets `HF_HUB_DISABLE_TELEMETRY=1` (via `setdefault`)
before `huggingface_hub` is imported, so TRL's trainer no longer sends
Hugging Face's anonymous usage ping the privacy page says does not
exist. Library use is not covered; the air-gap page says so. |
`cli/_config_load.py`, `cli/_dispatch.py` |
| #987 | `retention.enforce` was documented as a gate (`warn_on_excess`,
`block_on_excess` → `EXIT_EVAL_FAILURE`) but no command reads it. Docs,
field description, both wizards say it is reserved; the CLI warns when a
config sets either of the last two. | `config.py` (in place),
`cli/_config_load.py`, `wizard/_collectors.py` |
| #989 | `transformers>=5.3.0` was called the CVE floor but admitted
CVE-2026-5241 and CVE-2026-9856 (a crafted `chat_template` name writes
outside the save directory). Floor is now `>=5.10.1` (5.10.0 is yanked),
`huggingface_hub>=1.5.0`, the nightly floor job pins both, and tests
keep it from drifting. | `pyproject.toml`, `nightly.yml`, `tests/` |
| #990 | `fsdp_strategy: no_shard` / `hybrid_shard` passed `--dry-run`
and exited 2 after the model loaded (FSDP2 rejects them). They are now
rejected when the config loads (exit 1), and two knobs FSDP2 ignores
(`fsdp_auto_wrap: false`, `backward_post`) log a warning. | `config.py`
(in place), `cli/_config_load.py` |
| #992 | `forgelm purge --row-id` without `--output-dir` silently
pointed the memorisation check at the corpus directory. It now warns and
names the path. The purge module's line budget is raised by 8 with a
`budget_history` note (the project's documented option). |
`cli/subcommands/_purge.py`, `tools/check_module_size.py` |

`config.py` (0 lines of headroom on the module-size ratchet) and
`trainer.py` change only by in-place string edits; new warnings live in
`cli/_config_load.py`.

### Merge-conflict handling

The six branches touched `CHANGELOG.md`, `tests/test_cli.py` (two
appended test classes) and the import line in `cli/_dispatch.py`; all
three were resolved by keeping both sides (the import line now imports
`_disable_hub_telemetry` and `_warn_inert_fsdp_options`). The affected
test modules pass together (621 tests).

## Type

- [x] Bug fix
- [x] Dependency

## Testing

- The full `CLAUDE.md` gauntlet from a clean worktree of this branch:
exit 0.
- Per-fix tests as in the original pull requests, including: a real tiny
Llama on CPU for `rope_scaling` (linear, dynamic, yarn); the traversal
reproduction (**fails on transformers 5.3.0**, passes on 5.10.1); all 32
accepted FSDP variants building real `TrainingArguments`.
- transformers-facing test modules pass on a 5.10.1 / `huggingface_hub`
1.5.0 overlay (443 tests). The nightly minimum-versions job runs on its
schedule after merge; I did not dispatch it.

## Checklist

- [x] English and Turkish mirrors updated together
- [x] CHANGELOG entries under `[Unreleased]`
- [x] Tests added

🤖 Generated with [Claude Code](https://claude.com/claude-code)

## Summary by Sourcery

Harden CLI behavior and dependency compatibility across model loading,
privacy, retention, distributed training, and purge workflows.

Bug Fixes:
- Restore Transformers 5 compatibility for RoPE scaling and
sliding-window model configuration.
- Disable Hugging Face Hub telemetry by default for CLI runs while
preserving explicit operator opt-in.
- Warn when reserved retention enforcement modes are configured instead
of implying unsupported enforcement behavior.
- Reject unsupported FSDP1-only strategies during configuration and warn
about FSDP2 options that have no effect.
- Warn when purge uses the corpus directory as its implicit output
directory and clarify the memorisation-check scope.

Enhancements:
- Document the actual retention, FSDP, telemetry, and purge behavior in
English and Turkish user-facing documentation.

Build:
- Raise the minimum runtime dependencies to transformers>=5.10.1 and
huggingface_hub>=1.5.0 to address relevant security advisories.
- Update the nightly minimum-version job and packaging checks to enforce
the new dependency floors.

Tests:
- Add coverage for telemetry initialization, reserved retention
warnings, FSDP validation, purge output-directory warnings, Transformers
5 RoPE handling, and chat-template path traversal protection.

Chores:
- Increase the purge module size budget for the new output-directory
warning.

---------

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant