ci: parallelise the workflow and stop discarding the build cache - #202
Conversation
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>
|
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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (21)
✨ Finishing Touches📝 Generate docstrings
Comment |
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>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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>
There was a problem hiding this comment.
💡 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".
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>
Tiny Sweeper reviewTiny 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 Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Resolved this pass
Before mergeNone. How this fits togetherflowchart 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
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
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>
There was a problem hiding this comment.
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
| 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 |
There was a problem hiding this comment.
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
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 ·
| # 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 |
There was a problem hiding this comment.
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 ·
There was a problem hiding this comment.
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
| # 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 |
There was a problem hiding this comment.
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
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 ·
| # it rustdoc cannot see the very modules the docs point at. | ||
| - name: Doc lints | ||
| env: | ||
| RUSTDOCFLAGS: -D rustdoc::broken_intra_doc_links |
There was a problem hiding this comment.
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 ·
| 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 |
There was a problem hiding this comment.
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 ·
There was a problem hiding this comment.
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
Summary
CI takes ~23 minutes per run, and every PR commit triggers two of them. This
restructures the single 20-step
Rust SDKjob into four parallel jobs and fixesthe caching that the old layout was actively defeating.
Measured baseline, from the two runs on #196
(23m20s and 22m22s for the same commit):
What was slow, and why
Every run happened twice.
on: pushwith no branch filter fires for eachPR branch push alongside
on: pull_request. The concurrency group keyed ongithub.refdiffers between the two events, so neither cancelled the other —two full copies of a 23 minute suite per commit.
The 11m34s step was five cold builds in a row.
Test optional features independentlyrancargo cleanbefore each ofsqlite,toolsandmultimodal, because each feature selection links a different workspacegraph and they cannot share a target directory. Serialised in one job, that
is three from-scratch compiles of the dependency graph back to back.
The cache was being thrown away. Those
cargo cleancalls, plusFree build artifacts before coverage, meant that by the timeSwatinem/rust-cacheran its post step the target directory held only the coverage and rustdoc
artifacts. The build profile that the 6-minute
Buildsteps produce wasnever saved, so every run started those from cold.
What changed
on: pushis limited tomain. PR commits are built once, bypull_request. Mainline pushes still run, which is what populates the cachethat PR branches restore from. The concurrency group now uses
github.head_ref || github.refso a PR's runs coalesce and superseded commitscancel.
testmatrix —default,all-features,sqlite,tools,multimodal. They build concurrentlyinstead of serially, no
cargo cleanis needed because nothing shares atarget directory any more, and each leg carries its own
rust-cachekey so awarm cache survives from run to run.
lintis its own job.cargo clippyandcargo docstop at metadata andnever link, so fmt/clippy/doc-lints/machete now report in ~2 minutes rather
than behind 20 minutes of builds.
coverageis its own job with its own cache key. Instrumented objects arenot interchangeable with the test matrix's, which is exactly why the old
layout had to
cargo cleanbefore it.CARGO_INCREMENTAL=0(incremental only pays off acrossedits 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 targetsize). The coverage step already set the latter; it now applies everywhere.
CIgate job that depends on all the others, so branch protection canrequire one check instead of being edited whenever the matrix changes.
Measured effect
Latest run:
all green, 2m37s wall clock, against 23m20s on #196.
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 lintsandUnused dependenciescarriedcontinue-on-error: trueand a TODO. Both were reporting real annotations on every run, and both turned
out to be cheap to fix, so the
continue-on-erroris gone and they now block:cargo docwas run without--all-features, so links into feature-gateditems could not resolve — rustdoc could not see the modules the docs pointed
at. With
--all-featuresand 31 link targets corrected acrosstinyagents-graph,tinyagents-harnessandtinyagents-session, a cleancargo clean --doc && cargo doc --workspace --no-deps --all-featuresunder-D rustdoc::broken_intra_doc_linksnow exits 0. The TODO's "~160 warnings"was stale; the real count was 31.
Most were ordinary wrong paths (
Entry::Message→EntryKind::Message,crate::retry::RetryMiddleware→crate::middleware::library::RetryMiddleware,super::PrefixedToolSetfrom a sibling toolset member → thetoolsetre-export). Three classes needed a judgement call rather than a repath:
(
crate::subagent::SubAgentis intinyagents-orchestration, which dependson the harness, not the other way round) became plain code spans.
channel_from_config,boundary::CompiledGraph::advance)became plain code spans, matching how the same references are already
written elsewhere in those modules.
LoopState::final_errornames a fieldthat does not exist (a failed run reports through the driver's
Result),and
tinyagents_harness::memorywith itsChatHistory/ShortTermMemorynames 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_linkswarnings, adifferent lint that is not denied and was not in scope here.
cargo-machetewas walking the whole checkout includingvendor/, andits one finding (
tracinginvendor/tinytools/crates/tinytools-jev) is in avendored submodule that has to be fixed in its own repository. Scoped to
crateswithargs: crates, it is clean.No behaviour changed — the doc work is comments only.
The
Rust SDKstatus check no longer exists. If branch protection requires it,update the rule to require
CIinstead, otherwise PRs will block foreverwaiting 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