Skip to content

fix(ci): build the engine package before Typecheck, not just before coverage#5093

Merged
JSONbored merged 1 commit into
mainfrom
fix/ci-engine-build-order-typecheck
Jul 11, 2026
Merged

fix(ci): build the engine package before Typecheck, not just before coverage#5093
JSONbored merged 1 commit into
mainfrom
fix/ci-engine-build-order-typecheck

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

validate-code's "Build engine package" step ran after "Typecheck", so packages/gittensory-engine/dist/ (git-ignored, only materializes once that step runs — package.json's main/types point at dist/index.js/dist/index.d.ts) didn't exist yet when tsc --noEmit ran.

This was latent until PR #5082 (test/unit/miner-coding-agent-house-rules.test.ts) added the first real (non-vi.mock) import type {...} from "@jsonbored/gittensory-engine" in a root test file — needing the package specifier's dist/-backed types, not just its vi.mock-redirected runtime behavior — which failed TS2307: Cannot find module in CI while typechecking fine locally against a stale leftover dist/ from a previous local build.

The exact same ordering class was already fixed once, for "Test with coverage" — the #ci-engine-build-order comment right above the build step documents it: src/mcp/find-opportunities.ts imports committed miner-lib JS that itself imports @jsonbored/gittensory-engine, so any backend test run needs dist/ built first, not just a PR that touches packages/gittensory-engine/** directly. That fix moved the build step ahead of coverage, but it stayed after Typecheck — so Typecheck kept the exact same gap coverage used to have.

This moves "Build engine package" ahead of both "Restore TypeScript incremental build cache"/"Typecheck" and "Test with coverage" instead of just the latter, and updates the comment to explain both consumers.

Validation

  • Confirmed the failure mode directly: removed packages/gittensory-engine/dist locally and ran npm run typecheck — passed clean (proving the OLD order's local-vs-CI discrepancy was purely about a stale leftover dist/ masking the gap, not a false alarm).
  • Verified the fix the same way: removed dist/ again, ran npm run build --workspace @jsonbored/gittensory-engine (the exact command the moved step runs), then npm run typecheck in that order — passes clean, matching the new CI sequence.
  • npm run actionlint — clean on the modified workflow file.
  • Workflow-only change; no application code touched, no new tests needed.

Scope

  • Conventional Commit title.
  • Focused: 1 file (.github/workflows/ci.yml), reordering + comment update only.
  • No app code, no site//CNAME/VitePress.

Safety

  • No secrets/credentials touched.
  • Widens no permissions, adds no new external actions — pure step reordering within the existing job.
  • Verified locally with the exact commands CI runs, in the new order, before pushing.

Notes

Checked .github/workflows/release-selfhost.yml for the same class of bug — it already builds the engine package correctly (before its self-host bundling step), with its own #ci-engine-build-order (release pipeline) comment noting it's a separate instance of the same fix. No change needed there.

…overage

validate-code's "Build engine package" step ran after Typecheck, so
packages/gittensory-engine/dist/ (git-ignored, only materializes once
that step runs) didn't exist yet when tsc ran. This was latent until a
root test file added the first real (non-vi.mock) `import type` from
the "@jsonbored/gittensory-engine" package specifier -- needing its
dist/-backed types, not just its vi.mock-redirected runtime behavior --
which failed TS2307 in CI while passing locally against a stale
leftover dist/ (PR #5082). The exact same ordering class was already
fixed once for "Test with coverage" (#ci-engine-build-order); this
moves the build step ahead of both consumers instead of just one.

Verified by removing packages/gittensory-engine/dist locally, running
the build step, then typecheck in that order -- passes clean, matching
the new CI sequence.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.20%. Comparing base (e2bc19b) to head (09e9fa1).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5093   +/-   ##
=======================================
  Coverage   94.20%   94.20%           
=======================================
  Files         470      470           
  Lines       39736    39736           
  Branches    14501    14501           
=======================================
  Hits        37435    37435           
  Misses       1645     1645           
  Partials      656      656           
Flag Coverage Δ
shard-1 46.42% <ø> (ø)
shard-2 33.60% <ø> (ø)
shard-3 31.34% <ø> (ø)
shard-4 32.82% <ø> (ø)
shard-5 33.54% <ø> (ø)
shard-6 44.90% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 11, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-11 14:48:38 UTC

1 file · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · unstable

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This reorders a single CI workflow step: "Build engine package" is moved from after Typecheck (and after the tsbuildinfo cache restore) to before both Typecheck and Test with coverage, and the explanatory comment is moved/expanded accordingly. The fix directly addresses the described bug — packages/gittensory-engine/dist/ is gitignored and only materializes after this build step, so tsc --noEmit against a real (non-mocked) `import type` from the package specifier would fail TS2307 in CI while passing locally against a stale dist/. The diff is a pure reordering of an existing step (same `if` condition, same run command) plus comment relocation, so it carries very low risk of new breakage.

Blockers

  • PR description: this does not clearly close or link an eligible open issue; add the issue link or explain which maintainer-authorized issue this fixes.
Nits — 5 non-blocking
  • The comment is fairly long (10+ lines) for a single step; consider trimming to the essential causal chain now that it's stated twice historically in this file's git history.
  • Confirm the "Restore TypeScript incremental build cache" step (still after the build step per the diff) doesn't need engine dist/ to be present when populating .tsbuildinfo, though this is unlikely to matter since the build step now runs first.
  • Consider extracting the shared rationale into a short single-line comment with a link/anchor (already done via `#ci-engine-build-order`) rather than repeating the full multi-sentence history inline, to ease future maintenance if the step moves again.
  • .github/workflows/ci.yml:271: nit: the expanded inline comment is useful but dense enough that future edits to the workflow will be harder to scan; consider moving the historical detail into a shorter comment plus the relevant issue/PR references.
  • .github/workflows/ci.yml:271: keep the operational reason in the workflow comment and move the longer local-vs-CI narrative to the linked issue or PR discussion.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 46 registered-repo PR(s), 38 merged, 416 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 46 PR(s), 416 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ⚠️ ℹ️ Insufficient signal risk: clean · value: insufficient-signal — Nothing measurable for the structural-improvement analyzers on this PR (e.g. no code files changed).
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 46 PR(s), 416 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 11, 2026
@JSONbored JSONbored merged commit 6a04e8e into main Jul 11, 2026
18 checks passed
@JSONbored JSONbored deleted the fix/ci-engine-build-order-typecheck branch July 11, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant