docs(skills): use the POSIX read verbs in literary analysis, drop canvas - #1424
docs(skills): use the POSIX read verbs in literary analysis, drop canvas#1424phernandez wants to merge 1 commit into
Conversation
The pipeline was written before the POSIX surface existed, so a 138-chapter run never touched the tools that make it affordable. Add a Tools section with the two rules that compound at scale (never read a note to check a field; never read a whole file to reach one part), a chapter offset map built once so chapters are read by line range, --meta/--fields queries for finding what needs enriching and for coverage checks, and orphan checking framed around relation density rather than note count. Phase 5 becomes graph exploration: the canvas tool was removed, so the JSON Canvas instructions are replaced with traversal that produces synthesis notes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://cdn.statically.io/img/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: 57068c1c28
ℹ️ 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".
|
|
||
| ```bash | ||
| grep -n '^CHAPTER ' moby-dick.txt # or the work's heading pattern | ||
| bm cat moby-dick.txt --lines 4200-4890 # one chapter, not the whole text |
There was a problem hiding this comment.
When the source is the .txt file shown here, bm cat ... --lines cannot read it: bm cat --help defines its operand as a note identifier, while _apply_note_slice rejects entities without Markdown content with a 404 (src/basic_memory/api/v2/routers/knowledge_router.py:771-777). Non-Markdown files such as moby-dick.txt are raw resources, so the chapter-reading workflow fails at its central step; use a raw-file range reader or first ingest the text as a Markdown note.
Useful? React with 👍 / 👎.
| bm find --meta 'note_type=chapter' --fields chapter_number,pov,setting # coverage at a glance | ||
| bm find --meta 'note_type=character' --fields role,status # who is still a stub |
There was a problem hiding this comment.
These predicates return no Chapter or Character rows created by this skill because the schemas use note_type="Chapter", "Character", and "Theme", while find aliases note_type to frontmatter type and metadata equality is case-sensitive (src/basic_memory/mcp/tools/posix_tools.py:398-400, src/basic_memory/repository/sqlite_search_repository.py:944-947, and src/basic_memory/repository/postgres_search_repository.py:1200-1203). The same lowercase mismatch recurs in the validation and exploration examples, leaving the proposed work queues and coverage checks empty.
Useful? React with 👍 / 👎.
| bm find --meta 'note_type=chapter' --fields chapter_number # every chapter present? | ||
| bm find --meta 'note_type=chapter' --fields pov,setting # any missing required context? |
There was a problem hiding this comment.
For the 100+ chapter runs this skill targets, these commands inspect only the first 10 matches because bm find --help reports a default --page-size of 10 (src/basic_memory/schemas/directory.py:8, used by src/basic_memory/cli/commands/posix.py:741-744). Although the response can report a total, the displayed chapter numbers and projected fields cannot reveal sequence gaps or null fields after page one, so the checks do not prove completeness unless they iterate every page.
Useful? React with 👍 / 👎.
| ```bash | ||
| bm find --meta 'note_type=chapter' --fields chapter_number # every chapter present? | ||
| bm find --meta 'note_type=chapter' --fields pov,setting # any missing required context? | ||
| bm find --name '*.md' --meta 'note_type=character' # entity inventory vs. seed list |
There was a problem hiding this comment.
This exact inventory command exits with find: 'name' cannot combine with 'meta'; the bm find manual explicitly states that --name is refused alongside metadata mode and recommends scoping with the positional path instead (src/basic_memory/man/man1/find(1).md:50-55). Consequently, the documented entity-inventory coverage check never produces an inventory.
Useful? React with 👍 / 👎.
| ] | ||
| } | ||
| ```bash | ||
| bm tool build-context --url 'memory://characters/major/*' --depth 2 # the character web |
There was a problem hiding this comment.
bm tool build-context --help declares URL as a required positional argument and has no --url option; the command's own examples likewise use bm tool build-context memory://... (src/basic_memory/cli/commands/tool.py:985-990). Running the newly documented command therefore exits during argument parsing with No such option: --url, preventing the Phase 5 traversal.
Useful? React with 👍 / 👎.
1 participant
Why
The
memory-literary-analysispipeline predates the POSIX surface, so it is written entirely inwrite_note/edit_note/search_notes/list_directory. A 138-chapter run following it would never callfind --metaorcat --section— which means the planned Moby Dick re-run would measure nothing about the tools we just built and shipped. Updating the skill is a prerequisite for that eval, not polish.Also removes the canvas instructions: that tool was taken out.
What changed
--metapredicates and--fieldsprojection are for), and never read a whole file to reach one part of it. States the fallback plainly for anyone without the verbs.grep -nfor headings), then read chapters by line range instead of reloading the source text per chapter — the largest single read saving on a long work, and it survives context compaction.find --meta ... --fieldsinstead of re-reading every note to see which are thin. Null fields are the work queue.bm orphans— framed around relation density, since a pass that adds notes while leaving orphans has made the graph worse.analysis/synthesis notes.Verification
just package-check-skills— 15 skills validated. No canvas references remain anywhere in the packaged skills.Notes
Temporal-qualifier authoring guidance (SPEC-82) will be a follow-up to this once that lands — chapter
[event]observations and character[arc]observations are where narrative time gets authored, and that is what makes the Moby Dick run the validation for temporal semantics.🤖 Generated with Claude Code
https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp