Add weekly correction-outcome telemetry snapshot script#187
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent 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)
🧰 Additional context used📓 Path-based instructions (1)Gradata/tests/**/*.py📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Files:
🔇 Additional comments (1)
📝 Walkthrough
WalkthroughAdds a CLI script that reads NDJSON weekly correction events (file or stdin), skips malformed rows, classifies rows, aggregates totals and acceptance rate, computes top 5 normalized rule categories, emits a deterministic compact JSON snapshot, and includes docs, tests, and pytest/.gitignore wiring. ChangesWeekly Correction Snapshot Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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.20.0)OpenGrep fatal error (exit code 2): �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Gradata/tests/test_weekly_correction_snapshot.py (1)
1-77:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
pythonpath = ["src"]must includescriptsfor tests to load.The test file imports
from scripts import weekly_correction_snapshot, but pytest'spythonpathconfiguration only addsGradata/srcto the module search path. SinceGradata/scripts/is not included, the test module fails to import and never runs in CI.Update
Gradata/pyproject.tomlto add"scripts"to the pythonpath list, or relocateweekly_correction_snapshot.pyintosrc/if it's production code rather than a utility script.🤖 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/tests/test_weekly_correction_snapshot.py` around lines 1 - 77, The tests import scripts.weekly_correction_snapshot but the test PYTHONPATH only includes "src", so imports fail; fix by updating the project configuration to make the tests find the scripts package: either add "scripts" to the pythonpath list in pyproject.toml (ensure pythonpath = ["src","scripts"]) or move weekly_correction_snapshot.py into the src package and update any imports; reference the test file Gradata/tests/test_weekly_correction_snapshot.py and the module name weekly_correction_snapshot when applying the change.
🤖 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/docs/weekly-correction-snapshot.md`:
- Around line 26-27: The doc for acceptance_rate currently describes raw
division but the code uses round(..., 6); update the documentation text for
`acceptance_rate` to state that the value is computed as `accepted_graduations /
(accepted_graduations + rejection_count)` (or `0.0` if denominator is zero) and
is rounded to 6 decimal places (i.e., uses round(..., 6)). Reference the
`acceptance_rate` field and the rounding behavior so consumers know the exact
precision.
In `@Gradata/scripts/weekly_correction_snapshot.py`:
- Around line 81-84: The current loop can double-count rows because both
_is_graduation_accepted(row) and _is_rejection(row) are evaluated independently;
change the logic so a row counts as either accepted or rejected but not both
(e.g., use an if/elif or otherwise enforce mutual exclusivity between
_is_graduation_accepted(row) and _is_rejection(row) when updating
accepted_graduations and rejection_count) and keep acceptance_rate calculation
unchanged to use the corrected tallies.
---
Outside diff comments:
In `@Gradata/tests/test_weekly_correction_snapshot.py`:
- Around line 1-77: The tests import scripts.weekly_correction_snapshot but the
test PYTHONPATH only includes "src", so imports fail; fix by updating the
project configuration to make the tests find the scripts package: either add
"scripts" to the pythonpath list in pyproject.toml (ensure pythonpath =
["src","scripts"]) or move weekly_correction_snapshot.py into the src package
and update any imports; reference the test file
Gradata/tests/test_weekly_correction_snapshot.py and the module name
weekly_correction_snapshot when applying the change.
🪄 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: 70e4d16e-6dd0-4de0-a14d-1fcfa4946f55
📒 Files selected for processing (4)
.gitignoreGradata/docs/weekly-correction-snapshot.mdGradata/scripts/weekly_correction_snapshot.pyGradata/tests/test_weekly_correction_snapshot.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 macos-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest (py3.12)
- GitHub Check: pytest (py3.11)
🧰 Additional context used
📓 Path-based instructions (1)
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_weekly_correction_snapshot.py
🔇 Additional comments (1)
.gitignore (1)
174-178: Allowlist update is correct.The new exception cleanly keeps
Gradata/scripts/*ignored while allowing the weekly snapshot script to be versioned.
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.
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/tests/test_weekly_correction_snapshot.py`:
- Around line 57-77: Add a new test that exercises the explicit file-input
branch of snapshot.main by creating a temporary input file (using tmp_path)
containing the same payload lines as
test_main_emits_deterministic_json_with_skipped_rows, call snapshot.main with
the args ['--input', str(tmp_file)] instead of monkeypatching sys.stdin, assert
rc == 0, capture stdout via capsys, json.loads it and assert the same expected
dictionary (acceptance_rate, accepted_graduations, rejection_count,
skipped_rows, top_rule_categories, total_corrections). Ensure the test name
references the input path case (e.g.,
test_main_emits_deterministic_json_with_input_file) and reuse the same payload
string used in test_main_emits_deterministic_json_with_skipped_rows so behavior
is identical.
🪄 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: b3c29075-e23b-486a-be70-2909d11f4668
📒 Files selected for processing (4)
Gradata/docs/weekly-correction-snapshot.mdGradata/pyproject.tomlGradata/scripts/weekly_correction_snapshot.pyGradata/tests/test_weekly_correction_snapshot.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). (6)
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest windows-latest / py3.12
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/**/pyproject.toml
📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Maintain
dependencies = []in pyproject.toml — the base package is pure Python + stdlib with all heavy dependencies gated as optional extras: embeddings, gemini, encrypted, ranking, adapters-mem0
Files:
Gradata/pyproject.toml
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_weekly_correction_snapshot.py
🔇 Additional comments (3)
Gradata/pyproject.toml (1)
168-168: Pytest path update looks correct for script-module test imports.
pythonpath = ["src", "scripts"]is consistent with the new test module import pattern.Gradata/scripts/weekly_correction_snapshot.py (1)
81-86: Good fix for deterministic and non-contradictory outcome aggregation.Mutual exclusivity between accepted/rejected counts is now explicit, and output ordering remains deterministic.
Also applies to: 91-94, 124-126
Gradata/docs/weekly-correction-snapshot.md (1)
23-28: Schema documentation is aligned with implementation behavior.Field definitions, rounding precision, and category normalization are clearly and accurately described.
Also applies to: 36-36
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.
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.
1 participant
Summary
Validation