Relabel updated reports - #265
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReport updates now invalidate related gate answers and label results. Scans select updated reports, labeling reruns stale evaluations, status tools report stale records, and tests cover these behaviors. Design, implementation-plan, and troubleshooting documentation describe the updated timestamp rules. ChangesReport relabeling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Report
participant IncrementalScan
participant LabelingJob
participant label_report
participant GateAnswer
participant LabelResult
Report->>IncrementalScan: update updated_at
IncrementalScan->>LabelingJob: select report after checkpoint
LabelingJob->>label_report: process report
label_report->>GateAnswer: rerun stale gate
GateAnswer-->>label_report: return gate decision
label_report->>LabelResult: rerun stale label result
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution CodeRabbit couldn't update its existing comment. The review summary may be out of date. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
radis/labels/tasks.py (1)
21-21: 🚀 Performance & Scalability | 🔵 TrivialAdd a database index for
Report.updated_at.
Report.updated_atis used in scan-job scope filtering and checkpoint checks, butReporthas no modelindexesordb_index=Truefor this field, and its migrations do not add one. Add an index so incremental scans do not scan the full report table.🤖 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 `@radis/labels/tasks.py` at line 21, Add a database index for the Report.updated_at field, using the model’s existing indexing configuration or db_index declaration, and create the corresponding migration so the index is applied to existing databases.
🤖 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 `@AGENTS.md`:
- Line 228: Update the guidance around backfills and LABELING_SCAN_CRON to state
that existing reports require a backfill for a newly created label; a periodic
scan can queue labeling only when an eligible report has been created or updated
after the scan checkpoint. Preserve the note that any report update marks its
labels stale and triggers re-labeling.
In `@docs/superpowers/plans/2026-08-09-relabel-updated-reports.md`:
- Line 578: Update both fenced code examples at the affected documentation
sections to include a language identifier, such as text, immediately after each
opening fence. Ensure both fences satisfy markdownlint MD040 without changing
their example contents.
- Line 18: Replace every documented test command in the plan, including the
referenced occurrences, from `uv run pytest <path> -v` to the repository wrapper
format `uv run cli test -- <path>`, preserving any required verbosity or
arguments according to the documented command convention.
---
Nitpick comments:
In `@radis/labels/tasks.py`:
- Line 21: Add a database index for the Report.updated_at field, using the
model’s existing indexing configuration or db_index declaration, and create the
corresponding migration so the index is applied to existing databases.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5a85ed64-e88e-41a6-8ebb-2f8824b08ca6
📒 Files selected for processing (16)
AGENTS.mddocs/superpowers/plans/2026-08-09-relabel-updated-reports.mddocs/superpowers/specs/2026-08-09-relabel-updated-reports-design.mdradis/labels/admin.pyradis/labels/labeling.pyradis/labels/management/commands/labels_status.pyradis/labels/models.pyradis/labels/scope.pyradis/labels/tasks.pyradis/labels/tests/test_admin.pyradis/labels/tests/test_jobs.pyradis/labels/tests/test_labeling.pyradis/labels/tests/test_labels_status.pyradis/labels/tests/test_models.pyradis/labels/tests/test_scan.pyradis/labels/tests/test_scope.py
|
|
||
| - Confirm the label exists and is `active` | ||
| - Ensure a backfill has run or the periodic scan (`LABELING_SCAN_CRON`) has ticked since the label/report was created | ||
| - Ensure a backfill has run or the periodic scan (`LABELING_SCAN_CRON`) has ticked since the label was created or the report was created/updated (any report update marks its labels stale and triggers re-labeling) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not imply that a scan after label creation is sufficient.
radis/labels/tasks.py:incremental_label_scan queues work only when a report changed after the scan checkpoint. It does not inspect label creation time. A newly created label therefore requires a backfill unless a report update also occurs.
Update this guidance to state that a backfill is required for existing reports, while an eligible periodic scan can queue work after report updates.
Proposed wording
-- Ensure a backfill has run or the periodic scan (`LABELING_SCAN_CRON`) has ticked since the label was created or the report was created/updated (any report update marks its labels stale and triggers re-labeling)
+- Run a backfill after creating a label for existing reports. For report updates, an eligible periodic scan (`LABELING_SCAN_CRON`) can queue relabeling.📝 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.
| - Ensure a backfill has run or the periodic scan (`LABELING_SCAN_CRON`) has ticked since the label was created or the report was created/updated (any report update marks its labels stale and triggers re-labeling) | |
| - Run a backfill after creating a label for existing reports. For report updates, an eligible periodic scan (`LABELING_SCAN_CRON`) can queue relabeling. |
🤖 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 `@AGENTS.md` at line 228, Update the guidance around backfills and
LABELING_SCAN_CRON to state that existing reports require a backfill for a newly
created label; a periodic scan can queue labeling only when an eligible report
has been created or updated after the scan checkpoint. Preserve the note that
any report update marks its labels stale and triggers re-labeling.
| - Line length 100 (Ruff), Google Python Style, pyright basic mode. | ||
| - Keep docstrings/comments terse — match existing house style; comments only for constraints the code can't show. | ||
| - All work happens on the existing branch `relabel-updated-reports`. | ||
| - Test command: `uv run pytest <path> -v` from the project root (`/workspaces/adit-radis-workspace/projects/radis`). Tests marked `@pytest.mark.django_db` need the dev Postgres container running. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^|/)docs/src/superpowers/plans/2026-08-09-relabel-updated-reports\.md$|(^|/)uv.lock$|(^|/)pyproject\.toml$|(^|/)README'xargs -r -0 -n 1 echo
echo "== plan occurrences =="
if [ -f docs/superpowers/plans/2026-08-09-relabel-updated-reports.md ]; then
echo "plan lines count=$(wc -l < docs/superpowers/plans/2026-08-09-relabel-updated-reports.md)"
rg -n 'uv run (pytest|cli)|pytest' docs/superpowers/plans/2026-08-09-relabel-updated-reports.md || true
fi
echo "== cli/test command declarations =="
rg -n 'name\s*=\s*["'\'']?(pytest|test|cli)|uv run cli|pytest' pyproject.toml README.md uv.lock 2>/dev/null || true
echo "== project root command snippets =="
python3 - <<'PY'
from pathlib import Path
p = Path('pyproject.toml')
if p.exists():
text=p.read_text()
for needle in ['name = "cli"', 'name = "test"', '[project.scripts]', '[project.optional-dependencies]', 'uv run cli', 'uv run pytest']:
print('---', needle)
try:
idx=text.index(needle)
print(text[max(0, idx-120):idx+260])
except ValueError:
print('NOT FOUND')
PYRepository: openradx/radis
Length of output: 269
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== top-level files =="
git ls-files | sed -n '1,120p'
echo "== locate plan file variants =="
git ls-files | rg '2026-08-09-relabel-updated-reports\.md$|superpowers|plans' || true
echo "== locate configs =="
git ls-files | rg '(^|[\/])(pyproject\.toml|uv\.lock|README(\.md)?|tox\.ini|Makefile)$' || trueRepository: openradx/radis
Length of output: 4521
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== plan relevant command references =="
sed -n '15,25p;108,137p;206,216p;248,255p;313,322p;363,371p;436,444p;472,479p;523,531p;546,553p;586,594p' docs/superpowers/plans/2026-08-09-relabel-updated-reports.md
echo "== pyproject scripts/dependencies =="
sed -n '1,220p' pyproject.toml
echo "== root uv.lock package/entrypoint references =="
rg -n 'name\s*= "uv-run-test"|"uv-run-test"|name = "cli"|"cli"|pytest|test' pyproject.toml uv.lock 2>/dev/null || true
echo "== search repository for "uv run cli" and pytest wrapper declarations =="
rg -n 'uv run cli|uv run pytest|pytest run test|test' README.md CONTRIBUTING.md docs CONTRIBUTING.md AGENTS.md pyproject.toml 2>/dev/null || true
echo "== inspect cli.py structure =="
sed -n '1,260p' cli.pyRepository: openradx/radis
Length of output: 46152
🌐 Web query:
adit-radis-shared commands.test Typer pytest
💡 Result:
The adit-radis-shared repository is a shared library containing common Django apps, utilities, and infrastructure components used by the ADIT and RADIS projects [1][2]. Within the OpenRadX ecosystem, Typer is used for CLI commands [2], and pytest is the standard testing framework [3]. While there is no specific, public-facing command literally named commands.test in the repository, the project structure commonly employs pytest to execute test suites [3]. In development environments, such as the unified adit-radis-workspace, testing is typically handled via standard pytest commands or via wrappers provided by the dev container environment (often integrated with tools like uv) [4][5]. To run tests in projects utilizing this shared library, you would typically execute the following via your terminal: 1. Standard pytest execution (ensure you are in the project root or the directory containing your tests): pytest 2. If using the project's preferred toolchain (uv) as seen in recent repository configurations: uv run pytest 3. For specific testing configurations that might exist in the broader OpenRadX ecosystem, look for scripts in the project root or Makefile equivalents that invoke pytest, as these projects often use pytest-django and other plugins to handle complex test setups (e.g., database teardowns or async tasks) [3][6]. If you are encountering a specific "commands.test" reference, it may be a custom alias defined in the project's local Makefile, a custom Typer command implemented within the project, or a specific task defined in a task runner (like Procrastinate or a custom orchestration script) used within the ADIT/RADIS stack [2]. Check the project's pyproject.toml or Makefile for any defined script aliases.
Citations:
- 1: https://github.com/openradx/adit-radis-shared
- 2: https://github.com/openradx/radis/blob/main/AGENTS.md
- 3: Dependency Dashboard adit-radis-shared#9
- 4: https://github.com/openradx/adit-radis-workspace
- 5: openradx/adit@c3540c6
- 6: Error on teardown of test database in Django integration test procrastinate-org/procrastinate#1106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository docs for test command =="
sed -n '30,60p' AGENTS.md
sed -n '24,32p' docs/dev-docs/contributing.md
echo "== cli test command references =="
rg -n 'def test|commands\.test|test\(' cli.py pyproject.toml AGENTS.md README.md docs/dev-docs/contributing.md || true
echo "== package metadata for adit-radis-shared version and tests if any =="
rg -n 'adit-radio-shared|adit-radis-shared|commands\.test|def test' pyproject.toml uv.lock AGENTS.md cli.py || trueRepository: openradx/radis
Length of output: 3252
Use the documented uv run cli test wrapper for test steps.
cli.test is registered as the repository test command, and AGENTS.md documents tests under uv run cli test --. Replace the uv run pytest <path> command with uv run cli test -- <path> (or use the documented direct pytest exception format if it must be explicit), including lines 18, 112, 133, 211, 252, 318, 367, 440, 476, 527, 550, and 590.
🤖 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 `@docs/superpowers/plans/2026-08-09-relabel-updated-reports.md` at line 18,
Replace every documented test command in the plan, including the referenced
occurrences, from `uv run pytest <path> -v` to the repository wrapper format `uv
run cli test -- <path>`, preserving any required verbosity or arguments
according to the documented command convention.
Source: Learnings
|
|
||
| In the "Labels Not Appearing" section, change the second bullet from: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to both fenced examples.
markdownlint-cli2 reports MD040 at Lines 578 and 584. Add text or another suitable language identifier after each opening fence.
Also applies to: 584-584
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 578-578: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/superpowers/plans/2026-08-09-relabel-updated-reports.md` at line 578,
Update both fenced code examples at the affected documentation sections to
include a language identifier, such as text, immediately after each opening
fence. Ensure both fences satisfy markdownlint MD040 without changing their
example contents.
Source: Linters/SAST tools
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
radis/labels/tasks.py:21
- Using the generic
updated_athere makes every save a labeling event, not just a body change. In particular,_bulk_upsert_reportsunconditionally assignsexisting.updated_at = now(radis/reports/api/viewsets.py:153), so an identical ETL re-sync will put the entire corpus into scope and the new freshness checks will force gate and label LLM calls for every report. Key this window and staleness to an actual body revision (for example,body_updated_ator a persisted body hash updated only whenbodydiffers) so no-op re-upserts remain no-ops.
return Report.objects.filter(updated_at__gte=job.scan_from).order_by("pk")
| if g.id not in existing_gates | ||
| or existing_gates[g.id].generated_at < g.updated_at | ||
| or existing_gates[g.id].generated_at < report.updated_at |
Report Updates trigger labelling
closes #257
created_atwithupdated_atto query reports that need labelingSummary by CodeRabbit
Improvements
Documentation
Tests