feat(engine): cache ctx.ast() parse results within a single check run#487
Conversation
…closes #482) Add an astResults map to RunCaches, mirroring cachedGlob/cachedFileText, keyed on the NUL-joined (absPath, language, rev, comments) tuple. The promise is cached, so concurrent identical calls collapse into one in-flight parse/interpreter spawn. Rejected promises stay cached — a deliberate decision consistent with ctx.ast()'s fail-closed contract: every rule touching the same input fails fast with the identical error. The cheap argument-validation guardrails (path safety, language plausibility, feature guard) still run per call before the cache lookup, preserving ARCH-022's guardrail ordering on cache hits. The python doc-only-edit test now parses its "real edit" in a separate runChecks invocation: a mid-run working-tree edit is deliberately not observable within one run, matching readFile's existing cache semantics. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Deploying archgate-cli with
|
| Latest commit: |
9afa9df
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://168ca254.archgate-cli.pages.dev |
| Branch Preview URL: | https://feat-ast-parse-cache.archgate-cli.pages.dev |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_3a7edb75-f799-4086-953e-2b8bd7a252bf) |
A cached parse rejection is shared by every caller of the same input tuple, but its message interpolated the raw, closure-captured path from whichever caller populated the cache first. Since safePath() collapses aliased spellings (e.g. src/./a.ts and src/a.ts) into one cache entry, a later caller could receive an error mentioning another rule's spelling. Interpolate the normalized repo-relative path in everything thrown from the cached parse closure instead. The per-call guardrail errors ahead of the cache lookup still echo the caller's own spelling. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_3165095f-1834-42cf-a039-c82d8f2e1cdd) |
Code Coverage
Full HTML report available in workflow artifacts. Per-directory breakdown
|
## Problem `code-pull-request.yml` and `dco.yml` gate every job on `github.event.pull_request.draft == false`, but their `pull_request` triggers only listed `[opened, edited, synchronize, reopened]`. A PR opened as draft therefore got a run where every job was skipped — which made the "Validate Code" fan-in gate report failure — and marking the PR ready for review never triggered a fresh run, since `ready_for_review` was not in the trigger list. The PR stayed stuck with a spurious failed/skipped check until an unrelated push. This surfaced on #485, #486, and #487. ## Fix Add `ready_for_review` to the `pull_request` trigger `types` in both workflows, so promoting a draft to ready triggers a run whose jobs actually execute. Two-line diff, nothing else changed. ## Validation - `bun run validate` passes (lint, typecheck, format:check, 1558 tests, ADR check, knip, build check) - `bun run cli check` clean: 44/44 rules pass, 0 warnings Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
# archgate ## [0.50.0](v0.49.0...v0.50.0) (2026-07-17) ### Features * **engine:** cache ctx.ast() parse results within a single check run ([#487](#487)) ([40b39d3](40b39d3)), closes [#482](#482) * **engine:** ctx.findAstNodes() generic AST node collector ([#486](#486)) ([f14b73b](f14b73b)), closes [#483](#483) * **engine:** support { comments: true } for ctx.ast() Ruby ([#485](#485)) ([36b891f](36b891f)), closes [#484](#484) ### Bug Fixes * **ci:** run PR workflows when a draft is marked ready for review ([#488](#488)) ([ab1fc96](ab1fc96)), closes [#485](#485) [#486](#486) --- This PR was generated with [simple-release](https://github.com/TrigenSoftware/simple-release). <details> <summary>📄 Cheatsheet</summary> <br> You can configure the bot's behavior through a pull request comment using the `!simple-release/set-options` command. ### Command Format ````md !simple-release/set-options ```json { "bump": {}, "publish": {} } ``` ```` ### Useful Parameters #### Bump | Parameter | Type | Description | |-----------|------|-------------| | `version` | `string` | Force set specific version | | `as` | `'major' \| 'minor' \| 'patch' \| 'prerelease'` | Release type | | `prerelease` | `string` | Pre-release identifier (e.g., "alpha", "beta") | | `firstRelease` | `boolean` | Whether this is the first release | | `skip` | `boolean` | Skip version bump | | `byProject` | `Record<string, object>` | Per-project bump options for monorepos | #### Publish | Parameter | Type | Description | |-----------|------|-------------| | `skip` | `boolean` | Skip publishing | | `access` | `'public' \| 'restricted'` | Package access level | | `tag` | `string` | Tag for npm publication | ### Usage Examples #### Force specific version ````md !simple-release/set-options ```json { "bump": { "version": "2.0.0" } } ``` ```` #### Force major bump ````md !simple-release/set-options ```json { "bump": { "as": "major" } } ``` ```` #### Create alpha pre-release ````md !simple-release/set-options ```json { "bump": { "prerelease": "alpha" } } ``` ```` #### Publish with specific access and tag ````md !simple-release/set-options ```json { "bump": { "prerelease": "beta" }, "publish": { "access": "public", "tag": "beta" } } ``` ```` ### Access Restrictions The command can only be used by users with permissions: - repository owner - organization member - collaborator ### Notes - The last comment with `!simple-release/set-options` command takes priority - JSON must be valid, otherwise the command will be ignored - Parameters apply only to the current release execution - The command can be updated by editing the comment or adding a new one </details> <!-- Please do not edit this comment. simple-release-pull-request: true simple-release-branch-from: release simple-release-branch-to: main --> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Closes #482
Summary
ctx.ast()now memoizes parse results for the lifetime of a singlearchgate checkrun. A newastResultsmap onRunCaches(src/engine/runner.ts) mirrors the existingcachedGlob/cachedFileTextpattern: the promise is cached, so concurrent identical calls collapse into one in-flight parse/interpreter spawn, not just sequential ones.Design decisions
(absPath, language, rev ?? "working-tree", comments ?? false), NUL-joined (astCacheKey()insrc/engine/ast-support.ts). NUL cannot appear in a path, so distinct tuples can never collide.{ rev: "base" }vs working-tree and{ comments: true }vs false/omitted therefore cache independently;comments: falseand omitted share an entry (identical output).ctx.ast()is fail-closed per ARCH-022 — it throws, never returns a sentinel — so every rule touching the same input in a run fails fast with the identical error instead of re-paying the guardrail chain and subprocess spawn. Documented in a code comment at the cache site and asserted by tests (same error instance, single spawn).safePath, 2: language plausibility viaAST_LANGUAGE_EXTENSIONS, plus the{ comments }feature guard) still run on every call, before the cache lookup. Only the expensive work behind them (in-process parse, or guardrails 3–4: interpreter probe + guarded invocation) is wrapped in the cachedparseUncached()closure. This preserves ARCH-022's mandated guardrail ordering on cache hits too — an invalid path or implausible language fails per-call exactly as before — and theast-guardrail-orderingcompanion rule (which walksastImpl's body and asserts the four markers appear in order) still passes.RunCachesdoc comment and the rule-api docs now state rules must treat the returned tree as read-only.ctx.ast()'s signature, overloads, and return types are untouched. No cross-run/disk persistence: the cache lives and dies with onerunChecksinvocation, same lifetime asglobResults/fileText.runChecksinvocation; a mid-run working-tree edit is deliberately not observable within one run, matchingreadFile's existing per-run cache semantics.Docs
rule-api.mdxgained one short paragraph in theast()section documenting the per-run cache, the key tuple, the shared-error behavior, and the read-only contract — updated in all three locales (English, Norwegian bokmål, Brazilian Portuguese).Acceptance criteria
ctx.ast()calls with identical(path, language, rev, comments)within onearchgate checkrun result in exactly one interpreter spawn / parse —tests/engine/runner-ast-cache.test.ts: TS instance-identity test across three rules, plus a Python test spying onBun.spawnand counting exactly one-Iinvocation for three rules.Promise.alltest asserts both callers receive the same tree instance.{ rev: "base" }and working-tree parses of the same path are cached independently (never collide) — git-fixture test: base tree parsesfoo, working tree parsesbar, repeats within each revision share the same instance.{ comments: true }and{ comments: false }(or omitted) parses of the same path/rev are cached independently — comments variant carries acommentsarray, the plain one does not;comments: falseand omitted share (annotation: same tuple, identical output by construction).-Ispawn (Python,skipIfno interpreter, matching existing test conventions).Validation
bun run validate(lint, typecheck, format:check, tests, ADR check, knip, build check) passes;archgate checkreports 44/44 rules passing.