feat: measure agent install success#274
Conversation
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 Walkthrough
WalkthroughThis PR adds agent install attempt measurement recording to Gradata. A new module records per-agent install status, actions, and failure classifications to a JSONL file in the config directory. The CLI's install command now emits measurements for both successful and failed installs, and detects missing agent configurations to record docs-friction failures. Comprehensive tests cover success, code failure, and multi-agent scenarios, with documentation updates explaining the measurement fields and failure kinds. ChangesInstall measurement recording for agent integrations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.22.0)OpenGrep fatal error (exit code 2): �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Gradata/src/gradata/_install_measurement.py`:
- Around line 59-60: The append currently using path.open("a") can leave a
partial JSONL line; change it to use the project atomic-write helper (or
implement the atomic-write pattern) so each measurement is written atomically:
create a temp file in the same directory, write the original file contents plus
the new json.dumps(payload, sort_keys=True) + "\n" into the temp, fsync the
temp, close it and then os.replace the temp into place (or call the atomic
helper) instead of using path.open("a"); update the code locations referencing
path.open and payload to use this atomic-write flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4a526318-32da-4905-917d-0cc8a7b3e591
📒 Files selected for processing (5)
Gradata/README.mdGradata/docs/getting-started/install.mdGradata/src/gradata/_install_measurement.pyGradata/src/gradata/cli.pyGradata/tests/test_cli_install_agent.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest (py3.11)
- GitHub Check: pytest (py3.12)
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/src/**/*.py
📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Gradata/src/**/*.py: Prefersentence-transformersfor local embeddings,google-genaifor Gemini embeddings,cryptographyfor AES-GCM encrypted system.db,bm25sfor BM25 rule ranking, andmem0aifor external memory adapters — guard all optional dependency imports withtry / except ImportErrorat the call site, never at module level
Maintain strict layering: Layer 0 (Primitives: _types.py, _db.py, _events.py, _paths.py, _file_lock.py; Patterns: contrib/patterns/) must never import from Layer 1 (Enhancements: enhancements/, rules/) or Layer 2 (Public API: brain.py, cli.py, daemon.py, mcp_server.py)
Never use bareexcept: pass— use typed exceptions or at minimumlogger.warning(...)withexc_info=Trueto avoid silent failure in a memory product
Never import from out-of-scope sibling directories../Sprites/or../Hausgem/withingradata/*code — that is a layering bug
Never leak private-sibling paths into public docs/code — no references to../Sprites/,../Hausgem/, email addresses, OneDrive paths, or Sprites-specific examples from insidegradata/*
Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes
Files:
Gradata/src/gradata/cli.pyGradata/src/gradata/_install_measurement.py
Gradata/tests/**/*.py
📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Gradata/tests/**/*.py: SetBRAIN_DIRenvironment variable viatmp_pathin conftest.py for test isolation — ensure_paths.pymodule cache refreshes when callingBrain.init()directly inside tests
Add unit tests intests/test_*.pyfor every CI push without LLM calls (deterministic); mark integration tests with@pytest.mark.integrationand skip them by default (they hit real LLM APIs)
Files:
Gradata/tests/test_cli_install_agent.py
🧠 Learnings (1)
📚 Learning: 2026-04-17T17:18:07.439Z
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
Applied to files:
Gradata/README.md
🔇 Additional comments (9)
Gradata/src/gradata/cli.py (3)
458-463: LGTM!
467-479: LGTM!
490-507: LGTM!Gradata/tests/test_cli_install_agent.py (4)
222-227: LGTM!
230-241: LGTM!
244-257: LGTM!
260-276: LGTM!Gradata/README.md (1)
121-122: LGTM!Gradata/docs/getting-started/install.md (1)
107-117: LGTM!
| with path.open("a", encoding="utf-8") as f: | ||
| f.write(json.dumps(payload, sort_keys=True) + "\n") |
There was a problem hiding this comment.
Use atomic-write helper for JSONL append to prevent partial-write corruption.
The coding guidelines require: "Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes." While this is append-only JSONL, a crash during the write could leave a partial line that breaks downstream parsing. Consider using an atomic-write pattern (write to temp file, then rename) to ensure each measurement is fully written or not at all.
🛡️ Suggested atomic-write pattern
+import tempfile
+import shutil
+
def append_measurement(
result: InstallResult,
*,
brain_dir: Path,
source: str = "gradata install --agent",
failure_kind: FailureKind | None = None,
) -> dict[str, object]:
status = "success" if result.action != "failed" else "failure"
payload: dict[str, object] = {
"ts": datetime.now(UTC).isoformat(),
"source": source,
"agent": result.agent,
"status": status,
"action": result.action,
"failure_kind": failure_kind or classify_result(result),
"config_path": str(result.config_path),
"brain_dir": str(brain_dir),
"message": result.message,
}
path = measurement_path()
path.parent.mkdir(parents=True, exist_ok=True)
- with path.open("a", encoding="utf-8") as f:
- f.write(json.dumps(payload, sort_keys=True) + "\n")
+ # Atomic append: write to temp, then move
+ tmp_fd, tmp_path = tempfile.mkstemp(dir=path.parent, text=True)
+ try:
+ with os.fdopen(tmp_fd, "w", encoding="utf-8") as f:
+ # Read existing content if file exists
+ if path.exists():
+ f.write(path.read_text(encoding="utf-8"))
+ f.write(json.dumps(payload, sort_keys=True) + "\n")
+ shutil.move(tmp_path, path)
+ except Exception:
+ Path(tmp_path).unlink(missing_ok=True)
+ raise
return payloadAlternatively, use a project-standard atomic-write helper if one exists in the codebase.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| with path.open("a", encoding="utf-8") as f: | |
| f.write(json.dumps(payload, sort_keys=True) + "\n") | |
| import tempfile | |
| import shutil | |
| import os | |
| def append_measurement( | |
| result: InstallResult, | |
| *, | |
| brain_dir: Path, | |
| source: str = "gradata install --agent", | |
| failure_kind: FailureKind | None = None, | |
| ) -> dict[str, object]: | |
| status = "success" if result.action != "failed" else "failure" | |
| payload: dict[str, object] = { | |
| "ts": datetime.now(UTC).isoformat(), | |
| "source": source, | |
| "agent": result.agent, | |
| "status": status, | |
| "action": result.action, | |
| "failure_kind": failure_kind or classify_result(result), | |
| "config_path": str(result.config_path), | |
| "brain_dir": str(brain_dir), | |
| "message": result.message, | |
| } | |
| path = measurement_path() | |
| path.parent.mkdir(parents=True, exist_ok=True) | |
| # Atomic append: write to temp, then move | |
| tmp_fd, tmp_path = tempfile.mkstemp(dir=path.parent, text=True) | |
| try: | |
| with os.fdopen(tmp_fd, "w", encoding="utf-8") as f: | |
| # Read existing content if file exists | |
| if path.exists(): | |
| f.write(path.read_text(encoding="utf-8")) | |
| f.write(json.dumps(payload, sort_keys=True) + "\n") | |
| shutil.move(tmp_path, path) | |
| except Exception: | |
| Path(tmp_path).unlink(missing_ok=True) | |
| raise | |
| return payload |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Gradata/src/gradata/_install_measurement.py` around lines 59 - 60, The append
currently using path.open("a") can leave a partial JSONL line; change it to use
the project atomic-write helper (or implement the atomic-write pattern) so each
measurement is written atomically: create a temp file in the same directory,
write the original file contents plus the new json.dumps(payload,
sort_keys=True) + "\n" into the temp, fsync the temp, close it and then
os.replace the temp into place (or call the atomic helper) instead of using
path.open("a"); update the code locations referencing path.open and payload to
use this atomic-write flow.
Source: Coding guidelines
Summary
gradata install --agent ....status,action, andfailure_kind.code_failureand missing--agent allhost configs asdocs_friction.Paperclip: GRA-2507 / 4f4443e3-f6a4-4d47-99c7-f9e15e343cfe
Verification
PYTHONPATH=src env -u BRAIN_DIR -u GRADATA_BRAIN pytest -q tests/test_cli_install_agent.py tests/test_install_smoke_matrix.py tests/test_telemetry.py→ 52 passed, 2 skipped/home/olive/.local/bin/uvx ruff check src/gradata/_install_measurement.py src/gradata/cli.py tests/test_cli_install_agent.py→ All checks passedgradata install --agent allwrote JSONL rows for Codex success plus Claude Code/Hermes/Cursor docs_friction.