Fix: by-case ingestion incorrectly returns NOT_REQUIRED without a generated answer - #201
Closed
rkakumanuhmcts wants to merge 9 commits into
Closed
Fix: by-case ingestion incorrectly returns NOT_REQUIRED without a generated answer#201rkakumanuhmcts wants to merge 9 commits into
rkakumanuhmcts wants to merge 9 commits into
Conversation
…point (#191) * added discovery schdule initial files from claude * added repository method through Calude * updated claude docs through Claude * Fix CodeQL log-injection finding on DiscoverySchedulerController Log specific request fields (courtCentreId, courtRoomId, version) instead of the raw request DTO, addressing the java/log-injection alert flagged on PR #191. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: ravi kakumanu <ravik@ravis-MacBook-Pro-2.local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…and process scheduled ingestions for the dates (#192) * [DD-42880] implement nightly scheduler; calculate hearing days ahead and process scheduled ingestions for the dates
…w tasks for courtcentre configurations (#194) * calculate hearingDays ahead and dispatch ingestion workflow tasks for courtcentre configurations * removed Transactional to limit transaction to creating/inserting the jobmanager task * throw exception when no value for env variable CASEDOCUMENTKNOWLEDGE_SYSTEM_USER_ID * tidyup
…by-case) (#195) * DD-42807 - add manual by-case ingestion endpoint (POST /ingestions/start-by-case) Adds a synchronous entry point for the "Process IDPC" button on the AI Search page: given a single caseId it runs eligibility and IDPC-availability checks inline (via CheckCaseEligibilityTask/CheckIdpcAvailabilityAllDefendantsTask, same task classes the scheduled flow uses) and only responds once the outcome is known - STARTED, NOT_REQUIRED, or FAILED. Downstream JobManager tasks run at HIGH priority so manual clicks jump ahead of scheduled ingestion; the existing /ingestions/start endpoint is unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * DD-42807 - address code review findings on manual ingestion endpoint - Split the shared NOT_REQUIRED message: "case not eligible" now gets its own accurate message instead of reusing the "no newer IDPC version available, Answers already exists" text, which was misleading when the case was never found or has no defendants. - Strip CTX_SYNCHRONOUS_INVOCATION_KEY from job data before it reaches the dispatched RETRIEVE_MATERIAL_AND_UPLOAD job, so the sync-only flag never rides into the async job queue. - Add a test asserting HIGH priority actually propagates through to the dispatched retrieval job (the only point priority takes effect), and that the synchronous-invocation flag is stripped from its job data. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * DD-42807 - move eligibility/IDPC-availability rules into plain service classes Per review comment: reverts CheckCaseEligibilityTask and CheckIdpcAvailabilityAllDefendantsTask to thin JobManager task wrappers, and moves the reusable business logic into CaseEligibilityService and IdpcAvailabilityService respectively. Both the async scheduled flow (via the task classes) and the synchronous manual "Process IDPC" flow (via IngestionProcessorByCaseService) now call these services directly as plain methods, rather than IngestionProcessorByCaseService invoking CheckCaseEligibilityTask.execute(ExecutionInfo) through the task-manager contract. This removes the need for CTX_SYNCHRONOUS_INVOCATION_KEY entirely: since IngestionProcessorByCaseService never touches the task classes' execute() methods, there is no risk of the async job queue being triggered twice for the same case, so the flag (and its associated stripping-before-dispatch logic) is deleted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * DD-42807 - drop redundant case-eligibility step, share retrieval job-data building CaseEligibilityService/CheckCaseEligibilityTask added a round-trip that duplicated what IdpcAvailabilityService already establishes: an empty document list means nothing to ingest, whether that's because the case doesn't exist, has no defendants, or is already up to date. Both the scheduled flow (GetCasesForHearingTask now dispatches CHECK_IDPC_AVAILABILITY_ALL_DEFENDANTS directly) and the manual by-case flow (IngestionProcessorByCaseService) call IdpcAvailabilityService directly instead. Extracted RetrieveMaterialAndUploadJobDataService to build the per-document RETRIEVE_MATERIAL_AND_UPLOAD job data once, shared by both call sites, instead of duplicating that logic in two private dispatch methods. Removed the now-dead prosecution-case-eligibility client method, its DTOs and config, and the wiremock fixtures backing it; added a replacement failure stub on the court-document-search endpoint so the by-case FAILED integration scenario still exercises a real error path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * DD-42807 - inline user-id header lookup in startIngestionProcessByCase The intermediate headerName variable held the header's name, not the user id, which read as mislabeled per review. Inlining the lookup removes the confusion entirely — cppuid is the only variable left, and it holds the actual value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: ravi kakumanu <ravik@ravis-MacBook-Pro-2.local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…s using /hearing-cases-for-day (#196) * get hearing cases for day * passing integration tests * Updated integration test in NightlyDiscoverySchedulerLiveTes to assert CHECK_CASE_ELIGIBILITY task executed * Dedupe cases across the hearing days * removed static reference from the method * updated HearingClientImpl and response dto as per the response - returns list of caseIds * rebase with develop; discovery service now dispatches task CHECK_IDPC_AVAILABILITY_ALL_DEFENDANTS
…e ingestion startIngestionProcess in IngestionProcessorByCaseService returned IngestionProcessPhase.NOT_REQUIRED whenever no newer IDPC document was found, even when no answer had ever been generated for the case (e.g. answer generation failed after a previous ingestion). It now also checks CaseQueryStatusRepository for an ANSWER_AVAILABLE entry: only returns NOT_REQUIRED when an answer already exists, otherwise returns STARTED with a distinct message and dispatches nothing new. Fixes a latent bug surfaced by this change: CaseQueryStatusRepository's findByCaseId/findByCaseIdAndQueryId were broken derived queries that resolved against the entity's plain caseId getter instead of the embedded id, throwing UnknownPathException the first time they were actually invoked. Both now use explicit JPQL through caseQueryStatusId. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
Closing — re-raising against develop instead of main. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
IngestionProcessorByCaseService.startIngestionProcess(POST /ingestions/start-by-case) reportedIngestionProcessPhase.NOT_REQUIREDwhenever no newer IDPC document existed, even if no answer had ever been generated for the case (e.g. answer generation failed after a previous ingestion). It now also checksCaseQueryStatusRepositoryfor anANSWER_AVAILABLEentry:NOT_REQUIREDis only returned when an answer already exists; otherwise it returnsSTARTEDwith a distinct message ("No newer IDPC version is available; previous answers are still in the process of generating.") and dispatches nothing new.CaseQueryStatusRepository.findByCaseId/findByCaseIdAndQueryIdwere broken derived Spring Data queries — they resolved against the entity's plaincaseIdgetter instead of the@EmbeddedId, throwingUnknownPathExceptionthe first time they were actually invoked at runtime. Both now use explicit JPQL throughcaseQueryStatusId.caseId/.queryId.Test plan
IngestionProcessorByCaseServiceTest— added/updated unit tests covering: answer exists →NOT_REQUIRED; no answer yet →STARTED; noCaseQueryStatusrow at all →STARTED.IngestionProcessByCaseHttpLiveTest— split the oldNOT_REQUIREDscenario into two: ingested-but-no-answer-yet (STARTED) and ingested-with-answer (NOT_REQUIRED, now seeding acase_query_status(ANSWER_AVAILABLE)row).gradle test pmdMain pmdTest— green.gradle integration --tests IngestionProcessByCaseHttpLiveTest— all 4 scenarios pass against the real docker-compose stack.🤖 Generated with Claude Code