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 10 commits into
Conversation
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://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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e00c4e843c
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e00c4e843c
ℹ️ 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".
7f4aca5 to
ea34934
Compare
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>
…avior Every example in the POSIX rewrite was written without being run, and five did not work. All are now verified against a throwaway project: - Reading the source by line range needs the file inside the project and observed by an index pass; the skill documented neither. Ingesting it as .md silently shifts every line by the injected frontmatter, so an offset map built with grep -n would be wrong — keep it .txt. - note_type predicates need the authored casing (Chapter, not chapter), and the result row displays the snake-cased projection, so the output names a value that matches nothing. - find defaults to page-size 10 and caps at 200; coverage checks that claimed to prove completeness inspected the first ten rows. - --name cannot combine with --meta; scope with the positional path. - build-context takes URL positionally, not --url. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
The sequence check derived its range from the highest chapter number found, so a duplicate plus a missing tail reported missing: [] on an incomplete graph — the duplicate keeps the row count right and the absent chapter moves the goalpost. Reproduced with 138 rows numbered 1..137 and one duplicate. The check now takes the work's actual chapter count as $expected and reports duplicates alongside gaps; it passes only when has_more is false, missing is empty, and duplicates is empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
36b430f to
2148edd
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2148edd362
ℹ️ 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".
`cat --lines` is an output slice: the plain line-range path fetches the
full note through read_note_json_by_external_id and slices the payload
client-side (posix_tools.cat), and the server-side arm is no different —
knowledge_router._apply_note_slice cuts a full note response after cache
retrieval. Nothing does a ranged or partial read.
The literary-analysis skill framed line ranges as avoiding the read
("never read a whole file", "the single largest read saving"), which
could lead an agent to treat slicing as a cheap way to probe very large
files. Reword the line-slicing claims to what they actually buy: bounded
context, not bounded I/O.
Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dae05baf54
ℹ️ 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".
The casing guidance was inverted, and it made every metadata query in the skill return zero rows. `NoteType` carries a `BeforeValidator` running `to_snake_case`, so `write_note(note_type="Chapter")` is persisted as `type: chapter`. Confirmed against the real corpus: every chapter note in the Moby Dick project has `type: chapter`. The doc claimed the opposite — that a note authored as "Chapter" matches `--meta 'note_type=Chapter'` and not the lowercase form, and that the lowercase value a result row displays "is not the value to query with". Both backwards. Eight predicates followed that advice and would have matched nothing, silently, with exit 0 — the exact failure mode the section exists to warn about. Also stop `bm grep` from being presented as an exhaustive symbol search: it ranks semantically and pages at 10, so a symbol in 40 chapters comes back as 10. Use -F and a raised page size for symbol tracing, since the meaning shifts being hunted tend to be in the occurrences the default drops. Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dd4b585f1
ℹ️ 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".
The commands were fixed in 9dd4b58 but this adjacent paragraph still told readers the capitalized spelling was required, which is the advice that made every predicate match nothing. Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a31502e2b
ℹ️ 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".
The coverage check reported clean for an inventory holding a note the numbering does not account for — a prologue or epilogue typed as chapter lands at 0 or $expected+1, and missing/duplicates are both empty. Adds out_of_range to the report and the pass condition. Also note that --page-size raises grep's ceiling rather than removing it, so a symbol exceeding one page still needs --page walked to exhaustion. Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 732f101311
ℹ️ 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".
Verified in an isolated project holding one markdown note and one .txt: both index as entities, but a word present only in the .txt returns total: 0, while a word in both returns just the note. The source body is not in the searchable text. So a symbol search answers 'where have I written about this', not 'where does this appear in the book'. Says so, and points at the offset map for the latter. Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 792e599280
ℹ️ 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".
--page-size caps at 200, so the single-call check could never pass on a work with more than 200 chapters, and rerunning with --page 2 replaced the collected numbers instead of adding to them - reporting 1-200 missing on a complete corpus. Walks until has_more is false and checks the union. Verified against a stub returning 250 chapters over two pages: complete reports missing []; dropping chapter 210 reports missing [210]. Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d75b3a7af
ℹ️ 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".
A chapter note with no chapter_number comes back as null, and tonumber aborts the whole pipeline with 'null cannot be parsed as a number' - so the coverage check crashed on exactly the malformed inventory it exists to detect. Partition the rows first and report the offenders by title. Reproduced against a real project holding two numbered chapters and one without: the old form died on the null, the new one returns unnumbered: ["Chapter Three"], missing: [3]. Signed-off-by: phernandez <paul@basicmachines.co>
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