Skip to content

Commit f5035e8

Browse files
fix: impactful 'Low-tier' data-loss/crash bugs from the audit backlog
Triaged the audit's Low tier; these ~12 were really data-loss/crash bugs, not cosmetic. Fixed on this PR (deferred 4 larger items to issues #405-#408): - DOC-15: width/height attrs like '100%'/'50px' crashed int() (html/epub) or silently dropped the image (word). New shared scraper_utils.parse_leading_int. - DOC-13: table extraction dropped any body row equal to the header text; now skips <thead> rows STRUCTURALLY (shared helper + html_scraper copy). +tests. - DOC-11: EPUB section images with empty bytes wrote 0-byte PNGs + broken links; require non-empty bytes. - DOC-17: PDF image branch raised KeyError/TypeError (missing/empty data) and aborted the reference-file write; guard with .get + non-empty bytes. - DOC-14: checkpoint write is now atomic (temp + os.replace) so a second Ctrl-C can't truncate it and silently lose all crawl progress. - MED-14: video AI ref-cleaning overwrote the reference with a truncated reply; require stop_reason == 'end_turn'. - MED-17: Slack conversations_list is now paginated (was single-page; >200 channels truncated). - ENH-17: assembled PDF content was silently discarded when the base SKILL.md had no Code Examples / API Reference / Reference heading AND no footer; append at end. - MCP-15: install_skill aborts when packaging fails instead of fabricating a zip_path and uploading a missing file. - MCP-16: generate_config refuses to clobber an existing config without force=true (exposed on the FastMCP tool too). - ENH-15: prompt_file.write_text now uses encoding='utf-8' (was UnicodeEncodeError on non-UTF-8 locales for emoji/CJK prompts). - ADP-03: cap weaviate-client <4 (the adaptor uses the v3 API; v4 breaks it). MED-16 (Discord batch[-1]['id'] KeyError) was already fixed in the audit's batch 2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d60ff09 commit f5035e8

15 files changed

Lines changed: 161 additions & 32 deletions

pyproject.toml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ chroma = [
178178
]
179179

180180
weaviate = [
181-
"weaviate-client>=3.25.0",
181+
"weaviate-client>=3.25.0,<4",
182182
]
183183

184184
sentence-transformers = [
@@ -191,7 +191,7 @@ pinecone = [
191191

192192
rag-upload = [
193193
"chromadb>=0.4.0",
194-
"weaviate-client>=3.25.0",
194+
"weaviate-client>=3.25.0,<4",
195195
"sentence-transformers>=2.2.0",
196196
"pinecone>=5.0.0",
197197
]
@@ -267,7 +267,7 @@ all = [
267267
"google-cloud-storage>=2.10.0",
268268
"azure-storage-blob>=12.19.0",
269269
"chromadb>=0.4.0",
270-
"weaviate-client>=3.25.0",
270+
"weaviate-client>=3.25.0,<4",
271271
"pinecone>=5.0.0",
272272
"fastapi>=0.109.0",
273273
"sentence-transformers>=2.3.0",

src/skill_seekers/cli/agent_client.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ def _call_local(
448448
if output_file:
449449
full_prompt += f"\n\nWrite your response to: {resp_file}\n"
450450

451-
prompt_file.write_text(full_prompt)
451+
prompt_file.write_text(full_prompt, encoding="utf-8")
452452

453453
# Build command from preset
454454
cmd = []

src/skill_seekers/cli/chat_scraper.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -651,12 +651,21 @@ def _extract_slack_api(self) -> list[dict]:
651651
channel_ids = [self.channel]
652652
channel_names = {self.channel: self.channel}
653653
else:
654-
# List all accessible channels
655-
result = client.conversations_list(
656-
types="public_channel,private_channel",
657-
limit=200,
658-
)
659-
channels = result.get("channels", [])
654+
# List ALL accessible channels — paginate via next_cursor. A
655+
# single conversations_list call caps at the page limit and
656+
# silently truncates workspaces with >limit channels.
657+
channels = []
658+
cursor = None
659+
while True:
660+
result = client.conversations_list(
661+
types="public_channel,private_channel",
662+
limit=200,
663+
cursor=cursor,
664+
)
665+
channels.extend(result.get("channels", []))
666+
cursor = (result.get("response_metadata") or {}).get("next_cursor")
667+
if not cursor:
668+
break
660669
channel_ids = [ch["id"] for ch in channels]
661670
channel_names = {ch["id"]: ch.get("name", ch["id"]) for ch in channels}
662671
print(f" Found {len(channel_ids)} channel(s)")

src/skill_seekers/cli/doc_scraper.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,8 +372,14 @@ def save_checkpoint(self) -> None:
372372
}
373373

374374
try:
375-
with open(self.checkpoint_file, "w", encoding="utf-8") as f:
375+
# Atomic write: a second Ctrl-C during a direct open(...,'w') can
376+
# truncate the checkpoint, and load_checkpoint then silently starts
377+
# fresh — losing all crawl progress. Write a temp file, then
378+
# os.replace() it into place (atomic on the same filesystem).
379+
tmp_file = f"{self.checkpoint_file}.tmp"
380+
with open(tmp_file, "w", encoding="utf-8") as f:
376381
json.dump(checkpoint_data, f, indent=2)
382+
os.replace(tmp_file, self.checkpoint_file)
377383
logger.info(" 💾 Checkpoint saved (%d pages)", self.pages_scraped)
378384
except Exception as e:
379385
logger.warning(" ⚠️ Failed to save checkpoint: %s", e)

src/skill_seekers/cli/epub_scraper.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
from .skill_converter import SkillConverter
3232
from skill_seekers.cli.scraper_utils import score_code_quality as _score_code_quality
3333
from skill_seekers.cli.scraper_utils import extract_table_from_html as _extract_table_from_html
34+
from skill_seekers.cli.scraper_utils import parse_leading_int as _parse_leading_int
3435

3536
logger = logging.getLogger(__name__)
3637

@@ -603,7 +604,9 @@ def _generate_reference_file(self, _cat_key, cat_data, section_num, total_sectio
603604
img_filename = f"section_{sec_num}_img_{img_index}.png"
604605
img_path = os.path.join(assets_dir, img_filename)
605606

606-
if isinstance(img_data, (bytes, bytearray)):
607+
# Require NON-EMPTY bytes — b"" passed the isinstance
608+
# check and produced a 0-byte PNG plus a broken ![] link.
609+
if isinstance(img_data, (bytes, bytearray)) and len(img_data) > 0:
607610
with open(img_path, "wb") as img_file:
608611
img_file.write(img_data)
609612
f.write(f"![Image {img_index}](../assets/{img_filename})\n\n")
@@ -974,8 +977,8 @@ def _build_section(
974977
{
975978
"index": len(images),
976979
"data": b"", # EPUB images handled separately via manifest
977-
"width": int(elem.get("width", 0) or 0),
978-
"height": int(elem.get("height", 0) or 0),
980+
"width": _parse_leading_int(elem.get("width")),
981+
"height": _parse_leading_int(elem.get("height")),
979982
}
980983
)
981984
continue

src/skill_seekers/cli/html_scraper.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
from bs4 import BeautifulSoup, Comment, Tag
2727

2828
from .skill_converter import SkillConverter
29+
from skill_seekers.cli.scraper_utils import parse_leading_int as _parse_leading_int
2930
from skill_seekers.cli.scraper_utils import score_code_quality as _score_code_quality
3031

3132
logger = logging.getLogger(__name__)
@@ -863,12 +864,17 @@ def _extract_tables(self, table_elem: Tag) -> dict | None:
863864
if header_row:
864865
headers = [th.get_text(strip=True) for th in header_row.find_all(["th", "td"])]
865866

866-
# Body rows
867-
tbody = table_elem.find("tbody") or table_elem
868-
for row in tbody.find_all("tr"):
867+
# Body rows. Prefer an explicit <tbody>; otherwise take rows directly
868+
# under the table but skip <thead> rows STRUCTURALLY (not by value) so a
869+
# legitimate body row that duplicates the header text isn't dropped.
870+
tbody = table_elem.find("tbody")
871+
if tbody is not None:
872+
body_rows = tbody.find_all("tr")
873+
else:
874+
body_rows = [r for r in table_elem.find_all("tr") if r.find_parent("thead") is None]
875+
for row in body_rows:
869876
cells = [td.get_text(strip=True) for td in row.find_all(["td", "th"])]
870-
# Skip the header row we already captured
871-
if cells and cells != headers:
877+
if cells:
872878
rows.append(cells)
873879

874880
# If no explicit thead, use first row as header
@@ -908,8 +914,8 @@ def _extract_image_info(self, img_elem: Tag, source_file: Path) -> dict | None:
908914
"src": resolved_src,
909915
"alt": img_elem.get("alt", ""),
910916
"title": img_elem.get("title", ""),
911-
"width": int(img_elem.get("width", 0) or 0),
912-
"height": int(img_elem.get("height", 0) or 0),
917+
"width": _parse_leading_int(img_elem.get("width")),
918+
"height": _parse_leading_int(img_elem.get("height")),
913919
"data": b"", # Placeholder; actual image data loaded separately
914920
}
915921

src/skill_seekers/cli/pdf_scraper.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,12 +343,20 @@ def _generate_reference_file(self, _cat_key, cat_data, section_num, total_sectio
343343

344344
f.write("### Images\n\n")
345345
for img in page["images"]:
346+
# Guard the raw image data: a missing "data" key raised
347+
# KeyError and a non-bytes/empty value either crashed
348+
# (TypeError) or wrote a 0-byte PNG with a broken ![] link
349+
# — aborting the whole reference-file write.
350+
img_data = img.get("data")
351+
if not (isinstance(img_data, (bytes, bytearray)) and len(img_data) > 0):
352+
continue
353+
346354
# Save image to assets
347355
img_filename = f"page_{page['page_number']}_img_{img['index']}.png"
348356
img_path = os.path.join(assets_dir, img_filename)
349357

350358
with open(img_path, "wb") as img_file:
351-
img_file.write(img["data"])
359+
img_file.write(img_data)
352360

353361
# Add markdown image reference
354362
f.write(f"![Image {img['index']}](../assets/{img_filename})\n\n")

src/skill_seekers/cli/scraper_utils.py

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,21 @@
1515
import re
1616

1717

18+
def parse_leading_int(value, default: int = 0) -> int:
19+
"""Parse the leading integer from a dimension-ish value, defensively.
20+
21+
HTML/EPUB/Word width/height attributes can be ``"100%"``, ``"50px"``,
22+
``""`` or ``None``; a bare ``int("100%")`` raises ``ValueError`` (crashing
23+
image extraction) or silently drops the image. Returns the leading integer
24+
(``"100%"`` -> 100, ``"50px"`` -> 50) or ``default`` when there's none
25+
(``"auto"``/``""``/``None`` -> ``default``).
26+
"""
27+
if value is None:
28+
return default
29+
match = re.match(r"\s*(-?\d+)", str(value))
30+
return int(match.group(1)) if match else default
31+
32+
1833
def score_code_quality(code: str, *, notebook_mode: bool = False) -> float:
1934
"""Heuristic quality score for a code block (0.0-10.0).
2035
@@ -92,12 +107,18 @@ def extract_table_from_html(table_elem) -> dict | None:
92107
if header_row:
93108
headers = [th.get_text(strip=True) for th in header_row.find_all(["th", "td"])]
94109

95-
# Body rows
96-
tbody = table_elem.find("tbody") or table_elem
97-
for row in tbody.find_all("tr"):
110+
# Body rows. Prefer an explicit <tbody>; otherwise take rows directly under
111+
# the table but skip any that belong to <thead> — skipping STRUCTURALLY, not
112+
# by value, so a legitimate body row that merely duplicates the header text
113+
# isn't dropped.
114+
tbody = table_elem.find("tbody")
115+
if tbody is not None:
116+
body_rows = tbody.find_all("tr")
117+
else:
118+
body_rows = [r for r in table_elem.find_all("tr") if r.find_parent("thead") is None]
119+
for row in body_rows:
98120
cells = [td.get_text(strip=True) for td in row.find_all(["td", "th"])]
99-
# Skip the header row we already captured
100-
if cells and cells != headers:
121+
if cells:
101122
rows.append(cells)
102123

103124
# If no explicit thead, use first row as header

src/skill_seekers/cli/unified_skill_builder.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,14 +451,19 @@ def _synthesize_docs_github_pdf(self, skill_mds: dict[str, str]) -> str:
451451
if pdf_content_lines and insertion_index != -1:
452452
lines[insertion_index:insertion_index] = pdf_content_lines
453453
elif pdf_content_lines:
454-
# Append at end before footer
454+
# Append before the trailing footer if present, otherwise at the very
455+
# end. ENH-17: when the base SKILL.md had no Code Examples / API
456+
# Reference / Reference Documentation heading AND no trailing footer,
457+
# the assembled PDF content was silently discarded.
455458
footer_index = -1
456459
for i, line in enumerate(lines):
457460
if line.startswith("---") and i > len(lines) - 5:
458461
footer_index = i
459462
break
460463
if footer_index != -1:
461464
lines[footer_index:footer_index] = pdf_content_lines
465+
else:
466+
lines.extend(pdf_content_lines)
462467

463468
# Update reference documentation to include PDF
464469
final_content = "\n".join(lines)

src/skill_seekers/cli/video_scraper.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,14 @@ def _ai_clean_reference(ref_path: str, content: str, api_key: str | None = None)
304304
messages=[{"role": "user", "content": prompt}],
305305
)
306306
result = response.content[0].text
307-
if result and len(result) > len(content) * 0.5:
307+
# Require a COMPLETE response before overwriting the reference file: a
308+
# response truncated at max_tokens can still exceed 50% of the input yet
309+
# silently corrupt the reference. (stop_reason == "end_turn" => complete.)
310+
if (
311+
result
312+
and getattr(response, "stop_reason", None) == "end_turn"
313+
and len(result) > len(content) * 0.5
314+
):
308315
with open(ref_path, "w", encoding="utf-8") as f:
309316
f.write(result)
310317
logger.info(f"AI-cleaned reference: {os.path.basename(ref_path)}")

0 commit comments

Comments
 (0)