Skip to content

Commit 429c3d8

Browse files
fix(notebook,pipeline): B1/B2 wrapper ordering + safe list-column parser
Reorder all four teaching-notebook wrappers (run_heatmap_pipeline, run_tsne_pipeline, run_heatmap_network_plain_pipeline, run_heatmap_network_pipeline) to match the core pipeline contract: exclude codes before sentence extraction (B1), and rebuild filtered_sentences after group-label replacement so the co-occurrence / PMI / TF-IDF methods embed the replaced text (B2). Replace eval() on stringified codes/data_group cells with a bounded, list[str]-only parse_list_column_cell in vis_tool_core.py and all four notebook wrappers; it fails loud on tuples, malformed literals, non-string items, and oversized cells instead of executing arbitrary code. Add discriminating wrapper-ordering tests and list-column parser tests. Document the supported clustering method/metric pairs (PMI = cosine only) and the Ward-linkage clustered-heatmap design note in the README.
1 parent b4c2158 commit 429c3d8

5 files changed

Lines changed: 544 additions & 77 deletions

File tree

README.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,23 @@ This section also includes validation checks to:
334334

335335
These definitions are critical for interpretability in downstream visualization and clustering.
336336

337+
### Clustering Methods and Supported Distance Metrics
338+
339+
Word-to-word similarity can be computed with one of four methods, chosen with `clustering_method` and paired with a `distance_metric`:
340+
341+
- **1 — RoBERTa embeddings:** contextual embeddings compared with cosine similarity.
342+
- **2 — Co-occurrence:** `cosine` uses frequency-sensitive context vectors; the default uses the Jaccard index (set-based, binary overlap).
343+
- **3 — PMI (Positive Pointwise Mutual Information):** supported with `distance_metric = "cosine"` **only**. PMI builds context vectors whose similarity is meaningful under cosine; other metrics are not defined for this method, so pairing PMI with a non-cosine metric is an unsupported configuration rather than a defect. Use `"cosine"` with method 3.
344+
- **4 — TF-IDF weighted co-occurrence:** `cosine` (or `default`) uses cosine similarity; other values select an experimental unnormalized-sum variant.
345+
346+
Combining a method and metric outside these supported pairings — most notably PMI with a non-cosine metric — will not produce a usable similarity matrix. This is expected behavior: select a supported method/metric pair.
347+
348+
### Hierarchical Clustering (Ward Linkage) in the Clustered Heatmap
349+
350+
When `clustered = True`, the heatmap also shows a dendrogram-ordered version that groups words using hierarchical clustering (Ward linkage) applied to the rows and columns of the word-by-word similarity matrix. This places similar words next to one another and reveals nested groupings.
351+
352+
Ward linkage here operates on each word's full similarity *profile* (its row/column in the matrix). It is a deliberate choice for producing interpretable dendrograms and word groupings, not a transformation of the individual similarity values. The values shown in the standard (unclustered) heatmap are unchanged; the clustered view only reorders and annotates them.
353+
337354
## Troubleshooting
338355

339356
Here are solutions to common issues you might encounter:

function/vis_tool_core.py

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1732,6 +1732,69 @@ def repel_nodes(p,iterations=150,k=0.2):
17321732
bbox_to_anchor=(1.0,0.0),frameon=False, fontsize=16)
17331733
plt.axis('off'); plt.tight_layout(); return fig
17341734

1735+
#---------------------------------------------------------------------------------
1736+
# LIST-COLUMN PARSING (safe replacement for eval())
1737+
#---------------------------------------------------------------------------------
1738+
1739+
def _validate_str_items(items, max_items):
1740+
"""Validate a parsed sequence is a bounded list of strings; raise otherwise."""
1741+
if len(items) > max_items:
1742+
raise ValueError(f"List-column cell has {len(items)} items (max {max_items})")
1743+
for item in items:
1744+
if not isinstance(item, str):
1745+
raise ValueError(
1746+
f"List-column items must be strings, got {type(item).__name__}: {item!r}"
1747+
)
1748+
return list(items)
1749+
1750+
1751+
def parse_list_column_cell(value, max_items=1000, max_chars=100000):
1752+
"""Parse a stringified list-column cell (``codes`` / ``data_group``) into list[str].
1753+
1754+
Safe, bounded replacement for the previous ``eval()`` on list-column cells:
1755+
- missing / empty values become ``[]``;
1756+
- a bracketed list literal is parsed with ``ast.literal_eval`` and must be a
1757+
flat list of strings within the size bounds;
1758+
- a bare scalar string becomes a single-item list;
1759+
- a tuple / dict / set literal, a malformed literal, non-string items, or a
1760+
cell exceeding the size bounds raise ``ValueError`` (fail loud) rather than
1761+
silently mis-parsing or executing arbitrary code.
1762+
"""
1763+
if value is None:
1764+
return []
1765+
if isinstance(value, list):
1766+
return _validate_str_items(value, max_items)
1767+
if isinstance(value, str):
1768+
text = value.strip()
1769+
if not text:
1770+
return []
1771+
if text[0] == '[':
1772+
if len(text) > max_chars:
1773+
raise ValueError(
1774+
f"List-column cell exceeds {max_chars} chars ({len(text)})"
1775+
)
1776+
try:
1777+
parsed = ast.literal_eval(text)
1778+
except (ValueError, SyntaxError) as exc:
1779+
raise ValueError(f"Malformed list-column cell: {text[:80]!r}") from exc
1780+
if not isinstance(parsed, list):
1781+
raise ValueError(
1782+
f"List-column cell must be a list, got {type(parsed).__name__}: {text[:80]!r}"
1783+
)
1784+
return _validate_str_items(parsed, max_items)
1785+
if text[0] in '({':
1786+
raise ValueError(
1787+
f"List-column cell must be a list, not a {text[0]!r}-literal: {text[:80]!r}"
1788+
)
1789+
return [text]
1790+
try:
1791+
if pd.isna(value):
1792+
return []
1793+
except (TypeError, ValueError):
1794+
pass
1795+
return [str(value)]
1796+
1797+
17351798
#---------------------------------------------------------------------------------
17361799
# MAIN PIPELINE FUNCTION
17371800
#---------------------------------------------------------------------------------
@@ -1786,8 +1849,7 @@ def run_visuals_pipeline(input_data):
17861849
# Check if first non-null value is a string that looks like a list
17871850
sample = df[col].dropna().iloc[0] if not df[col].dropna().empty else None
17881851
if isinstance(sample, str) and (sample.startswith('[') or ',' in sample):
1789-
df[col] = df[col].apply(lambda x: eval(x) if isinstance(x, str) and x.strip() else
1790-
([] if pd.isna(x) else [x]))
1852+
df[col] = df[col].apply(parse_list_column_cell)
17911853

17921854
# Apply Metadata Filters
17931855
if input_data.projects and 'project' in df.columns:

tests/test_list_column_parser.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
"""Bounded list-column parser (GR-EVAL): safe replacement for eval() on codes/data_group.
2+
3+
``parse_list_column_cell`` replaces an ``eval()`` call that previously ran on every
4+
stringified ``codes`` / ``data_group`` cell. These tests pin the contract: it parses a
5+
bracketed list of strings, treats missing/scalar values sensibly, and fails loud on
6+
tuples, malformed literals, non-string items, and oversized cells.
7+
"""
8+
9+
from __future__ import annotations
10+
11+
import numpy as np
12+
import pytest
13+
14+
15+
def test_parses_bracketed_string_list(vis_module) -> None:
16+
assert vis_module.parse_list_column_cell("['nebeker', 'background']") == [
17+
"nebeker",
18+
"background",
19+
]
20+
21+
22+
def test_parses_single_item_list(vis_module) -> None:
23+
assert vis_module.parse_list_column_cell("['interview']") == ["interview"]
24+
25+
26+
def test_passes_through_existing_list(vis_module) -> None:
27+
assert vis_module.parse_list_column_cell(["a", "b"]) == ["a", "b"]
28+
29+
30+
def test_bare_scalar_string_becomes_single_item(vis_module) -> None:
31+
assert vis_module.parse_list_column_cell("interview") == ["interview"]
32+
33+
34+
@pytest.mark.parametrize("empty", [None, "", " ", np.nan, float("nan")])
35+
def test_missing_values_become_empty_list(vis_module, empty) -> None:
36+
assert vis_module.parse_list_column_cell(empty) == []
37+
38+
39+
def test_tuple_literal_is_rejected(vis_module) -> None:
40+
with pytest.raises(ValueError):
41+
vis_module.parse_list_column_cell("('a', 'b')")
42+
43+
44+
def test_dict_literal_is_rejected(vis_module) -> None:
45+
with pytest.raises(ValueError):
46+
vis_module.parse_list_column_cell("{'a': 1}")
47+
48+
49+
def test_non_string_items_are_rejected(vis_module) -> None:
50+
with pytest.raises(ValueError):
51+
vis_module.parse_list_column_cell("[1, 2, 3]")
52+
53+
54+
def test_malformed_literal_fails_loud(vis_module) -> None:
55+
with pytest.raises(ValueError):
56+
vis_module.parse_list_column_cell("['unclosed")
57+
58+
59+
def test_oversized_cell_is_rejected(vis_module) -> None:
60+
oversized = str([f"code_{i}" for i in range(1001)])
61+
with pytest.raises(ValueError):
62+
vis_module.parse_list_column_cell(oversized)
63+
64+
65+
def test_no_arbitrary_code_execution(vis_module) -> None:
66+
"""A code-injection payload must not execute; it is not a list literal."""
67+
with pytest.raises(ValueError):
68+
vis_module.parse_list_column_cell("[__import__('os').getcwd()]")

0 commit comments

Comments
 (0)