Skip to content

fix: refresh now surfaces externally-generated files in new pakages#1026

Merged
wenytang-ms merged 5 commits into
mainfrom
fix/914
Jun 5, 2026
Merged

fix: refresh now surfaces externally-generated files in new pakages#1026
wenytang-ms merged 5 commits into
mainfrom
fix/914

Conversation

@wenytang-ms
Copy link
Copy Markdown
Contributor

@wenytang-ms wenytang-ms commented Jun 2, 2026

The source package-root load only did a shallow (DEPTH_ONE) refresh, so files written to disk by external generators into brand-new packages never appeared in the Java Projects view, even after Refresh.

Manual Refresh now deep-refreshes (DEPTH_INFINITE) and closes the package root so the Java Model re-enumerates its package-fragment list. Auto-refresh passes the changed resource URI to the server via PackageParams.syncPaths, which scopes the refresh to the nearest existing ancestor subtree instead of the whole source root, falling back to a full deep refresh when a path cannot be resolved.

  • PackageParams: add syncPaths field + accessors
  • PackageCommand: targeted refresh branch + findNearestExistingResource helper
  • packageRootNode: stash pending sync paths, snapshot/clear in loadData
  • syncHandler: onDidCreate stashes the created URI (cycle-safe NodeKind check)
  • add E2E plans for manual deep refresh and targeted auto-refresh

#914


Verification (local E2E)

Both regression plans pass locally: manual-refresh 21/21, targeted auto-refresh 16/16. CI on this PR is green.

1. Manual Refresh — correctness

An external generator writes a .java file into a brand-new package com.mycompany.app.gen, then the user invokes Refresh.

Before — the new package is absent (tree shows only com.mycompany.app, com.mycompany.app1, module-info.java):

manual-1-before-absent

After Refresh — the new package com.mycompany.app.gen and its Gen914InNewPkg class are now visible:

manual-2-after-present

2. Targeted Auto-refresh — performance optimization (syncPaths)

A file is written into a brand-new package com.mycompany.app.autogen. No manual Refresh is invoked — only the file-watcher + scoped syncPaths refresh.

Before — the new package is absent:

autorefresh-1-before-absent

After (auto-refresh only)com.mycompany.app.autogen and Gen914AutoNewPkg appear without any manual Refresh:

autorefresh-2-after-present

…ages

The source package-root load only did a shallow (DEPTH_ONE) refresh, so files
written to disk by external generators into brand-new packages never appeared in
the Java Projects view, even after Refresh.

Manual Refresh now deep-refreshes (DEPTH_INFINITE) and closes the package root so
the Java Model re-enumerates its package-fragment list. Auto-refresh passes the
changed resource URI to the server via PackageParams.syncPaths, which scopes the
refresh to the nearest existing ancestor subtree instead of the whole source
root, falling back to a full deep refresh when a path cannot be resolved.

- PackageParams: add syncPaths field + accessors
- PackageCommand: targeted refresh branch + findNearestExistingResource helper
- packageRootNode: stash pending sync paths, snapshot/clear in loadData
- syncHandler: onDidCreate stashes the created URI (cycle-safe NodeKind check)
- add E2E plans for manual deep refresh and targeted auto-refresh
wenytang-ms and others added 3 commits June 3, 2026 15:57
CI runs the autotest harness with the LLM screenshot judge enabled, which the
local --no-llm runs skipped. Four state-check steps sat on a wait action while
asserting a state that was already true (the package/class had appeared in an
earlier refresh step). The LLM judge diffed the before/after screenshots, saw no
visual delta, and failed the steps -- even though the deterministic verifyTreeItem
DOM assertion passed. This produced a spurious 18/20 in CI vs 20/20 locally.

Remove the natural-language �erify: field from those pure state-check steps and
rely solely on the deterministic �erifyTreeItem assertion, which is the actual
behaviour under test. Steps where a visible change genuinely occurs within their
own window keep their �erify:.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…914)

The Windows-UI run still failed (19/20): the LLM screenshot judge could not
confirm the insertLineInFile step because the written file is not visible in the
collapsed Explorer (autoReveal off), a flaky judgment that passed on Linux but
failed on Windows. Remove �erify: from all steps with no reliable visual
signal (file writes, manual refresh, view focus); keep it only on expandTreeItem
steps, which produce a clear visual change. Deterministic verifyTreeItem remains
the real assertion. Also trim the verbose header/inline comments.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Each plan only asserted the new package was PRESENT after refresh, never that it
was ABSENT before — the missing "before" half of the before/after comparison.
Add a deterministic verifyTreeItem (visible: false) baseline proving the package
does not exist prior to the file write, so the later appearance is a genuine
refresh-driven transition rather than a pre-existing or leftover node. Uses the
DOM check (waitForTreeItemGone), so no LLM-judge flakiness. Verified locally:
manual 21/21, auto-refresh 16/16.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses #914 by ensuring the Java Projects view reflects externally-generated (out-of-band) files that land in new packages. It does so by (1) deep-refreshing source roots for manual refresh and forcing Java Model re-enumeration, and (2) optimizing auto-refresh by sending “changed URIs” (syncPaths) so the server can deep-refresh only the nearest existing ancestor subtree (falling back to a full deep refresh when needed).

Changes:

  • Add syncPaths plumbing from the VS Code watcher → package root node → JDTLS extension (PackageParams) to enable targeted refresh on auto-refresh.
  • Change server-side source-root refresh behavior to deep-refresh (DEPTH_INFINITE) and close the package fragment root to force package-fragment re-enumeration.
  • Add E2E regression plans covering manual deep refresh and targeted auto-refresh.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/e2e-plans/java-dep-refresh-generated-files.yaml Adds an E2E plan validating manual Refresh surfaces newly generated files in new/existing packages.
test/e2e-plans/java-dep-autorefresh-targeted.yaml Adds an E2E plan validating watcher-driven targeted auto-refresh surfaces a new package without manual Refresh.
src/views/packageRootNode.ts Tracks pending created-URI sync paths per package root and forwards them to the server on next load.
src/syncHandler.ts Stashes newly created URIs onto the nearest package root so auto-refresh can scope server refresh.
jdtls.ext/.../PackageParams.java Adds syncPaths to the request payload for server-side scoped refresh.
jdtls.ext/.../PackageCommand.java Implements deep refresh + root close for source roots and scoped subtree refresh using syncPaths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/e2e-plans/java-dep-refresh-generated-files.yaml Outdated
Comment thread test/e2e-plans/java-dep-autorefresh-targeted.yaml Outdated
Comment thread src/views/packageRootNode.ts
…targets

- packageRootNode: restore the pending syncPaths snapshot if getPackageData rejects so a transient server error does not drop a targeted refresh

- PackageCommand: dedup resolved refresh targets via LinkedHashSet so multiple files in the same new package only refresh the subtree once

- e2e plans: align Maven section id to lowercase 'maven' for consistency

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@wenytang-ms wenytang-ms merged commit 7be414f into main Jun 5, 2026
6 checks passed
@wenytang-ms wenytang-ms deleted the fix/914 branch June 5, 2026 01:56
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.

3 participants