Add archive-first CLI workflows#57
Conversation
Summary by CodeRabbit
WalkthroughThis PR introduces SpineDigest v0.2.0, reframing the tool around a managed 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: 13
🧹 Nitpick comments (6)
docs/en/ai-agents.md (2)
59-63: ⚡ Quick winAddress repetitive sentence beginnings in the "Avoid" section.
Lines 60–63 start successive list items with "Do not". Consider varied phrasing to improve readability.
Example rewording to vary structure
## Avoid -- Do not unzip `.sdpub` for routine retrieval. -- Do not inspect `database.db` unless building external tooling or debugging internals. -- Do not run a full-archive ready build just because a user asked a question about the archive. -- Do not present SpineDigest as a natural-language QA layer; the agent answers after reading archive context. +- Avoid unzipping `.sdpub` for routine retrieval. +- Never inspect `database.db` unless building external tooling or debugging internals. +- Skip full-archive ready builds in response to casual user questions about the archive. +- Do not present SpineDigest as a natural-language QA layer; the agent answers after reading archive context.🤖 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/ai-agents.md` around lines 59 - 63, The "Avoid" section has four consecutive list items (lines 60-63) that all begin with "Do not", creating repetitive sentence structure. Reword at least two of these items to use different phrasing patterns (such as imperative verbs like "Avoid", "Never", "Skip", or "Refrain from") while preserving the original meaning and the directive nature of the guidance. This will improve readability through structural variety without changing the content or intent.
39-45: ⚡ Quick winAddress repetitive sentence beginnings in the execution strategy.
Lines 40–45 start successive sentences with the same verb ("Use"). Consider varying the sentence structure for readability.
Example rewording to vary structure
## Recommended Execution Strategy -1. For an unknown archive, run `status` and `index`. +1. Start with an unknown archive by running `status` and `index`. -2. Use `find` or `ls` to discover stable object ids. -3. Use `page` to read one object. -4. Use `evidence` before quoting or making source-backed claims. -5. Use `links`, `backlinks`, `path`, or `map` to navigate graph context. -6. Use `export` only when the user needs a projection. +2. Discover stable object ids with `find` or `ls`. +3. Read one object using `page`. +4. Before quoting or making source-backed claims, run `evidence`. +5. Navigate graph context with `links`, `backlinks`, `path`, or `map`. +6. Export projections only when the user needs them.🤖 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/ai-agents.md` around lines 39 - 45, The execution strategy list in the docs/en/ai-agents.md file contains repetitive sentence beginnings where lines 40-44 all start with "Use". Vary the sentence structure for better readability by rephrasing these items to use different grammatical constructions. Consider using alternative patterns like imperative verbs (e.g., "Run", "Execute"), gerunds (e.g., "Finding or listing"), or prepositional phrases (e.g., "To discover", "For navigating") while maintaining the same meaning and functionality described in each instruction. Reference lines 39 and 45 which already demonstrate varied sentence structures that can serve as models for the rewording.docs/zh-CN/ai-agents.md (1)
56-56: ⚡ Quick winFix grammatical issue with adjective usage on line 56.
The phrase "导入/源是安全第一步" is grammatically awkward. When using "是" with an adjective descriptor, clarify the relationship.
Suggested fix
-Import/source 是安全第一步。Graph 和 summary 阶段可能调用 LLM provider。 +导入/源是安全的第一步。Graph 和 summary 阶段可能调用 LLM provider。Or, if preserving the English phrasing:
-Import/source 是安全第一步。Graph 和 summary 阶段可能调用 LLM provider。 +Import/source 很安全，是第一步。Graph 和 summary 阶段可能调用 LLM provider。🤖 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/ai-agents.md` at line 56, The phrase "是安全第一步" (in the context of "Import/source 是安全第一步") is grammatically awkward because the structure of "是" paired with the adjective descriptor lacks clarity. Revise this phrase to clarify the relationship between the subject and the descriptor. The intended meaning should express that importing/sourcing is either a secure first step or the first step in security. Restructure the sentence to use either a more natural adjective placement or a more explicit construction that clearly connects "import/source" with the concept of being a security-focused first step.data/help/topics/command.jinja (1)
25-37: ⚡ Quick winIncomplete command shapes documentation.
The "Command shapes" section documents 12 of the 17 archive-first commands but omits:
grep,links,backlinks,path, andmap. While these may be less commonly used, users would benefit from seeing the expected syntax for all commands in one reference.📝 Proposed addition
Add the missing command shapes after line 34:
spinedigest evidence <archive.sdpub> <id> [--json] + spinedigest links <archive.sdpub> <node:id> [--json] + spinedigest backlinks <archive.sdpub> <node:id> [--json] spinedigest related <archive.sdpub> <node:id> [--json] + spinedigest path <archive.sdpub> <from:node:id> <to:node:id> [--json] + spinedigest map <archive.sdpub> [--json] spinedigest pack <archive.sdpub> <id> [--budget <chars>] [--json] + spinedigest grep <archive.sdpub> <query> [--json] spinedigest export <archive.sdpub> --output-format <format> [--output <path>]🤖 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 25 - 37, The Command shapes section in the documentation is incomplete, missing syntax definitions for five archive-first commands: grep, links, backlinks, path, and map. Add the command shape documentation for these five missing commands following the same format as the existing commands (showing the command name, archive parameter, and relevant options). Insert these additional command shapes after the export command definition to provide complete documentation of all 17 archive-first commands in one reference.data/help/commands/archive/status.jinja (1)
14-14: 💤 Low valueClarify the distinction between archive and config status commands.
The note mentions "bare
spinedigest statusshows config status," but this help page documentsspinedigest status <archive.sdpub>. Users may be confused about when to use which form. Consider expanding the note to explicitly state: "To check CLI configuration status instead of archive status, runspinedigest statuswithout an archive path."🤖 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/status.jinja` at line 14, The note on line 14 currently states "bare `spinedigest status` shows config status" which is ambiguous and may confuse users about which command variant to use. Replace this note with a clearer explanation that explicitly distinguishes the two forms: state that running `spinedigest status` without an archive path checks CLI configuration status, while the command documented on this page (`spinedigest status <archive.sdpub>`) checks archive status instead. This will help users understand when each form is appropriate.data/help/commands/archive/path.jinja (1)
8-8: ⚡ Quick winInconsistent placeholder notation for node IDs across help templates.
Both
path.jinjaandrelated.jinjause<node:id>embedding the type prefix inside angle brackets, while other help templates (pack.jinja,page.jinja) use<id>and demonstrate the prefix in examples. This inconsistency may confuse users about placeholder syntax.
data/help/commands/archive/path.jinja#L8: Change<node:id>to<id>and clarify in the contract or example that both arguments must be node IDs.data/help/commands/archive/related.jinja#L8: Change<node:id>to<id>to match the convention used in other templates.🤖 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/path.jinja` at line 8, Standardize the placeholder notation for node ID arguments to match the convention used in other help templates. In data/help/commands/archive/path.jinja at line 8, change both occurrences of `<node:id>` to `<id>` to align with the notation in pack.jinja and page.jinja templates. Apply the same change in data/help/commands/archive/related.jinja at line 8, replacing `<node:id>` with `<id>`. This ensures consistent placeholder syntax across all help templates and clarifies that both arguments are node IDs.
🤖 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/import.jinja`:
- Around line 10-14: The help documentation for the import command in
import.jinja contains a contradiction between the Usage section (line 8) which
lists --llm and --prompt as available options, and the Contract section (lines
10-14) which states "LLM: no". Resolve this contradiction by either removing
--llm and --prompt from the Usage section if they are not actually supported by
the import command, or update the Contract section to accurately describe how
these flags are handled (e.g., whether they are parsed but unused, or used in a
specific processing stage) to match what the parseArchiveArguments function in
src/cli/args.ts actually implements.
In `@data/help/commands/archive/pack.jinja`:
- Line 14: Update the text on line 14 in the pack.jinja file to reflect all
available command sources that produce object IDs. The current text mentions
only "index, ls, find, page, or links" but the implementation also supports
additional commands: grep, evidence, backlinks, related, and map. Expand the
list in the help text to include all these commands so the documentation
accurately describes all valid ID sources.
In `@data/help/commands/archive/page.jinja`:
- Line 18: The example in the archive page template uses the fragment:1:0 format
for the spinedigest page command, but this format is not documented in the
general object ID reference sections. To resolve this inconsistency, either
update the example in the archive page template to use the documented
sentence:<serial>:<fragment>:<index> format instead of fragment:1:0, or add the
fragment:<serial>:<id> format definition to the general object ID documentation
sections in both the root.jinja and ai.jinja files to document that both formats
are supported.
In `@data/help/commands/archive/path.jinja`:
- Line 13: The `path` command currently accepts the `--json` flag in its
argument parser but ignores it during execution, creating inconsistency with the
documented behavior stating `JSON: no`. You must choose one of two approaches to
resolve this: either implement JSON output support for the `path` command to
align with similar commands like `pack`, `page`, `evidence`, `links`,
`backlinks`, and `related` (returning structured path data in JSON format when
the flag is used), or explicitly reject the `--json` flag in the argument parser
configuration for the `path` command to ensure the documented behavior matches
the actual behavior and prevent users from using an unsupported flag. Update the
documentation accordingly once you choose your approach.
In `@data/help/topics/command.jinja`:
- Around line 27-28: The help documentation uses inconsistent stage terminology
across multiple locations, creating confusion about the relationship between
build targets and archive states. In data/help/topics/command.jinja lines 27-28,
the build and estimate commands document stages as `source|graph|summary|ready`,
but in data/help/topics/command.jinja line 40, the direct digest command
documents stages as `planned|sourced|graphed|summarized`, and in
data/help/topics/format.jinja lines 26-36, the "Readiness by archive stage"
section also uses `planned/sourced/graphed/summarized` while mentioning "ready
builds". Choose one canonical set of stage names and update all three locations
to use the same terminology consistently, ensuring the stage values align across
the build command documentation, the digest command documentation, and the
archive readiness description.
In `@docs/zh-CN/quickstart.md`:
- Around line 101-103: The spinedigest status command example is missing the
required archive path argument. Update the command from spinedigest status to
include an explicit path to the .sdpub archive file as an argument, so readers
can successfully run the command without errors. This aligns with the
archive-first pattern established elsewhere in the documentation.
In `@src/cli/archive.ts`:
- Around line 213-214: The fetch call for args.sourcePath in archive.ts lacks a
timeout mechanism, allowing slow or unresponsive endpoints to block the CLI
indefinitely. Add an AbortController to implement a timeout for the fetch
operation. Create an AbortController instance, set up a timeout that will abort
the request after a reasonable duration (e.g., 30 seconds), and pass the abort
signal to the fetch call's options parameter. Ensure the timeout is cleared if
the fetch completes successfully to prevent resource leaks.
- Around line 221-226: The URL import branch unconditionally calls
response.text() which assumes the source is text-based, but args.inputFormat may
specify a non-text format (like binary), corrupting the source data before
conversion. Add a check on args.inputFormat before the response.text() call: if
the format is non-text, either reject the URL import with an appropriate error
message, or use response.arrayBuffer() and byte-preserving handling instead.
Only call response.text() and formatFetchedUrlSource() when the input format is
actually text-based.
In `@src/cli/args.ts`:
- Around line 297-299: The `--budget` and `--confirm` flags are currently
registered globally in the parseArgs function, which causes them to be silently
accepted by all commands including help, status, and sdpub where they are not
valid. Remove these flag definitions from the global argument registration in
parseArgs (around line 297-299 and 367-369 in src/cli/args.ts) and instead
register them only in the command-specific argument definitions for the commands
that actually support them, such as the convert command. This will ensure that
using these flags with unsupported commands properly rejects them with an error
rather than silently ignoring the invalid usage.
- Around line 2525-2545: Remove the unsupported "fragments" entry from the
isArchiveAction type guard function. The "fragments" action is not part of the
CLIArchiveAction type definition and is not handled in parseArchiveArguments, so
it should not be included in the type guard check. Simply delete the line
containing value === "fragments" || from the function to prevent unhandled
runtime paths for spinedigest fragments commands.
In `@src/facade/archive-view.ts`:
- Around line 428-444: The packArchiveContext function accepts a budget
parameter but does not use it to limit the returned evidence and links, making
the budget feature non-functional. After retrieving the evidence and links from
readArchiveEvidence and listAllArchiveLinks respectively, apply the budget
constraint to limit the size or quantity of these results before returning them
in the ArchivePack object. This ensures that the budget parameter actually acts
as a limiter for the pack command output.
- Around line 462-496: The estimate logic in the calculateEstimate function has
three issues: First, when targetStage is "source" or "sourced", targetCalls is
correctly set to 0 but inputTokens and outputTokens are still calculated from
the full words value, creating a mismatch between expected calls and computed
tokens/cost. Second, when calculating targetCalls for non-source stages, the
logic includes pendingSummary for all cases, but this overestimates for "graph"
targets which should only count pendingGraph and planned. Third, the
recommendation logic only checks explicitly for "high" risk and treats all other
cases (including "medium") with a "low enough" message, failing to provide
appropriate guidance for medium-risk scenarios. Fix these by: adjusting the
inputTokens and outputTokens calculations to be stage-aware (set to 0 for
source/sourced stages, or recalculate appropriately for other stages), excluding
pendingSummary from targetCalls when targetStage is "graph", and updating the
recommendation ternary chain to explicitly handle the "medium" risk case with a
distinct message that reflects medium-level caution.
In `@src/facade/index.ts`:
- Around line 17-33: The barrel export in the archive facade is missing two
public APIs from archive-view.ts. Add `formatFragmentId` (a function) and
`ArchiveSourceFragment` (a type) to the export statement(s) in the index.ts file
so that consumers of the facade can access these exports from the public API.
---
Nitpick comments:
In `@data/help/commands/archive/path.jinja`:
- Line 8: Standardize the placeholder notation for node ID arguments to match
the convention used in other help templates. In
data/help/commands/archive/path.jinja at line 8, change both occurrences of
`<node:id>` to `<id>` to align with the notation in pack.jinja and page.jinja
templates. Apply the same change in data/help/commands/archive/related.jinja at
line 8, replacing `<node:id>` with `<id>`. This ensures consistent placeholder
syntax across all help templates and clarifies that both arguments are node IDs.
In `@data/help/commands/archive/status.jinja`:
- Line 14: The note on line 14 currently states "bare `spinedigest status` shows
config status" which is ambiguous and may confuse users about which command
variant to use. Replace this note with a clearer explanation that explicitly
distinguishes the two forms: state that running `spinedigest status` without an
archive path checks CLI configuration status, while the command documented on
this page (`spinedigest status <archive.sdpub>`) checks archive status instead.
This will help users understand when each form is appropriate.
In `@data/help/topics/command.jinja`:
- Around line 25-37: The Command shapes section in the documentation is
incomplete, missing syntax definitions for five archive-first commands: grep,
links, backlinks, path, and map. Add the command shape documentation for these
five missing commands following the same format as the existing commands
(showing the command name, archive parameter, and relevant options). Insert
these additional command shapes after the export command definition to provide
complete documentation of all 17 archive-first commands in one reference.
In `@docs/en/ai-agents.md`:
- Around line 59-63: The "Avoid" section has four consecutive list items (lines
60-63) that all begin with "Do not", creating repetitive sentence structure.
Reword at least two of these items to use different phrasing patterns (such as
imperative verbs like "Avoid", "Never", "Skip", or "Refrain from") while
preserving the original meaning and the directive nature of the guidance. This
will improve readability through structural variety without changing the content
or intent.
- Around line 39-45: The execution strategy list in the docs/en/ai-agents.md
file contains repetitive sentence beginnings where lines 40-44 all start with
"Use". Vary the sentence structure for better readability by rephrasing these
items to use different grammatical constructions. Consider using alternative
patterns like imperative verbs (e.g., "Run", "Execute"), gerunds (e.g., "Finding
or listing"), or prepositional phrases (e.g., "To discover", "For navigating")
while maintaining the same meaning and functionality described in each
instruction. Reference lines 39 and 45 which already demonstrate varied sentence
structures that can serve as models for the rewording.
In `@docs/zh-CN/ai-agents.md`:
- Line 56: The phrase "是安全第一步" (in the context of "Import/source 是安全第一步") is
grammatically awkward because the structure of "是" paired with the adjective
descriptor lacks clarity. Revise this phrase to clarify the relationship between
the subject and the descriptor. The intended meaning should express that
importing/sourcing is either a secure first step or the first step in security.
Restructure the sentence to use either a more natural adjective placement or a
more explicit construction that clearly connects "import/source" with the
concept of being a security-focused first step.
🪄 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: c120c7dc-6e8e-455a-8dbe-01232abdf75d
📒 Files selected for processing (45)
README.mdREADME_zh-CN.mddata/help/commands/archive/backlinks.jinjadata/help/commands/archive/build.jinjadata/help/commands/archive/estimate.jinjadata/help/commands/archive/evidence.jinjadata/help/commands/archive/export.jinjadata/help/commands/archive/find.jinjadata/help/commands/archive/grep.jinjadata/help/commands/archive/import.jinjadata/help/commands/archive/index.jinjadata/help/commands/archive/links.jinjadata/help/commands/archive/ls.jinjadata/help/commands/archive/map.jinjadata/help/commands/archive/pack.jinjadata/help/commands/archive/page.jinjadata/help/commands/archive/path.jinjadata/help/commands/archive/related.jinjadata/help/commands/archive/status.jinjadata/help/commands/root.jinjadata/help/topics/ai.jinjadata/help/topics/command.jinjadata/help/topics/format.jinjadata/help/topics/overview.jinjadata/help/topics/recipe.jinjadata/help/topics/runtime.jinjadata/help/topics/sdpub.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.mdpackage.jsonsrc/cli/archive.tssrc/cli/args.tssrc/cli/help.tssrc/cli/main.tssrc/facade/archive-view.tssrc/facade/index.tstest/cli/archive.test.tstest/cli/args.test.tstest/facade/archive-view.test.ts
| Contract: | ||
| - Mutates: yes, writes the archive path. | ||
| - LLM: no; import stops at source stage. | ||
| - stdin: supported with --input-format markdown or txt. | ||
| - JSON: no. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the import command's argument parser to confirm whether --llm is actually supported
# and understand its interaction with the "source stage" claim.
# Check parseArchiveArguments for "import" action and validate --llm handling
rg -A 30 'action.*===.*"import"' src/cli/args.tsRepository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Find the import.jinja file and explore codebase structure
fd -type f -name "import.jinja" | head -20Repository: oomol-lab/spinedigest
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Check if src/cli/args.ts exists and explore cli directory structure
fd -type f "args\.(ts|js)" | head -20Repository: oomol-lab/spinedigest
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Search for --llm flag usage in relation to import command
rg "\-\-llm" --type ts --type js | grep -i import | head -20Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Find the import.jinja file - simpler approach
fd "import.jinja"Repository: oomol-lab/spinedigest
Length of output: 105
🏁 Script executed:
#!/bin/bash
# List files in the data/help/commands directory if it exists
ls -la data/help/commands/archive/ 2>/dev/null || echo "Directory not found, searching..."
fd -type f -path "*help*commands*" | head -20Repository: oomol-lab/spinedigest
Length of output: 1389
🏁 Script executed:
#!/bin/bash
# Search for --llm flag usage and import context
rg "\-\-llm" | head -30Repository: oomol-lab/spinedigest
Length of output: 4023
🏁 Script executed:
#!/bin/bash
# Search for the word "source stage" to understand the contract
rg "source stage" | head -20Repository: oomol-lab/spinedigest
Length of output: 506
🏁 Script executed:
#!/bin/bash
# Read the full import.jinja file to see context
cat -n data/help/commands/archive/import.jinjaRepository: oomol-lab/spinedigest
Length of output: 750
🏁 Script executed:
#!/bin/bash
# Find and examine the CLI argument parser
fd "args" | grep -E "\.(ts|js)$" | head -10Repository: oomol-lab/spinedigest
Length of output: 103
🏁 Script executed:
#!/bin/bash
# Search for how import command handles arguments
rg "import" | grep -i "args\|parse" | head -20Repository: oomol-lab/spinedigest
Length of output: 1462
🏁 Script executed:
#!/bin/bash
# Read the args parser to see how import handles arguments
cat -n src/cli/args.ts | head -100Repository: oomol-lab/spinedigest
Length of output: 3471
🏁 Script executed:
#!/bin/bash
# Search for "import" in args.ts and surrounding context
rg -B 5 -A 15 'import.*command\|"import"' src/cli/args.ts | head -60Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Look for where import or add command arguments are parsed
rg -n "add.*action\|import.*action" src/cli/args.ts | head -20Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search for functions that parse archive or chapter arguments
rg -n "function\|const.*=.*\(" src/cli/args.ts | grep -i "parse\|archive\|chapter" | head -20Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Get the file size and check the entire args.ts
wc -l src/cli/args.tsRepository: oomol-lab/spinedigest
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Search for where chapter arguments with action "add" are parsed
rg -n '"add"' src/cli/args.ts -B 3 -A 10 | head -50Repository: oomol-lab/spinedigest
Length of output: 1327
🏁 Script executed:
#!/bin/bash
# Look for the function that parses chapter arguments
rg -n "parseSdpubChapterArguments\|CLISdpubChapterArguments" src/cli/args.ts | head -20Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/antml:
# Search for where llmJSON is assigned for chapter actions
rg -n "llmJSON.*=\|action.*===.*\"add\"" src/cli/args.ts -A 5 | head -40Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Read the section where "add" action is handled, around line 1478
sed -n '1475,1550p' src/cli/args.tsRepository: oomol-lab/spinedigest
Length of output: 2594
🏁 Script executed:
#!/an/bash
# Search for where chapter arguments are returned after parsing
rg -n "return.*{" src/cli/args.ts | grep -A 2 -B 2 "1478"Repository: oomol-lab/spinedigest
Length of output: 147
🏁 Script executed:
#!/an/bash
# Look for the return statement for chapter arguments parsing
sed -n '1475,1600p' src/cli/args.ts | grep -n "return\|llmJSON\|action"Repository: oomol-lab/spinedigest
Length of output: 2744
🏁 Script executed:
#!/bin/bash
# Search for where the main "import" command (not chapter add) is parsed
rg -n "import" src/cli/args.ts | grep -i "command\|action\|archive" | head -30Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check what sdpub subcommands exist
rg -n "SDPUB_SUBCOMMANDS\|sdpub.*subcommand" src/cli/help.ts | head -20Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Look for where the help is generated for archive commands
rg -n "renderArchiveCommandHelpText\|import" src/cli/help.ts | head -20Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search for the main command parsing - where "import" might be parsed as a main archive command
rg -n "parseCLIArguments\|function parse" src/cli/args.ts | head -30Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Look for where positional arguments (like archive path) are handled
rg -n "positionals\|remaining" src/cli/args.ts | head -30Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check if import is a top-level command or part of a subcommand
rg -n "archive|import" src/cli/help.ts -B 2 -A 2 | head -40Repository: oomol-lab/spinedigest
Length of output: 872
🏁 Script executed:
#!/bin/bash
# Find the CLIArchiveAction type definition
rg -n "type CLIArchiveAction\|export.*CLIArchiveAction" src/cli/args.ts -A 15Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search for where archive actions are parsed - looking for the main parseArchiveArguments or similar
rg -n "CLIArchiveAction|parseArchive" src/cli/args.ts | head -20Repository: oomol-lab/spinedigest
Length of output: 928
🏁 Script executed:
#!/bin/bash
# Look for the main command parsing flow
sed -n '1,200p' src/cli/args.ts | tail -100Repository: oomol-lab/spinedigest
Length of output: 2415
🏁 Script executed:
#!/bin/bash
# Read the parseArchiveArguments function, specifically the "import" case
sed -n '725,850p' src/cli/args.tsRepository: oomol-lab/spinedigest
Length of output: 4158
🏁 Script executed:
#!/bin/bash
# Look for the "import" case within parseArchiveArguments
rg -n '"import"' src/cli/args.ts -B 2 -A 20 | grep -A 20 "parseArchiveArguments"Repository: oomol-lab/spinedigest
Length of output: 47
Contradiction confirmed: Contract contradicts Usage regarding --llm and --prompt support.
The import command's argument parser (src/cli/args.ts, parseArchiveArguments function) explicitly parses both --llm and --prompt flags and includes them in the returned arguments. However, line 12 of import.jinja states "LLM: no; import stops at source stage," which contradicts line 8's Usage section that lists [--llm <json>] [--prompt <text>] as available options.
Either clarify the Contract section to explain the actual role of these flags (e.g., whether they are accepted but unused, or used in a specific context), or remove them from Usage if they are truly not supported.
🤖 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/import.jinja` around lines 10 - 14, The help
documentation for the import command in import.jinja contains a contradiction
between the Usage section (line 8) which lists --llm and --prompt as available
options, and the Contract section (lines 10-14) which states "LLM: no". Resolve
this contradiction by either removing --llm and --prompt from the Usage section
if they are not actually supported by the import command, or update the Contract
section to accurately describe how these flags are handled (e.g., whether they
are parsed but unused, or used in a specific processing stage) to match what the
parseArchiveArguments function in src/cli/args.ts actually implements.
| - Mutates: no. | ||
| - LLM: no. | ||
| - JSON: yes. | ||
| - Input must be a known id from index, ls, find, page, or links. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check which archive commands produce object IDs in their output
# Search for archive command implementations that format or return IDs
rg -A 10 'id.*:.*node|chapter|fragment|sentence|summary|meta' src/cli/archive.ts src/facade/archive-view.tsRepository: oomol-lab/spinedigest
Length of output: 48196
🏁 Script executed:
cat -n data/help/commands/archive/pack.jinjaRepository: oomol-lab/spinedigest
Length of output: 704
🏁 Script executed:
# Check which archive commands are implemented and whether they produce IDs
rg -B 2 -A 5 'case "(grep|evidence|backlinks|related|map)"' src/cli/archive.tsRepository: oomol-lab/spinedigest
Length of output: 1866
🏁 Script executed:
# Check the page command implementation
rg -B 2 -A 8 'case "page"' src/cli/archive.tsRepository: oomol-lab/spinedigest
Length of output: 348
Expand the ID source list to include additional commands that produce object IDs.
The contract on line 14 lists "index, ls, find, page, or links" as ID sources, but the implementation supports several more commands that produce valid object IDs: grep, evidence, backlinks, related, and map. Update the list to reflect all available sources of object IDs, or clarify if only a subset are intended as primary entry points.
🤖 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 14, Update the text on line 14
in the pack.jinja file to reflect all available command sources that produce
object IDs. The current text mentions only "index, ls, find, page, or links" but
the implementation also supports additional commands: grep, evidence, backlinks,
related, and map. Expand the list in the help text to include all these commands
so the documentation accurately describes all valid ID sources.
|
|
||
| Examples: | ||
| spinedigest page book.sdpub chapter:1 | ||
| spinedigest page book.sdpub fragment:1:0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether the code uses "fragment:" or "sentence:" prefix for evidence IDs
rg -n "fragment:|sentence:" src/facade/archive-view.ts src/cli/archive.tsRepository: oomol-lab/spinedigest
Length of output: 587
🏁 Script executed:
# Check context around the fragment: and sentence: generation functions
sed -n '510,530p' src/facade/archive-view.tsRepository: oomol-lab/spinedigest
Length of output: 642
🏁 Script executed:
# Check what the `spinedigest page` command actually outputs
grep -n "page" src/cli/archive.ts | head -20Repository: oomol-lab/spinedigest
Length of output: 1110
🏁 Script executed:
# Check the full help template structure in ai.jinja and root.jinja to see documented ID formats
cat -n data/help/topics/ai.jinja | sed -n '35,50p'Repository: oomol-lab/spinedigest
Length of output: 813
🏁 Script executed:
# Check root.jinja for documented ID formats
cat -n data/help/commands/root.jinja | sed -n '40,55p'Repository: oomol-lab/spinedigest
Length of output: 912
Fragment ID format undocumented in general object ID reference.
The example uses fragment:1:0, but the general object ID definitions in data/help/commands/root.jinja (line 47) and data/help/topics/ai.jinja (line 41) document only sentence:<serial>:<fragment>:<index>, not fragment:.... While the spinedigest page command does support both formats, documenting fragment: only in this template while omitting it from the general reference creates confusion. Update the example to use the documented sentence: format, or add fragment:<serial>:<id> to the object ID definitions in root.jinja and ai.jinja.
🤖 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/page.jinja` at line 18, The example in the archive
page template uses the fragment:1:0 format for the spinedigest page command, but
this format is not documented in the general object ID reference sections. To
resolve this inconsistency, either update the example in the archive page
template to use the documented sentence:<serial>:<fragment>:<index> format
instead of fragment:1:0, or add the fragment:<serial>:<id> format definition to
the general object ID documentation sections in both the root.jinja and ai.jinja
files to document that both formats are supported.
| Contract: | ||
| - Mutates: no. | ||
| - LLM: no. | ||
| - JSON: no. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if path command handles --json flag
rg -A 20 'case "path"' src/cli/archive.ts | rg -i 'json|writeOutput'Repository: oomol-lab/spinedigest
Length of output: 47
🏁 Script executed:
# Explore the repository structure
git ls-files | head -20Repository: oomol-lab/spinedigest
Length of output: 650
🏁 Script executed:
# Find the path command documentation and implementation
fd -t f "path" --path-separator / | grep -E "(jinja|archive|path)" | head -20Repository: oomol-lab/spinedigest
Length of output: 145
🏁 Script executed:
# Check the help file mentioned in the review
cat data/help/commands/archive/path.jinjaRepository: oomol-lab/spinedigest
Length of output: 411
🏁 Script executed:
# Find where archive commands are implemented
fd -t f "archive" --path-separator / | grep -E "\.(ts|js)$" | head -20Repository: oomol-lab/spinedigest
Length of output: 306
🏁 Script executed:
# Read the archive implementation to find the path command
cat -n src/cli/archive.ts | head -100Repository: oomol-lab/spinedigest
Length of output: 3752
🏁 Script executed:
# Search for path command handling in archive.ts
rg "path" src/cli/archive.ts -A 10 -B 2Repository: oomol-lab/spinedigest
Length of output: 2419
🏁 Script executed:
# Check all archive commands and their JSON support
rg "JSON" data/help/commands/archive/ -lRepository: oomol-lab/spinedigest
Length of output: 733
🏁 Script executed:
# Check the args definition for archive commands
cat -n src/cli/args.ts | grep -A 100 "archive"Repository: oomol-lab/spinedigest
Length of output: 28682
🏁 Script executed:
# Check which archive commands declare JSON support
rg "JSON:" data/help/commands/archive/Repository: oomol-lab/spinedigest
Length of output: 984
🏁 Script executed:
# Find and read the rejectArchiveNonReadFlags function
rg "rejectArchiveNonReadFlags" src/cli/args.ts -B 5 -A 15Repository: oomol-lab/spinedigest
Length of output: 5098
🏁 Script executed:
# Read the rest of the rejectArchiveNonReadFlags function
rg "function rejectArchiveNonReadFlags" src/cli/args.ts -A 30Repository: oomol-lab/spinedigest
Length of output: 874
Add JSON output support or explicitly reject the --json flag for the path command.
The path command currently accepts the --json flag in the argument parser but the implementation ignores it and always outputs plain text. The documentation correctly states JSON: no, but this is inconsistent with similar read commands (pack, page, evidence, links, backlinks, related) which all support JSON output. Either implement JSON output (returning structured path data) or reject the flag explicitly in argument parsing to match documented behavior.
🤖 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/path.jinja` at line 13, The `path` command
currently accepts the `--json` flag in its argument parser but ignores it during
execution, creating inconsistency with the documented behavior stating `JSON:
no`. You must choose one of two approaches to resolve this: either implement
JSON output support for the `path` command to align with similar commands like
`pack`, `page`, `evidence`, `links`, `backlinks`, and `related` (returning
structured path data in JSON format when the flag is used), or explicitly reject
the `--json` flag in the argument parser configuration for the `path` command to
ensure the documented behavior matches the actual behavior and prevent users
from using an unsupported flag. Update the documentation accordingly once you
choose your approach.
| spinedigest build <archive.sdpub> [--stage <source|graph|summary|ready>] [--chapter <id>] [--confirm] | ||
| spinedigest estimate <archive.sdpub> [--stage <source|graph|summary|ready>] [--json] |
There was a problem hiding this comment.
Inconsistent stage terminology across help documentation. The archive-first commands (build, estimate) use stage values source|graph|summary|ready, but the direct digest command and readiness descriptions use planned|sourced|graphed|summarized. This creates confusion about the relationship between build targets and archive states.
data/help/topics/command.jinja#L27-L28: archive commands document--stage <source|graph|summary|ready>data/help/topics/command.jinja#L40-L40: direct digest command documents--stage <planned|sourced|graphed|summarized>data/help/topics/format.jinja#L26-L36: "Readiness by archive stage" section usesplanned/sourced/graphed/summarized(lines 27-30) but mentions "ready builds" (line 34) creating internal conflict
Align all documentation to use one canonical set of stage names, or clearly explain the distinction between build stage targets and archive readiness states.
📍 Affects 2 files
data/help/topics/command.jinja#L27-L28(this comment)data/help/topics/command.jinja#L40-L40data/help/topics/format.jinja#L26-L36
🤖 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 27 - 28, The help documentation
uses inconsistent stage terminology across multiple locations, creating
confusion about the relationship between build targets and archive states. In
data/help/topics/command.jinja lines 27-28, the build and estimate commands
document stages as `source|graph|summary|ready`, but in
data/help/topics/command.jinja line 40, the direct digest command documents
stages as `planned|sourced|graphed|summarized`, and in
data/help/topics/format.jinja lines 26-36, the "Readiness by archive stage"
section also uses `planned/sourced/graphed/summarized` while mentioning "ready
builds". Choose one canonical set of stage names and update all three locations
to use the same terminology consistently, ensuring the stage values align across
the build command documentation, the digest command documentation, and the
archive readiness description.
| budget: { | ||
| type: "string", | ||
| }, |
There was a problem hiding this comment.
--budget / --confirm are now silently accepted by non-archive commands.
These flags are globally registered in parseArgs, but only convert explicitly rejects them. help, status, and sdpub paths currently accept them without effect, which hides user mistakes instead of surfacing invalid command usage.
Also applies to: 367-369
🤖 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/args.ts` around lines 297 - 299, The `--budget` and `--confirm` flags
are currently registered globally in the parseArgs function, which causes them
to be silently accepted by all commands including help, status, and sdpub where
they are not valid. Remove these flag definitions from the global argument
registration in parseArgs (around line 297-299 and 367-369 in src/cli/args.ts)
and instead register them only in the command-specific argument definitions for
the commands that actually support them, such as the convert command. This will
ensure that using these flags with unsupported commands properly rejects them
with an error rather than silently ignoring the invalid usage.
| function isArchiveAction(value: string | undefined): value is CLIArchiveAction { | ||
| return ( | ||
| value === "backlinks" || | ||
| value === "build" || | ||
| value === "estimate" || | ||
| value === "evidence" || | ||
| value === "fragments" || | ||
| value === "export" || | ||
| value === "find" || | ||
| value === "grep" || | ||
| value === "import" || | ||
| value === "index" || | ||
| value === "links" || | ||
| value === "ls" || | ||
| value === "map" || | ||
| value === "page" || | ||
| value === "pack" || | ||
| value === "path" || | ||
| value === "related" || | ||
| value === "status" | ||
| ); |
There was a problem hiding this comment.
Remove unsupported fragments from archive action detection.
isArchiveAction returns true for "fragments", but "fragments" is not part of CLIArchiveAction and is not handled in parseArchiveArguments. This creates an unhandled runtime path for spinedigest fragments ... / help fragments.
Suggested fix
function isArchiveAction(value: string | undefined): value is CLIArchiveAction {
return (
value === "backlinks" ||
value === "build" ||
value === "estimate" ||
value === "evidence" ||
- value === "fragments" ||
value === "export" ||
value === "find" ||🤖 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/args.ts` around lines 2525 - 2545, Remove the unsupported "fragments"
entry from the isArchiveAction type guard function. The "fragments" action is
not part of the CLIArchiveAction type definition and is not handled in
parseArchiveArguments, so it should not be included in the type guard check.
Simply delete the line containing value === "fragments" || from the function to
prevent unhandled runtime paths for spinedigest fragments commands.
| export async function packArchiveContext( | ||
| document: Document, | ||
| id: string, | ||
| budget: number, | ||
| ): Promise<ArchivePack> { | ||
| const anchor = await readArchivePage(document, id); | ||
| const [evidence, links] = await Promise.all([ | ||
| readArchiveEvidence(document, id), | ||
| listAllArchiveLinks(document, id), | ||
| ]); | ||
|
|
||
| return { | ||
| anchor, | ||
| budget, | ||
| evidence, | ||
| links, | ||
| }; |
There was a problem hiding this comment.
pack budget is currently ignored in the packed result.
Line [431] accepts budget, but Lines [434]-[444] always return full evidence/links and only echo the number back. That makes spinedigest pack ... --budget ... non-functional as a limiter.
🤖 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/archive-view.ts` around lines 428 - 444, The packArchiveContext
function accepts a budget parameter but does not use it to limit the returned
evidence and links, making the budget feature non-functional. After retrieving
the evidence and links from readArchiveEvidence and listAllArchiveLinks
respectively, apply the budget constraint to limit the size or quantity of these
results before returning them in the ArchivePack object. This ensures that the
budget parameter actually acts as a limiter for the pack command output.
| const targetCalls = | ||
| targetStage === "source" || targetStage === "sourced" | ||
| ? 0 | ||
| : Math.max(0, pendingGraph + pendingSummary + planned); | ||
| const inputTokens = Math.ceil(words * 1.5); | ||
| const outputTokens = Math.ceil(words * 0.35); | ||
| const risk = | ||
| inputTokens > 1_000_000 || targetCalls > 100 | ||
| ? "high" | ||
| : inputTokens > 150_000 || targetCalls > 20 | ||
| ? "medium" | ||
| : "low"; | ||
|
|
||
| return { | ||
| estimatedCostUsd: { | ||
| max: roundMoney( | ||
| (inputTokens / 1_000_000) * 6 + (outputTokens / 1_000_000) * 18, | ||
| ), | ||
| min: roundMoney( | ||
| (inputTokens / 1_000_000) * 1 + (outputTokens / 1_000_000) * 3, | ||
| ), | ||
| }, | ||
| estimatedLlmCalls: targetCalls, | ||
| estimatedTime: { | ||
| maxSeconds: targetCalls * 120, | ||
| minSeconds: targetCalls * 30, | ||
| }, | ||
| estimatedTokens: { | ||
| input: inputTokens, | ||
| output: outputTokens, | ||
| }, | ||
| recommendation: | ||
| risk === "high" | ||
| ? "Do not run a full build in an interactive agent session; build a scoped chapter first." | ||
| : "Estimate is low enough for an interactive build if the user expects LLM-backed work.", |
There was a problem hiding this comment.
Estimate logic is not stage-aware and can report misleading cost/risk.
- Lines [463]-[465] set
estimatedLlmCallsto0forsource/sourced, but Lines [466]-[483] still compute non-zero tokens/cost from full source words. - For
graph, Line [465] includespendingSummary, which overestimates calls for a graph-only target. - Lines [493]-[496] label non-high risk as “low enough,” so medium risk gets a low-risk recommendation string.
Suggested directional fix
- const targetCalls =
- targetStage === "source" || targetStage === "sourced"
- ? 0
- : Math.max(0, pendingGraph + pendingSummary + planned);
- const inputTokens = Math.ceil(words * 1.5);
- const outputTokens = Math.ceil(words * 0.35);
+ const targetCalls =
+ targetStage === "source" || targetStage === "sourced"
+ ? 0
+ : targetStage === "graph"
+ ? Math.max(0, pendingGraph + planned)
+ : Math.max(0, pendingGraph + pendingSummary + planned);
+
+ const inputTokens = targetCalls === 0 ? 0 : Math.ceil(words * 1.5);
+ const outputTokens = targetCalls === 0 ? 0 : Math.ceil(words * 0.35);
@@
- recommendation:
- risk === "high"
- ? "Do not run a full build in an interactive agent session; build a scoped chapter first."
- : "Estimate is low enough for an interactive build if the user expects LLM-backed work.",
+ recommendation:
+ risk === "high"
+ ? "Do not run a full build in an interactive agent session; build a scoped chapter first."
+ : risk === "medium"
+ ? "Prefer a scoped build first, then expand if needed."
+ : "Estimate is low enough for an interactive build if the user expects LLM-backed work.",🤖 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/archive-view.ts` around lines 462 - 496, The estimate logic in the
calculateEstimate function has three issues: First, when targetStage is "source"
or "sourced", targetCalls is correctly set to 0 but inputTokens and outputTokens
are still calculated from the full words value, creating a mismatch between
expected calls and computed tokens/cost. Second, when calculating targetCalls
for non-source stages, the logic includes pendingSummary for all cases, but this
overestimates for "graph" targets which should only count pendingGraph and
planned. Third, the recommendation logic only checks explicitly for "high" risk
and treats all other cases (including "medium") with a "low enough" message,
failing to provide appropriate guidance for medium-risk scenarios. Fix these by:
adjusting the inputTokens and outputTokens calculations to be stage-aware (set
to 0 for source/sourced stages, or recalculate appropriately for other stages),
excluding pendingSummary from targetCalls when targetStage is "graph", and
updating the recommendation ternary chain to explicitly handle the "medium" risk
case with a distinct message that reflects medium-level caution.
| export { | ||
| estimateArchiveBuild, | ||
| findArchiveObjects, | ||
| formatChapterId, | ||
| formatEdgeId, | ||
| formatNodeId, | ||
| formatSentenceId, | ||
| formatSummaryId, | ||
| getArchiveIndex, | ||
| listAllArchiveLinks, | ||
| listArchiveLinks, | ||
| listArchiveObjects, | ||
| listRelatedArchiveObjects, | ||
| packArchiveContext, | ||
| readArchiveEvidence, | ||
| readArchivePage, | ||
| } from "./archive-view.js"; |
There was a problem hiding this comment.
Archive facade barrel is missing part of the new public surface.
src/facade/archive-view.ts exports formatFragmentId and ArchiveSourceFragment, but they are not re-exported here (Lines [17]-[33], [56]-[66]). This creates an avoidable API contract gap for ../facade/index.js consumers.
Proposed fix
export {
estimateArchiveBuild,
findArchiveObjects,
formatChapterId,
formatEdgeId,
+ formatFragmentId,
formatNodeId,
formatSentenceId,
formatSummaryId,
getArchiveIndex,
@@
export type {
ArchiveEstimate,
ArchiveFindField,
ArchiveFindHit,
ArchiveIndex,
ArchiveListItem,
ArchiveListKind,
ArchiveObjectType,
ArchivePack,
ArchivePage,
+ ArchiveSourceFragment,
} from "./archive-view.js";Also applies to: 56-66
🤖 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/index.ts` around lines 17 - 33, The barrel export in the archive
facade is missing two public APIs from archive-view.ts. Add `formatFragmentId`
(a function) and `ArchiveSourceFragment` (a type) to the export statement(s) in
the index.ts file so that consumers of the facade can access these exports from
the public API.
1 participant
Summary
Verification