feat: ranked memory search + supersede detector#3
Conversation
search.py: BM25 ranking via stdlib sqlite3 FTS5 + link-graph rerank over the nightly vault-graph.json, IDF coverage weighting, stale-status demotion, graph recall of 1-hop neighbors. Schema/language-agnostic, zero extra deps, degrades to a naive substring scan if FTS5 is unavailable. Reuses common.parse_frontmatter. Run 'search.py --selftest' for the built-in assert check. supersede.py: deterministic same-entity contradiction detector. Groups cards by entity, flags conflicting scalar fields (company/role/status/...), writes .graph/supersede-candidates.json for the nightly rollup to resolve (dry-run by default; --apply stamps status=superseded conservatively).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds two new scripts under ChangesRanked Memory Search Script
Stacked Contradiction Supersede Scan
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant search_memory
participant SQLiteFTS5
participant VaultGraph
CLI->>search_memory: query, vault, limit, scope
search_memory->>search_memory: load_docs and content_tokens
search_memory->>SQLiteFTS5: bm25_search(fts_query)
SQLiteFTS5-->>search_memory: ranked docs or empty
search_memory->>search_memory: naive_search fallback
search_memory->>VaultGraph: load_graph
VaultGraph-->>search_memory: nodes and edges
search_memory->>search_memory: bfs_distances and reranking
search_memory-->>CLI: ranked results
Related issues: None referenced. Related PRs: None referenced. Suggested labels: enhancement, scripts Suggested reviewers: None Poem: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ede141364
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| vault_dir = Path(args[0]) | ||
| apply = "--apply" in args | ||
| verbose = "--verbose" in args | ||
| schema = load_schema(vault_dir / ".claude" / "skills" / "autograph" / "schema.json") |
There was a problem hiding this comment.
When a vault follows the documented/common layout with schema.json at the vault root, this explicit path raises FileNotFoundError before the scan runs, so supersede.py <vault> never writes .graph/supersede-candidates.json unless the user has also mirrored the schema into .claude/skills/autograph. Please use the same schema resolution as the other scripts or accept a schema argument.
Useful? React with 👍 / 👎.
| old_rel = cand["superseded_candidates"][0] | ||
| cur_rel = cand["current"] |
There was a problem hiding this comment.
If two duplicate cards both lack updated/created/last_accessed values, or their dates tie, scan() still picks current from sorted walk order and this block rewrites the other card as superseded. In that common two-card conflict scenario, --apply silently chooses a winner arbitrarily, contradicting the documented “clear newer-by-date winner” and can mark the wrong card superseded.
Useful? React with 👍 / 👎.
| for f in CONFLICT_FIELDS: | ||
| vals = {} | ||
| for c in cards: | ||
| v = str(c["fields"].get(f, "")).strip() |
There was a problem hiding this comment.
If any duplicate group contains a markdown file without YAML frontmatter, parse_frontmatter() returns None; this loop then calls .get on that None value and aborts the entire dry-run before writing the report. Vaults can contain ordinary notes without frontmatter, so please coerce missing fields to {} before storing or reading them.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
skills/autograph/scripts/supersede.py (3)
68-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
Path.stem/Pathoperations over string splitting for path/name handling.
.split("/")[-1].replace(".md", "")andcur_rel.replace('.md', '')assume POSIX separators and a single, terminal.mdoccurrence;Path(rp).stemis more robust and clearer.♻️ Suggested fix
- "entity": ordered[0]["path"].split("/")[-1].replace(".md", ""), + "entity": Path(ordered[0]["path"]).stem,- fields["superseded_by"] = f"[[{cur_rel.replace('.md', '')}]]" + fields["superseded_by"] = f"[[{Path(cur_rel).stem}]]"Also applies to: 93-93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/autograph/scripts/supersede.py` at line 68, Use Path-based name extraction in supersede.py instead of manual string splitting/replacement. Update the logic around ordered[0]["path"] and cur_rel so entity/name derivation uses Path(...).stem (or equivalent Path operations) rather than split("/")[-1].replace(".md", "") and .replace(".md", ""), keeping the existing behavior while making it cross-platform and more robust.
46-50: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBlind
except Exception: continuesilently swallows read/parse failures.Both loops silently skip unreadable/malformed cards with no diagnostic output, which could mask corruption or encoding issues in the vault without any trace, as flagged by static analysis (S112/BLE001).
🛡️ Suggested fix
- try: - fields = parse_frontmatter((vault_dir / rp).read_text(encoding="utf-8"))[0] - except Exception: - continue + try: + fields = parse_frontmatter((vault_dir / rp).read_text(encoding="utf-8"))[0] + except Exception as e: + print(f"supersede: skipping unreadable card {rp}: {e}", file=sys.stderr) + continueAlso applies to: 86-89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/autograph/scripts/supersede.py` around lines 46 - 50, The broad exception handling in supersede.py is hiding read and frontmatter parse failures in the loops that load card metadata, so replace the blind except Exception: continue in the logic around parse_frontmatter and the related loop at the later location with narrower handling and a diagnostic path. Use the existing processing flow in the supersede script to log or report the specific error and the affected path rp before skipping, so unreadable or malformed cards are visible instead of silently ignored.Source: Linters/SAST tools
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNon-English comments reduce accessibility for the broader team.
Cyrillic comments (flagged by Ruff RUF002/RUF003 as ambiguous characters) are fine functionally but may hinder maintainability for contributors who don't read Russian.
Also applies to: 61-61, 78-78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/autograph/scripts/supersede.py` at line 30, Replace the Cyrillic/Russian comments in supersede.py with clear English comments so the intent is accessible to the whole team and avoids Ruff RUF002/RUF003 ambiguity; update the comment near the top of the file and the other flagged comment locations in the same module, keeping the meaning intact while rewriting them in English.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/autograph/scripts/search.py`:
- Around line 277-278: The no-match test in search_memory is currently
tautological, so it never verifies the graceful empty-result path. Replace the
always-true assertion near search_memory with a real check on the returned count
from the no-match query, using the existing search_memory(v, "zzzznomatch",
limit=5) call, so the test fails if the function does not return zero matches.
- Around line 117-121: The bm25 ranking in search.py is using the wrong weight
alignment because the UNINDEXED path column is still counted, so the intended
title/meta/tags/body priorities are shifted. Update the SELECT query in the
search flow that calls bm25(d, ...) to include a leading 0.0 weight for path,
keeping the remaining weights aligned with the indexed columns and preserving
the current LIMIT behavior.
In `@skills/autograph/scripts/supersede.py`:
- Around line 100-108: The main() flow in supersede.py is hardcoding a schema
path instead of using the loader’s default discovery behavior, which can make
the script fail even when a valid schema exists elsewhere in the vault. Update
the schema loading in main() to rely on load_schema()’s default search logic,
and if needed accept an optional schema path in the same style as the other
scripts so schema resolution stays flexible.
---
Nitpick comments:
In `@skills/autograph/scripts/supersede.py`:
- Line 68: Use Path-based name extraction in supersede.py instead of manual
string splitting/replacement. Update the logic around ordered[0]["path"] and
cur_rel so entity/name derivation uses Path(...).stem (or equivalent Path
operations) rather than split("/")[-1].replace(".md", "") and .replace(".md",
""), keeping the existing behavior while making it cross-platform and more
robust.
- Around line 46-50: The broad exception handling in supersede.py is hiding read
and frontmatter parse failures in the loops that load card metadata, so replace
the blind except Exception: continue in the logic around parse_frontmatter and
the related loop at the later location with narrower handling and a diagnostic
path. Use the existing processing flow in the supersede script to log or report
the specific error and the affected path rp before skipping, so unreadable or
malformed cards are visible instead of silently ignored.
- Line 30: Replace the Cyrillic/Russian comments in supersede.py with clear
English comments so the intent is accessible to the whole team and avoids Ruff
RUF002/RUF003 ambiguity; update the comment near the top of the file and the
other flagged comment locations in the same module, keeping the meaning intact
while rewriting them in English.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bcfd0c4f-29d1-4298-a2e0-aac7f8036ee2
📒 Files selected for processing (2)
skills/autograph/scripts/search.pyskills/autograph/scripts/supersede.py
…bustness
- search.py: bm25() needs one weight per column INCLUDING the UNINDEXED path
slot; the 4-weight form shifted meta/tags left by one and mis-ranked them
- supersede.py: resolve schema via load_schema() fallback chain (schema.json →
schema.example.json), degrade to {} instead of raising on a fresh vault so
the scan actually runs
- supersede.py: tolerate notes without frontmatter (coerce None → {})
- supersede.py: --apply only supersedes on a genuinely newer winner, never on a
date tie / missing dates
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/autograph/scripts/supersede.py`:
- Around line 86-95: The exception handling in supersede.py around
parse_frontmatter/read_text is too broad and silently hides file read or parse
failures. Update the try/except in the supersede flow to log a diagnostic that
includes the current path and/or old path plus the exception details before
continuing, so skipped candidates from read/parse errors are distinguishable
from the normal newer-date filter in the _card_date comparison.
- Around line 114-120: Pass vault_dir into load_schema() in supersede.py so
schema resolution starts from the current vault instead of CWD or the skill
directory. Update the schema-loading flow to use the vault-specific path first,
then fall back to the shared example schema only if that lookup fails, keeping
the existing graceful {} fallback behavior in the exception path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ec5c632-d399-46b8-b573-c72f54957dec
📒 Files selected for processing (2)
skills/autograph/scripts/search.pyskills/autograph/scripts/supersede.py
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/autograph/scripts/search.py
| cur_path = vault_dir / cur_rel | ||
| try: | ||
| cur_fields = parse_frontmatter(cur_path.read_text(encoding="utf-8"))[0] or {} | ||
| fields, body, lines = parse_frontmatter(old_path.read_text(encoding="utf-8")) | ||
| except Exception: | ||
| continue | ||
| fields = fields or {} | ||
| # Only apply on a genuinely newer winner — never guess on a tie or missing dates. | ||
| if _card_date(cur_fields) <= _card_date(fields): | ||
| continue |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Good fix, but silent blind excepts hide read/parse failures.
The strictly-newer date check at Lines 93-95 correctly closes the tie/missing-date gap called out in the commit message. However, Lines 90-91 swallow any exception (missing file, permission error, malformed frontmatter) from reading two files and silently continue, with no diagnostic output. For a --apply run that mutates files, an operator has no way to tell "candidate skipped because winner isn't newer" from "candidate skipped because reading a card raised an exception."
🩹 Suggested logging
try:
cur_fields = parse_frontmatter(cur_path.read_text(encoding="utf-8"))[0] or {}
fields, body, lines = parse_frontmatter(old_path.read_text(encoding="utf-8"))
- except Exception:
+ except Exception as e:
+ print(f"supersede: skip {old_rel} (read/parse error: {e})", file=sys.stderr)
continue📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cur_path = vault_dir / cur_rel | |
| try: | |
| cur_fields = parse_frontmatter(cur_path.read_text(encoding="utf-8"))[0] or {} | |
| fields, body, lines = parse_frontmatter(old_path.read_text(encoding="utf-8")) | |
| except Exception: | |
| continue | |
| fields = fields or {} | |
| # Only apply on a genuinely newer winner — never guess on a tie or missing dates. | |
| if _card_date(cur_fields) <= _card_date(fields): | |
| continue | |
| cur_path = vault_dir / cur_rel | |
| try: | |
| cur_fields = parse_frontmatter(cur_path.read_text(encoding="utf-8"))[0] or {} | |
| fields, body, lines = parse_frontmatter(old_path.read_text(encoding="utf-8")) | |
| except Exception as e: | |
| print(f"supersede: skip {old_rel} (read/parse error: {e})", file=sys.stderr) | |
| continue | |
| fields = fields or {} | |
| # Only apply on a genuinely newer winner — never guess on a tie or missing dates. | |
| if _card_date(cur_fields) <= _card_date(fields): | |
| continue |
🧰 Tools
🪛 Ruff (0.15.20)
[error] 90-91: try-except-continue detected, consider logging the exception
(S112)
[warning] 90-90: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/autograph/scripts/supersede.py` around lines 86 - 95, The exception
handling in supersede.py around parse_frontmatter/read_text is too broad and
silently hides file read or parse failures. Update the try/except in the
supersede flow to log a diagnostic that includes the current path and/or old
path plus the exception details before continuing, so skipped candidates from
read/parse errors are distinguishable from the normal newer-date filter in the
_card_date comparison.
Source: Linters/SAST tools
- supersede.py: resolve schema from vault_dir first (then example/{}), so a scan
against an arbitrary vault never picks up a different vault's schema
- supersede.py: exclude already-superseded cards from the conflict report, so
resolved conflicts don't re-surface every nightly run
- search.py: drop the tautological 'or True' in the no-match selftest assertion
1 participant
Adds two schema-agnostic engine scripts consumed by downstream vaults (Iva, agent-second-brain).
search.py — ranked recall
Replaces raw-grep memory recall with BM25 ranking:
sqlite3FTS5 (zero extra deps, no native build), column weights title/meta > tags > body.graph/vault-graph.json(BFS proximity + 1-hop graph-recall + incoming-link boost)status: superseded/archived/...demoted ×0.3 (findable but ranked below current)common.parse_frontmatter; no hardcoded card typespython3 search.py "<query>" --vault DIR [--json]→{count, engine, hits:[{file,score,status,confidence,snippet}]}. Built-in check:search.py --selftest.supersede.py — contradiction detector
Ported from the production copy running in Iva's nightly doctor. Groups same-entity cards, flags conflicting scalar fields, writes
.graph/supersede-candidates.jsonfor the nightly rollup to resolve (dry-run by default).Verified end-to-end against
graph.py healthoutput (node shape, graph-recall).Summary by CodeRabbit