Skip to content

feat: opt-in telemetry + crash reporting (#616) - #621

Merged
frankbria merged 8 commits into
mainfrom
feature/issue-616-opt-in-telemetry
Jun 12, 2026
Merged

feat: opt-in telemetry + crash reporting (#616)#621
frankbria merged 8 commits into
mainfrom
feature/issue-616-opt-in-telemetry

Conversation

@frankbria

Copy link
Copy Markdown
Owner

Summary

Implements #616: Launch: opt-in telemetry + crash reporting.

  • codeframe/core/telemetry.py (headless): consent state at ~/.codeframe/telemetry.json (enabled/prompted/anonymous_id), resolution chain CODEFRAME_TELEMETRY env → DO_NOT_TRACK → config file → default off; command/crash event builders; fire-and-forget httpx sender that never raises.
  • cf config telemetry on|off|status — new cf config Typer group; status shows effective state and which override produced it.
  • Entry wrapper (codeframe/cli/telemetry_runtime.py + main() in app.py): one-time opt-in prompt (interactive TTY only, default No), command timing, exit-code capture, unhandled-exception crash capture. All three entry points (cf/codeframe console scripts, python -m codeframe, python -m codeframe.cli) route through it. Exit codes and exceptions propagate unchanged.
  • Privacy hardening: command names are resolved against the registered Typer command tree, so raw argv (paths, ids, typos) can never leak; crash reports carry only in-package traceback frames with relativized paths and omit exception messages.
  • Minimal backend: scripts/telemetry_collector.py — self-hostable ~50-line FastAPI JSONL appender (cheapest thing that works for beta volume).
  • PRIVACY.md (repo root, linked from README): what is/isn't collected, consent precedence, endpoint, 90-day retention, deletion path.
  • Suite hermeticity: root conftest sets CODEFRAME_TELEMETRY=off so tests never prompt or send.

Acceptance Criteria

  • Opt-in only: one-time prompt on first run, default off; cf config telemetry on|off; honored everywhere including non-interactive runs (env var)
  • Anonymous usage events (command name, duration, success/failure, version, OS) — no project content, no prompts, no file paths
  • Crash reporting: unhandled-exception reports with stack trace + version, gated behind the same opt-in
  • PRIVACY.md documenting exactly what is collected, where it goes, and retention
  • Backend minimal: self-hostable JSONL collector

Test Plan

  • Unit tests written first (TDD) — 70 new tests across core/CLI/collector
  • Full v2 suite passing (pytest tests/ --ignore=tests/e2e -m v2 exit 0) + ruff + mypy (191 files) clean
  • Diff coverage 91% on changed lines (≥85% gate)
  • Internal code review (advisory) completed — no Critical/Major defects
  • Cross-family review pass: codex — one P2 finding (python -m codeframe bypassed wrapper) fixed in a0ea9da
  • Test mutation sanity check: 5 mutations (default-on, message leak, argv leak, re-prompt, send-when-disabled) — all killed by tests

Known Limitations / Intentionally Deferred

  • Handled failures carry no stack trace: commands that catch their own exceptions and raise typer.Exit(1) (the common CodeFRAME pattern) produce a failed command event but no crash event — crash reports are for unhandled exceptions only, per the issue contract.
  • Collector is unauthenticated and unthrottled beyond a 100-event batch cap — acceptable for beta volume per the issue ("cheapest thing that works"); add auth/rate limiting before any public scale-up.
  • Up to 1s added exit latency on opted-in commands when the collector is slow/unreachable (bounded daemon-thread join); opted-out runs are unaffected.
  • telemetry.codeframe.dev default endpoint is not yet provisioned — until it is deployed, opted-in sends fail silently (by design); self-hosters use CODEFRAME_TELEMETRY_ENDPOINT.
  • Installed environments need a reinstall/uv sync for the console-script change (:app:main) to take effect.

Implementation Notes

  • Plan was self-authored (issue had no implementation plan comment) and approved interactively before implementation.
  • DO_NOT_TRACK is honored as an extra off-switch (console DNT convention) — explicit CODEFRAME_TELEMETRY=on still wins as the more specific signal.

Closes #616

Test User added 6 commits June 12, 2026 11:22
…616)

- ~/.codeframe/telemetry.json (enabled/prompted/anonymous_id), atomic writes
- Resolution: CODEFRAME_TELEMETRY env > DO_NOT_TRACK > file > default OFF
- Command + crash event builders; crash frames limited to in-package paths,
  exception messages omitted (privacy)
- send_events never raises; fire-and-forget daemon-thread variant
…crash capture (#616)

- main() console-script entry wraps app(): one-time opt-in prompt
  (interactive TTY only, default No), command timing, exit-code capture,
  unhandled-exception crash event; exceptions/exit codes propagate unchanged
- Command names resolved against the registered Typer tree — raw argv tokens
  can never leak into telemetry
- Suite-wide CODEFRAME_TELEMETRY=off setdefault in root conftest
- scripts/telemetry_collector.py: self-hostable FastAPI endpoint appending
  events to JSONL (cheapest backend for beta volume), 100-event batch cap
- PRIVACY.md: exactly what is/isn't collected, consent precedence, endpoint,
  90-day retention, deletion path; linked from README
…dex review)

All three entry points (console scripts, python -m codeframe,
python -m codeframe.cli) now share the same main() wrapper.
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@frankbria, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 2 hours, 14 minutes, and 31 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f2b74bba-6ad4-4ef0-988c-07129116c74e

📥 Commits

Reviewing files that changed from the base of the PR and between 667b996 and 68637c4.

📒 Files selected for processing (18)
  • CLAUDE.md
  • PRIVACY.md
  • README.md
  • codeframe/__main__.py
  • codeframe/cli/__main__.py
  • codeframe/cli/app.py
  • codeframe/cli/config_commands.py
  • codeframe/cli/telemetry_runtime.py
  • codeframe/core/telemetry.py
  • docs/CLI_WIREFRAME.md
  • docs/QUICKSTART.md
  • pyproject.toml
  • scripts/telemetry_collector.py
  • tests/cli/test_config_telemetry_cli.py
  • tests/cli/test_telemetry_entry.py
  • tests/conftest.py
  • tests/core/test_telemetry.py
  • tests/scripts/test_telemetry_collector.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/issue-616-opt-in-telemetry

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

@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code Review — feat: opt-in telemetry + crash reporting (#616)

This is a well-architected, privacy-first implementation. The design decisions are sound: default-off, sanitized crash frames, no args/paths leaking, fire-and-forget transport, and complete test isolation via conftest. A few things worth addressing before merge.


Correctness / Minor Bugs

1. Double config read in _print_status() (config_commands.py)

Two independent load_config() calls (one explicit, one inside is_enabled()) means a concurrent cf config telemetry on could cause config.anonymous_id and effective to disagree. Harmless in practice, but trivially fixed — resolve effective from the already-loaded config.enabled after checking env overrides, rather than calling is_enabled() separately.


2. Collector has no concurrent write protection (scripts/telemetry_collector.py)

with LOG_PATH.open("a") as f:
    for event in batch.events:
        f.write(json.dumps({...}) + "\n")

Under async uvicorn, two simultaneous requests will both enter this block. Each f.write() call is a separate syscall — lines from two requests can interleave at the OS level. A corrupted JSONL line would cause silent data loss when you later jq the file. Simple fix: batch the writes into a single f.write("".join(...)) call so each request is one syscall, or add fcntl.flock for strong exclusion. For low beta volume this is unlikely to trigger, but worth fixing before the collector goes live.


Test Coverage Gap

3. test_frames_only_include_codeframe_package passes vacuously (tests/core/test_telemetry.py)

_raise_and_capture() raises ValueError from test code (outside the codeframe package), so _sanitize_frames returns [] and the for frame in event["frames"] assertion iterates over nothing. The filtering logic is actually covered by test_in_package_frames_are_captured_relative, but this test gives a false signal of coverage. Either drop it or make the empty-frames case explicit:

assert event["frames"] == []  # exception raised outside package; all frames filtered

Design Observations (non-blocking)

4. resolve_command_name uses typer.main.get_command() (telemetry_runtime.py)

This is a semi-internal Typer API. The choice is correct — it's the only way to get the registered command tree for safe argv filtering — but a comment explaining why a private API is used deliberately would help future readers.

5. ensure_config TOCTOU: benign

Two concurrent first-run invocations can both see no config file and both write, with the second write's anonymous_id winning. Acceptable for beta; worth a comment so nobody "fixes" it by introducing a lock and inadvertently blocking CLI startup.

6. Unprovisioned endpoint (acknowledged)

PRIVACY.md correctly documents that telemetry.codeframe.dev isn't yet deployed and opted-in sends fail silently. Just ensure it's live before any public announcement.


What's done well

  • Architecture compliance: core/telemetry.py is fully headless (no FastAPI), CLI wraps it correctly per CLAUDE.md rules.
  • Privacy by design: crash frames are package-only and relativized; exception messages stripped; argv never touches payloads; command names validated against the registered tree.
  • Atomic config writes: mkstemp + os.replace is the right pattern, consistent with atomic_write_json used elsewhere in the server layer.
  • Suite hermeticity: CODEFRAME_TELEMETRY=off in root conftest ensures no subprocess-based lifecycle test can ever prompt or send.
  • Exit code propagation: run() handles SystemExit, KeyboardInterrupt/Abort, and BaseException correctly — the CLI's exit code is never changed by telemetry.
  • Mutation test coverage: the five mutations listed in the PR description (default-on, message leak, argv leak, re-prompt, send-when-disabled) are a strong correctness signal.

Summary

Issues #2 (collector concurrency) and #3 (vacuous test) are worth a quick fix. #1 is trivial cleanup. None are blockers for a beta rollout, but #2 could produce a silently corrupted log file under concurrent load — recommend addressing before the collector goes live.

…ingle-write, explicit empty-frames assertion (#621)
@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown

Follow-up Review — All Prior Issues Resolved

All three items from my first review have been addressed in the current diff:

#1_print_status() double config read fixed.
The function now reads load_config() once and derives effective inline, with the explanatory comment added. The potential anonymous_id/effective disagreement is gone.

#2 — Collector concurrent write interleaving fixed.
Single f.write(lines) call per request with the "One write call per request" comment. This is the right fix — each request is now one OS syscall, so lines from concurrent requests cannot interleave.

#3 — Vacuous test giving false coverage signal fixed.
test_frames_only_include_codeframe_package now explicitly asserts assert event["frames"] == [] with a docstring explaining why the list is empty. The intent is now clear and the assertion is meaningful.


The two non-blocking design observations remain outstanding but do not warrant a re-request:

  • typer.main.get_command() semi-internal API — still no comment explaining the deliberate private-API choice. Low urgency; a one-liner would help future readers but will not affect correctness.
  • ensure_config TOCTOU — still no "benign by design" comment. Same: low urgency.

No blocking issues remain. The implementation is solid: privacy-by-design, correct exit-code propagation, hermetic test suite, and the collector concurrency fix lands before the endpoint goes live. Ready to merge.

@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown

Third-pass Review — No New Findings

Scanned the full diff again. The three corrected issues from review #1 remain fixed, and no new issues have been introduced since review #2.

The two outstanding non-blocking observations are still present:

  • typer.main.get_command() semi-internal API — still no explanatory comment at the call site (telemetry_runtime.py:355). A one-liner like # typer.main.get_command is the only way to walk the registered command tree for safe argv filtering would help future readers but is not a correctness concern.
  • ensure_config TOCTOU — still no note that the benign race is intentional (core/telemetry.py:579). Low urgency.

Nothing has changed to block merging. Previous verdict stands: ready to merge.

@frankbria

Copy link
Copy Markdown
Owner Author

Final Triage Summary (PR #621)

Cutoff: 2026-06-12T19:45:16Z (last push, docs sync commit 68637c4)
New findings since cutoff: 0 (claude-bot follow-up at 19:44:10Z, pre-cutoff, confirmed all prior findings resolved — "Ready to merge")

Fixed

  • [claude-bot] _print_status() double config read → fixed in 21cb38f (single load, effective state derived inline)
  • [claude-bot] collector concurrent write interleaving → fixed in 21cb38f (single f.write per request)
  • [claude-bot] vacuous test_frames_only_include_codeframe_package → fixed in 21cb38f (explicit frames == [] assertion; fails under mutation)
  • [codex] python -m codeframe bypassed telemetry wrapper → fixed in a0ea9da

Skipped (with justification)

  • [claude-bot] add comment on typer.main.get_command() private-API use — nitpick: resolve_command_name's docstring already states the purpose; the API choice is the only way to read the registered tree
  • [claude-bot] add "benign by design" comment on ensure_config TOCTOU — nitpick: worst case is one orphaned anonymous id; a lock here could block CLI startup, which is the worse trade
  • [coderabbitai (app)] no review posted (org rate limit) — covered by the CodeRabbit CLI review run pre-PR: no findings; CodeRabbit CI check: pass

Hard gates

  • CI: all checks green on 68637c4 (backend, frontend, lint+mypy, hardcoded-URLs, claude-review, CodeRabbit, GitGuardian)
  • Demo: all 5 acceptance criteria verified with outcome evidence (local collector, fresh HOME, PTY prompt, crash report, gating)

@frankbria
frankbria merged commit 670c910 into main Jun 12, 2026
11 checks passed
@frankbria
frankbria deleted the feature/issue-616-opt-in-telemetry branch June 12, 2026 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Launch: opt-in telemetry + crash reporting

1 participant