Skip to content

fix(cli): keep reason's summary off stdout when writing the closure - #6641

Merged
jeswr merged 3 commits into
mainfrom
fix/cli-reason-stdout
Oct 5, 2026
Merged

jeswr merged 3 commits into
mainfrom
fix/cli-reason-stdout

Conversation

@jeswr

@jeswr jeswr commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Requested by Jesse · project thread

Summary

Before: sparq-cli reason data.n3 turtle n3 /dev/stdout printed 3 triples after n3 reasoning on stdout ahead of the closure, so the output was not valid N-Triples.

After: when an output path is given, the summary goes to stderr and stdout carries only the closure. Without an output path, the count is the command's only result and stays on stdout, so reason <file> <fmt> rdfs behaves as before.

How: one branch in cmd_reason; the datalog and EL CLI tests and bench/deep-taxonomy/run.sh now read the summary from stderr. New reason_to_dev_stdout_writes_only_the_closure_to_stdout contract test.

Fixes #6466

Base gate (always required)

  • cargo clippy -p sparq-cli --all-targets --features el,datalog -- -D warnings clean
  • Touched code formatted
  • cli_contract, el_cli, datalog_cli tests pass

Ratchets and conventions

  • No ratchet lowered.

Benchmark

No hot path changes. The diff only moves sparq reason's summary line from stdout to stderr and updates the two bench scripts to read it from there, so no before/after run was needed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01C1gMf8RJim1LzuKapZNpZR


Generated by Claude Code


Generated by Claude Code

…6466)

With an output path the closure is the data product and may itself be stdout
(/dev/stdout), so the '<n> triples after <profile> reasoning' summary now goes
to stderr. Without an output path the count is the only result and stays on
stdout.

Fixes #6466

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C1gMf8RJim1LzuKapZNpZR
@jeswr jeswr self-assigned this Oct 5, 2026
@github-actions github-actions Bot added area:bench workspace crate/area area:sparq-cli crate/surface conflict-partition labels Oct 5, 2026

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

The gate check (ci-summary) failed by timing out, not from a test failure. When it gave up, the clippy, MSRV, C1, benchmark and coverage jobs on this head were still queued behind a backed-up runner pool (about 170 queued runs repo-wide). Every check that did run passed. Nothing in this diff is involved. The fix is the merge-path change the maintainer's roadmap work is making (a fast required check replacing the polling gate), so I'm not re-running gate into the same queue. This PR stays watched.


Generated by Claude Code

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sparq engine

Details
Benchmark suite Current: b093d3f Previous: 674f50b Ratio
store_bytes_per_triple 88 bytes 88 bytes 1
dict_bytes_per_term 53 bytes 53 bytes 1
comp_store_bytes_per_triple 44 bytes 44 bytes 1
store_bytes_per_triple_small 88 bytes 88 bytes 1
wasm_bundle_bytes 1562916 bytes 1562916 bytes 1

This comment was automatically generated by workflow using github-action-benchmark.

@jeswr

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head b093d3f832d5. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [medium] Stream change breaks the OWL sameAs benchmark — crates/sparq-cli/src/main.rs:1239
    bench/owl-sameas/run.sh:78–79 invokes reason with an output path but still extracts the triple count from stdout. This change leaves stdout empty, so its grep pipeline fails under set -euo pipefail, aborting the benchmark on the first tier. The full scripts/ci-bench.sh run consequently fails too. Update the OWL sameAs runner to read the summary from stderr, as this PR already does for deep-taxonomy.

Verdict: Fix the remaining benchmark consumer before merging.

@jeswr

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head 4b9241cedac9. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [medium] Summary stream change breaks the OWL sameAs benchmark — crates/sparq-cli/src/main.rs:1239
    bench/owl-sameas/run.sh:78–79 invokes reason with an output file but still extracts triples after owl reasoning from stdout. This change leaves that stream empty, so its grep pipeline fails and set -euo pipefail immediately terminates the runner. Consequently, the full scripts/ci-bench.sh run also fails despite a correct closure.

    Update the OWL sameAs runner to read the summary from stderr, matching the deep-taxonomy runner changed in this PR.

Verdict: Fix the remaining benchmark consumer before merging.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C1gMf8RJim1LzuKapZNpZR

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed in 34a4772. bench/owl-sameas/run.sh now reads <N> triples after owl reasoning from stderr, as deep-taxonomy does. I checked every other script in the repo that consumes this line: the site demo runs reason without an output path, so it still gets the line on stdout.


Generated by Claude Code

@jeswr

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head 34a4772566a9. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

No correctness, security, soundness or design problems found.

Verdict: The PR is safe to merge as is.

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

Local ci-fast gate (GitHub Actions incident; Jesse approved local-gate merges at 20:19Z).

Tested PR head 34a4772566 merged with main b1ef23c365, running the same steps as .github/workflows/ci-fast.yml:

merge-with-main: clean (f9696a71)
clippy: pass
tests: pass
doctests: pass
conformance: pass (1225 pass + 4 divergence = 1229 >= 1229)
RESULT: GREEN 22:04Z

Main was re-checked immediately before merging.


Generated by Claude Code

@jeswr
jeswr merged commit cb9ec99 into main Oct 5, 2026
8 checks passed
@jeswr
jeswr deleted the fix/cli-reason-stdout branch October 5, 2026 23:52
jeswr added a commit that referenced this pull request Oct 9, 2026
* docs: bring skills and crate READMEs back in line with main

Audit every skills/*/SKILL.md and crates/*/README.md against the public
API on main after this week's merges, and fix the drift:

- publish status: the 12-crate crates.io set (#6663/#6646) — jsonld,
  engine-serialize and engine-service are now published; unpublished
  crates lose crates.io/docs.rs badges and `"0.1"` install snippets
- default-members (#6649): introspect, canon, substrate claims
- zk-query-proofs / mpc / usage-control-policy recipes now compile
  against current signatures (prover_toml_for, verify_manifest,
  revoke_prover_toml, AttestedStatusRef::clear, Session fields)
- cli: reason summary to stderr (#6641), dump streaming (#4313),
  jsonld-compact, bench --json, flag list
- substrate: #3825 float comparison boundary fixed by #6671
- vc: proof-option validation (#6589) and EdDSA config change (#6585)
- lws: per-resource WAC on notifications (#6388), reclaim race (#6675)
- stale API names, test paths, floors, see-also links; router lists
  trust-graph
- trust-expression spec: last github.com/jeswr/sparq link

Closes #6327

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUY53os6oSKvmT9gCqdzud

* ci(notation3tests): save rust-cache only on main

docs-quality quick-gates' cache-posture test fails on every PR (main
too) because this step had no save-if.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUY53os6oSKvmT9gCqdzud

* docs: one git source for collaborating crates in EL/QL and Arrow recipes

Mixing a crates.io sparq-core/engine with a git or path sparq-reason-el,
sparq-reason-ql or sparq-arrow yields distinct Dict/Query/QueryResult
types, so the recipes now take every collaborating crate from git.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUY53os6oSKvmT9gCqdzud

* docs: strict SERVICE egress in the quickstart; strip model tags

- sparq-engine-service quickstart used with_service_egress_allow, which
  only blocks private addresses; it now uses
  with_service_egress_policy(true, ..) so only the listed host is dialled,
  as the comment says.
- Remove leftover model tags and "Model:" notes from skills/ and crate
  READMEs (#6673 removed the convention).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUY53os6oSKvmT9gCqdzud

* docs: redo the model-tag strip without touching code syntax

The previous strip also deleted every `()` on lines carrying a tag and
every " ()" across the touched files, breaking examples such as
`.text()`, `.arrayBuffer()` and `Result<(), String>`. Revert it and
strip only the tags themselves (plus "Model:" notes); `()` counts and
spacing punctuation are now identical to before the strip in every
touched file. Keeps the strict SERVICE egress quickstart.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUY53os6oSKvmT9gCqdzud

* docs: restore academic-paper fence; vendored spargebra in the PROV recipe

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUY53os6oSKvmT9gCqdzud

* docs: fix markdownlint hits left by the model-tag strip

Spaces inside bold markers in skills/mpc, a doubled blank line in two
READMEs, and an orphaned provenance comment in sparq-trust.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUY53os6oSKvmT9gCqdzud

* docs: PROV recipes declare sparq-core/oxrdf; router CLI and JSON-LD status match main

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUY53os6oSKvmT9gCqdzud

* docs(zk): verify_manifest API line lists all ten parameters

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUY53os6oSKvmT9gCqdzud

* docs(skills): router entries are each skill's own frontmatter description

The router carried frozen copies of old per-skill descriptions and status
notes that had drifted from the skills (a JS import subpath the package
no longer exports, SHACL strict validation and features reported as
missing, stale CLI/JSON-LD notes). Each entry now reproduces the
skill's current frontmatter description verbatim.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUY53os6oSKvmT9gCqdzud

* docs(jsonld): CLI dump has --context only; framing is planned

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUY53os6oSKvmT9gCqdzud

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:bench workspace crate/area area:sparq-cli crate/surface conflict-partition

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug]: log information is written to stdout when n3 reasoning

2 participants