Structsense skills#5
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the structsense-skills project, which provides a model-agnostic, multi-stage pipeline for structured information extraction (such as NER, resource extraction, and schema-driven extraction) along with integration guides, prompts, schemas, and helper scripts. The review feedback highlights three critical issues in the newly added scripts: first, map_masked_offsets_to_original in mask_pass.py ignores the placeholder_map parameter, which breaks offset mapping when placeholders alter text length; second, sorting validation errors in json_repair.py can trigger a TypeError due to mixed-type comparisons in path elements; and third, the judge score summarization in group_by_entity.py incorrectly treats boolean values as numeric scores.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def map_masked_offsets_to_original(masked_text: str, | ||
| new_items: Iterable[dict], | ||
| placeholder_map: list[dict], | ||
| original_text: str, | ||
| start_key: str = "start", | ||
| end_key: str = "end", | ||
| surface_key: str = "entity", | ||
| ) -> list[dict]: | ||
| """Translate offsets reported in `masked_text` back to `original_text`. | ||
|
|
||
| Because placeholders are padded to the same length as the spans they | ||
| replace, character offsets in masked_text equal those in original_text | ||
| — UNLESS a span was shorter than its placeholder text. The function | ||
| re-verifies each item against `original_text[start:end]` and, on | ||
| mismatch, searches for the surface form in the original text. | ||
| """ | ||
| from .span_validator import repair_span # local import to avoid cycle | ||
| out = [] | ||
| for it in new_items: | ||
| s = it.get(start_key) | ||
| e = it.get(end_key) | ||
| surface = it.get(surface_key) | ||
| if s is None or e is None or surface is None: |
There was a problem hiding this comment.
The map_masked_offsets_to_original function completely ignores the placeholder_map parameter. When placeholders shift the text length (e.g., short entities like "C4" replaced by longer placeholders like "[E0]"), the offsets in masked_text will not align with original_text.
Since the shift is not adjusted, the direct match check original_text[s:e] == surface fails, and the function falls back to repair_span. However, repair_span checks if the sentence (which still contains [En] placeholders) is in original_text, which fails. It then falls back to original_text.index(surface), mapping the entity to its first occurrence in the document. This breaks the exhaustive NER requirement by losing the correct mention positions and duplicating the first mention.
Use the placeholder_map to calculate the cumulative shift and accurately map the offsets back to the original text.
def map_masked_offsets_to_original(masked_text: str,
new_items: Iterable[dict],
placeholder_map: list[dict],
original_text: str,
start_key: str = "start",
end_key: str = "end",
surface_key: str = "entity",
) -> list[dict]:
from .span_validator import repair_span # local import to avoid cycle
out = []
sorted_pmap = sorted(placeholder_map, key=lambda p: p["masked_start"])
def to_orig_offset(masked_offset: int) -> int:
shift = 0
for p in sorted_pmap:
if masked_offset >= p["masked_end"]:
orig_len = p["orig_end"] - p["orig_start"]
masked_len = p["masked_end"] - p["masked_start"]
shift += (orig_len - masked_len)
else:
break
return masked_offset + shift
for it in new_items:
s = it.get(start_key)
e = it.get(end_key)
surface = it.get(surface_key)
if s is None or e is None or surface is None:
continue
orig_s = to_orig_offset(s)
orig_e = to_orig_offset(e)
if 0 <= orig_s < orig_e <= len(original_text) and original_text[orig_s:orig_e] == surface:
new_it = dict(it)
new_it[start_key] = orig_s
new_it[end_key] = orig_e
out.append(new_it)
else:
repaired = repair_span(original_text, it)
if repaired is not None:
out.append(repaired)
return out| current = parsed | ||
|
|
||
| for _ in range(max_attempts + 1): | ||
| errs = sorted(validator.iter_errors(current), key=lambda e: list(e.absolute_path)) |
There was a problem hiding this comment.
Sorting validator.iter_errors using list(e.absolute_path) as the key can raise a TypeError in Python 3 if the paths contain mixed types (e.g., integer indices for lists and string keys for dictionaries). Comparing an integer and a string during sorting is not supported.
To prevent this, convert all path elements to strings (or another uniform type) before sorting.
| errs = sorted(validator.iter_errors(current), key=lambda e: list(e.absolute_path)) | |
| errs = sorted(validator.iter_errors(current), key=lambda e: tuple(str(p) for p in e.absolute_path)) |
| def _summarize_judge(scores: list[float]) -> dict[str, Optional[float]]: | ||
| finite = [s for s in scores if isinstance(s, (int, float))] | ||
| if not finite: |
There was a problem hiding this comment.
In Python, isinstance(True, int) evaluates to True because bool is a subclass of int. If scores contains any boolean values, they will be incorrectly treated as numeric scores (1.0 or 0.0) and included in the calculations.
Explicitly exclude boolean values from the finite list comprehension.
| def _summarize_judge(scores: list[float]) -> dict[str, Optional[float]]: | |
| finite = [s for s in scores if isinstance(s, (int, float))] | |
| if not finite: | |
| def _summarize_judge(scores: list[float]) -> dict[str, Optional[float]]: | |
| finite = [s for s in scores if isinstance(s, (int, float)) and not isinstance(s, bool)] | |
| if not finite: |
Pi is a CLI coding agent with native Agent Skills support and a built-in bash tool, so it discovers the skill and runs the pipeline directly — same integration story as Claude Code. Wired the new guide into the SKILL.md connecting list and the README directory tree + platform table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pi only scans skill search paths at startup, so a skill copied in mid-session isn't detected until /reload (or a restart). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
To maintain alphabetical order in the directory tree structure, pi-dev.md should be placed at the end of the connecting/ list. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Reframe the label list as preferred suggestions and allow coining the most appropriate label when none fits, instead of forcing a poor match or Other. Add a tuning knob to revert to closed-taxonomy behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…abel field description Gemini Suggestion Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Gemini Suggestion Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Update neuroscience ner prompt
Add Pi (pi.dev) connection guide for structsense-skills
This skill implements StructSense and can be considered a replica of the original implementation. It reuses portions of the StructSense codebase.
The skill has been tested with both Claude Code and the Claude Desktop application. However, when running it through the Claude Desktop application, I was unable to access the required tools.