Skip to content

[codex] Refactor 0.5 integration hardening and typed API cleanup#734

Merged
ObservedObserver merged 118 commits into
mainfrom
refactor-0.5
Jun 20, 2026
Merged

[codex] Refactor 0.5 integration hardening and typed API cleanup#734
ObservedObserver merged 118 commits into
mainfrom
refactor-0.5

Conversation

@ObservedObserver

Copy link
Copy Markdown
Member

Summary

This PR is the refactor-0.5 integration hardening branch. It is a large but intentionally scoped cleanup of PygWalker’s public API surface, adapter behavior, frontend/backend communication contracts, and regression coverage.

The main goal is to make the refactor mergeable by preserving the existing user-facing integrations while making the internals easier to reason about and test. During review and strict browser regression testing, we also found and fixed several correctness issues that were either introduced by the refactor or exposed by running real integrations end to end.

At a high level this PR:

  • Adds the public Walker object API and supports reusing Walker objects across notebook, webserver, Streamlit, static HTML, and component-style entrypoints.
  • Moves the large pygwalker.api.pygwalker implementation into smaller services for rendering, props building, spec handling, data bridging, chart export, display, and communication.
  • Makes frontend/backend communication payloads typed and validated instead of relying on loose dictionaries.
  • Introduces the unified computation option and fixes the resolution semantics so None still allows DataBridge to apply its existing automatic size heuristic.
  • Defaults Jupyter rendering to anywidget while preserving legacy transport warnings and compatibility paths.
  • Adds pyarrow Table input support and broadens public tabular-input regression coverage.
  • Hardens CI and package-build checks around pytest, frontend type checking, frontend rebuilds, notebook execution, and repo hygiene.
  • Fixes integration bugs found during real browser regression testing across Jupyter, Streamlit, Gradio, Marimo, webserver, static HTML, Component, and Reflex.

Why this changed

The branch started as a refactor of the PygWalker runtime shape, but the code review made clear that the high-risk areas were not cosmetic structure changes. The real risks were behavioral compatibility across integrations, ambiguous computation-mode resolution, loose communication payloads, and test gaps that allowed silent regressions.

So the final scope was deliberately narrowed around merge-critical correctness:

  • keep public behavior stable where possible;
  • fail clearly where an integration cannot support a mode safely;
  • add focused tests for the behavior that reviewers flagged;
  • validate the integrations by running real applications in Chrome instead of relying only on unit tests or mocked notebook execution.

Some cleanup items were intentionally deferred. For example, deriving every reject-default baseline from constructor signatures is valuable follow-up work, but the reviewer confirmed the current duplicated defaults are not a live bug. The duplicate frontend model cleanup and compression helper cleanup are also lower-risk than the integration correctness work in this branch.

Major backend/API changes

Public Walker object support

This PR adds a first-class Walker object API and routes existing entrypoints through shared validation/reuse helpers.

Notable behavior:

  • pyg.walk(...) can create and render a PygWalker/Walker-backed object.
  • Existing Walker objects can be reused in supported environments.
  • Static HTML and component rendering now reject live kernel/cloud computation where no live backend exists, instead of silently generating broken output.
  • Reflex now explicitly rejects kernel/cloud computation and directs users to browser computation, because the live backend path cannot safely resolve the required gid in that integration.

Refactored rendering and service boundaries

The large API implementation was split into smaller modules:

  • render orchestration
  • props construction
  • spec management
  • data bridge logic
  • chart export
  • Jupyter display handling
  • communication handlers
  • desktop/cloud/data/spec/upload communication services

This reduces the blast radius of changes and makes the integration-specific behavior easier to test directly.

Unified computation option

The branch adds and hardens the shared computation-mode resolver.

Important final semantics:

  • connector-backed inputs resolve to kernel computation;
  • explicit computation options resolve to the requested boolean mode;
  • unspecified/no-signal computation resolves to None, not False, so DataBridge can still run its existing automatic size heuristic;
  • JupyterConvert explicitly overrides to False, which is the intended static/export behavior.

This was one of the most important review fixes because the regression was silent: a wrong False value can disable automatic kernel mode without throwing an error.

DataBridge and tabular input hardening

This PR adds broader handling and tests for empty and public tabular inputs, including pandas-like, modin, polars, spark, database, cloud dataset, and pyarrow parser paths.

Fixes include:

  • empty tabular input handling;
  • modin index handling regression coverage;
  • pyarrow Table support;
  • public dataset docs and tests aligned with actual accepted inputs;
  • clearer unsupported dataset errors.

Spec handling hardening

Spec parsing and spec IO paths were tightened. One important correctness fix changes empty-string handling to short-circuit only when spec is a real string:

  • isinstance(spec, str) and spec == ""

This avoids ambiguous equality checks for array-like values.

Major frontend/communication changes

Typed communication protocol

Frontend communication envelopes and backend communication responses are now typed and validated. Unknown actions and malformed payloads are rejected more deliberately.

This includes typed payloads for:

  • cloud upload communication;
  • chart communication;
  • save communication;
  • data query communication;
  • empty requests/responses;
  • transport envelopes.

Frontend telemetry and privacy behavior

Telemetry is now lazier and more explicit:

  • frontend telemetry is loaded only after opt-in;
  • telemetry client imports are lazy-loaded on the Python side;
  • privacy defaults were adjusted to update-only behavior;
  • telemetry exit hangs were fixed.

Frontend build/tooling hardening

The frontend build moved to Vite 6 and CI now enforces frontend type checking. Package builds also rebuild frontend assets, which prevents stale dist artifacts from hiding source/build mismatch.

Integration fixes found by strict regression testing

After the code review fixes, we ran real integration apps and used Chrome/Playwright to interact with the actual UI. This exposed several issues that unit tests alone did not catch.

Streamlit browser computation data source fix

Problem found:

  • Streamlit browser computation could render the PygWalker frame but the Data tab/dragged chart had no usable local data.

Fix:

  • pygwalker/api/streamlit.py now sends serialized origin_data_source into component props for browser computation.
  • Kernel computation sends an empty data source because it should fetch through the live backend path.
  • Serialization uses DataFrameEncoder, including timestamp-safe handling.

Streamlit pure chart local rendering fix

Problem found:

  • StreamlitRenderer.chart() in browser computation rendered a blank chart with a frontend console error similar to TypeError: A is not a function.

Root cause:

  • The frontend pure renderer was treating local/browser chart rendering like remote/kernel rendering.

Fix:

  • app/src/index.tsx now branches in PureRednererApp:
    • kernel uses remote computation;
    • browser/local uses rawData={props.dataSource}.
  • tests/test_frontend_contracts.py now locks this local-data contract.

Component static live-computation rejection

Problem found:

  • Component static HTML paths could accept kernel/cloud computation even though the static output has no live backend.

Fix:

  • Component.explorer().to_html() and Component.profiling().to_html() now reject kernel/cloud computation with a clear ValueError.
  • Pure chart static output remains supported where it can be rendered with local data.

Reflex live-computation rejection

Problem found:

  • Reflex kernel/cloud paths could fail at runtime because live computation requires a gid/backend path that the Reflex integration does not safely provide.

Fix:

  • Reflex get_component now rejects kernel/cloud computation clearly and supports browser computation.

Code review fixes addressed

The second-pass reviewer confirmed the main fixes as correct and complete. The key reviewed items addressed in this branch include:

  • computation resolver preserves None for unspecified mode so DataBridge auto-detection still runs;
  • every entrypoint passes the resolved computation value directly into PygWalker(kernel_computation=...) without branching on it as a bool first;
  • JupyterConvert correctly overrides computation to False;
  • Streamlit and other adapters pass data_source correctly in both reuse and main branches;
  • spec empty-string detection avoids ambiguous array-like comparisons;
  • modin empty/index regression is covered;
  • computation None regression is covered.

Deferred reviewer items are deliberate:

  • deriving reject-default baselines from constructor signatures should be prioritized in a follow-up cleanup PR, but the current baselines were checked and are not a live bug;
  • pyarrow nullable integer inference is a behavior to document/test later rather than fix in this PR, because changing to pandas Arrow dtypes would ripple through type inference branches;
  • a composed >16MB pyg.walk(...) kernel-mode integration test would be a valuable follow-up guard, but the current PR already covers the resolver and DataBridge heuristic independently plus real browser integrations.

Strict integration regression validation

The final regression pass used real applications and real browser interaction. The purpose was to verify that integrations start, render PygWalker, and support actual chart creation/interaction rather than just importing or serving HTML.

Browser validation was performed with Google Chrome launched through Playwright because the Chrome MCP plugin was unavailable in this environment with a sandbox metadata error. This still used a real Chrome browser, real local apps, and real drag/drop UI interaction.

Jupyter / anywidget

Validated:

  • real JupyterLab app;
  • real notebook UI execution via Run All Cells;
  • browser-computation anywidget rendering;
  • kernel-computation anywidget rendering;
  • drag/drop chart creation;
  • local source file and built ESM bundle provenance.

Evidence captured:

  • STRICT_PYGWALKER_FILE pointed to this checkout's pygwalker/__init__.py;
  • STRICT_BUNDLE_PATH pointed to this checkout's pygwalker/templates/dist/pygwalker-app.es.js;
  • widget and bundle SHA256 values matched;
  • no relevant console errors during the validated run.

Streamlit

Validated:

  • real Streamlit app;
  • browser explorer rendering and drag/drop chart creation;
  • kernel explorer rendering and drag/drop chart creation;
  • public explorer rendering;
  • table rendering with 72 rows;
  • viewer rendering from spec;
  • StreamlitRenderer.chart() after the pure-renderer fix.

The Streamlit chart bug was found during this pass, fixed, rebuilt, and revalidated successfully.

Gradio

Validated:

  • real Gradio app;
  • browser computation rendering and drag/drop chart creation;
  • kernel computation rendering and drag/drop chart creation;
  • no relevant console errors.

Marimo

Validated:

  • real Marimo app;
  • browser computation widget rendering and drag/drop chart creation;
  • kernel computation widget rendering and drag/drop chart creation;
  • public Walker widget rendering and drag/drop chart creation.

A Marimo websocket keepalive assertion appeared after closing the browser session. It was treated as shutdown cleanup noise, not an app-runtime failure, because the clean session rendered and interacted correctly.

Webserver

Validated:

  • real webserver app;
  • browser computation rendering and drag/drop chart creation;
  • kernel computation rendering and drag/drop chart creation;
  • public mode rendering and drag/drop chart creation;
  • no relevant console errors.

Static HTML / Component

Validated regenerated static exports for:

  • to_html explorer output;
  • to_table_html table output;
  • to_render_html renderer output;
  • component().bar().to_html();
  • component().explorer().to_html();
  • component().profiling().to_html().

The only observed static-server noise was a /favicon.ico 404; the generated HTML artifacts themselves loaded and rendered.

Reflex

Validated:

  • real Reflex app;
  • browser computation rendering and drag/drop chart creation;
  • explicit kernel/cloud rejection message;
  • no relevant console errors during the validated browser path.

Automated validation

Final local gates run after the last integration fix:

venv/bin/python -m pytest tests -q
yarn typecheck
yarn build:app
git status --short

Results:

  • 293 passed
  • frontend typecheck passed;
  • frontend app build passed;
  • working tree clean.

Review focus

The highest-value review areas are:

  • computation-mode resolution and adapter behavior around None, False, browser, kernel, and cloud modes;
  • integration-specific rejection behavior for static Component and Reflex live computation;
  • Streamlit local/browser data source handling;
  • frontend pure renderer local-data branch;
  • typed communication payload compatibility;
  • public Walker reuse behavior across adapters;
  • CI/build workflow changes that now enforce frontend and notebook regressions.

Follow-up work intentionally not included

Recommended follow-ups after this PR:

  • derive adapter reject-default baselines from constructor signatures to avoid future defaults drift;
  • add an explicit pyarrow nullable integer inference test documenting current behavior;
  • add an end-to-end large-data pyg.walk(...) integration assertion that confirms DataBridge chooses kernel mode through the composed public API;
  • optionally clean up duplicate frontend model definitions and compression helpers if still useful after this branch lands.

@ObservedObserver
ObservedObserver marked this pull request as ready for review June 20, 2026 15:34

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 55bb6d0d06

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread pygwalker/communications/protocol.py Outdated
action: str
data: Dict[str, Any] = Field(default_factory=dict)
rid: Optional[str] = None
gid: Optional[str] = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Allow integer gids in comm envelopes

The public APIs still accept gid: Union[int, str], and PropsBuilder sends that value back to the frontend as id. When a user passes an integer gid under pydantic v2, the frontend posts an envelope with a numeric gid; this new validation rejects it before dispatching to the registered handler, so save/data/export requests fail even though integer gids are documented as supported. Make the envelope accept int | str or coerce before validation.

Useful? React with 👍 / 👎.

Comment thread app/src/index.tsx Outdated
Comment on lines +627 to +631
const propsRef = React.useRef<IAppProps | null>(null);
if (!propsRef.current) {
propsRef.current = streamlitProps.args as IAppProps;
propsRef.current.visSpec = FormatSpec(propsRef.current.visSpec, propsRef.current.rawFields);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Refresh Streamlit props on reruns

When Streamlit reruns a script with the same component key, withStreamlitConnection re-renders this component with new streamlitProps.args; keeping only the first args in this ref means later changes to visSpec, dataSource, rawFields, or defaultTab are ignored and the app keeps showing the previous dataset/spec. This regresses normal Streamlit reruns where users update inputs without changing gid/key.

Useful? React with 👍 / 👎.

@ObservedObserver
ObservedObserver merged commit 2b2c520 into main Jun 20, 2026
14 checks passed
@ObservedObserver
ObservedObserver deleted the refactor-0.5 branch June 20, 2026 16:33
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.

1 participant