Skip to content

ci: parallelise the workflow and stop discarding the build cache - #202

Merged
senamakel merged 12 commits into
mainfrom
ci-parallelize
Sep 23, 2026
Merged

senamakel merged 12 commits into
mainfrom
ci-parallelize

Conversation

@senamakel

@senamakel senamakel commented Sep 23, 2026

Copy link
Copy Markdown
Member

Summary

CI takes ~23 minutes per run, and every PR commit triggers two of them. This
restructures the single 20-step Rust SDK job into four parallel jobs and fixes
the caching that the old layout was actively defeating.

Measured baseline, from the two runs on #196
(23m20s and 22m22s for the same commit):

Step Time
Check formatting 4s
Clippy + Clippy all features 57s
Build 3m08s
Build all features 3m00s
Test + Test all features 48s
Test optional features independently 11m34s
Verify line coverage (incl. install) 2m42s
Doc lints + machete 35s

What was slow, and why

  1. Every run happened twice. on: push with no branch filter fires for each
    PR branch push alongside on: pull_request. The concurrency group keyed on
    github.ref differs between the two events, so neither cancelled the other —
    two full copies of a 23 minute suite per commit.

  2. The 11m34s step was five cold builds in a row. Test optional features independently ran cargo clean before each of sqlite, tools and
    multimodal, because each feature selection links a different workspace
    graph and they cannot share a target directory. Serialised in one job, that
    is three from-scratch compiles of the dependency graph back to back.

  3. The cache was being thrown away. Those cargo clean calls, plus Free build artifacts before coverage, meant that by the time Swatinem/rust-cache
    ran its post step the target directory held only the coverage and rustdoc
    artifacts. The build profile that the 6-minute Build steps produce was
    never saved, so every run started those from cold.

What changed

  • on: push is limited to main. PR commits are built once, by
    pull_request. Mainline pushes still run, which is what populates the cache
    that PR branches restore from. The concurrency group now uses
    github.head_ref || github.ref so a PR's runs coalesce and superseded commits
    cancel.
  • The five feature selections became a test matrixdefault,
    all-features, sqlite, tools, multimodal. They build concurrently
    instead of serially, no cargo clean is needed because nothing shares a
    target directory any more, and each leg carries its own rust-cache key so a
    warm cache survives from run to run.
  • lint is its own job. cargo clippy and cargo doc stop at metadata and
    never link, so fmt/clippy/doc-lints/machete now report in ~2 minutes rather
    than behind 20 minutes of builds.
  • coverage is its own job with its own cache key. Instrumented objects are
    not interchangeable with the test matrix's, which is exactly why the old
    layout had to cargo clean before it.
  • Global build env: CARGO_INCREMENTAL=0 (incremental only pays off across
    edits on one machine; in CI it costs codegen time and inflates the cache) and
    CARGO_PROFILE_{DEV,TEST}_DEBUG=0 (debug info dominates link time and target
    size). The coverage step already set the latter; it now applies everywhere.
  • New CI gate job that depends on all the others, so branch protection can
    require one check instead of being edited whenever the matrix changes.

Measured effect

Latest run:
all green, 2m37s wall clock, against 23m20s on #196.

Job Time
Lint 1m31s
Test (default) 2m16s
Test (tools) 2m17s
Test (all-features) 2m25s
Test (multimodal) 2m25s
Test (sqlite) 2m29s
Coverage 2m15s
CI (gate) 2s
Total wall clock 2m37s

And one run per commit instead of two. Cold-cache machine time goes up somewhat
(each matrix leg recompiles shared dependencies), but warm-cache time drops
sharply now that the build profile is actually retained between runs instead of
being cargo cleaned away before the cache is saved.

Fixing the two checks that only pretended to pass

Both Doc lints and Unused dependencies carried continue-on-error: true
and a TODO. Both were reporting real annotations on every run, and both turned
out to be cheap to fix, so the continue-on-error is gone and they now block:

  • cargo doc was run without --all-features, so links into feature-gated
    items could not resolve — rustdoc could not see the modules the docs pointed
    at. With --all-features and 31 link targets corrected across
    tinyagents-graph, tinyagents-harness and tinyagents-session, a clean
    cargo clean --doc && cargo doc --workspace --no-deps --all-features under
    -D rustdoc::broken_intra_doc_links now exits 0. The TODO's "~160 warnings"
    was stale; the real count was 31.

    Most were ordinary wrong paths (Entry::MessageEntryKind::Message,
    crate::retry::RetryMiddlewarecrate::middleware::library::RetryMiddleware,
    super::PrefixedToolSet from a sibling toolset member → the toolset
    re-export). Three classes needed a judgement call rather than a repath:

    • Links to items in crates the harness does not depend on
      (crate::subagent::SubAgent is in tinyagents-orchestration, which depends
      on the harness, not the other way round) became plain code spans.
    • Links to private items (channel_from_config, boundary::CompiledGraph::advance)
      became plain code spans, matching how the same references are already
      written elsewhere in those modules.
    • Two references were simply stale: LoopState::final_error names a field
      that does not exist (a failed run reports through the driver's Result),
      and tinyagents_harness::memory with its ChatHistory/ShortTermMemory
      names a module that no longer exists anywhere in the workspace — that role
      now belongs to tinyagents-session. Both were reworded.

    The remaining rustdoc output is private_intra_doc_links warnings, a
    different lint that is not denied and was not in scope here.

  • cargo-machete was walking the whole checkout including vendor/, and
    its one finding (tracing in vendor/tinytools/crates/tinytools-jev) is in a
    vendored submodule that has to be fixed in its own repository. Scoped to
    crates with args: crates, it is clean.

No behaviour changed — the doc work is comments only.

⚠️ Follow-up required before merge

The Rust SDK status check no longer exists. If branch protection requires it,
update the rule to require CI instead, otherwise PRs will block forever
waiting on a check that is never reported.

Commands run locally

  • python3 -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))" — workflow parses.

No Rust source changed; the actual verification is this PR's own CI run.

Summary by CodeRabbit

  • Documentation
    • Clarified and corrected API references across the documentation, including where conversation history is managed.
  • Chores
    • Updated CI to run checks on the main branch and pull requests, cancel superseded runs, and run test configurations in parallel.
    • Added stricter documentation and dependency checks; CI now reports failures across linting, tests, and coverage.

senamakel and others added 2 commits September 23, 2026 12:04
The CI workflow is split into separate lint, test, and coverage jobs so that each feature set builds concurrently rather than serially with intervening clean steps. Environment variables are added to disable incremental compilation and debug info, reducing build times and cache sizes. The push trigger is restricted to the main branch to avoid duplicate runs on pull request commits, and concurrency grouping is adjusted to use head_ref for pull requests so superseded commits cancel each other.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The job name for the "all features" matrix entry contained a space, which caused issues with certain CI tooling that expects hyphenated identifiers. The name is changed to "all-features" to ensure compatibility.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 18739ddf-f383-4bee-8576-bf8e2a1474e2

📥 Commits

Reviewing files that changed from the base of the PR and between 157186c and 43f19ca.

📒 Files selected for processing (21)
  • .github/workflows/ci.yml
  • crates/tinyagents-graph/src/agent_loop/mod.rs
  • crates/tinyagents-graph/src/agent_loop/types.rs
  • crates/tinyagents-graph/src/channel/registry.rs
  • crates/tinyagents-graph/src/channel/types.rs
  • crates/tinyagents-graph/src/checkpoint/types.rs
  • crates/tinyagents-graph/src/compiled/mod.rs
  • crates/tinyagents-harness/src/agent_loop/tools.rs
  • crates/tinyagents-harness/src/context/mod.rs
  • crates/tinyagents-harness/src/context/types.rs
  • crates/tinyagents-harness/src/error.rs
  • crates/tinyagents-harness/src/host/mod.rs
  • crates/tinyagents-harness/src/lib.rs
  • crates/tinyagents-harness/src/limits/types.rs
  • crates/tinyagents-harness/src/runtime/types.rs
  • crates/tinyagents-harness/src/summarization/types.rs
  • crates/tinyagents-harness/src/tool/deferred/types.rs
  • crates/tinyagents-harness/src/tool/toolset/combined/types.rs
  • crates/tinyagents-harness/src/tool/toolset/prefixed/types.rs
  • crates/tinyagents-session/src/entry_tree/legacy.rs
  • crates/tinyagents-session/src/transcript/test.rs
 ___________________________________________________________________________________
< The best way to predict the future is to implement it. - David Heinemeier Hansson >
 -----------------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

senamakel and others added 5 commits September 23, 2026 12:19
Update several rustdoc links to use fully-qualified paths or plain code spans where the original link targets were not in scope, preventing broken documentation links and keeping the docs accurate.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reword the doc comment on the `finished` field to explain that a run ending in an error reports the error through the driver's `Result` rather than through this struct, making the terminal outcome semantics clearer.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Update documentation links across multiple modules to use fully-qualified paths instead of relative crate paths, ensuring they resolve correctly in generated documentation and preventing broken links when items are re-exported or restructured.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The module-level documentation in the harness crate no longer references the `memory` module, which has been moved to the separate `tinyagents-session` crate. The comment now correctly describes that transcript persistence lives in that external crate rather than being part of the harness itself.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…xedToolSet

The doc comments for `CombinedToolSet` and `PrefixedToolSet` used relative `super::` paths to reference each other, which would break when the documentation is rendered outside the module hierarchy. Changed both references to use the full `crate::` path so the links resolve correctly in generated documentation.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
@senamakel
senamakel marked this pull request as ready for review September 23, 2026 09:20
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 23, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-23T10:42:19.732611Z 43f19ca New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

…endency check

The doc lint step now passes `--all-features` to `cargo doc` so that intra-doc links into feature-gated items resolve correctly, and the `continue-on-error` flag has been removed since the previous workaround for broken links is no longer needed. The unused dependency check is now scoped to the `crates` directory to avoid scanning vendored submodules, and its `continue-on-error` flag has been dropped as the workspace findings have been cleaned up.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 14e2f95611

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/ci.yml Outdated
Updated three doc comments in the legacy module to reference `crate::entry_tree::EntryKind::Message` instead of the previous `Entry::Message` path, ensuring the documentation links resolve correctly after a refactor of the entry tree types.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
@tinysweeper

tinysweeper Bot commented Sep 23, 2026

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 14 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Changes requested
Priority: high
Reviewed head: 43f19ca69840
Updated: 1790163318 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 19 Active findings 0
Tests 1 Noted findings 0
Documentation 0 Resolved findings 54
Configuration 1 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Resolved this pass

  • Pin checkout to an immutable revision
  • Pin actions/checkout to a real version
  • Pin the cargo-machete action to an immutable revision
  • Pin the Rust toolchain action to an immutable revision
  • Pin the Rust cache action to an immutable revision
  • Pin the llvm-cov installer to an immutable revision
  • Key pull-request concurrency by PR number
  • Fix and pin the llvm-cov installer to an immutable revision
  • Do not make broken intra-doc links block merges without fixing them first
  • Pin checkout to an immutable revision
  • Do not make known rustdoc warnings gate CI
  • Pin the cargo-machete action to an immutable revision
  • Pin the Rust toolchain action to an immutable revision
  • Pin the Rust cache action to an immutable revision
  • Key pull-request concurrency by PR number
  • Pin actions/checkout to a real version
  • Pin the llvm-cov installer to an immutable revision
  • Keep known unused-dependency findings non-blocking
  • Do not make broken intra-doc links block merges without fixing them first
  • Pin checkout to an immutable revision
  • Pin the cargo-machete action to an immutable revision
  • Pin the Rust toolchain action to an immutable revision
  • Pin the Rust cache action to an immutable revision
  • Pin the llvm-cov installer to an immutable revision
  • Key pull-request concurrency by PR number
  • critical — Pin actions/checkout to a real version
  • critical — Fix and pin the llvm-cov installer to an immutable revision
  • Do not make broken intra-doc links block merges without fixing them first
  • high — Do not make broken intra-doc links block merges without fixing them first
  • high — Do not make known rustdoc warnings gate CI
  • high — Do not make known rustdoc warnings gate CI
  • high — Do not make broken intra-doc links block merges without fixing them first
  • Pin checkout to an immutable revision
  • Pin the cargo-machete action to an immutable revision
  • Pin the Rust toolchain action to an immutable revision
  • Pin the Rust cache action to an immutable revision
  • Pin the llvm-cov installer to an immutable revision
  • Key pull-request concurrency by PR number
  • Pin actions/checkout to a real version
  • Fix and pin the llvm-cov installer to an immutable revision
  • Keep known unused-dependency findings non-blocking
  • Do not make broken intra-doc links block merges without fixing them first
  • Do not make known rustdoc warnings gate CI
  • Pin checkout to an immutable revision
  • Do not make known rustdoc warnings gate CI
  • Pin the cargo-machete action to an immutable revision
  • Pin the Rust toolchain action to an immutable revision
  • Pin the Rust cache action to an immutable revision
  • Pin the llvm-cov installer to an immutable revision
  • Key pull-request concurrency by PR number
  • Pin actions/checkout to a real version
  • Fix and pin the llvm-cov installer to an immutable revision
  • Keep known unused-dependency findings non-blocking
  • Do not make broken intra-doc links block merges without fixing them first

Before merge

None.

How this fits together

flowchart LR
  n0["LoopState<br/>changed"]:::changed
  n1["Send"]:::impacted
  n2["model_node"]:::impacted
  n3["AgentHarness"]:::impacted
  n4["Channel"]:::impacted
  n5["RunContext"]:::impacted
  n6["spawn"]:::impacted
  n2 -->|uses| n0
  n2 -->|uses| n1
  n2 -->|uses| n3
  n2 -->|uses| n5
  n3 -->|uses| n1
  n4 -->|uses| n1
  n4 -->|implements| n1
  n5 -->|uses| n1
  n6 -->|uses| n1
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Failure
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 21 files; 2 findings. (2 already reported on an earlier push) _The code index is behind this pull request (indexed at `3ff4bcefdf9e`), so retrieved context may be out of date._ _3 memory call(s) failed (model: cortex: v1/answer: timed out after 20s), so this review saw part of what the engine holds._

security

  • Conclusion: Failure
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 21 files; 2 findings. (2 already reported on an earlier push) (2 earlier finding(s) still open) _The code index is behind this pull request (indexed at `3ff4bcefdf9e`), so retrieved context may be out of date._ _3 memory call(s) failed (model: cortex: v1/answer: timed out after 20s), so this review saw part of what the engine holds._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: This pull request restructures the CI workflow into parallel jobs (lint, test matrix, coverage) with pinned actions and per-leg caches, fixes numerous intra-doc links across the workspace, and improves a concurrent test to avoid a race condition. All prior findings from earlier cycles are resolved. No new defects introduced. _The code index is behind this pull request (indexed at `3ff4bcefdf9e`), so retrieved context may be out of date._ _3 memory call(s) failed (model: cortex: v1/answer: timed out after 20s), so this review saw part of what the engine holds._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: This PR parallelizes the CI workflow into separate lint, test matrix, coverage, and gate jobs, fixes caching so build artifacts are retained, and corrects all 31 broken intra-doc links and the unused-dependency finding, removing the previous `continue-on-error` workarounds. All earlier findings are fixed and no new problems are introduced. _The code index is behind this pull request (indexed at `3ff4bcefdf9e`), so retrieved context may be out of date._ _3 memory call(s) failed (model: cortex: v1/answer: timed out after 20s), so this review saw part of what the engine holds._

e2e

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No end-to-end harness in this repository: no e2e test files and no e2e workflow.
Evidence and run details
  • Models: ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash
  • Spend: $0.050537
  • Tokens: 1001989 input · 29403 output · 143030 cached · 1202 embedding
Head State Pass summary
3ff4bcefdf9e changes requested 14 active finding(s), 0 resolved finding(s) (at 1790156457)
3ff4bcefdf9e changes requested 15 active finding(s), 29 resolved finding(s) (at 1790157753)
43f19ca69840 changes requested 3 active finding(s), 72 resolved finding(s) (at 1790162603)
43f19ca69840 changes requested 4 active finding(s), 63 resolved finding(s) (at 1790162829)
43f19ca69840 changes requested 0 active finding(s), 54 resolved finding(s) (at 1790163318)

tinysweeper 0.1.0

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Requesting changes: 2 lane(s) blocking, worst finding is critical.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0391 · 741,680 in / 26,935 out · 38,344 cached (5%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 1,129 embedded
critique:    $0.0214 · 437,842 in / 12,785 out · 31,588 cached (7%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security:    $0.0126 · 263,152 in / 5,671 out  · 5,476 cached (2%)  · gpt-5.6-luna
tests:       $0.0020 · 19,643 in  / 1,322 out  · 0 cached (0%)      · deepseek/deepseek-v4-flash
description: $0.0019 · 11,489 in  / 5,021 out  · 1,280 cached (11%) · deepseek/deepseek-v4-flash

Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
@tinysweeper tinysweeper Bot added the priority: p0 Drop what you are doing. Data loss, a live break, or an exploitable hole. label Sep 23, 2026

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Requesting changes: 3 lane(s) blocking, worst finding is critical.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0544 · 964,500 in / 37,100 out · 125,168 cached (13%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash, deepseek-v4-flash · 1,129 embedded
critique:    $0.0293 · 540,593 in / 15,099 out · 71,409 cached (13%)  · gpt-5.6-luna, deepseek/deepseek-v4-flash
security:    $0.0213 · 379,128 in / 9,983 out  · 44,031 cached (12%)  · gpt-5.6-luna
tests:       $0.0025 · 21,549 in  / 3,572 out  · 1,792 cached (8%)    · deepseek/deepseek-v4-flash
description: $0.0004 · 13,395 in  / 5,433 out  · 0 cached (0%)        · deepseek-v4-flash

Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
senamakel and others added 3 commits September 23, 2026 13:28
Pin all third-party GitHub Actions to specific commit SHAs instead of version tags or branch names, adding the original tag as a comment for readability. This follows supply-chain security best practices by preventing a compromised tag from being silently updated in future workflow runs.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The concurrency group key was changed from `github.head_ref` to `github.event.pull_request.number` to prevent pull requests from different forks that share the same head branch name from being placed in the same concurrency group, which could cause one PR's required checks to be incorrectly cancelled by another.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
@senamakel
senamakel merged commit 293782d into main Sep 23, 2026
8 of 9 checks passed
@senamakel
senamakel deleted the ci-parallelize branch September 23, 2026 11:08

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Requesting changes: 3 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0168 · 239,457 in / 15,857 out · 12,924 cached (5%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 1,202 embedded
critique:    $0.0059 · 97,982 in  / 5,247 out  · 6,459 cached (7%)  · gpt-5.6-luna, deepseek/deepseek-v4-flash
security:    $0.0052 · 90,154 in  / 3,894 out  · 3,649 cached (4%)  · gpt-5.6-luna
tests:       $0.0026 · 24,317 in  / 2,960 out  · 1,536 cached (6%)  · deepseek/deepseek-v4-flash
description: $0.0017 · 16,252 in  / 1,723 out  · 1,280 cached (8%)  · deepseek/deepseek-v4-flash

Comment thread .github/workflows/ci.yml
run: cargo build --workspace --all-targets
# `--all-features` so links into feature-gated items resolve; without
# it rustdoc cannot see the very modules the docs point at.
- name: Doc lints

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

priority high security confident

Do not make known rustdoc warnings gate CI

The previous workflow explicitly marked this check as non-blocking because the workspace has roughly 160 existing broken intra-doc links. Moving it into the new lint job without continue-on-error makes those known warnings fail the required CI status and blocks every merge until unrelated documentation is fixed. Restore non-blocking behavior or fix all existing warnings before enabling the gate.


Additional tests observation

priority high confident

Do not make broken intra-doc links block merges without fixing them first

[RULE] ci-regression

The old CI kept this step with continue-on-error: true because there were ~160 existing broken intra-doc link warnings across the workspace. This revision removes the soft-fail and makes it a hard gate, but only fixes links in the files it touches. The remaining broken links elsewhere will cause the lint job to fail, blocking all merges until they are all cleaned up. Either fix every broken link across the workspace in this PR, or restore continue-on-error: true with the TODO comment until a dedicated cleanup covers the rest.

[RULE] nonblocking-known-ci-failure ·

Comment thread .github/workflows/ci.yml
# Scoped to `crates`, this repo's own workspace: an unscoped run also
# walks `vendor/`, where it reports findings against the vendored
# submodules that have to be fixed in their own repositories.
- name: Unused dependencies

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

priority high security confident

Keep known unused-dependency findings non-blocking

The old step used continue-on-error because the workspace already has known unused-dependency findings. This replacement runs the pinned action as a required lint without that exemption, so the new lint job will fail until unrelated existing findings are cleaned up, blocking all merges.

[RULE] nonblocking-known-ci-failure ·

@tinysweeper tinysweeper Bot added priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition. and removed priority: p0 Drop what you are doing. Data loss, a live break, or an exploitable hole. labels Sep 23, 2026

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Requesting changes: 3 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0680 · 1,060,865 in / 32,599 out · 61,161 cached (6%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 1,202 embedded
critique:    $0.0330 · 572,965 in   / 14,402 out · 34,559 cached (6%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security:    $0.0294 · 436,699 in   / 7,489 out  · 12,778 cached (3%) · gpt-5.6-luna
tests:       $0.0028 · 24,206 in    / 3,880 out  · 1,792 cached (7%)  · deepseek/deepseek-v4-flash
description: $0.0019 · 16,141 in    / 3,139 out  · 1,280 cached (8%)  · deepseek/deepseek-v4-flash

Comment thread .github/workflows/ci.yml
# Scoped to `crates`, this repo's own workspace: an unscoped run also
# walks `vendor/`, where it reports findings against the vendored
# submodules that have to be fixed in their own repositories.
- name: Unused dependencies

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

priority high critique confident

Keep known unused-dependency findings non-blocking

The previous step was explicitly non-blocking because the workspace has existing unused-dependency findings. This revision removes continue-on-error, so cargo-machete will make the lint job fail on that pre-existing debt and block every merge. Restore non-blocking behavior until those findings are cleaned up, or remove the exception only together with the dependency fixes.


Additional tests observation

priority high confident

Keep known unused-dependency findings non-blocking

[RULE] known-issues-gate-ci

The workspace previously had continue-on-error: true on this step because of existing unused-dependency findings across the workspace. This change removes that guard without fixing the findings. If any such findings remain, the lint job will fail and block merges. Either fix all unused-dependency issues in this PR or add back continue-on-error: true.

[RULE] known-warning-gate ·

Comment thread .github/workflows/ci.yml
# it rustdoc cannot see the very modules the docs point at.
- name: Doc lints
env:
RUSTDOCFLAGS: -D rustdoc::broken_intra_doc_links

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

priority high security confident

Do not make broken intra-doc links block merges without fixing them first

This change removes the previous non-blocking behavior for the known broken intra-doc-link set and places the check in the required lint job. Existing broken links therefore prevent the aggregate ci job from succeeding even when the code and tests pass. Repair the links before enforcing this gate, or preserve the non-blocking behavior.

[RULE] broken-intra-doc-links-gate ·

Comment thread .github/workflows/ci.yml
run: cargo build --workspace --all-targets
# `--all-features` so links into feature-gated items resolve; without
# it rustdoc cannot see the very modules the docs point at.
- name: Doc lints

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

priority high tests confident

Do not make known rustdoc warnings gate CI

The workspace has approximately 160 existing broken intra-doc link warnings (as noted in earlier review context). This change removes the continue-on-error: true from the doc lints step without fixing those links, so the step will now fail the lint job and block merges on pre-existing issues. Either fix the broken links in this PR or restore continue-on-error: true until they are resolved.

[RULE] known-issues-gate-ci ·

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Requesting changes: 2 lane(s) blocking, worst finding is low.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0505 · 1,001,989 in / 29,403 out · 143,030 cached (14%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 1,202 embedded
critique:    $0.0265 · 536,881 in   / 13,148 out · 67,799 cached (13%)  · gpt-5.6-luna, deepseek/deepseek-v4-flash
security:    $0.0202 · 413,982 in   / 7,526 out  · 42,207 cached (10%)  · gpt-5.6-luna
tests:       $0.0010 · 24,230 in    / 2,907 out  · 24,064 cached (99%)  · deepseek/deepseek-v4-flash
description: $0.0020 · 16,165 in    / 3,234 out  · 0 cached (0%)        · deepseek/deepseek-v4-flash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant