fix(gate): doc-warnings documents private items, so broken pub(crate) doc links fail the gate (#2336) - #2354
Merged
Merged
Conversation
…nks are checked Rustdoc only checks intra-doc links inside items it documents, so the doc-warnings gate step — cargo doc --no-deps with -D warnings — was blind to every broken link in a private module, private function, or pub(crate) item. Adding --document-private-items surfaced 34 broken references across nine crates, all pre-existing on main: paths that do not resolve from their scope, links to renamed or deleted items (render_transcript, RepoBackend::push, fetch_url, RESUME_GRACE, BlockDraft::without_local_content), links into #[cfg(test)] modules that can never resolve in a doc build, and prose like argv[0] or sha256:<hex> that rustdoc parses as links or HTML. Every site is repaired in this commit — a resolving target where one exists (rustdoc resolves links with normal visibility rules from the linking scope, so only pub(crate)-or-wider targets are linkable across modules), prose with backticks where none can (test modules, fully-private items in sibling modules). The accept.rs twins stay byte-identical for their drift test. The flag flips in the same commit in all three normative homes (Makefile doc-warnings, ci.yml, CONTRIBUTING.md), so main never sees a red window between repairs and enforcement. --keep-going rides along on the same command: rustdoc bails at the first failing crate, which has repeatedly masked the next layer's break (#1823); with it the gate reports every crate in one run. Closes #2336
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's GuideThis PR updates the Rustdoc gate to document and check private items, ensures the gate keeps running across all crates, and then fixes all intra-doc links and doc-comment references exposed by that stricter check across multiple crates without changing runtime behavior. Sequence diagram for the updated rustdoc gate command checking private itemssequenceDiagram
actor Dev
participant Makefile
participant CI
participant Cargo
participant Rustdoc
Dev->>Makefile: make doc-warnings
Makefile->>Cargo: cargo doc $(CARGO_SCOPE) --no-deps --document-private-items --keep-going
Dev->>CI: push / PR
CI->>Cargo: cargo doc --workspace --no-deps --document-private-items --keep-going
Cargo->>Rustdoc: run with RUSTDOCFLAGS="-D warnings"
loop each_crate_in_workspace
Rustdoc->>Crate: build_docs_with_private_items
Crate-->>Rustdoc: intra_doc_links (public + pub(crate) + private)
alt doc_warnings_present
Rustdoc-->>Cargo: emit_warning_as_error_for_crate
note over Rustdoc,Cargo: command continues to next crate due to --keep-going
else no_doc_warnings
Rustdoc-->>Cargo: success_for_crate
end
end
Cargo-->>Dev: exit 0 if all crates clean
Cargo-->>CI: exit 101 if any crate fails gate
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
macanderson
added a commit
that referenced
this pull request
Aug 8, 2026
…e query actually narrowed (#2333) (#2356) ## What Domain overlap becomes an admission channel again — this time against a scope **the query itself narrowed**, which is the property that was missing when it shipped and the reason it was removed. A recall now carries two domain scopes (`RecallScope`): | scope | source | filters | ranks | **admits** | |---|---|---|---|---| | `session` | the workspace vocabulary (`Domains::names()`) | ✅ | ✅ | ❌ | | `query` | the domains owning the files **this goal named** | ❌ | ❌ | ✅, conditionally | Admission requires `query` to be a **non-empty proper subset** of `session` (`evidence::scope_is_query_conditional`). ## Why the proper-subset condition is the whole fix If the two scopes are equal, "shares a domain with the query" and "is in scope at all" are the *same predicate* — and the second is already the filter every candidate passed to get here. The rung would admit the entire in-scope corpus, which is precisely what it did: the session hands recall its whole vocabulary and nothing narrowed it, so the gate admitted every tagged node on every prompt, and reflection write-back tags every episode whose turn touched files. That was #2289 reopened at full width, and it is why PR #2298 removed the rung outright rather than patching it. **A scope must narrow to discriminate.** Narrowing `session` instead was the tempting one-value version and is strictly worse: `session` also drives `node_ids_excluded_by_scope`, so a narrower value silently starts **suppressing** memories rather than merely declining to admit them. Hence two fields, not one. ## How - **`RecallScope`** (`crates/stella-context/src/retrieval/scope.rs`) — the two-scope type. `recall_scoped_excluding` takes it; `recall` and `recall_scoped` stay as conveniences and document that they can never admit on domain, so a caller with no per-query scope gets the conservative behavior *by construction* rather than by remembering to ask. - **Derivation** (`ScopedStore::query`, `crates/stella-cli/src/contextgraph.rs`) — `query_domain_scope` maps the goal's anchors through `Domains::domains_for_path`. Anchors are already workspace-relative paths (`goal_path_anchors`), which is exactly what that function consumes, so no URI parsing is involved. An anchor in any other spelling matches no prefix and drops out — narrowing the scope *less*, which can only make the gate more reluctant to admit. That is the safe direction. `ScopedStore` now holds the `Domains` taxonomy rather than just its names; `ContextQuery` stays workspace-agnostic, so the wire contract is untouched. - **No extra I/O.** A query scope that narrows a non-empty session scope implies the corpus tag map was already loaded for the overlap ranking, so the evidence projection is a filter over rows in hand, never a second scan. ## Witness **In the store** — one corpus, one prompt, a single variable changed: | test | query scope | result | |---|---|---| | `a_full_vocabulary_domain_scope_is_not_evidence` | the whole vocabulary | **0 frames**, `no_evidence_cut` 5 | | `a_narrowed_domain_scope_is_evidence` | one domain of it | exactly that domain's `note-0`, `note-3`; other three refused | No note shares a distinctive term with the prompt, so domain overlap is the only channel that can fire — these read the rung directly. The narrowed case is a true fail→pass witness: with the channel disabled it returns `[]` against the expected `["note-0", "note-3"]`, verified by neutering `scope_is_query_conditional` and re-running. `recall_scoped_alone_never_admits_on_domain` pins the convenience API's posture. Four unit tests pin the predicate itself, including that repeats cannot fake a narrowing (set semantics, not list length) and that a scope reaching outside the vocabulary is not a subset. **In the CLI** — six tests pin the derivation (`query_domain_scope_derivation`), including the sweeping goal that names a file in every domain and therefore narrows nothing. That degenerate case is pinned on **both** sides of the boundary deliberately: the two halves have to agree on it, and it is the exact predicate that shipped inverted. ## File-size ratchet `RecallScope` pushed `retrieval.rs` to 1519 lines, past the 1500 limit, and the file is not grandfathered — so it was **split, not baselined** (AGENTS.md § God files). 1444 now. The split follows a real seam rather than the line count: `retrieval::scope` owns domain scoping entire — the type plus the two projections the corpus tag map feeds, `overlap_ranking` (ranks, against `session`) and `evidence_ids` (admits, against `query`). Putting them side by side is the point. They read the same map through different scopes, that asymmetry is the subtlest thing in retrieval, and it has already been got wrong once. Both are pure functions over loaded rows, matching the `evidence` and `ranking` siblings; the parent keeps the query that loads the map. ## Docs The three prose sites that describe the channels move together, as they must: the `evidence.rs` module doc, the `require_evidence` doc comment in `crates/stella-cli/src/settings/context.rs`, and `docs/spec/adaptive-context/adaptive-context.md` §6.1 — which now states the general rule rather than just this instance: admission is strictly narrower than ranking, and a predicate the query does not vary cannot be an admission rung. ## Verification - `cargo test -p stella-context` — 175 passed, 0 failed. - `cargo test -p stella-cli` — 1500 unit + 12 integration binaries, 0 failed. - `cargo clippy -p stella-context -p stella-cli --all-targets -- -D warnings` — clean. - `RUSTDOCFLAGS="-D warnings" cargo doc` — clean (exit 0). - `cargo fmt --all --check` — clean. - `make guards-fast` — all guards green, `check-file-size` and `check-module-reachability` included. ## Notes on CI **`main` is red independently of this branch.** `shellcheck` fails on `scripts/test-arena-scripts.sh` (arrived via #2328/#2351), which reds every open PR whatever it changed. Filed as **#2355** with the fix; deliberately **not** folded in here, since a shared red gate that several PRs each "helpfully" repair is its own failure mode. This diff touches no shell script. **Forward-checked against #2354.** That PR (open) tightens `doc-warnings` to `--document-private-items`, which surfaces doc links inside `pub(crate)` items. Ran this branch under the stricter flag: **zero errors in any code this PR adds or touches** — `scope.rs`, `evidence.rs`, `evidence_tests.rs`, `contextgraph.rs`, and every added line of `retrieval.rs` are clean. The 9 that do fire are all pre-existing on `main` in files this PR does not touch (`candidates.rs`, `ranking.rs`, `ann.rs`, `store/domain.rs`, `store/schema.rs`, and `retrieval.rs`'s own module doc), and are exactly what #2354 repairs. So this merges green in either order. Closes #2333 Refs #2289
5 tasks
Owner
Author
macanderson
added a commit
that referenced
this pull request
Aug 8, 2026
…check and the gate are green again (#2355) (#2363) ## What & why `main` is red: the `shellcheck` gate step fails on `scripts/test-arena-scripts.sh` (arrived with #2328, extended by #2351) with nine `SC2016`/`SC2028` info-level findings, and since `shellcheck` is a `GATE_STEPS` entry and a required check, every open PR is red regardless of its diff (observed on #2354, which touches no shell script). The findings are false positives about intent — the generator `echo`s single-quoted lines so that `$1`, `$STUB_LOG` and `\n` land literally in the generated stub. This PR takes issue #2355's preferred fix (option 1): emit the stub with a **quoted heredoc** (`cat <<'STUB'`), whose content is literal by construction, so both codes stop applying and the intent is self-evident to reader and linter alike. No disables, no gate widening, step name untouched (`check-gate-parity.sh` unaffected). Bonus the heredoc buys for free: the old form was interpreter-dependent — under `sh` (XSI `echo`), the `\n` in the `printf` line expands and splits the format string across two lines, exactly the hazard `SC2028` names. Benign in practice (the script runs under bash, and `printf "%s<newline>"` behaves the same), but now it cannot happen at all. Closes #2355 ## The witness - [x] The gate step itself is the witness: `make shellcheck` **fails on `main`** (exit 1, nine findings) and **passes here** (verified locally). The generated stub is proven **byte-identical** under bash — both generator forms were run side by side and compared with `cmp` — and `bash scripts/test-arena-scripts.sh` passes **20/20**, including the stub-consuming crash/handoff classification checks. ## The gate - [x] `make shellcheck` green locally; `bash scripts/test-arena-scripts.sh` 20/20 - [x] Shell-only diff — no Rust compiled; clippy/test unaffected and left to CI - [x] `Closes #2355` appears both above and as a commit trailer ## Nothing left behind - [x] There is nothing: the fix is the issue's own prescribed option 1, and the interpreter-dependence observation above is fixed by the same change. Related pre-existing alerts remain tracked in #2121. ## Anything reviewers should know? Landed on its own from a fresh `main` per the issue's constraint (a red shared gate must not be repaired inside unrelated PRs — the #2004 merge-skew lesson). Once this merges, open PRs (e.g. #2354) need only a re-run / branch update to go green. ## Summary by Sourcery Emit the arena launch stub using a quoted heredoc to restore shellcheck gate success while preserving the generated script’s behavior. Bug Fixes: - Resolve shellcheck SC2016/SC2028 findings in scripts/test-arena-scripts.sh that were causing the shared gate to fail on main. Enhancements: - Generate the launcher stub via a quoted heredoc to make the intended literal content clearer and avoid interpreter-dependent \n handling.
macanderson
added a commit
that referenced
this pull request
Aug 8, 2026
…ate one (#2365) ## What & why **`main` is red, and every open PR is red with it.** One line fixes it. `WorkspaceProbe::diff` is public and its doc comment linked `[`Self::ignores`]`, which is private. `rustdoc::private_intra_doc_links` is denied under `-D warnings`, so `cargo doc` fails the required `fmt + clippy + test` job: ``` error: public documentation for `diff` links to private item `Self::ignores` --> crates/stella-tools/src/shell_touch.rs:349:39 error: could not document `stella-tools` ``` Reproduced on main's own runs at `ad92643b` and `298b2705`, not just on a PR head. ## How it got in #2344 introduced the link. Its branch predated #2354, so the `cargo doc` gate that ran against it was the older command, and the failure it *did* report was read as the pre-existing shellcheck breakage that #2363 was already fixing. ## One thing worth a follow-up thought on #2336 / #2354 `--document-private-items` does **not** silence this lint for a public → private link. rustdoc's own note says "this link will resolve properly if you pass `--document-private-items`" — and both `make doc-warnings` and `ci.yml` already pass it, at the exact commits that failed. So the hint is misleading for this direction. That does not weaken #2354; the gate caught a real defect. It just means the working rule is narrower than the hint suggests: **a public item cites a private helper in prose, never as an intra-doc link.** `pub(crate) → pub(crate)` links, which #2354 was about, are unaffected. ## The fix The link becomes prose. No API change, no behavior change — deliberately the smallest possible diff, because an unbreak PR that also does something else is how a red `main` stays red longer. ## Witness - [ ] This PR includes a witness test None, and none is possible: the failing check *is* the witness. `cargo doc -D warnings` fails on `main` at this commit's parent and passes here — a witness test cannot assert about a rustdoc lint, and the gate already does. ## Gate Not run locally — a Terminal-Bench match is executing on this machine and a workspace build would contend for CPU, which is how a trial acquires a false timeout. `make guards-fast` is green; the compile tiers are CI's. ## Ground-rule check - [x] No I/O added to `stella-core`; no new deps - [x] No new outbound network calls - [x] No new cross-boundary types ## Summary by Sourcery Bug Fixes: - Resolve rustdoc private_intra_doc_links failure by replacing a link to the private ignores helper with plain prose in the diff method documentation.
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.
What & why
Rustdoc does not check intra-doc links inside items it is not documenting, so the
doc-warningsgate step —cargo doc --no-depsunder-D warnings— was blind to everybroken link in a private module, private function, or
pub(crate)item. In a repositorywhere doc comments are load-bearing (invariants are cited by number from Rust doc
comments; type doc comments are the wire contract), that is the documented failure mode of
the citation scheme itself.
This PR turns the light on and fixes everything it reveals, in one atomic change so
mainnever has a red window between repairs and enforcement:
--document-private-itemsto the gate command in its three normative homes:the
Makefiledoc-warningstarget (stillCARGO_SCOPE-aware),ci.yml's rawinvocation, and
CONTRIBUTING.md's gate block (check-gate-parity.shstays green).This is the same posture rust-lang/cargo's CI takes for its internal docs.
--keep-goingto the same command: rustdoc bails at the first failing crate,which has repeatedly masked the next dependency layer's break (docs(stella-core): unbreak main — drop private intra-doc links in LoopVerdict::evidence #1823 was diagnosed one
layer at a time). With it, one gate run reports every crate.
(
stella-diag,stella-context,stella-store,stella-core,stella-graph,stella-observatory,stella-tools,stella-serve,stella-tui). The issue'sestimate was right: the five known
stella-contexterrors were one crate of a muchlarger population, including links whose targets had been renamed or deleted —
render_transcript→render_transcript_window,RepoBackend::push→push_branch,fetch_url→fetch_raw,RESUME_GRACE→ the configuredresume_grace/DEFAULT_RESUME_GRACE, andBlockDraft::without_local_content, a mechanism that nolonger exists (the doc now describes the real one: decomposition maps
AttachmentSource::Datato no local preimage).Closes #2336
The repair rules (for review)
Rustdoc resolves intra-doc links with ordinary Rust name-resolution visibility from the
linking scope —
--document-private-itemsdoes not change that. So:pub/pub(crate)(or reachable through apub(crate) usere-export,e.g.
crate::store::domains_by_node) gets a real link target; display text is unchanged.subtree (this is why
stella-graph's already-qualifiedcrate::store::index_oneandcrate::walk::DENY_DIRSfailed). Those become prose backticks, usually with a link tothe visible containing module — the same shape fix(stella-store,repo): unbreak main — rustdoc private-link + obsolete event.rs ratchet entry #1965/fix(stella-cli): unbreak main — unresolved SkipReason doc link in daemon::boot #1970 used.
[tests::…]references point into#[cfg(test)]modules that never exist in a docbuild → prose backticks.
argv[0],agent[0],"sha256:<hex>"→ backticked so rustdoc stops parsing them aslinks/HTML.
accept.rstwins (stella-observatory/stella-serve) received byte-identicaledits, keeping
the_two_copies_of_this_policy_have_not_driftedgreen.driver.rs,deck_ui.rs,registry.rs,stella-store/lib.rs) were editedstrictly in place — zero added lines.
The witness
main, passes here) — the gate commanditself is the witness:
RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --document-private-items --keep-goingexits 101 onmain(34 errors, ninecrates failing) and 0 on this branch (verified locally, full workspace).
The gate
cargo fmt --check(clean locally)cargo clippy --workspace --all-targets -- -D warnings— left to CI (doc-comment-onlyRust changes; this machine deliberately does not run workspace-wide builds)
cargo test --workspace— left to CI, same reasonCONTRIBUTING.md gate block)
Closes #2336appears both above and as a commit trailerAlso run locally: the full flagged doc build (exit 0) and
check-gate-parity.sh(OK).Cost of the flag (the issue asked): on an identical warm cache, forcing a full re-doc
of all 21 workspace crates — old command 28.1s wall, new command 28.4s wall (~1%; the
extra rustdoc work hides inside build parallelism). Pre-push hook and CI impact is noise.
Nothing left behind
already tracked as deps: two open Dependabot alerts — js-yaml (high, website) and h2 (medium, terminal_bench_analysis) #2121 (with website: a stale package-lock.json beside pnpm-lock.yaml fails every Dependabot run and raises phantom high-severity alerts #2263 covering the phantom-alert cause), and
everything this change surfaced is fixed here.
Ground-rule check
stella-core(doc comments only); no new depsAnything reviewers should know?
make docs(the human browse target) deliberately keeps building without--document-private-items: whether browsed docs should include every private item is areading-experience preference, and "now vs right" on that is the maintainer's call. The
gate and CI are what must see private items, and now do.
deck_ui/create.rs's module doc resolves some of itslink fragments at the parent module's scope (rustdoc merges outer
///-on-moddocswith inner
//!docs and loses the span). The three affected links now usecrate-absolute targets, which resolve identically from either scope.
demo-scenario.shand the ultra-audit skill config still use the plain command — bothare stress/audit harnesses, not the gate, and were left alone on purpose.
Summary by Sourcery
Tighten the rustdoc gate to cover private items and keep running on multiple failures, and repair all intra-doc links and documentation references surfaced by the new checks across the workspace.
Enhancements:
Build:
doc-warningstarget to run rustdoc with--document-private-itemsand--keep-goingso the gate enforces clean docs for public and private items.pub(crate)docs are checked and multiple failing crates are reported in a single run.Documentation: