feat(casework): court-record binder for accused binds and case dates - #442
feat(casework): court-record binder for accused binds and case dates#442gaurav-karki wants to merge 27 commits into
Conversation
… one PATCH Adds get_courtcase and list_hearings (read the NGM composite-key detail route and its hearings sub-resource, paging by page number the same way get_court_case_entities does) plus patch_case, which sends scalar fields and a whole-list path (e.g. entities) in ONE conditional PATCH -- the fix for the two-request/one-ETag failure build_replace_ops documents from enrich_card's 2026-08-04 smoke run.
…luck The existing test's non-deciding row was filtered out before max() ever ran, so the assertion passed identically under max(), decided[0], or decided[-1]. Add a case with two फैसला-passing rows at different dates, asserted in both list orders, so only an actual max-by-date comparison can satisfy it.
…ate them Adds the three-rung ladder for Task 4: a row's own nes_id is a pure copy, then exact name equality within the person prefix (never the scored resolver, which would pick a namesake by rank), then create-and-bind with run-scoped de-duplication. Fixes two defects found in the plan's own test snippet: the create payload was missing the server-required "slug" field, and one collision-path assertion hardcoded an IRI that doesn't match entity_slug's real (schwa-preserving) transliteration of the same name.
…esolve_defendant from ever raising Review round 1 findings: - CRITICAL: exact_person_match ignored CandidateList.complete, so a single hit inside a search window that stopped on relevance (not exhaustion) read as a confirmed unique match. That is how a namesake gets bound as accused on a corruption case -- the exact failure this ladder exists to prevent. Now a lone hit from an incomplete window falls through to creation instead of binding. - IMPORTANT: resolve_defendant called exact_person_match unguarded, so a transient search-API error would propagate and kill the whole run. Wrapped it the same way the create POST already was. - IMPORTANT: _is_person's nested-category behavior (person/politician IS a person, personnel/organization are NOT) had no direct coverage. Also corrects _is_person's docstring (it does not use startswith) and drops a file-path citation from a comment.
…o entities key, and require every reference to plainly acquit - plan_case now merges via current_entity_binds (PATCH shape), not the raw read list, which keyed every existing bind as (nes_id, "") and appended duplicate accused binds while re-sending outcome. - Refuse a case payload missing the "entities" key outright, matching enrich_related_entities.plan_case_entities's own guard against the same destructive-replace hazard. - bind_outcome now requires every court reference to have decided, using the same two-source truth end_date already relies on, and refuses to call a hearing cell a plain acquittal when it also carries आंशिक or ठहर.
…sspelled qualifiers before testing them - Add the missing positive-branch test: every reference deciding a plain सफाई, one via its own hearing row and the other via the case_status fallback, asserts ACQUITTED. Nothing before this proved the rewrite's all(decided) and all(acquitted) branch survives; every prior multi-reference test only asserted CHARGED. - Normalise the hearing decision_type cell through courts.case_status._order_key before testing for आंशिक/ठहर qualifiers, so a documented misspelling (आंशीक) can't slip a partial conviction past the guard and read as a plain acquittal. Reuses the same normalisation outcome_from_hearings already applies to this identical field, rather than hand-copying the spelling table.
… and log why every skip happened - select-skip events now carry `plan.skips` as `detail`, so an operator replaying *.events.jsonl can tell "no court reference at all" from "every reference on it 404'd" instead of seeing an identical line for both. - _log_plan routes per-reference read failures to court_read/unreadable instead of dates/no_source -- a broken read is not a date fact. - the 412-vs-rejected split now checks isinstance(exc, HTTPError) and exc.code == 412 instead of a substring test on the exception's own message, which a slug like case-...-cr-0412 could spuriously match on the unrelated no-ETag ValueError. - casework.ledger.NON_OUTCOME_STATUSES gains "dry_run" so a clean --dry-run patch event stops folding into the "what did we change, when" audit, the same way bind_materials' "planned" already does. Covered by new tests for each: a skip-state detail check, a two-court- reference partial-read case, an --apply 412 conflict plus its companion success path, a slug-containing-412 regression proof, a dry-run test that now falls through to the entity-creation rung instead of short-circuiting on an existing nes_id, and a ledger test for the new non-outcome status. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…help text STAGES/TIERS both gain court_record so test_stage_names_match_llm_tier_names stays pinned. case_start_date/case_end_date help text now says what the field actually holds (court registration date on 46 of 48 published cases with a readable court record; deciding hearing date on 29 of 29), not the "alleged incident" convention nobody uses.
…ing the ledger
The court-record binder had two paths that bound a person the ladder had just
declined to identify. On a 409 the create kept the IRI it computed BEFORE the
POST and bound it -- but that branch fires precisely when rung 2 refused ("13
person entities carry this exact name", or the truncation veto), so the bind
went to whoever already owned the slug. And `run_entities`, shared across cases
by design, was keyed on the bare name, so two different people sharing a name on
two cases collapsed into one entity carrying both accusations.
Both now fail safe. The 409 path binds nothing and reports the taken IRI for a
human; the run key is name AND the court party row's address. This trades bind
coverage for safety and is an interim ruling -- see the fix report for the
alternatives (re-search and compare, or an explicit review queue).
Three more corrections:
- An unreadable prefix list (a transient 502 at run start) reported every
defendant as needing a prefix that is not creatable -- false for a prefix as
ordinary as `person`. It now says the list was never read, retry the case,
matching `enrich_related_entities._cannot_create`.
- Every intermediate event now reports `ok` with its classification in the
detail, the way every sibling enricher does. Adding `dry_run` to
NON_OUTCOME_STATUSES achieved nothing while `bind_plan`/`merged` still landed
in the "what did we change, when" audit for every dry-run case. A
`nothing-to-do` case gains a terminal `idempotency`/`already` event so it does
not vanish from the ledger instead.
- `court_record.py` no longer claims to be unwired, `defendant_names` and
`_accused_binds` share one `is_defendant`/`party_name` pair rather than two
copies of the filter, and the unread `CHARGED` is gone.
The guard-wiring pin covers `enrich_court_record` now, plus `enrich_card` and
`enrich_description` which had drifted out of it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ions OA, RE and the W* writ codes name a government office in the court record's defendant column, not a person. case_number_code() reads the case-type letters out of the court case number, and _accused_binds now skips a whole record whose code isn't on the CR/CB/FJ/"" allow-list before it ever reads that record's parties -- start_date/end_date are untouched, since the filter is on binding only.
… review debt _log_plan had no branch for _accused_binds' new record-level skip, so it fell into the dates/no_source bucket and the bind_plan summary claimed 0 defendants where the record had named one and been declined. Add _NON_PROSECUTION_SKIP_PREFIX alongside _COURT_READ_FAILURE_PREFIX and route on it; the bind_plan line now counts resolved defendants and says how many references were skipped as non-prosecution. Also: drop the self-referential BINDABLE_CODES test, collapse the duplicate _court_record test builder into _record, and cut the same rationale from being written three times across two docstrings and a comment block.
Two same-name defendants on separate court cases collapsed into one NES entity through the exact-match rung, putting one person's accusation on another's record. defendant_name_index/held_names build a cross-case name index from every selected case's court record; a name in the held set gets a report row and no bind item, while the case's other defendants and dates are unaffected. Also adds the automated _log_plan routing test the non- prosecution skip was still missing.
A held row inflated len(plan.rows) into three consumers that all meant "defendants this run resolved" -- the bind_plan summary, the review file's accused+N, and the nothing-to-do idempotency detail all overcounted by the held row and never mentioned the hold. A nothing-to-do case whose only outstanding item is a held name also logged ledger status "already", which casework.ledger.build_ledger records as a completed outcome; it now logs the distinct held_for_review instead. Also makes held a required keyword (no more held=None silently reopening the same-name collapse), fixes the held-check to membership (is not None) rather than truthiness, keys the per-case defendant dedup on the normalised name, and trims a docstring paragraph and a dead fallback string flagged in review.
…w status
main() split into two passes: pass 1 reads every selected case's court record
and builds the cross-case held-name index once, pass 2 re-reads each case for
a fresh ETag and plans it against that same index -- closing the gap where
held={} left the Task 2 hold inert in a real run. Ships a held-names JSON
file beside the review file for a human to rule on. Also fixes review rows
always reading would-patch regardless of the case's actual outcome, and
renames a shadowed `detail` variable flagged in the Task 2 review
… index write_held_file crashed after the PATCH landed whenever the review directory didn't exist yet, since main() called it before review.write() -- the only thing that ever created it -- losing the review file, the held file, and the run footer. Also logs a run-level held_index event (cases selected vs readable vs names indexed vs held) so a pass-1 read failure or a narrowed selection (--limit/--slug/--court-case/--batch-csv) is visible instead of silently weakening the hold, pins get_case_with_etag/get_courtcase call counts, and rewires the three held-behavior tests onto the real two-pass index instead of a monkeypatched one
…tion The held_index WARNING only fired for --limit/--slug/--court-case/ --batch-csv, never when a pass-1 read failure shrank readable_cases below the selection -- the exact scenario a transient 5xx on one case creates, leaving no signal beyond error=1 in the footer. Now fires on either cause, names which one applied, and logs at logging.WARNING so it's visible to level-based filtering, not just to grepping detail text
case_number_code returned "" -- the pre-FY073 prosecution bucket -- for anything it couldn't parse, not just the genuine legacy all-digit shape. `W-081-0037`, `RE-081-1730`, `079_CR_0151`, and a Devanagari-transliterated number all fell into BINDABLE_CODES by accident, inverting the allow-list rule Task 1 was written around. Only `^[0-9]+-[0-9]+-[0-9]+$` (matching `93-068-0194`) now maps to "", everything else this can't read returns UNPARSEABLE, which is not in BINDABLE_CODES so it skips and logs like any other unrecognised code.
…ld index can't see
narrowed checked --limit/--slug/--court-case/--batch-csv but not
--fiscal-year -- the selector an operator scoping a whole campaign is most
likely to use, so a fiscal-year-scoped run got no WARNING though the same
shrink under --limit did. Added to the predicate and the reason text.
The held_index detail also said nothing about select_cases's own
ENRICHABLE_STATES gate, so a clean, unrestricted, unshrunk run read as "the
index is complete" when a PUBLISHED case -- exactly where a confirmed
accused bind already lives -- is structurally invisible to it regardless of
any CLI flag. States the limit on every run now, and corrects the module
docstring's claim that an unrestricted sweep "is what actually makes the
hold protective"; it does not, on its own, see past the state gate.
Also:
- resolved_count = len(plan.rows) - held_count counted a how="failed" row
(an unslugabble name, a search error, a slug collision) as resolved, so
"accused+N" and the bind_plan summary overcounted by one phantom bind per
failure. Factored into _rung_counts, shared by _log_plan and main.
- Pass 1 keyed court_records on `case.get("slug") or ""` with no guard, so
two slug-less cases would both land on "" and pass 2 would plan both
against whichever record set was written last. Refuse to index a
slug-less case instead.
- _accused_binds read its new entity's citation off records[0] before the
per-record prosecution filter ran, so a person created for a CR defendant
could be cited to an OA/RE/writ material that never names them. Now taken
from the first record that actually passes the filter.
- A dry run's "would create" resolution implied the printed IRI is the one
--apply would bind, but dry_run never reaches the EntityAlreadyExists
handler -- on the COMMON path for a common name (this slug already
belongs to someone the ladder declined to identify), --apply refuses the
bind instead. The reason now says that plainly.
…swallow it `nes_id or reason` discarded the reason on every row carrying both, which is exactly the two whose reason is a warning: a dry run's "would create" (--apply refuses it when the slug is taken) and "reused from this run". Both read as a settled bind in the events file a caseworker approves from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…written The selection gate (ENRICHABLE_STATES, DRAFT+IN_REVIEW) and the write gate (REQUIRED_WRITE_STATE, DRAFT alone) are deliberately different widths, and conflating them would start writing to cases already under human review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, push a new commit or reopen this pull request to trigger a review.
|
Warning Review limit reached
Next review available in: 30 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis change adds a zero-LLM ChangesCourt record enrichment
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant court_record_for_case
participant CaseworkApi
participant plan_case
participant resolve_defendant
CLI->>court_record_for_case: Read court references
court_record_for_case->>CaseworkApi: Fetch details, hearings, and parties
CLI->>plan_case: Plan dates and defendant binds
plan_case->>resolve_defendant: Resolve or create person entity
plan_case->>CaseworkApi: Apply conditional case PATCH
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
PR Reviewer Guide 🔍(Review updated until commit 45a24cd)Here are some key observations to aid the review process:
🛠️ Relevant configurations:
[config] enable_ai_metadata: False
custom_model_max_tokens: 200000
git_provider: github
output_relevant_configurations: True
model: openai/cx/gpt-5.5
ENABLE_AUTO_APPROVAL: True
custom_reasoning_model: False
fallback_models: ['openai/cx/gpt-5.4-mini']
is_auto_command: True
publish_output: True
publish_output_progress: True
progress_gif_url:
progress_gif_width: 48
verbosity_level: 0
use_extra_bad_extensions: False
log_level: DEBUG
use_wiki_settings_file: True
use_repo_settings_file: True
use_global_settings_file: True
extra_config_url:
disable_auto_feedback: False
ai_timeout: 120
response_language: en-US
repo_context_files: ['AGENTS.md']
repo_context_from_default_branch: True
repo_context_max_lines: 500
max_description_tokens: 500
max_commits_tokens: 500
max_model_tokens: 32000
model_token_count_estimate_factor: 0.3
patch_extension_skip_types: ['.md', '.txt']
allow_dynamic_context: True
max_extra_lines_before_dynamic_context: 10
patch_extra_lines_before: 5
patch_extra_lines_after: 1
cli_mode: False
large_patch_policy: clip
duplicate_prompt_examples: False
seed: -1
temperature: 0.2
ignore_pr_title: ['^\\[Auto\\]', '^Auto', '^Bump ', '^chore\\(deps\\)']
ignore_pr_target_branches: []
ignore_pr_source_branches: []
ignore_pr_labels: []
ignore_pr_authors: []
ignore_repositories: []
ignore_language_framework: []
restricted_mode: False
reasoning_effort: medium
enable_claude_extended_thinking: False
extended_thinking_budget_tokens: 2048
extended_thinking_max_output_tokens: 4096
claude_extended_thinking_models_override: []
extract_issue_from_branch: True
branch_issue_regex:
enable_custom_labels: False
[pr_reviewer] require_ticket_analysis_review: False
require_score_review: False
require_tests_review: True
require_estimate_effort_to_review: True
require_can_be_split_review: False
require_security_review: True
require_estimate_contribution_time_cost: False
require_todo_scan: False
publish_output_no_suggestions: True
persistent_comment: True
extra_instructions: Focus on: logic errors and edge cases; security/authz regressions; missing error handling;
Django/DRF correctness (migrations, N+1 queries, transaction/atomicity, serializer & permission gaps).
Do NOT comment on formatting, import order, or naming — ruff handles those in CI.
num_max_findings: 3
final_update_message: True
enable_review_labels_security: True
enable_review_labels_effort: True
require_all_thresholds_for_incremental_review: False
minimal_commits_for_incremental_review: 0
minimal_minutes_for_incremental_review: 0
enable_intro_text: True
enable_help_text: False
|
PR Code Suggestions ✨Explore these optional code suggestions:
🛠️ Relevant configurations:
[config] enable_ai_metadata: False
custom_model_max_tokens: 200000
git_provider: github
output_relevant_configurations: True
model: openai/cx/gpt-5.5
ENABLE_AUTO_APPROVAL: True
custom_reasoning_model: False
fallback_models: ['openai/cx/gpt-5.4-mini']
is_auto_command: True
publish_output: True
publish_output_progress: True
progress_gif_url:
progress_gif_width: 48
verbosity_level: 0
use_extra_bad_extensions: False
log_level: DEBUG
use_wiki_settings_file: True
use_repo_settings_file: True
use_global_settings_file: True
extra_config_url:
disable_auto_feedback: False
ai_timeout: 120
response_language: en-US
repo_context_files: ['AGENTS.md']
repo_context_from_default_branch: True
repo_context_max_lines: 500
max_description_tokens: 500
max_commits_tokens: 500
max_model_tokens: 32000
model_token_count_estimate_factor: 0.3
patch_extension_skip_types: ['.md', '.txt']
allow_dynamic_context: True
max_extra_lines_before_dynamic_context: 10
patch_extra_lines_before: 5
patch_extra_lines_after: 1
cli_mode: False
large_patch_policy: clip
duplicate_prompt_examples: False
seed: -1
temperature: 0.2
ignore_pr_title: ['^\\[Auto\\]', '^Auto', '^Bump ', '^chore\\(deps\\)']
ignore_pr_target_branches: []
ignore_pr_source_branches: []
ignore_pr_labels: []
ignore_pr_authors: []
ignore_repositories: []
ignore_language_framework: []
restricted_mode: False
reasoning_effort: medium
enable_claude_extended_thinking: False
extended_thinking_budget_tokens: 2048
extended_thinking_max_output_tokens: 4096
claude_extended_thinking_models_override: []
extract_issue_from_branch: True
branch_issue_regex:
enable_custom_labels: False
[pr_code_suggestions] commitable_code_suggestions: False
dual_publishing_score_threshold: -1
focus_only_on_problems: True
extra_instructions: Prefer a few high-impact, project-specific suggestions over many generic ones.
Skip style/formatting (ruff-enforced) and changes under cases/migrations/.
enable_help_text: False
enable_chat_text: False
persistent_comment: True
max_history_len: 4
publish_output_no_suggestions: True
suggestions_score_threshold: 0
new_score_mechanism: True
new_score_mechanism_th_high: 9
new_score_mechanism_th_medium: 7
auto_extended_mode: True
num_code_suggestions_per_chunk: 3
max_number_of_calls: 3
parallel_calls: True
final_clip_factor: 0.8
decouple_hunks: False
demand_code_suggestions_self_review: False
code_suggestions_self_review_text: **Author self-review**: I have reviewed the PR code suggestions, and addressed the relevant ones.
approve_pr_on_self_review: False
fold_suggestions_on_self_review: True
num_code_suggestions: 4
|
PR Agent Walkthrough 🤖Welcome to the PR Agent, an AI-powered tool for automated pull request analysis, feedback, suggestions and more. Here is a list of tools you can use to interact with the PR Agent:
(1) Note that each tool can be triggered automatically when a new PR is opened, or called manually by commenting on a PR. (2) Tools marked with [*] require additional parameters to be passed. For example, to invoke the |
|
Auto-approved PR |
![jawafdehi-pr-agent[bot]](https://avatars.githubusercontent.com/u/241969530?s=60&v=4){kind=small}
The help text was genuinely wrong -- `case_start_date` read "when the alleged incident began" while 46 of 48 published cases use the court registration date, which is also what the binder writes. But the fix is independent of the binder, and it dragged the only cases/ change and a migration number into an otherwise self-contained casework/ branch, where 0056 is a merge hazard for a cosmetic edit. Goes back as its own PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Persistent review updated to latest commit 7df92ab |
…ecord-binder # Conflicts: # casework/common/llm.py # casework/common/pipeline.py
|
Persistent review updated to latest commit d3a2477 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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
🧹 Nitpick comments (6)
tests/casework/test_api.py (1)
1069-1077: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a short-first-page case to this pagination test.
The fixture returns exactly 100 rows on page 1, so the test only proves that a full page is followed. It cannot detect the truncation risk described on
casework/common/api.pyLine 414-421: a server that capspage_sizebelow 100 makes page 1 short and the loop returns after one request. Add a case where page 1 is short but carriesnext, so the test pins the intended termination rule.🤖 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 `@tests/casework/test_api.py` around lines 1069 - 1077, Extend test_list_hearings_follows_pages_by_number with a short-first-page fixture where page 1 has fewer than 100 results and includes a next-page indicator, while page 2 contains additional results. Assert both pages are followed and all rows are returned, preserving the existing full-page pagination coverage.tests/casework/test_court_record.py (1)
183-204: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider pinning a hearings-side and parties-side read failure.
Both failure tests inject the exception into
detail, so they only exercise the first of the three reads. A regression that movedlist_hearingsorget_court_case_entitiesoutside thetryblock would still pass. One extra case with the exception stored inhearingswould close that.🤖 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 `@tests/casework/test_court_record.py` around lines 183 - 204, Extend the court record failure tests around court_record_for_case to cover read failures from the hearings and parties/entity lookups, not only detail. Add cases injecting the HTTPError into the _FullApi hearings and get_court_case_entities data, and assert the affected reference is skipped without raising while other valid records remain returned.casework/enrich_court_record.py (4)
984-1002: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winOn
--dry-run, pass 2 re-reads every case for an ETag it never uses.Pass 1 already fetched
case_detailat Line 920 and discards it. Pass 2 fetches it again for a fresh ETag. Ifargs.dry_runis set, no PATCH is sent, so the fresh ETag has no purpose. Keeping the pass-1case_detailand reusing it on dry runs halves the case reads for the most common run mode. A full-corpus dry run currently pays about 3,000 avoidable round trips.Keep the re-read for
--apply, where a staleIf-Matchwould 412.🤖 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 `@casework/enrich_court_record.py` around lines 984 - 1002, Update the pass-1/pass-2 case processing around readable_cases and plan_case so --dry-run retains and reuses the pass-1 case_detail instead of calling api.get_case_with_etag again. Keep the existing pass-2 re-read and fresh ETag acquisition for apply runs, while ensuring dry-run planning still receives the retained case_detail and appropriate ETag value.
899-932: 🩺 Stability & Availability | 🔵 TrivialConsider a request-rate control for the pass-1 sweep.
Pass 1 issues one case read plus up to three court reads per court reference, with no delay and no retry policy. Across the full corpus that is several thousand sequential requests against the NGM read plane. The client has no backoff for a 429 or a 5xx:
court_record_for_caseturns any failure into a skip, so a transient rate limit silently costs those cases their defendants and their dates for the whole run.Two options, in order of value:
- Treat 429 and 5xx differently from 404 in
court_record_for_case, so a throttled read is retried rather than converted into a permanent skip.- Add an optional inter-request delay flag, validated with
casework.common.cli.nonneg_float.Based on learnings: "ensure numeric retry/interval/circuit-breaker flags validate non-negativity at argument parsing time by using
casework.common.cli.nonneg_int/casework.common.cli.nonneg_float".🤖 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 `@casework/enrich_court_record.py` around lines 899 - 932, Update court_record_for_case to distinguish transient 429 and 5xx responses from permanent 404s, retrying transient failures with bounded backoff before skipping the case. Preserve existing permanent-failure behavior, and ensure any retry or interval CLI options use nonneg_int or nonneg_float validation so negative values are rejected during argument parsing.Source: Learnings
317-319: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winKeep rung 1 inside the person prefix policy.
Rung 1 currently accepts any canonical entity IRI, including
organization/...orpersonnel/.... Since this module is meant to stay insidepersonand rung 2 filters with_is_person, add that guard at line 319 before binding the court row'snes_idasaccused.🛡️ Proposed fix
row_nes_id = (row_nes_id or "").strip() - if row_nes_id and is_valid_entity_iri(row_nes_id): + if row_nes_id and is_valid_entity_iri(row_nes_id) and _is_person(row_nes_id): return Resolution(row_nes_id, "nes_id")🤖 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 `@casework/enrich_court_record.py` around lines 317 - 319, Restrict rung 1 in the resolution logic to person entities by adding the existing _is_person guard alongside is_valid_entity_iri before returning Resolution for row_nes_id. Keep non-person IRIs from being bound as the accused, while preserving the current valid-person resolution path.
107-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove
_order_keyout of the private cross-app import.
_order_keyis a private symbol that is only used bycasework.enrich_court_recordoutsidecourts/, and its use is only justified by the shared spelling table it drives. Export a public helper fromcourts.case_statusand import that instead, so this parser is not coupled to private court scraper internals.🤖 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 `@casework/enrich_court_record.py` at line 107, Replace the private _order_key import in the casework parser with a public helper exported by courts.case_status that provides the same shared spelling-table ordering behavior, and update the courts-side definition/export accordingly. Keep parse_case_status unchanged and remove the cross-app dependency on _order_key.Source: Coding guidelines
🤖 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 `@casework/common/api.py`:
- Around line 414-421: The list_hearings paginator in
casework/common/api.py:414-421 should stop based on data.get("next") or an empty
batch, not on batch length, matching get_court_case_entities and iter_cases;
preserve extending all fetched results. Update
tests/casework/test_api.py:1069-1077 by adding next values to page fixtures and
covering a short first page that still has a next link, ensuring pagination
continues until next is absent.
In `@casework/enrich_court_record.py`:
- Around line 539-543: Update the party_name docstring in
casework/court_record.py to remove or correct the claim that both de-duplication
paths key on the exact returned string. Document the actual behavior: the
enricher uses normalise_name(name), while court_record.defendant_names uses the
raw stripped name.
---
Nitpick comments:
In `@casework/enrich_court_record.py`:
- Around line 984-1002: Update the pass-1/pass-2 case processing around
readable_cases and plan_case so --dry-run retains and reuses the pass-1
case_detail instead of calling api.get_case_with_etag again. Keep the existing
pass-2 re-read and fresh ETag acquisition for apply runs, while ensuring dry-run
planning still receives the retained case_detail and appropriate ETag value.
- Around line 899-932: Update court_record_for_case to distinguish transient 429
and 5xx responses from permanent 404s, retrying transient failures with bounded
backoff before skipping the case. Preserve existing permanent-failure behavior,
and ensure any retry or interval CLI options use nonneg_int or nonneg_float
validation so negative values are rejected during argument parsing.
- Around line 317-319: Restrict rung 1 in the resolution logic to person
entities by adding the existing _is_person guard alongside is_valid_entity_iri
before returning Resolution for row_nes_id. Keep non-person IRIs from being
bound as the accused, while preserving the current valid-person resolution path.
- Line 107: Replace the private _order_key import in the casework parser with a
public helper exported by courts.case_status that provides the same shared
spelling-table ordering behavior, and update the courts-side definition/export
accordingly. Keep parse_case_status unchanged and remove the cross-app
dependency on _order_key.
In `@tests/casework/test_api.py`:
- Around line 1069-1077: Extend test_list_hearings_follows_pages_by_number with
a short-first-page fixture where page 1 has fewer than 100 results and includes
a next-page indicator, while page 2 contains additional results. Assert both
pages are followed and all rows are returned, preserving the existing full-page
pagination coverage.
In `@tests/casework/test_court_record.py`:
- Around line 183-204: Extend the court record failure tests around
court_record_for_case to cover read failures from the hearings and
parties/entity lookups, not only detail. Add cases injecting the HTTPError into
the _FullApi hearings and get_court_case_entities data, and assert the affected
reference is skipped without raising while other valid records remain returned.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 072960fa-97a2-4f46-9777-14d60374ff50
📒 Files selected for processing (12)
casework/README.mdcasework/common/api.pycasework/common/llm.pycasework/common/pipeline.pycasework/court_record.pycasework/enrich_court_record.pycasework/ledger.pytests/casework/test_api.pytests/casework/test_build_api_guard_wiring.pytests/casework/test_court_record.pytests/casework/test_enrich_court_record.pytests/casework/test_ledger.py
…erson list_hearings exited on `len(batch) < 100`, but config.settings uses plain PageNumberPagination with no page_size_query_param -- page_size is ignored and every page is 20 rows, so the loop always stopped after page 1. Measured: 3 of 77 sampled FY078/079 cases carry more than 20 hearings (max 27), and a deciding फैसला row in the dropped tail silently changes case_end_date and bind_outcome. Also from review: rung 1 copied the court row's nes_id on IRI validity alone, the one path by which an office could be bound as the accused individual; a dry run re-read every case for an If-Match it can never send; and party_name's docstring still claimed both de-dup paths key on the same string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks — the pagination finding was real, and worse than described. Addressed in 45a24cd.
The root cause is not "if the endpoint caps page_size". Measured on production, anonymous GET, over every 4th FY078/079 census case: ~4% of cases silently lost hearings, and Now exits on Test rewritten as you asked, plus two more: the production shape (20 rows with
Nitpicks:
1618 tests pass, |
|
Persistent review updated to latest commit 45a24cd |
1 participant
User description
Adds
casework/enrich_court_record.py: a standalone control-plane enricher that reads acase's court record, fills
case_start_date/case_end_date, and binds the court'sdefendants to the case as
accused. No Django model imports — HTTP only, like every othercasework/stage.Why this is possible at all
Measured against production, anonymous
GETonly, across all 307 cases in the FY078/079census.
registration_date_ad(the start date)case_statusstringcase_start_datealready setThe rule reproduces what caseworkers already do by hand: across the 62 published cases,
case_start_dateequals the court registration date on 46 of 48, andcase_end_dateequals the deciding hearing date on 29 of 29.
How it writes
PATCH /cases/<slug>with anIf-MatchETag.entitiesis a whole-list-replace path, sothe binder reads the current list, merges on
(nes_id, relationship_type), and sends thewhole list back — an existing bind is never dropped and never duplicated. A 412 is logged
and skipped, never retried or forced. Writes are gated to
DRAFT(REQUIRED_WRITE_STATE);--allow-remote-writesis required for any non-loopback PATCH.The two wrong-person paths this closes
Binding the wrong person as
accusedon a corruption case is defamation of a namedindividual, so both were measured rather than assumed.
1. Writ and appeal cases name government offices in the defendant column. Every one of
the 14
OAreferences is filed againstनेपाल सरकार; theW*references name things likeमुख्यमन्त्री तथा मन्त्रिपरिषदको कार्यालय प्रदेश नं.१ विराटनगर. Without a filter the bindercreates a
personentity for a ministry and marks it accused.case_number_codeclassifies the court number and only prosecutions bind —CR(2,804refs),
CB(9),FJ(1), and the pre-FY073 no-code format like93-068-0194(139 refs on135 cases, which a naive "must contain
-CR-" rule would have silently dropped). Anythingunparseable skips rather than binds. The filter applies to binding only: the 16 cases whose
only reference is a writ still get both dates.
2. Name-only identity, with nothing to disambiguate it.
addressnes_idrun_entitiesis keyed on(name, address)and no row has an address, so every defendanttakes the address-less path. Refusing to reuse an address-less hit does not fix it: once
case A creates entity X for name N, case B's
exact_person_matchfinds exactly one N andbinds X through the ordinary rung, no cache involved.
So a name appearing on more than one case in the run is held — reported with the other
cases it appears on, never auto-bound — and written to a
<review>.held.jsonfile for ahuman to rule on. Names on exactly one case bind normally.
A 25-case production dry run confirmed the resolution rate holds at scale: 0 of 138
defendants matched an existing NES person. Entity creation is not an edge case here, it
is the normal path, which is what makes the hold load-bearing.
Known limitations — please read before running
--applyThe hold is scoped to one run. It only sees names repeated among the cases selected
in that run, so
--limit,--slug,--court-case,--batch-csvand--fiscal-yearall narrow it. Running in small batches switches the protection off: two cases naming
the same person in different batches will both bind. The
held_indexevent logs the scopeand warns at
WARNINGlevel whenever the selection is narrowed, but the warning cannotprevent it. A follow-up adding a
--held-names <file>flag — precompute the collision listonce over the whole corpus, load it in every batch — would make batching safe.
The index cannot see PUBLISHED cases. It is built from
select_for_run, which filtersto
ENRICHABLE_STATES = ("DRAFT", "IN_REVIEW"). Published cases carry confirmed accusedbinds and are invisible to the collision check. This is deliberate — published cases are
not to be touched — and the
held_indexline states it explicitly.A pass-1 read failure shrinks the index. A case whose court record fails to read
contributes no defendants, so another case sharing a name with it binds instead of holding.
Made visible (the warning fires on
readable != selected), not repaired.Full-corpus runs sit close to the rate limit. Measured 8.8 requests per case, ~4,470
requests/hour against a 5,000/hour ceiling. A ~2,900-case sweep is ~26,000 requests over
~6 hours at 89% of budget, and a throttled NES search degrades silently into "could not
resolve". Pace it or raise the limit before attempting one.
Testing
1,426 tests in
tests/casework/,ruff check .andtyclean.Every task went through an independent review and fix rounds, plus a whole-branch review.
The reviews found — and this branch fixes — a held file that crashed the run after the
PATCHes landed (destroying the review file), held defendants counted as "resolved" and
recorded in the ledger as a completed stage, review rows that read
would-patcheven underMode: APPLIED, and a merge against the API's read shape that would have duplicated bindsand 400'd on every case already carrying one.
A local end-to-end smoke test against a throwaway sqlite server covers all four fixture
shapes (shared defendant, writ case, pre-FY073 number, ordinary case) plus dry-run →
apply → idempotent-rerun. Verdict PASS.
No
--applyhas ever been run against production.🤖 Generated with Claude Code
PR Type
Enhancement, Tests
Description
Add
court_recordenricherFill case dates from court records
Bind accused via cautious person resolution
Extend API, pipeline, ledger tests
Diagram Walkthrough
File Walkthrough
4 files
Add court-record enrichment CLIRead full court record dataAdd courtcase reads and patch helperTreat court dry-runs as non-outcomes3 files
Register court_record pipeline stageRegister non-LLM stage tierMigrate corrected date help text2 files
Document court-record enricher usageUpdate case date help text5 files
Cover court-record enrichment planningCover court record reads and codesCover courtcase API helpersCover court dry-run ledger behaviorPin guard wiring for all enrichers🛠️ Relevant configurations:
These are the relevant configurations for this tool:
[config]
[pr_description]
Summary by CodeRabbit