Skip to content

Defer oversized Java-fix coverage - #9116

Open
kimeta wants to merge 10 commits into
oracle:masterfrom
kimeta:forge/java-fix-coverage-handoff
Open

Defer oversized Java-fix coverage#9116
kimeta wants to merge 10 commits into
oracle:masterfrom
kimeta:forge/java-fix-coverage-handoff

Conversation

@kimeta

@kimeta kimeta commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Closes #9115

What changes after a Java repair

A Java-fix issue should stop being a compatibility issue once its compilation or JVM-runtime failure is repaired. Continuing directly into every uncovered dynamic-access class can turn a narrow repair into a very long coverage run for libraries such as hibernate-core.

This change keeps small reports inline and moves oversized coverage into the normal library-update workflow under §WF-java-fail-fix-workflow.3:

  • Javac and Java-run use the existing FORGE_DYNAMIC_ACCESS_CHUNK_CLASS_THRESHOLD.
  • Reports at or below the threshold continue through composite exploration.
  • Reports above the threshold skip exploration while preserving the successful repair.
  • Publication opens one new library-update-request for the fixed version and reuses that same issue on publication retries.
  • The same behavior applies when a library-update-request routes through Javac or Java-run repair.
  • Javac and Java-run default to their Terra composite strategies when no explicit override is supplied.

Why the threshold check happens after repair

The dynamic-access report cannot be trusted before the primary repair because the target version does not compile or run far enough to produce it. The composite is the first shared point where both Java modes know that repair succeeded, so it checks the normal report and either runs exploration or skips it.

There is no separate coverage-handoff report, metrics field, schema entry, or workflow status. The composite records an oversized skip in the normal continuation marker together with the uncovered-class count and threshold. Publication reads that recorded decision instead of regenerating the report. It creates one fixed-version issue for the repair, records its number before the remaining publication steps, and reuses it on retries. If issue creation succeeds immediately before the marker save is interrupted, publication recovers the exact issue by its repair reference and coordinate. After the repair reaches master, the ordinary library-update-request path regenerates coverage and applies its existing chunk-selection logic.

How the work stays ordered

repair succeeds -> report exceeds threshold -> repair PR
                                      |
                                      +-> new library-update-request (In Progress)
                                                     |
repair PR reaches master ----------------------------+-> Todo -> normal chunking

The repair PR keeps Fixes: #<repair>, adds Refs: #<coverage>, states that exploration was skipped, and carries Forge-Unblocks-Issue: #<coverage> under §GIT-pr-body. Forge parks the new issue in In Progress until the repair merges, then clears its assignees and moves it to Todo. Chunking therefore starts through the existing workflow from the fixed version on the default branch.

Generated output stays focused

Java-fix PRs now show only strategy, agent, model, token, and iteration metrics. When coverage is deferred, that section immediately follows the summary and includes the uncovered-class count, threshold, new issue link, and unblock trailer. Forge revision details and the bounded test-source diff follow it; library-generation statistics and coverage comparisons are omitted. Publication also no longer discards a successful repair when a dynamic-access category loses full coverage; that trade-off remains visible for review.

The new issue keeps the fixed coordinate in its title and has one sentence explaining that Forge opened it while resolving the repair issue because the dynamic-access class count exceeded the threshold. Issue #9119 was updated to this format.

How defaults remain mode-specific

The dispatcher previously materialized a generic work-queue strategy even when no override was requested. It now omits --strategy-name in that case so each selected driver applies its own default, while the existing continuation machinery retains the resolved driver strategy. Explicit overrides and resumed strategy identities still win. This keeps the declarative strategy bundles aligned with §STRAT-forge-predefined-strategy-contract.

@kimeta
kimeta requested review from jormundur00 and vjovanov July 27, 2026 19:35

@jormundur00 jormundur00 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found three issues that need to be addressed before this is safe to merge.

[P1] Make continuation-marker writes atomic

forge/utility_scripts/continuation_marker.py writes the marker directly with open(..., "w") followed by json.dump. If the process is interrupted during that write after the follow-up issue has been created, the existing marker is truncated. The preserved-marker loader then catches the parse error and returns no marker, while the resumable issue is skipped because it no longer has valid continuation state. This can leave the repair unresumable and the coverage issue parked in In Progress.

Please write to a temporary file and atomically replace the marker (with flush/fsync as appropriate), and add a regression test that simulates interruption during persistence.

[P2] Forward strategy overrides through library-update javac/Java-run routes

The library-update routing in forge_metadata.py constructs the javac-fix and Java-run-fix argv without --strategy-name, even though invoke_pipeline records and uses an explicit or resumed strategy override for marker creation and preflight. This allows the marker/preflight to identify one strategy while the driver actually executes its mode default.

Please either forward a compatible override to these routed drivers or consistently derive marker/preflight state from the route-selected strategy that will actually run.

[P2] Preserve deferral state in the documented direct-CLI flow

The docs advertise --dynamic-access-class-threshold N for direct CLI use and state that publication opens the follow-up issue. Without the dispatcher, however, there is normally no continuation marker, so the strategy phase update is a no-op. The javac-fix and Java-run publishers only handle deferral when explicit follow-up issue/count/threshold arguments are supplied, which the documented two-step direct flow does not create or pass. The resulting PR can omit the handoff and present statistics as though exploration ran.

Please either make this option explicitly issue-driven only, or persist the deferral result in metrics/marker state and have publication consume it to create the follow-up.

@kimeta

kimeta commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Re [P1] atomic continuation-marker writes: I don't think this is reachable, so I'm leaving save() as is.

Resume does not read the on-disk marker. resolve_issue_continuation_marker goes through load_continuation_marker_from_branch, which reads the marker out of git on the pushed preserved branch (forge_metadata.py:4136, git show <ref>:forge/<marker>). That is also the loader that catches the parse error and returns None.

A process interrupted during save() never reaches the following commit and push, so the preserved branch still carries the last intact marker. The truncated bytes only ever exist in a worktree that dies with the run. For the described outcome — repair unresumable, coverage issue parked in In Progress — the corrupt file would have to be committed and pushed, which cannot happen on an interruption.

For completeness, the Forge-local loader load_continuation_marker (utility_scripts/continuation_marker.py:43) does not swallow JSONDecodeError either, so a corrupt local marker fails loudly in publication rather than silently degrading to "no marker".

The regression test as requested is also not expressible: a run cannot be killed mid-write and then assert on its own state afterwards.

@kimeta

kimeta commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Both [P2]s: the strategy one is what resolve_workflow_default_strategy_name already does, marker and preflight now take the strategy from the route-selected driver instead of DEFAULT_WORK_QUEUE_STRATEGY_NAME, and forwarding an override isn't implementable since strategy names are mode-scoped, so a dynamic_access_* name would be executed as the javac workflow. The direct-CLI one is unreachable: nothing in CI, scripts, or the E2E fixtures invokes the Java-fix drivers directly (every caller goes through forge_metadata.py, which always creates a marker).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bound Java-fix work after primary repair

2 participants