Add sdpub coordination and queue workflow#65
Merged
Conversation
Summary by CodeRabbit
WalkthroughThis PR replaces the Possibly Related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
docs/en/cli.md (1)
15-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCLI reference still advertises removed
buildcommand.Line 15 and Lines 59-68 still define
spinedigest buildand build-centric stage guidance, which conflicts with the queue-based workflow introduced in this PR. Update these sections to document queue equivalents so the reference stays executable.Also applies to: 59-68
🤖 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 `@docs/en/cli.md` at line 15, The CLI documentation in the spinedigest build command reference on line 15 and the build-centric stage guidance on lines 59-68 reference a command that has been removed in favor of a queue-based workflow. Replace the spinedigest build command syntax and all related stage guidance with documentation of the queue equivalents. Update both sections to reflect the new queue-based workflow so that the documented commands are actually executable and match the implementation changes introduced in this PR.docs/en/quickstart.md (1)
50-60:⚠️ Potential issue | 🟠 Major | ⚡ Quick winQuickstart still instructs deprecated
buildflow instead of queue workflow.Lines 50-60 and Lines 87-90 continue to describe knowledge generation as direct
buildcommands. Please migrate this to the queue lifecycle (queue add ... --accept-cost, thenqueue list/status/watch) so first-run guidance matches current CLI behavior.Also applies to: 87-90
🤖 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 `@docs/en/quickstart.md` around lines 50 - 60, The quickstart documentation in the "Build Knowledge" section uses deprecated spinedigest build commands instead of the current queue workflow. Replace the direct spinedigest build ./book.sdpub --stage graph --confirm command with the new queue lifecycle: use spinedigest queue add ./book.sdpub --stage graph --accept-cost to add the job, then follow with spinedigest queue list, spinedigest queue status, or spinedigest queue watch to monitor progress. Apply this same change to both the general workflow example (around lines 50-60) and the scoped work example showing the --chapter 3 flag (around lines 87-90) so all instructions reflect the current queue-based workflow instead of the deprecated direct build approach.README_zh-CN.md (1)
78-82:⚠️ Potential issue | 🔴 CriticalBuild command example should be replaced with queue workflow.
Line 81 shows
spinedigest build ./book.sdpub --stage graph --confirm, which should be updated to use the queue-based system per the PR objectives. Replace this example withspinedigest queue addor the appropriate queue command.🤖 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 `@README_zh-CN.md` around lines 78 - 82, The example command in the code block demonstrates the old build approach using spinedigest build with flags, but it should be updated to use the new queue-based system as per the PR objectives. Replace the spinedigest build command example with the appropriate queue command (such as spinedigest queue add) to reflect the updated workflow for building derived knowledge when confirming LLM costs.README.md (1)
78-82:⚠️ Potential issue | 🔴 CriticalBuild command example should be replaced with queue workflow.
The code block at lines 78–82 documents
spinedigest build ./book.sdpub --stage graph --confirm. Per the PR objectives, thebuildcommand is removed and replaced with a queue-based system. Update this example to usespinedigest queue addor the appropriate queue command instead.🤖 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 `@README.md` around lines 78 - 82, The README.md documentation at lines 78-82 contains an outdated example using the spinedigest build command, which has been removed and replaced with a queue-based system per the PR objectives. Replace the example command `spinedigest build ./book.sdpub --stage graph --confirm` with the appropriate queue workflow command, such as `spinedigest queue add` or the correct queue command that achieves the same goal of building derived knowledge. Ensure the documentation accurately reflects the new queue-based workflow for users.docs/zh-CN/cli.md (1)
15-15:⚠️ Potential issue | 🔴 CriticalBuild command documentation should be removed or replaced with queue workflow.
Line 15 still documents
spinedigest build <archive.sdpub> ..., but per the PR objectives, thebuildcommand is being removed and replaced with a queue-based system. This command reference and any associated help text should be updated to point tospinedigest queueinstead.🤖 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 `@docs/zh-CN/cli.md` at line 15, The CLI documentation in docs/zh-CN/cli.md still contains the old spinedigest build command reference that is being removed in this PR. Replace the documentation line showing spinedigest build with its archive parameter and options with documentation for the new spinedigest queue command that replaces it. Ensure the updated documentation accurately reflects the queue-based workflow instead of the deprecated build command.docs/zh-CN/quickstart.md (1)
50-60:⚠️ Potential issue | 🔴 CriticalBuild command examples should be replaced with queue-based workflow.
The "## 5. 构建知识" section (lines 50–60) still shows
spinedigest buildcommand examples. Per the PR objectives, thebuildcommand is removed and replaced with a queue-based system. Update this section to demonstrate the newspinedigest queue addworkflow instead.🤖 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 `@docs/zh-CN/quickstart.md` around lines 50 - 60, The section titled "## 5. 构建知识" contains outdated command examples using the spinedigest build command, which has been removed and replaced with a queue-based workflow. Replace both command examples (the full build example and the chapter-specific build example) with the equivalent spinedigest queue add commands that demonstrate how to queue the same operations using the new queue-based system.
🧹 Nitpick comments (3)
src/facade/sdpub-coordinator.ts (1)
437-612: ⚖️ Poor tradeoffDuplicated utility functions across coordinator and build-queue modules.
Functions like
isProcessAlive,delay,getString,getNumber,getOptionalString,createArchiveKeyare duplicated betweensdpub-coordinator.tsandbuild-queue.ts. Consider extracting to a shared internal utility module to reduce maintenance burden.🤖 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 `@src/facade/sdpub-coordinator.ts` around lines 437 - 612, Extract the utility functions isProcessAlive, delay, getString, getNumber, getOptionalString, getOptionalNumber, and createArchiveKey from sdpub-coordinator.ts into a new shared internal utility module. Import and use these functions from the new module in both sdpub-coordinator.ts and build-queue.ts to eliminate duplication and improve maintainability across these modules.src/facade/build-queue.ts (1)
767-802: 💤 Low valueStale job recovery emits requeued event with old job reference.
At line 794,
appendBuildJobEvent(job, ...)uses the oldjobobject (which hasstate: "running") but emits a "requeued" event withstate: "queued". The event is written correctly, but the job object is stale after the UPDATE. This is minor since the event itself contains the correct state, but for consistency the pattern elsewhere fetches the updated job before emitting.🤖 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 `@src/facade/build-queue.ts` around lines 767 - 802, In the recoverStaleBuildJobs function, after the UPDATE statement modifies the build_jobs table to set state to 'queued', fetch the updated job record from the database before calling appendBuildJobEvent. Currently the function passes the stale job object (which still has state: "running") to appendBuildJobEvent, but for consistency with the pattern used elsewhere in the codebase, you should query the database to get the refreshed job object with its updated state and pass that refreshed job object to the appendBuildJobEvent call instead of the original job variable.test/cli/archive-chapter.test.ts (1)
96-96: ⚡ Quick winAdd explicit assertions for new build-job guard behavior.
Line 96 adds the mock, but the updated mutation paths should also assert
assertNoActiveBuildJobscalls (includingchapterIdsshape andrequiresTargetcases) so these safety checks don’t regress silently.🧪 Example assertion pattern
+expect(assertNoActiveBuildJobs).toHaveBeenCalledWith( + expect.objectContaining({ + archivePath: "/tmp/book.sdpub", + chapterIds: expect.any(Array), + }), +);🤖 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 `@test/cli/archive-chapter.test.ts` at line 96, The mock for assertNoActiveBuildJobs is set up on line 96 but lacks verification assertions to ensure it's being called correctly. Add explicit test assertions after the mutation calls to verify that the assertNoActiveBuildJobs mock was invoked with the expected parameters, specifically checking the chapterIds shape and ensuring the mock is called in both the requiresTarget true and false scenarios. Use the vi.fn() mock's call history to assert the function was called once with the correct arguments structure.
🤖 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 `@data/help/commands/archive/pack.jinja`:
- Line 8: The usage signature for the spinedigest pack command on line 8
documents a positional <selector> argument, but the examples on lines 18-19 use
flag-based selector syntax. Update the usage line to replace the positional
<selector> parameter with a flag-based equivalent (such as [--selector
<selector>]) to match what the examples actually demonstrate, ensuring users
copy the correct syntax from the documentation.
In `@data/help/commands/root.jinja`:
- Line 47: The documentation for the spinedigest pack command on line 47
currently shows selector as a positional argument `<selector>`, but the actual
CLI implementation uses flag-based selector targeting instead. Remove the
positional `<selector>` argument and replace it with the appropriate flag form
(such as `--selector <value>` or similar flag syntax that matches the actual CLI
implementation) so that users can copy and paste the documented command without
errors.
In `@data/help/topics/ai.jinja`:
- Line 31: The spinedigest grep command example on line 31 is missing the
required --type flag that is mandated by the search-lens contract. Update the
grep example to include the --type flag with an appropriate type value. The
command should show the full required syntax including the --type parameter so
that documentation accurately reflects the actual command requirements.
- Line 41: The pack command example on line 41 uses legacy positional selector
syntax with the <selector> parameter that no longer matches the current archive
selector contract. Update the example to replace the positional selector
argument with the appropriate selector flags (such as filter or include flags)
that align with the current archive selector specification. Remove the
positional <selector> placeholder and substitute it with the correct flag-based
selector syntax to provide users with an accurate, up-to-date example of the
pack command usage.
In `@data/help/topics/command.jinja`:
- Around line 46-47: The command shape examples for spinedigest page,
spinedigest read, and spinedigest pack subcommands are showing positional
selector arguments, but the actual CLI parsing requires explicit selector flags.
Replace the positional <selector> placeholder in these command examples with the
appropriate explicit flag syntax (such as --selector or similar flag format used
by the actual CLI). This change should be applied to all three subcommand
examples to ensure the documentation accurately reflects the supported CLI
parsing behavior.
In `@docs/en/ai-agents.md`:
- Around line 43-44: In the documentation section covering expensive operations
(around lines 43-44 and 59-68), replace all references to the `spinedigest
build` command and `--confirm` flag with the new queue-based workflow. Update
the guidance to show users how to use `queue add` to submit expensive
operations, `queue list` and `queue watch` to monitor operations, and
`--accept-cost` to acknowledge costs instead of the deprecated build
confirmation pattern. Ensure the documentation clearly explains that graph or
summary operations should now be submitted through the queue workflow rather
than direct build commands.
In `@docs/zh-CN/ai-agents.md`:
- Line 43: The AI agents guide in docs/zh-CN/ai-agents.md contains three
outdated references to the legacy spinedigest build command that need to be
updated to use the queue-based workflow. First, replace the phrase "graph 或
summary build" on line 43 with language describing expensive queue operations.
Second, update the step 11 reference from "build 前先 estimate" to reflect the
queue command workflow. Third, replace the entire "构建流程" (Build Flow) section
(lines 61-68) which contains example build commands with new examples
demonstrating the spinedigest queue add command with appropriate flags like
--chapter, --node, --fragment, --summary, --meta book, --to summary, and
--accept-cost, along with spinedigest queue watch commands to monitor the job.
Ensure all examples reflect the flag-based selector syntax already documented in
the guide.
In `@docs/zh-CN/cli.md`:
- Line 19: Add documentation for the `--id <ids>` parameter in the "对象 ID"
section of the CLI documentation. The `--id` parameter filters collection items
by their stable ID and requires exactly one `--type` to be specified. Document
the valid ID formats for each type with concrete examples such as `--id
chapter:3` for chapters and `--id node:abc123` for nodes, explaining how users
should format their ID queries based on the object type they are filtering.
In `@src/cli/args.ts`:
- Around line 808-815: The case block for "clean" and "worker" actions is
currently including llmJSON in the returned args object when values.llm is
defined, but the execution paths for these actions do not consume this flag,
creating a misleading no-op option. Remove the conditional spread of llmJSON
from the return statement for these two actions so that the args object only
contains the action field, ensuring the --llm flag is not accepted for queue
clean and queue worker commands.
- Around line 756-781: The switch case for queue actions (status, pause, resume,
cancel, boost) currently accepts and includes the watch-only flags `--from` and
`--jsonl` in the returned arguments, even though these flags should only be
valid for the watch action. Add validation after the parseWatchFrom call to
check if values.from or values.jsonl are defined for these non-watch actions,
and throw an error with a helpful message using withHelpRoute indicating that
these flags are only valid for the watch action. Only include the from and jsonl
fields in the returned args object when these flags are provided for the watch
action.
In `@src/cli/queue.ts`:
- Around line 213-235: The assertJobStillRunning checks in the job processing
flow (such as before the generateChapterSummary call and its related operations)
occur before the actual flush/commit of editable session changes, creating a
race condition where canceled or paused jobs can still write data. Capture a
version or lease token from the job state when calling getBuildJob for the
initial assertJobStillRunning check, then validate that this version token
remains unchanged in the same critical section where the editable session is
flushed or committed (e.g., within generateChapterSummary or where its results
are persisted). This ensures the cancellation check and the data flush are
atomic and cannot be separated by job state changes. Apply this pattern to all
similar job processing blocks including graph generation (referenced in lines
239-243).
In `@src/facade/spine-digest.ts`:
- Around line 116-118: The error message thrown when a chapter summary is
missing contains outdated command guidance that references the removed direct
build flow. Update the error message in the throw statement to replace the stale
`spinedigest build` command hint with the correct queue command syntax that
reflects the new command flow, ensuring the message provides users with the
accurate guidance for generating the missing summary.
In `@src/output/epub/book.ts`:
- Line 76: The error message for missing chapter summary on line 76 still
references the outdated `spinedigest build` command that has been replaced with
a queue flow in this PR. Update the error message text in the string to replace
the guidance that tells users to run `spinedigest build <archive.sdpub> --stage
summary --confirm` with the new queue flow command instead, ensuring the error
message directs users to the current workflow for handling missing summaries.
In `@src/output/plain-text.ts`:
- Line 69: The error message at line 69 in the plain-text.ts file references the
removed spinedigest build command in its guidance text. Update the error message
that starts with "Chapter ${item.serialId} summary is missing" to remove the
reference to running spinedigest build and keep only the valid alternative
instruction about using spinedigest page to inspect the chapter. Ensure the
remaining guidance is clear and helpful without referencing the deprecated build
workflow.
In `@src/serial.ts`:
- Line 588: The error message in src/serial.ts for the missing chapter summary
contains a stale command reference to `spinedigest build <archive.sdpub> --stage
summary --confirm` which is no longer valid with the queue-based workflow
introduced in this PR. Update the error message string that includes `Chapter
${serialId} summary is missing` to reference the correct command or workflow
that users should follow for the new queue-based system, ensuring the
remediation instructions are accurate and actionable for users encountering this
error.
---
Outside diff comments:
In `@docs/en/cli.md`:
- Line 15: The CLI documentation in the spinedigest build command reference on
line 15 and the build-centric stage guidance on lines 59-68 reference a command
that has been removed in favor of a queue-based workflow. Replace the
spinedigest build command syntax and all related stage guidance with
documentation of the queue equivalents. Update both sections to reflect the new
queue-based workflow so that the documented commands are actually executable and
match the implementation changes introduced in this PR.
In `@docs/en/quickstart.md`:
- Around line 50-60: The quickstart documentation in the "Build Knowledge"
section uses deprecated spinedigest build commands instead of the current queue
workflow. Replace the direct spinedigest build ./book.sdpub --stage graph
--confirm command with the new queue lifecycle: use spinedigest queue add
./book.sdpub --stage graph --accept-cost to add the job, then follow with
spinedigest queue list, spinedigest queue status, or spinedigest queue watch to
monitor progress. Apply this same change to both the general workflow example
(around lines 50-60) and the scoped work example showing the --chapter 3 flag
(around lines 87-90) so all instructions reflect the current queue-based
workflow instead of the deprecated direct build approach.
In `@docs/zh-CN/cli.md`:
- Line 15: The CLI documentation in docs/zh-CN/cli.md still contains the old
spinedigest build command reference that is being removed in this PR. Replace
the documentation line showing spinedigest build with its archive parameter and
options with documentation for the new spinedigest queue command that replaces
it. Ensure the updated documentation accurately reflects the queue-based
workflow instead of the deprecated build command.
In `@docs/zh-CN/quickstart.md`:
- Around line 50-60: The section titled "## 5. 构建知识" contains outdated command
examples using the spinedigest build command, which has been removed and
replaced with a queue-based workflow. Replace both command examples (the full
build example and the chapter-specific build example) with the equivalent
spinedigest queue add commands that demonstrate how to queue the same operations
using the new queue-based system.
In `@README_zh-CN.md`:
- Around line 78-82: The example command in the code block demonstrates the old
build approach using spinedigest build with flags, but it should be updated to
use the new queue-based system as per the PR objectives. Replace the spinedigest
build command example with the appropriate queue command (such as spinedigest
queue add) to reflect the updated workflow for building derived knowledge when
confirming LLM costs.
In `@README.md`:
- Around line 78-82: The README.md documentation at lines 78-82 contains an
outdated example using the spinedigest build command, which has been removed and
replaced with a queue-based system per the PR objectives. Replace the example
command `spinedigest build ./book.sdpub --stage graph --confirm` with the
appropriate queue workflow command, such as `spinedigest queue add` or the
correct queue command that achieves the same goal of building derived knowledge.
Ensure the documentation accurately reflects the new queue-based workflow for
users.
---
Nitpick comments:
In `@src/facade/build-queue.ts`:
- Around line 767-802: In the recoverStaleBuildJobs function, after the UPDATE
statement modifies the build_jobs table to set state to 'queued', fetch the
updated job record from the database before calling appendBuildJobEvent.
Currently the function passes the stale job object (which still has state:
"running") to appendBuildJobEvent, but for consistency with the pattern used
elsewhere in the codebase, you should query the database to get the refreshed
job object with its updated state and pass that refreshed job object to the
appendBuildJobEvent call instead of the original job variable.
In `@src/facade/sdpub-coordinator.ts`:
- Around line 437-612: Extract the utility functions isProcessAlive, delay,
getString, getNumber, getOptionalString, getOptionalNumber, and createArchiveKey
from sdpub-coordinator.ts into a new shared internal utility module. Import and
use these functions from the new module in both sdpub-coordinator.ts and
build-queue.ts to eliminate duplication and improve maintainability across these
modules.
In `@test/cli/archive-chapter.test.ts`:
- Line 96: The mock for assertNoActiveBuildJobs is set up on line 96 but lacks
verification assertions to ensure it's being called correctly. Add explicit test
assertions after the mutation calls to verify that the assertNoActiveBuildJobs
mock was invoked with the expected parameters, specifically checking the
chapterIds shape and ensuring the mock is called in both the requiresTarget true
and false scenarios. Use the vi.fn() mock's call history to assert the function
was called once with the correct arguments structure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f43495b-99f0-4e34-8eca-14ef59476e09
📒 Files selected for processing (67)
README.mdREADME_zh-CN.mddata/help/commands/archive/backlinks.jinjadata/help/commands/archive/build.jinjadata/help/commands/archive/estimate.jinjadata/help/commands/archive/find.jinjadata/help/commands/archive/grep.jinjadata/help/commands/archive/links.jinjadata/help/commands/archive/list.jinjadata/help/commands/archive/pack.jinjadata/help/commands/archive/page.jinjadata/help/commands/archive/path.jinjadata/help/commands/archive/read.jinjadata/help/commands/archive/related.jinjadata/help/commands/maintenance/chapter.jinjadata/help/commands/maintenance/chapter/add.jinjadata/help/commands/maintenance/chapter/list.jinjadata/help/commands/maintenance/chapter/reset.jinjadata/help/commands/maintenance/chapter/set-source.jinjadata/help/commands/maintenance/chapter/set-summary.jinjadata/help/commands/maintenance/chapter/status.jinjadata/help/commands/queue.jinjadata/help/commands/root.jinjadata/help/topics/ai.jinjadata/help/topics/command.jinjadata/help/topics/config-file.jinjadata/help/topics/config.jinjadata/help/topics/env.jinjadata/help/topics/format.jinjadata/help/topics/overview.jinjadata/help/topics/recipe.jinjadata/help/topics/runtime.jinjadata/help/topics/task.jinjadata/help/topics/troubleshoot.jinjadocs/en/ai-agents.mddocs/en/cli.mddocs/en/quickstart.mddocs/zh-CN/ai-agents.mddocs/zh-CN/cli.mddocs/zh-CN/quickstart.mdsrc/cli/archive-chapter.tssrc/cli/archive.tssrc/cli/args.tssrc/cli/help.tssrc/cli/io.tssrc/cli/main.tssrc/cli/queue.tssrc/facade/archive-view.tssrc/facade/build-queue.tssrc/facade/chapter.tssrc/facade/index.tssrc/facade/sdpub-coordinator.tssrc/facade/spine-digest-file.tssrc/facade/spine-digest.tssrc/output/epub/book.tssrc/output/plain-text.tssrc/serial.tstest/cli/archive-chapter.test.tstest/cli/archive.test.tstest/cli/args.test.tstest/cli/io.test.tstest/cli/main.test.tstest/cli/queue.test.tstest/facade/build-queue.test.tstest/facade/chapter-graph.test.tstest/facade/spine-digest-file.test.tstest/facade/spine-digest.test.ts
💤 Files with no reviewable changes (1)
- data/help/commands/archive/build.jinja
|
|
||
| Usage: | ||
| spinedigest pack <archive.sdpub> <id> [--budget <chars>] [--json] | ||
| spinedigest pack <archive.sdpub> <selector> [--budget <chars>] [--json] |
There was a problem hiding this comment.
Fix selector syntax mismatch between Usage and Examples.
Line 8 documents a positional <selector>, but Lines 18-19 use flag selectors. Please make the usage signature match the examples to avoid incorrect copy/paste commands.
Proposed doc fix
- spinedigest pack <archive.sdpub> <selector> [--budget <chars>] [--json]
+ spinedigest pack <archive.sdpub> (--chapter <id>|--node <id>|--fragment <chapter>:<fragment>|--summary <id>|--meta book) [--budget <chars>] [--json]Also applies to: 18-19
🤖 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 `@data/help/commands/archive/pack.jinja` at line 8, The usage signature for the
spinedigest pack command on line 8 documents a positional <selector> argument,
but the examples on lines 18-19 use flag-based selector syntax. Update the usage
line to replace the positional <selector> parameter with a flag-based equivalent
(such as [--selector <selector>]) to match what the examples actually
demonstrate, ensuring users copy the correct syntax from the documentation.
| spinedigest backlinks <archive.sdpub> --node <id> [--json] | ||
| spinedigest related <archive.sdpub> --node <id> [--json] | ||
| spinedigest path <archive.sdpub> --from <id> --to <id> --chapter <id> | ||
| spinedigest pack <archive.sdpub> <selector> [--budget <chars>] [--json] |
There was a problem hiding this comment.
Pack usage still shows an unsupported positional selector.
Line 47 documents pack with <selector>, but CLI parsing is flag-selector based for object targeting. Please switch this to the flag form to avoid a failing copy/paste command.
Suggested doc fix
- spinedigest pack <archive.sdpub> <selector> [--budget <chars>] [--json]
+ spinedigest pack <archive.sdpub> (--chapter <id>|--node <id>|--fragment <chapter>:<fragment>|--summary <id>|--meta book) [--budget <chars>] [--json]📝 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.
Suggested change
| spinedigest pack <archive.sdpub> <selector> [--budget <chars>] [--json] | |
| spinedigest pack <archive.sdpub> (--chapter <id>|--node <id>|--fragment <chapter>:<fragment>|--summary <id>|--meta book) [--budget <chars>] [--json] |
🤖 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 `@data/help/commands/root.jinja` at line 47, The documentation for the
spinedigest pack command on line 47 currently shows selector as a positional
argument `<selector>`, but the actual CLI implementation uses flag-based
selector targeting instead. Remove the positional `<selector>` argument and
replace it with the appropriate flag form (such as `--selector <value>` or
similar flag syntax that matches the actual CLI implementation) so that users
can copy and paste the documented command without errors.
| spinedigest find <archive.sdpub> <query> --type fragment | ||
| spinedigest page <archive.sdpub> fragment:<serial>:<fragment> | ||
| spinedigest page <archive.sdpub> --fragment <chapter>:<fragment> | ||
| spinedigest grep <archive.sdpub> <exact phrase> |
There was a problem hiding this comment.
grep example is missing the required --type flag.
Line 31 currently documents a command form that conflicts with the required search-lens contract.
Suggested doc fix
- spinedigest grep <archive.sdpub> <exact phrase>
+ spinedigest grep <archive.sdpub> <exact phrase> --type fragment📝 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.
Suggested change
| spinedigest grep <archive.sdpub> <exact phrase> | |
| spinedigest grep <archive.sdpub> <exact phrase> --type fragment |
🤖 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 `@data/help/topics/ai.jinja` at line 31, The spinedigest grep command example
on line 31 is missing the required --type flag that is mandated by the
search-lens contract. Update the grep example to include the --type flag with an
appropriate type value. The command should show the full required syntax
including the --type parameter so that documentation accurately reflects the
actual command requirements.
| spinedigest related <archive.sdpub> --node <id> | ||
| - Need a reusable context projection: | ||
| spinedigest pack <archive.sdpub> <id> --budget 5000 | ||
| spinedigest pack <archive.sdpub> <selector> --budget 5000 |
There was a problem hiding this comment.
pack example still uses legacy positional selector syntax.
Line 41 should use selector flags to match the current archive selector contract.
Suggested doc fix
- spinedigest pack <archive.sdpub> <selector> --budget 5000
+ spinedigest pack <archive.sdpub> --node <id> --budget 5000📝 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.
Suggested change
| spinedigest pack <archive.sdpub> <selector> --budget 5000 | |
| spinedigest pack <archive.sdpub> --node <id> --budget 5000 |
🤖 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 `@data/help/topics/ai.jinja` at line 41, The pack command example on line 41
uses legacy positional selector syntax with the <selector> parameter that no
longer matches the current archive selector contract. Update the example to
replace the positional selector argument with the appropriate selector flags
(such as filter or include flags) that align with the current archive selector
specification. Remove the positional <selector> placeholder and substitute it
with the correct flag-based selector syntax to provide users with an accurate,
up-to-date example of the pack command usage.
Comment on lines
+46
to
+47
| spinedigest page <archive.sdpub> <selector> [--json] | ||
| spinedigest read <archive.sdpub> <selector> |
There was a problem hiding this comment.
Command shapes for page/read/pack still show unsupported positional selectors.
These lines should use explicit selector flags so examples match actual CLI parsing behavior.
Suggested doc fix
- spinedigest page <archive.sdpub> <selector> [--json]
- spinedigest read <archive.sdpub> <selector>
+ spinedigest page <archive.sdpub> (--chapter <id>|--node <id>|--fragment <chapter>:<fragment>|--summary <id>|--meta book) [--json]
+ spinedigest read <archive.sdpub> (--chapter <id>|--node <id>|--fragment <chapter>:<fragment>|--summary <id>|--meta book)
@@
- spinedigest pack <archive.sdpub> <selector> [--budget <chars>] [--json]
+ spinedigest pack <archive.sdpub> (--chapter <id>|--node <id>|--fragment <chapter>:<fragment>|--summary <id>|--meta book) [--budget <chars>] [--json]Also applies to: 49-49
🤖 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 `@data/help/topics/command.jinja` around lines 46 - 47, The command shape
examples for spinedigest page, spinedigest read, and spinedigest pack
subcommands are showing positional selector arguments, but the actual CLI
parsing requires explicit selector flags. Replace the positional <selector>
placeholder in these command examples with the appropriate explicit flag syntax
(such as --selector or similar flag format used by the actual CLI). This change
should be applied to all three subcommand examples to ensure the documentation
accurately reflects the supported CLI parsing behavior.
Comment on lines
+213
to
+235
| const latestJob = await getBuildJob(job.jobId); | ||
|
|
||
| assertJobStillRunning(latestJob); | ||
| if (latestJob.target === "graph" || details.stage === "summarized") { | ||
| return; | ||
| } | ||
| if (details.stage !== "graphed") { | ||
| details = await getChapterDetails(document, job.chapterId); | ||
| } | ||
| if (details.stage !== "graphed") { | ||
| throw new Error( | ||
| `Chapter ${job.chapterId} is ${details.stage}. Cannot generate summary.`, | ||
| ); | ||
| } | ||
|
|
||
| await reporter.stepStarted("summary"); | ||
| details = await generateChapterSummary(document, job.chapterId, { | ||
| llm, | ||
| }); | ||
| await reporter.updateWords({ summaryWords: details.words }); | ||
| await reporter.stepCompleted("summary"); | ||
| assertJobStillRunning(await getBuildJob(job.jobId)); | ||
| }, |
There was a problem hiding this comment.
Make cancellation checks atomic with archive flush.
assertJobStillRunning is a pre-flush check; job state can change immediately afterward, so canceled/paused jobs can still flush graph/summary writes.
A robust fix is to add a fencing token / lease version and validate it in the same critical section that commits or flushes the editable session, so state and write visibility cannot race.
Also applies to: 239-243
🤖 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 `@src/cli/queue.ts` around lines 213 - 235, The assertJobStillRunning checks in
the job processing flow (such as before the generateChapterSummary call and its
related operations) occur before the actual flush/commit of editable session
changes, creating a race condition where canceled or paused jobs can still write
data. Capture a version or lease token from the job state when calling
getBuildJob for the initial assertJobStillRunning check, then validate that this
version token remains unchanged in the same critical section where the editable
session is flushed or committed (e.g., within generateChapterSummary or where
its results are persisted). This ensures the cancellation check and the data
flush are atomic and cannot be separated by job state changes. Apply this
pattern to all similar job processing blocks including graph generation
(referenced in lines 239-243).
Comment on lines
116
to
118
| throw new Error( | ||
| `Chapter ${serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> chapter:${serialId}\`.`, | ||
| `Chapter ${serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${serialId}\`.`, | ||
| ); |
There was a problem hiding this comment.
Update stale command hint in summary-missing error message.
Line 117 still tells users to run spinedigest build, but this PR removes direct build flow in favor of queue commands, so this guidance is now incorrect.
Suggested fix
- `Chapter ${serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${serialId}\`.`,
+ `Chapter ${serialId} summary is missing. Queue summary generation with \`spinedigest queue add <archive.sdpub> --chapter ${serialId} --target summary --accept-cost\`, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${serialId}\`.`,📝 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.
Suggested change
| throw new Error( | |
| `Chapter ${serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> chapter:${serialId}\`.`, | |
| `Chapter ${serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${serialId}\`.`, | |
| ); | |
| throw new Error( | |
| `Chapter ${serialId} summary is missing. Queue summary generation with \`spinedigest queue add <archive.sdpub> --chapter ${serialId} --target summary --accept-cost\`, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${serialId}\`.`, | |
| ); |
🤖 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 `@src/facade/spine-digest.ts` around lines 116 - 118, The error message thrown
when a chapter summary is missing contains outdated command guidance that
references the removed direct build flow. Update the error message in the throw
statement to replace the stale `spinedigest build` command hint with the correct
queue command syntax that reflects the new command flow, ensuring the message
provides users with the accurate guidance for generating the missing summary.
| if (summary === undefined) { | ||
| throw new Error( | ||
| `Chapter ${item.serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> chapter:${item.serialId}\`.`, | ||
| `Chapter ${item.serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${item.serialId}\`.`, |
There was a problem hiding this comment.
Replace removed build guidance in the missing-summary error.
Line 76 still tells users to run spinedigest build ..., but this PR replaces build with queue flow. In this blocking error path, that sends users to a stale command.
💡 Suggested fix
- `Chapter ${item.serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${item.serialId}\`.`,
+ `Chapter ${item.serialId} summary is missing. Queue a summary job before export (for example, \`spinedigest queue add <archive.sdpub> --chapter ${item.serialId} --target summary --accept-cost\`), or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${item.serialId}\`.`,📝 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.
Suggested change
| `Chapter ${item.serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${item.serialId}\`.`, | |
| `Chapter ${item.serialId} summary is missing. Queue a summary job before export (for example, \`spinedigest queue add <archive.sdpub> --chapter ${item.serialId} --target summary --accept-cost\`), or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${item.serialId}\`.`, |
🤖 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 `@src/output/epub/book.ts` at line 76, The error message for missing chapter
summary on line 76 still references the outdated `spinedigest build` command
that has been replaced with a queue flow in this PR. Update the error message
text in the string to replace the guidance that tells users to run `spinedigest
build <archive.sdpub> --stage summary --confirm` with the new queue flow command
instead, ensuring the error message directs users to the current workflow for
handling missing summaries.
| if (summary === undefined) { | ||
| throw new Error( | ||
| `Chapter ${item.serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> chapter:${item.serialId}\`.`, | ||
| `Chapter ${item.serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${item.serialId}\`.`, |
There was a problem hiding this comment.
Replace removed build command in the error guidance.
Line 69 still tells users to run spinedigest build ..., but this workflow is removed in this PR. That guidance is now invalid and can strand users on the recovery path.
💡 Proposed fix
- `Chapter ${item.serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${item.serialId}\`.`,
+ `Chapter ${item.serialId} summary is missing. Queue a summary job first, e.g. \`spinedigest queue add <archive.sdpub> --chapter ${item.serialId} --target summary --accept-cost\`, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${item.serialId}\`.`,📝 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.
Suggested change
| `Chapter ${item.serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${item.serialId}\`.`, | |
| `Chapter ${item.serialId} summary is missing. Queue a summary job first, e.g. \`spinedigest queue add <archive.sdpub> --chapter ${item.serialId} --target summary --accept-cost\`, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${item.serialId}\`.`, |
🤖 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 `@src/output/plain-text.ts` at line 69, The error message at line 69 in the
plain-text.ts file references the removed spinedigest build command in its
guidance text. Update the error message that starts with "Chapter
${item.serialId} summary is missing" to remove the reference to running
spinedigest build and keep only the valid alternative instruction about using
spinedigest page to inspect the chapter. Ensure the remaining guidance is clear
and helpful without referencing the deprecated build workflow.
| if (summary === undefined) { | ||
| throw new Error( | ||
| `Chapter ${serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> chapter:${serialId}\`.`, | ||
| `Chapter ${serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${serialId}\`.`, |
There was a problem hiding this comment.
Update stale build-command remediation text here as well.
Line 588 still references spinedigest build ..., which conflicts with the queue-based workflow introduced in this PR and gives users an invalid recovery command.
💡 Suggested fix
- `Chapter ${serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${serialId}\`.`,
+ `Chapter ${serialId} summary is missing. Queue a summary job before export (for example, \`spinedigest queue add <archive.sdpub> --chapter ${serialId} --target summary --accept-cost\`), or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${serialId}\`.`,📝 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.
Suggested change
| `Chapter ${serialId} summary is missing. Run \`spinedigest build <archive.sdpub> --stage summary --confirm\` before export, or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${serialId}\`.`, | |
| `Chapter ${serialId} summary is missing. Queue a summary job before export (for example, \`spinedigest queue add <archive.sdpub> --chapter ${serialId} --target summary --accept-cost\`), or inspect the chapter with \`spinedigest page <archive.sdpub> --chapter ${serialId}\`.`, |
🤖 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 `@src/serial.ts` at line 588, The error message in src/serial.ts for the
missing chapter summary contains a stale command reference to `spinedigest build
<archive.sdpub> --stage summary --confirm` which is no longer valid with the
queue-based workflow introduced in this PR. Update the error message string that
includes `Chapter ${serialId} summary is missing` to reference the correct
command or workflow that users should follow for the new queue-based system,
ensuring the remediation instructions are accurate and actionable for users
encountering this error.
This was referenced Jun 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation