Skip to content

[pull] master from graphistry:master#122

Merged
pull[bot] merged 1 commit into
admariner:masterfrom
graphistry:master
Oct 17, 2025
Merged

[pull] master from graphistry:master#122
pull[bot] merged 1 commit into
admariner:masterfrom
graphistry:master

Conversation

@pull
Copy link
Copy Markdown

@pull pull Bot commented Oct 17, 2025

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

…ds (#797)

* fix(plot): Fix dataset_id invalidation in encoding and metadata methods

Dataset IDs were not being invalidated when encoding or metadata attributes
changed, causing stale ID reuse and incorrect server-side uploads.

**Problem**: After upload, modifying encodings (encode_point_color, bind with
point_*/edge_* params) or metadata (name, description) would reuse the old
dataset_id, sending outdated visualization configurations to the server.

**Root cause**: Invalidation was only implemented for DataFrame-changing
methods (.nodes(), .edges(), .graph()) but not for encoding/metadata methods.

**Fixed methods** (5 total, ~10 LOC):
- __encode() helper: Invalidates dataset_id after modifying _complex_encodings
  → Fixes all 7+ .encode_*() methods (encode_point_color, encode_edge_color,
    encode_point_size, encode_point_icon, encode_edge_icon, encode_point_badge,
    encode_edge_badge) with single choke-point fix
- encode_axis(): Invalidates dataset_id after modifying _complex_encodings
- name(): Invalidates dataset_id after modifying _name
- description(): Invalidates dataset_id after modifying _description
- bind(): Conditionally invalidates dataset_id when changing encodings
  → Preserves IDs when explicitly setting them via bind(dataset_id='...')
  → Invalidates when changing encodings via bind(point_color='...', etc.)

**Analysis approach**: Used Python AST analysis to identify all methods
modifying dataset-relevant attributes, confirming manual audit findings.

**Testing**: Added comprehensive test suite with 11 tests covering:
- 5 bug reproduction tests (now passing)
- 2 correct behavior tests (settings, bind with IDs)
- 2 file ID invalidation tests (already working)
- 2 real-world mixed scenario tests

**Impact**: Server-side uploads now correctly detect dataset changes,
preventing visualization artifacts from stale encodings and metadata.

**Documentation**: Added pyre-check setup guide (ai/prompts/PYRE_ANALYSIS.md)
and tool selection guidance (ai/README.md) demonstrating AST-based analysis
as practical alternative for custom code patterns.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs(pyre): Enable pyre type checking by excluding hop.py

Pyre-check hangs when analyzing graphistry/compute/hop.py due to the
hop() function's complexity (384 lines, nested loops, runtime TypeVar
resolution). Excluding this single file allows pyre to successfully
analyze the entire codebase.

Changes:
- Exclude compute/hop.py from pyre analysis in .pyre_configuration
- Include tests/ in pyre analysis (previously excluded)
- Add .pyre/ cache directory to .gitignore
- Document exclusion rationale in hop.py docstring
- Update PYRE_ANALYSIS.md with working configuration and quick start
- Add pyre investigation results to PLAN.md template

Results:
- Pyre now analyzes 3711 functions (including tests) in ~2.5 seconds
- Previously hung at 1631/1709 functions indefinitely
- No other files require exclusion

The hop() function exceeds pyre's hard-coded complexity limit for deep
analysis. Mypy handles this file without issues.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(plot): Fix dataset_id invalidation in style() and addStyle()

Both style() and addStyle() modify the _style attribute which affects
visualization rendering. These methods should invalidate dataset_id to
ensure proper server-side upload behavior.

Found by Pysa call graph analysis - these methods use indirect
modification via bind() that AST pattern matching missed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* test: Add comprehensive dataset_id invalidation test suite

Add 25 tests covering dataset_id and file_id invalidation logic:
- 11 core tests for basic invalidation scenarios
- 3 Pysa-discovered bug tests (style, addStyle, infer_labels)
- 8 parametric tests covering ALL encoding methods
- 3 bind() edge case tests (conditional invalidation)

Uses parametric testing (@pytest.mark.parametrize) to ensure all
encoding methods are tested and future methods are easily added.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: Streamline PYRE_ANALYSIS.md (82% reduction)

Reduce from 373 to 65 lines by:
- Removing generic pyre documentation
- Focusing on PyGraphistry-specific commands
- Adding timeout debugging section
- Using $(pwd) instead of absolute paths
- Removing future work section

Addresses PR feedback for more concise, actionable documentation.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* chore: Add Pysa call graph extraction tooling

Add ai/assets/pysa_extract_callers.py for extracting caller
information from Pysa's call-graph.json output. Moved from gitignored
plans/ to committed ai/assets/ for team reuse.

Update .pyre_configuration with taint_models_path for Pysa analysis.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: Remove project-specific content from PYRE_ANALYSIS.md

Remove Status line (project-specific findings) and redundant Pysa
Workflow section. Keep as reusable workflow guide focused on commands
and gotchas.

Addresses PR feedback on lines 3 and 25.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: Shorten pyre exclusion note in hop.py

Reduce verbose 5-line explanation to concise 1-line note.
Still conveys essential information about pyre exclusion.

Addresses PR feedback on line 9.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: Remove gitignored path from .pyre_configuration

Remove taint_models_path referencing plans/ (gitignored directory).
This path would break pyre for other developers.

Taint models are not needed for call graph analysis (documented
workflow in PYRE_ANALYSIS.md). Configuration now works on all systems.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: Document optional taint models usage

Keep .pyre_configuration clean (no gitignored paths).
Document how users can optionally add taint models locally.

Taint models are experimental and user-specific, not committed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: Improve Quick Commands section in PYRE_ANALYSIS.md

Add numbered workflow and missing commands:
1. Generate call graph (existing)
2. Verify call graph generated (new)
3. Extract callers with single/multiple method examples (enhanced)

Makes the workflow clearer and more actionable.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
@pull pull Bot locked and limited conversation to collaborators Oct 17, 2025
@pull pull Bot added the ⤵️ pull label Oct 17, 2025
@pull pull Bot merged commit 56ac00e into admariner:master Oct 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant