Skip to content

Cache package metadata across sessions and fix Polars member completion - #825

Merged
renkun-ken merged 12 commits into
masterfrom
codex/fix-polars-dataframe-completion
Oct 30, 2026
Merged

renkun-ken merged 12 commits into
masterfrom
codex/fix-polars-dataframe-completion

Conversation

@renkun-ken

@renkun-ken renkun-ken commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Polars conversions and chained results could lose their inferred member surface, leaving completion, signature help and hover unavailable. Long reassignment chains also repeated analysis after edits, and slow or contended hosts could exhaust the initial time allowance. Package metadata preparation repeated substantial work across saves and server sessions.

Prepare S3 method return summaries and preserve namespace ownership through constructors, member calls, assignments and aliases. For unresolved inputs, providers offer common known members without asserting a concrete receiver type. Signature help and hover show an uncertainty note; conflicting implementations and local overrides suppress those details. Registered methods are compared with the owning namespace's actual closures, including their environments. Foreign closure environments do not establish namespace-owned generic behavior.

Cold member analysis starts at 250 ms and extends the same traversal as needed, up to two seconds of CPU time and five seconds elapsed. The existing 20,000-node allowance remains shared, with a 128-level request depth bound; explicit caller deadlines remain strict. Completed receivers and inert binding summaries are reused across providers, edits and cursor positions. Native class projections validate the receiver's owning namespace even when alias recovery starts in a generic document context. Only the verified declared surface survives partial argument analysis.

A shared analysis-cache facility provides bounded domains, explicit semantic contexts and independent serialized results. Replacing a context, capacity, namespace or schema allocates an isolated store, so stale handles cannot write into the current store. Corrupt or incompatible entries become misses. Member/S3 binding summaries validate metadata identity and revision, attachments, registrations and exact source dependencies. Local signatures and documentation reuse exact input text, and formatting changes invalidate cached signatures. Parse workers reuse unchanged comment blocks; NAMESPACE declarations validate current contents before reusing parsed facts.

A general persistent cache serves member models, startup datasets, S3 catalogs, documentation summaries and rendered help. Keys cover package/dependency content, loaded namespace and registry state, R version, cache schema and the loaded server implementation, including nested inert constants and R6 methods. R/native checksums are captured at server load rather than inferred from files replaced later. Package file checksums are reused while file identity, size, modification time and change time remain unchanged. Disabled or uncacheable contexts reject reads and writes even with previously computed paths. Payloads are checksummed, writes are atomic, and storage is bounded at 512 entries/256 MiB.

Help caches use the database actually resolved, including its alias indexes, before returning memory or disk results. Missing topics are cached under that identity too. Content-addressed session copies prevent R's internal lazy-load cache from reusing replaced help databases; no internal R flush API is used. Documentation and rendered-help memory caches, fingerprint memos and temporary database copies are bounded. Rendered help tracks locale and renderer options; namespace signatures and diagnostic stubs refresh after function replacement. The README documents cache configuration and compatibility.

Validation:

  • The broader member, completion, signature, hover, S3, inlay, hierarchy and parser suite passed. Relevant cache, native projection, member provider, S4/R6 and Polars groups were rerun after the final fixes. Optional skips cover unavailable S7 capabilities and datawizard fixtures.
  • Regressions cover stale shared handles, changed capacities, corrupt payloads, disabled caching, incompatible schema/server identities, live server constants, loaded native identity, same-version package replacement preserving size/mtime, two-library help resolution, database and alias replacement, missing topics, formatting changes and function replacement. Neutral fixtures are used; the reported financial/SQL demo is not a test case.
  • Local Polars measurements put uncached preparation around 7.5–8.2 seconds, cache creation around 8.8 seconds, warm disk hits around 400 ms, and loaded-namespace validation around 350–370 ms. That validation runs in metadata workers. Warm shared-cache reads were about 15 microseconds; documentation identity checks add roughly 1 ms to warm help lookups.
  • A compiled neutral chain took about 0.96 seconds / 8,593 inference nodes initially, then about 1 ms / two nodes after argument edits. A built-server regression exposed repeated roughly 0.8-second inference when the outer context did not select the receiver namespace; the ownership fix restored reuse. After loading, repeated edits returned 89 completion members in 109–245 ms including parsing/client communication, signature help in 5–7 ms, and valid hover output.
  • R CMD check --no-tests --no-manual --no-build-vignettes reported no errors, warnings or notes; tests ran separately. Changed files add no lint findings compared with the preceding PR head, and git diff --check passed.

The remote SSH host itself was not accessed. Contention and long-chain behavior were exercised locally with synthetic fixtures.

Review points 1–4 completed in 17d2fea:

  • Added bounded worker-local function-syntax and binding-fact memos. Mutable namespace/captured values are still inspected; body/formals and namespace/producer identity validate reuse. Serial Polars 2.0.0 comparisons reduced warm disk hits from 373 to 211 ms and namespace validation from 336 to 179 ms.
  • Retained and revalidated the closure-environment ownership and actual-help-library/database/alias invalidation fixes. Corrected portable text-help assertions responsible for the prior Windows check failure.
  • Added reproducible inst/benchmarks/package-cache.R and inst/benchmarks/member-budget.R, with methods and measured results in the benchmark README. Budget policies use identical metadata, parsed input and fresh caches; cold, retry and completed warm requests are distinguished.
  • Enforced CPU/wall caps before extended scheduling deadlines and stopped exhausted argument/statement work. Dots expansion avoids repeated list copying. The 25,000-argument case now stops at 20,001 visits in about 213 ms, compared with about 1,513 ms previously.
  • Normal chains measured 685/694 ms with fixed/adaptive policies. Injected 25 ms stalls caused fixed-deadline timeouts around 1,016 ms; adaptive requests completed around 2,363 ms and later cache hits were around 1 ms. Extreme 150 ms stalls reached the five-second wall limit; the retry completed from retained prefixes, followed by cache hits below 1 ms. The deliberate cold-path wait and checkpoint overshoot are documented.
  • Broad provider and focused final regressions passed; the compiled server returned all 89 completion members, signature help and hover after edits. R CMD check had no errors, warnings or notes. Fresh CI is tracked for this revision.

@renkun-ken renkun-ken changed the title Fix Polars DataFrame inference and later member completion Cache package metadata across sessions and fix Polars member completion Oct 29, 2026
@renkun-ken
renkun-ken requested a review from eitsupi October 29, 2026 16:20

@eitsupi eitsupi 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.

Reviewed by ChatGPT. I found a few concerns worth addressing before merging:

  1. Persistent cache validation overhead ("R/package-cache.R")
    "package_metadata_namespace_state()" traverses and snapshots namespace bindings even on cache hits. This could make cache validation itself expensive. Could you provide benchmarks comparing uncached preparation, cold cache creation, and warm cache hits, particularly for large packages like Polars?

  2. S3 method identity ("R/member-metadata.R")
    External method detection compares function syntax, but identical function bodies can behave differently in different closure environments. Could this incorrectly reuse return summaries when an external registration replaces a method with an implementation having the same syntax but a different environment? Please consider checking function ownership/identity and adding a regression test.

  3. Help cache invalidation ("R/package-cache.R", "R/utils.R")
    File identity relies on size and mtime, and "get_help()" derives package identity via "find.package()" rather than the actual help-file location. This may allow stale results when multiple library paths contain the same package or when file contents change without metadata changes.

  4. Inference latency ("R/member-completion.R")
    The adaptive budget now permits up to five seconds of elapsed time. Since member inference runs during interactive requests, could we verify the impact on responsiveness under cold-cache or pathological cases? It would also be worth evaluating this change independently from the caching improvements.

More generally, this PR combines S3 inference improvements, document-level reuse, persistent package caching, and budget changes. These seem independently reviewable, and splitting them would make performance regressions and future maintenance easier to manage.

@renkun-ken renkun-ken left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Cache review fixes are committed in da70d5f.

The review found stale shared-store handles, ignored capacity changes, corrupt-entry exceptions, incomplete server fingerprints, stale help/library identities, unbounded help memory, and repeated native receiver analysis when alias recovery did not select the owning namespace. These are now covered by focused regressions. Same-syntax method replacements with different closure environments also invalidate ownership assumptions.

Polars warm disk hits remain around 400 ms in metadata workers versus several seconds to rebuild. Warm requests consume prepared snapshots. The built-server alias case now returns signature help in 5–7 ms after edits, down from roughly 0.8 seconds; completion and hover remain available. The initial loading/analysis cost remains a cold cost.

Local cache/member/provider/S4/R6/Polars tests pass. The broader provider suite also passed, with optional dependency/capability skips. R CMD check reported no errors, warnings or notes; changed files add no lint findings. Integration with current master is clean. Fresh CI for this commit is running; the prior green checks do not validate the new commit.

@renkun-ken renkun-ken left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review points 1–4 from #825 (review) are addressed in 17d2fea (including the prior fixes in da70d5f).

  1. Package-cache validation: reuse canonical function syntax and unchanged binding facts in bounded worker-local memos. Mutable bindings, captured values and namespace/producer identity are still inspected, so this does not assume that an unchanged package version implies unchanged runtime metadata. Serial Polars 2.0.0 measurements on R 4.6.1/macOS arm64 reduced warm disk hits from 373 to 211 ms and loaded-namespace validation from 336 to 179 ms. Uncached preparation was 6,830/6,744 ms and cache creation 7,224/6,778 ms before/after. The reproducible script is inst/benchmarks/package-cache.R; the scope and raw CPU/elapsed measurements are documented in inst/benchmarks/README.md.

  2. S3 ownership: actual registered closures are compared with their owning namespace bindings, including their environments. The existing same-syntax/different-environment regression passes with the new memos, together with namespace replacement, captured-value and mutable-registry invalidation checks. Getter and deferred-binding fixtures remain unevaluated.

  3. Help invalidation: the actual help-library/database and alias indexes determine identities. Existing two-library and content-replacement regressions pass. The new Windows failure was an assertion against a backspace-formatted text title; assertions now inspect the unformatted description, validating the same refresh behavior on both text and rich-help paths.

  4. Adaptive-budget responsiveness: inst/benchmarks/member-budget.R compares a fixed one-second elapsed deadline with the adaptive policy using identical prepared metadata/parsed input and fresh caches. Normal chains took 685/694 ms before/after the policy change. With injected 25 ms stalls, fixed cold requests timed out around 1,016 ms while adaptive requests completed around 2,363 ms and later requests used cached results in about 1 ms. With extreme 150 ms stalls, the first adaptive request hit the five-second wall limit, a retry reused completed prefixes and finished, and the third request was below 1 ms. The deliberate cold-path tradeoff and checkpoint overshoot are documented. CPU/wall caps now apply before an extended scheduling deadline; explicit caller deadlines remain strict. Exhausted wide calls stop visiting arguments, and dots expansion avoids repeated list copying. The 25,000-argument case now stops at 20,001 visits in about 213 ms, versus about 1,513 ms previously. Node/depth-bound and recursive cases remain conservative and do not cache incomplete receivers.

Validation: the broad member/completion/signature/hover/S3/inlay/hierarchy/parser suite passed with five optional capability/dependency skips. Focused final regressions and built-server completion/signature/hover checks passed. R CMD check reported no errors, warnings or notes; no new lint findings or whitespace errors. Integration with current master is clean. Fresh CI is required for this new SHA and is being checked separately from prior runs.

@renkun-ken
renkun-ken requested a review from eitsupi October 30, 2026 09:50
@renkun-ken
renkun-ken merged commit cf116dd into master Oct 30, 2026
18 checks passed

@renkun-ken renkun-ken left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed the failing macOS check in 1e39745. All 18 hosted checks now pass at this exact SHA, including all three R CMD check platforms and all eight coverage shards.

The server log showed S7 member metadata ready after 8.36 seconds, while the constructor signature request remained blocked until S3 catalogs arrived at 11.11 seconds, beyond the fixture's ten-second readiness timeout. Verified constructors now use member readiness independently of S3 catalogs; missing member metadata and possible namespace masking still preserve the wait. The S7 fixture allows 30 seconds for cold setup, while after-edit requests continue to check their first response without retries. Timed-out test requests remove their callbacks so late cancellation replies cannot fail a subsequent request.

Regression coverage verifies draining a queued constructor request with S3 catalogs still absent, preserving waits for missing/masking metadata, and ignoring late replies after test-request timeouts. Local provider/S7/client/S3 tests and the package check pass; current-master integration is clean and the checkout is clean.

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.

2 participants