Skip to content

Commit 53fb296

Browse files
queeliusclaude
andcommitted
fix: bugs and code quality in analyses system
- Fix MCP save_analysis always reporting "updated" (checked existence after upsert) - Fix SPA analysis section using global DB instead of db parameter - Fix parser crash on non-dict YAML frontmatter - Extract _save_tags/_fetch_tags helpers in db.py (DRY notes + analyses) - Move json import to module level in server.py - Modernize test fixtures: tmp_path instead of tempfile + manual cleanup - Add edge case tests: unclosed frontmatter, non-dict YAML, empty body Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent df7bef3 commit 53fb296

5 files changed

Lines changed: 132 additions & 142 deletions

File tree

src/chartfold/analysis_parser.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ def parse_analysis_file(path: str | Path) -> dict:
4646
if len(parts) >= 3:
4747
yaml_text = parts[1]
4848
content = parts[2].lstrip("\n")
49-
frontmatter = yaml.safe_load(yaml_text) or {}
49+
parsed_yaml = yaml.safe_load(yaml_text)
50+
frontmatter = parsed_yaml if isinstance(parsed_yaml, dict) else {}
5051

5152
title = frontmatter.pop("title", None)
5253
if not title:

src/chartfold/db.py

Lines changed: 27 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,27 @@ def last_load_counts(self, source: str) -> dict[str, int] | None:
231231
"source_assets": row.get("source_assets_count", 0),
232232
}
233233

234+
# --- Tag helpers (shared by notes and analyses) ---
235+
236+
def _save_tags(self, table: str, fk_col: str, fk_id: int, tags: list[str]) -> None:
237+
"""Replace all tags for a record: delete existing, insert new."""
238+
self.conn.execute(f"DELETE FROM {table} WHERE {fk_col}=?", (fk_id,))
239+
for tag in tags:
240+
clean = tag.strip()
241+
if clean:
242+
self.conn.execute(
243+
f"INSERT OR IGNORE INTO {table} ({fk_col}, tag) VALUES (?, ?)",
244+
(fk_id, clean),
245+
)
246+
247+
def _fetch_tags(self, table: str, fk_col: str, fk_id: int) -> list[str]:
248+
"""Fetch sorted tags for a record."""
249+
rows = self.query(
250+
f"SELECT tag FROM {table} WHERE {fk_col} = ? ORDER BY tag",
251+
(fk_id,),
252+
)
253+
return [r["tag"] for r in rows]
254+
234255
# --- Personal notes CRUD ---
235256

236257
def save_note(
@@ -254,7 +275,6 @@ def save_note(
254275
"WHERE id=?",
255276
(title, content, now, ref_table, ref_id, note_id),
256277
)
257-
self.conn.execute("DELETE FROM note_tags WHERE note_id=?", (note_id,))
258278
else:
259279
# Create new note
260280
cursor = self.conn.execute(
@@ -265,13 +285,7 @@ def save_note(
265285
# lastrowid is always int after INSERT
266286
note_id = cursor.lastrowid or 0
267287

268-
for tag in tags:
269-
clean_tag = tag.strip()
270-
if clean_tag:
271-
self.conn.execute(
272-
"INSERT OR IGNORE INTO note_tags (note_id, tag) VALUES (?, ?)",
273-
(note_id, clean_tag),
274-
)
288+
self._save_tags("note_tags", "note_id", note_id, tags)
275289

276290
return note_id
277291

@@ -285,11 +299,7 @@ def get_note(self, note_id: int) -> dict | None:
285299
if not rows:
286300
return None
287301
note = rows[0]
288-
tag_rows = self.query(
289-
"SELECT tag FROM note_tags WHERE note_id = ? ORDER BY tag",
290-
(note_id,),
291-
)
292-
note["tags"] = [r["tag"] for r in tag_rows]
302+
note["tags"] = self._fetch_tags("note_tags", "note_id", note_id)
293303
return note
294304

295305
def search_notes_personal(
@@ -333,13 +343,8 @@ def search_notes_personal(
333343
tuple(params),
334344
)
335345

336-
# Attach tags to each result
337346
for row in rows:
338-
tag_rows = self.query(
339-
"SELECT tag FROM note_tags WHERE note_id = ? ORDER BY tag",
340-
(row["id"],),
341-
)
342-
row["tags"] = [r["tag"] for r in tag_rows]
347+
row["tags"] = self._fetch_tags("note_tags", "note_id", row["id"])
343348

344349
return rows
345350

@@ -379,7 +384,6 @@ def save_analysis(
379384
"category=?, summary=?, source=?, updated_at=? WHERE id=?",
380385
(title, content, frontmatter_json, category, summary, source, now, analysis_id),
381386
)
382-
self.conn.execute("DELETE FROM analysis_tags WHERE analysis_id=?", (analysis_id,))
383387
else:
384388
cursor = self.conn.execute(
385389
"INSERT INTO analyses (slug, title, content, frontmatter, "
@@ -389,13 +393,7 @@ def save_analysis(
389393
)
390394
analysis_id = cursor.lastrowid or 0
391395

392-
for tag in tags:
393-
clean_tag = tag.strip()
394-
if clean_tag:
395-
self.conn.execute(
396-
"INSERT OR IGNORE INTO analysis_tags (analysis_id, tag) VALUES (?, ?)",
397-
(analysis_id, clean_tag),
398-
)
396+
self._save_tags("analysis_tags", "analysis_id", analysis_id, tags)
399397

400398
return analysis_id
401399

@@ -409,11 +407,7 @@ def get_analysis(self, slug_or_id: str | int) -> dict | None:
409407
if not rows:
410408
return None
411409
analysis = rows[0]
412-
tag_rows = self.query(
413-
"SELECT tag FROM analysis_tags WHERE analysis_id = ? ORDER BY tag",
414-
(analysis["id"],),
415-
)
416-
analysis["tags"] = [r["tag"] for r in tag_rows]
410+
analysis["tags"] = self._fetch_tags("analysis_tags", "analysis_id", analysis["id"])
417411
return analysis
418412

419413
def search_analyses(
@@ -458,11 +452,7 @@ def search_analyses(
458452
)
459453

460454
for row in rows:
461-
tag_rows = self.query(
462-
"SELECT tag FROM analysis_tags WHERE analysis_id = ? ORDER BY tag",
463-
(row["id"],),
464-
)
465-
row["tags"] = [r["tag"] for r in tag_rows]
455+
row["tags"] = self._fetch_tags("analysis_tags", "analysis_id", row["id"])
466456

467457
return rows
468458

src/chartfold/mcp/server.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
from __future__ import annotations
88

9+
import json
910
import os
1011
import re
1112

@@ -674,8 +675,6 @@ def save_analysis(
674675
frontmatter_yaml: Optional YAML string with additional metadata fields.
675676
Will be parsed and stored as JSON for json_extract() queries.
676677
"""
677-
import json
678-
679678
db = _get_db()
680679
try:
681680
tag_list = [t.strip() for t in tags.split(",") if t.strip()] if tags else []
@@ -690,6 +689,7 @@ def save_analysis(
690689
except Exception:
691690
frontmatter_json = frontmatter_yaml # Store as-is if not valid YAML
692691

692+
existing_before = db.get_analysis(slug)
693693
saved_id = db.save_analysis(
694694
slug=slug,
695695
title=title,
@@ -701,9 +701,7 @@ def save_analysis(
701701
source=source,
702702
)
703703

704-
existing = db.get_analysis(slug)
705-
is_update = existing and existing["id"] == saved_id
706-
return {"id": saved_id, "slug": slug, "status": "updated" if is_update else "created"}
704+
return {"id": saved_id, "slug": slug, "status": "updated" if existing_before else "created"}
707705
finally:
708706
db.close()
709707

src/chartfold/spa/js/sections.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1095,7 +1095,7 @@ const Sections = {
10951095
var data = [];
10961096
var fromDB = false;
10971097
try {
1098-
var dbRows = DB.query("SELECT slug, title, category, summary, content, source FROM analyses ORDER BY updated_at DESC");
1098+
var dbRows = db.query("SELECT slug, title, category, summary, content, source FROM analyses ORDER BY updated_at DESC");
10991099
if (dbRows.length > 0) {
11001100
data = dbRows.map(function(r) {
11011101
return { slug: r.slug, title: r.title, body: r.content, filename: r.slug + '.md', category: r.category, summary: r.summary };

0 commit comments

Comments
 (0)