Skip to content

v1.5.1 — polish: docs accuracy, test reliability, edge-case observability - #6

Merged
fabio-dee merged 23 commits into
mainfrom
release/v1.5.1
Apr 27, 2026
Merged

fabio-dee merged 23 commits into
mainfrom
release/v1.5.1

Conversation

@fabio-dee

@fabio-dee fabio-dee commented Apr 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

Polish release. No user-visible behavior changes. No safety invariants (INV-S1..INV-S6) touched. No interactive picker TUI files modified.

  • Documentation accuracy: README v1.6 roadmap section rewritten as an abstract CLI/JSON API contract; CHANGELOG ## [1.5.1] entry added.
  • Audit-trail invariant: packages/internal/src/remediation/purge.ts documents the "if it isn't journaled, it didn't happen" contract and locks it with in-source tests against manifest_write_failed: regressions.
  • Test reliability + edge-case observability: B1–B6 + remaining Tier A (A2–A5) + opportunistic Tier C (C1, C2, C4, C5) — 12 atomic per-file fixes spanning test helpers, banner formatting, scan-memory Windows-path normalization, change-plan canonical-ID call sites, glyph NO_COLOR semantics, JSON-SCHEMA MD028 fix, change-plan table arrow alignment, bundle-size-check error formatting, pagination-500 test header.

Scope

  • 18 commits between v1.5.0 (b26ae7e) and HEAD on release/v1.5.1.
  • pnpm verify green (typecheck + lint + build + 1788 tests + format:check).
  • Files in packages/internal/src/remediation/{bust,restore,reclaim,manifest,checkpoint}.ts — UNTOUCHED.
  • Interactive picker TUI files — UNTOUCHED.

Test plan

  • pnpm verify green on CI
  • Spot-check the README v1.6 section reads as a stability contract, not a feature spec
  • CHANGELOG ## [1.5.1] entry rendered correctly
  • No regressions in archive / restore / reclaim flows (smoke test)

Summary by CodeRabbit

  • New Features

    • Human-readable bundle-size messages and improved UI column alignment/spacing.
  • Documentation

    • README expanded with interactive picker example and clarified v1.6 CLI/JSON API framing.
    • Clarified no-color behavior and refreshed domain-folder examples.
    • Added changelog entry for v1.5.1.
  • Bug Fixes

    • Improved test reliability, tightened assertions, and fixed timing/termination edge cases.
  • Chores

    • Package version bumped to 1.5.1 and added targeted regression/test coverage.

Fabio-D added 18 commits April 27, 2026 16:22
Replace the example domain-folder name in README.md's negative-list
narrative without changing scanner behavior.
- Replace concrete subcommand examples with stability guarantees
- Preserve link to docs/JSON-SCHEMA.md and apiVersion mention
- Removes release-blocker leaks ahead of v1.5.1
- Extend top-of-file block-comment with "Audit-trail invariant" subsection
- Formalize the philosophical contract: "if it isn't journaled, it didn't happen"
- Document why manifest_write_failed: failures count identically to disk-mutation failures in the all-failed gate
- No production code changes; documentation-only addition
…ests

- Add in-source tests asserting writeOp throw produces a failure record carrying the manifest_write_failed: prefix and counted in the failure tally (not success tally)
- Add companion single-op test confirming all-failed gate fires identically for manifest_write_failed: as for disk-mutation failures
- Locks Option (a): carving manifest_write_failed: out of the all-failed gate is now a regression
Inserted '---' separator between the manifest-casing exception blockquote
and the pre-dispatch validation envelope blockquote. Full-file scan found
no additional MD028 instances.
Note: the file contains 4 it() cases, not 5 as the design source assumed.
The header now mirrors reality — tests are the source of truth.
- Insert ## [1.5.1] - 2026-04-27 section above v1.5.0 header
- Three subsections: Changed (README + _glyphs doc), Fixed (purge audit-
  trail, JSON-SCHEMA MD028, change-plan canonical-ID, scan-memory Windows
  paths), Tests (B1-B6 + Tier C fold-ins)
- v1.5.0 entry preserved byte-for-byte
@coderabbitai

coderabbitai Bot commented Apr 27, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@fabio-dee has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 51 minutes and 22 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0662c94e-0da7-41f2-b577-e340b8ff6a6b

📥 Commits

Reviewing files that changed from the base of the PR and between 32d6c5e and 7abe543.

📒 Files selected for processing (1)
  • README.md
📝 Walkthrough

Walkthrough

Patch release v1.5.1: documentation and README clarifications, test reliability and alignment fixes, small UI/formatting tweaks, replacement of hard-coded canonical-id literals in tests with helper calls, and added regression tests (including Windows path-normalization and purge audit-trail scenarios).

Changes

Cohort / File(s) Summary
Release & Version
CHANGELOG.md, apps/ccaudit/package.json
Adds v1.5.1 changelog entry and bumps apps/ccaudit version from 1.5.0 → 1.5.1.
README & Schema Docs
README.md, docs/JSON-SCHEMA.md
Reframes v1.6 CLI/JSON API contract text, updates README examples and domain list, swaps em dashes for hyphens, and inserts a visual separator in JSON schema docs.
NO_COLOR / Glyphs
packages/terminal/src/tui/_glyphs.ts
Docs updated to accurately describe NO_COLOR behavior (non-empty string disables color); predicate logic unchanged.
Test Helpers & Fixtures
apps/ccaudit/src/__tests__/_test-helpers.ts, apps/ccaudit/src/__tests__/fixtures/tmux-e2e.ts
Renames maxWaitMs→graceMs in docs, prevents post-timeout Promise resolve in runCcauditGhost, and removes redundant ANSI stripping in TmuxE2ESession.waitForText.
Tests — Comments & Assertions
apps/ccaudit/src/__tests__/pagination-500.test.ts, apps/ccaudit/src/__tests__/restore-corrupt-manifest.test.ts, apps/ccaudit/src/__tests__/restore-json-envelope.test.ts
Aligns top-of-file comments with actual assertions, replaces non-null assertion with explicit null check, and expands envelope JSDoc to document expected fields.
Remediation & Purge Tests
packages/internal/src/remediation/change-plan.ts, packages/internal/src/remediation/purge.ts
Tests now derive canonical IDs via canonicalItemId(...) instead of literals; adds “audit-trail invariant” docs and tests for executePurge behavior when journal writeOp fails (multi-op vs single-op outcomes).
Scanner Tests
packages/internal/src/scanner/scan-memory.ts
Adds tests ensuring Windows backslash-to-forward-slash normalization for import-chain name generation (handles mixed separators and nested paths).
UI / Output Formatting
apps/ccaudit/scripts/bundle-size-check.mjs, packages/terminal/src/tables/change-plan.ts, packages/terminal/src/tui/_force-partial-banner.ts
Adds formatBytes helper for human-readable budget display; aligns ARCHIVE table columns using padEnd(8); removes leading-space from zero-protected suffix and ensures single-space insertion when needed (unit test added).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I nibbled docs and tests today,
Swapped slashes, trimmed a double-space away,
IDs now fetched the helper's song,
Purge checks sing where writes go wrong,
V1.5.1 hops along — hooray! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the PR's main focus: a v1.5.1 polish release addressing documentation accuracy, test reliability, and edge-case observability improvements across the codebase.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch release/v1.5.1

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.

❤️ Share

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

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
README.md (1)

34-34: ⚠️ Potential issue | 🟡 Minor

Update README "Current release" and "Current package version" to 1.5.1.

The CHANGELOG adds a ## [1.5.1] entry and apps/ccaudit/package.json is at 1.5.1, but README.md still advertises v1.5.0 at line 34 and line 686. Update both so users see the released version.

📝 Suggested fixes

Line 34:

-Current release: **v1.5.0** — interactive archive picker, interactive restore picker, fuzzy-match restore by name, archive purge. See [CHANGELOG.md](./CHANGELOG.md).
+Current release: **v1.5.1** — interactive archive picker, interactive restore picker, fuzzy-match restore by name, archive purge. See [CHANGELOG.md](./CHANGELOG.md).

Line 686:

-- Current package version: **1.5.0**
+- Current package version: **1.5.1**
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 34, Update the two occurrences of the displayed version in
README: change the "Current release: **v1.5.0**" text (the "Current release"
line) and the "Current package version" value (the "Current package version"
line) from v1.5.0 to v1.5.1 so the README matches the changelog and
apps/ccaudit/package.json; search for the literal "v1.5.0" or the headings
"Current release" and "Current package version" and replace with "v1.5.1".
🧹 Nitpick comments (5)
apps/ccaudit/src/__tests__/fixtures/tmux-e2e.ts (1)

145-168: LGTM — redundant strip removed, invariant preserved.

The mapping ansi: opts.stripAnsi === false correctly delegates ANSI handling to capture():

  • stripAnsi unset/true → ansi: false → capture() returns stripped output.
  • stripAnsi: false → ansi: true → capture() returns raw bytes.

So lastCapture = raw is in the desired shape in both branches and matching/error reporting are unaffected.

Optional nit: the local name raw is a little misleading in the default path (where capture() has already stripped ANSI). Renaming to something like captured would more accurately reflect the dual-state semantics, but it's purely cosmetic.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/ccaudit/src/__tests__/fixtures/tmux-e2e.ts` around lines 145 - 168, The
code in waitForText correctly delegates ANSI handling to capture, but the local
variable name raw is misleading because capture may already strip ANSI; rename
raw to captured (and update its usage where assigned to lastCapture and where
it's declared) and adjust the adjacent comment to reflect that capture returns
either stripped or raw output depending on opts.stripAnsi (via ansi:
opts.stripAnsi === false), leaving all logic and the matching/error message
unchanged; update references to raw, lastCapture assignment, and the comment
inside waitForText accordingly.
apps/ccaudit/scripts/bundle-size-check.mjs (2)

17-26: Minor formatting inconsistency in formatBytes non-finite fallback.

The non-finite branch returns ${bytes}B (no space) while the byte branch on line 25 returns ${bytes} B (with space). Not currently reachable for BUDGET_BYTES, but worth normalizing to keep output uniform.

♻️ Optional refactor
-  if (!Number.isFinite(bytes)) return `${bytes}B`;
+  if (!Number.isFinite(bytes)) return `${bytes} B`;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/ccaudit/scripts/bundle-size-check.mjs` around lines 17 - 26, The
non-finite branch in function formatBytes returns `${bytes}B` (no space) while
other branches use a space before "B"/"KB"/"MB"; update the non-finite return to
use a space (e.g. `${bytes} B`) so output formatting is consistent in
formatBytes, and ensure the change is applied only inside the formatBytes
function to normalize spacing across all branches.

96-99: Optional: apply formatBytes to phase-local FAIL message too.

The primary FAIL message on line 60 now reports a human-readable budget, but the phase-local FAIL on line 98 still emits raw ${phaseBudget}B. Using formatBytes(phaseBudget) here keeps the messages consistent.

♻️ Optional refactor
-      `[bundle-size] FAIL: phase-local delta exceeds ${phaseBudget}B (${phaseDelta} > ${phaseBudget})`,
+      `[bundle-size] FAIL: phase-local delta exceeds ${formatBytes(phaseBudget)} (${phaseDelta} > ${phaseBudget})`,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/ccaudit/scripts/bundle-size-check.mjs` around lines 96 - 99, The
phase-local FAIL log uses raw bytes ("${phaseBudget}B") instead of the
human-readable formatter; update the console.error in the branch that checks if
(phaseDelta > phaseBudget) to use formatBytes(phaseBudget) (and keep phaseDelta
as-is or format if desired) so messages match the primary FAIL format—look for
the variables phaseDelta, phaseBudget and the formatBytes function in
bundle-size-check.mjs and replace the raw `${phaseBudget}B` insertion with
formatBytes(phaseBudget).
CHANGELOG.md (1)

36-53: Minor: categorization mismatch in ### Fixed vs ### Tests.

The scan-memory.ts bullet (lines 36–38) explicitly notes the normalization "was already correct; the test locks it" — i.e., it's regression coverage, not a behavior fix. Since you've already introduced a dedicated ### Tests section directly below, moving this entry there would be more consistent and avoid implying a user-visible fix landed.

Also, ### Tests isn't part of the Keep a Changelog 1.1.0 category set referenced at the top of the file (Added/Changed/Deprecated/Removed/Fixed/Security). Not a blocker — many projects extend the schema — but worth a thought if you want to stay strictly conformant.

📝 Suggested move
 ### Fixed
@@
 - `change-plan.ts`: replaced hard-coded canonical-ID literal strings with
   calls to the exported `canonicalItemId(...)` helper, eliminating drift
   risk if the format string changes.
-- `scan-memory.ts`: added Windows path-normalization regression coverage
-  (in-source test). The normalization itself was already correct; the
-  test locks it.

 ### Tests

+- `scan-memory.ts`: added Windows path-normalization regression coverage
+  (in-source test). The normalization itself was already correct; the
+  test locks it.
 - Test-helper polish: replaced a dead `void killed` no-op with a real
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CHANGELOG.md` around lines 36 - 53, The changelog places the `scan-memory.ts`
bullet under `### Fixed` but it describes regression test coverage, so move that
bullet into the existing `### Tests` section (or change its phrasing to
explicitly say "test/regression coverage" rather than a fix); update the
`scan-memory.ts` line to read that it adds Windows path-normalization regression
coverage (in-source test) and remove any wording that implies a user-visible
fix; optionally, if you want strict Keep a Changelog conformance, replace or
omit the `### Tests` heading and instead add the bullet under an appropriate
official category (e.g., `Changed` or a dedicated testing note) so the entry's
intent matches its section.
README.md (1)

704-718: LGTM — abstract contract framing is the right call for an unreleased v1.6.

Removing the concrete ccaudit game ... subcommand list and the external command: "game.*" envelope guarantee in favor of apiVersion framing avoids prematurely committing to a surface that isn't shipped yet. The "not part of the contract" negative bullets (TUI, tables, env vars, internal TS, path-shaped canonical IDs) read as a clear stability boundary.

One small thought: the section header dropped "game" but the surrounding paragraph doesn't say what is covered (commands? envelopes? both?). If the v1.6 release notes aren't ready yet, consider a one-line "scope: report commands' --json output" hint so embedders reading this today have a rough mental model. Optional.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 704 - 718, The header "v1.6 stable CLI/JSON API
contract" and paragraph mention `apiVersion` and `--json` but don't state what
is in-scope; add a single clarifying line under that paragraph such as "Scope:
report commands' `--json` output (CLI subcommands' JSON envelopes including
`apiVersion` must adhere to docs/JSON-SCHEMA.md)" so readers know the contract
covers command JSON envelopes; reference the header "v1.6 stable CLI/JSON API
contract", the `--json` flag, `apiVersion`, and docs/JSON-SCHEMA.md when adding
this line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/ccaudit/src/__tests__/_test-helpers.ts`:
- Around line 10-18: The docstring is inaccurate: update it to state that the
poll loop is bounded by the hard-coded earlyExitMs (earlyExitMs = 1000) rather
than graceMs, and that graceMs is only the final configurable sleep added after
the poll completes (default 300ms), not the duration that the poll runs;
reference the variables earlyExitMs and graceMs in the docstring and remove the
contradictory phrase that the poll runs until graceMs then a 300ms grace is
added.

In `@README.md`:
- Line 621: Add "data-engineering" to the DOMAIN_STOP_FOLDERS set so the
explicit Tier 2 stop-list matches the README's claim; update the corresponding
unit test that asserts the stop-list length (and expected entries) from 18 to 19
to include the new "data-engineering" entry. Modify the DOMAIN_STOP_FOLDERS
constant and the test that validates its count/contents (the test referencing
DOMAIN_STOP_FOLDERS) so they both include "data-engineering" to restore dual
enforcement described in the README.

---

Outside diff comments:
In `@README.md`:
- Line 34: Update the two occurrences of the displayed version in README: change
the "Current release: **v1.5.0**" text (the "Current release" line) and the
"Current package version" value (the "Current package version" line) from v1.5.0
to v1.5.1 so the README matches the changelog and apps/ccaudit/package.json;
search for the literal "v1.5.0" or the headings "Current release" and "Current
package version" and replace with "v1.5.1".

---

Nitpick comments:
In `@apps/ccaudit/scripts/bundle-size-check.mjs`:
- Around line 17-26: The non-finite branch in function formatBytes returns
`${bytes}B` (no space) while other branches use a space before "B"/"KB"/"MB";
update the non-finite return to use a space (e.g. `${bytes} B`) so output
formatting is consistent in formatBytes, and ensure the change is applied only
inside the formatBytes function to normalize spacing across all branches.
- Around line 96-99: The phase-local FAIL log uses raw bytes ("${phaseBudget}B")
instead of the human-readable formatter; update the console.error in the branch
that checks if (phaseDelta > phaseBudget) to use formatBytes(phaseBudget) (and
keep phaseDelta as-is or format if desired) so messages match the primary FAIL
format—look for the variables phaseDelta, phaseBudget and the formatBytes
function in bundle-size-check.mjs and replace the raw `${phaseBudget}B`
insertion with formatBytes(phaseBudget).

In `@apps/ccaudit/src/__tests__/fixtures/tmux-e2e.ts`:
- Around line 145-168: The code in waitForText correctly delegates ANSI handling
to capture, but the local variable name raw is misleading because capture may
already strip ANSI; rename raw to captured (and update its usage where assigned
to lastCapture and where it's declared) and adjust the adjacent comment to
reflect that capture returns either stripped or raw output depending on
opts.stripAnsi (via ansi: opts.stripAnsi === false), leaving all logic and the
matching/error message unchanged; update references to raw, lastCapture
assignment, and the comment inside waitForText accordingly.

In `@CHANGELOG.md`:
- Around line 36-53: The changelog places the `scan-memory.ts` bullet under `###
Fixed` but it describes regression test coverage, so move that bullet into the
existing `### Tests` section (or change its phrasing to explicitly say
"test/regression coverage" rather than a fix); update the `scan-memory.ts` line
to read that it adds Windows path-normalization regression coverage (in-source
test) and remove any wording that implies a user-visible fix; optionally, if you
want strict Keep a Changelog conformance, replace or omit the `### Tests`
heading and instead add the bullet under an appropriate official category (e.g.,
`Changed` or a dedicated testing note) so the entry's intent matches its
section.

In `@README.md`:
- Around line 704-718: The header "v1.6 stable CLI/JSON API contract" and
paragraph mention `apiVersion` and `--json` but don't state what is in-scope;
add a single clarifying line under that paragraph such as "Scope: report
commands' `--json` output (CLI subcommands' JSON envelopes including
`apiVersion` must adhere to docs/JSON-SCHEMA.md)" so readers know the contract
covers command JSON envelopes; reference the header "v1.6 stable CLI/JSON API
contract", the `--json` flag, `apiVersion`, and docs/JSON-SCHEMA.md when adding
this line.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4274f389-93d5-42e7-ad51-6b369a6aa845

📥 Commits

Reviewing files that changed from the base of the PR and between b26ae7e and 8c16009.

📒 Files selected for processing (16)
  • CHANGELOG.md
  • README.md
  • apps/ccaudit/package.json
  • apps/ccaudit/scripts/bundle-size-check.mjs
  • apps/ccaudit/src/__tests__/_test-helpers.ts
  • apps/ccaudit/src/__tests__/fixtures/tmux-e2e.ts
  • apps/ccaudit/src/__tests__/pagination-500.test.ts
  • apps/ccaudit/src/__tests__/restore-corrupt-manifest.test.ts
  • apps/ccaudit/src/__tests__/restore-json-envelope.test.ts
  • docs/JSON-SCHEMA.md
  • packages/internal/src/remediation/change-plan.ts
  • packages/internal/src/remediation/purge.ts
  • packages/internal/src/scanner/scan-memory.ts
  • packages/terminal/src/tables/change-plan.ts
  • packages/terminal/src/tui/_force-partial-banner.ts
  • packages/terminal/src/tui/_glyphs.ts

Comment on lines 10 to 18
/**
* Wait for the spawned picker subprocess to reach its blocking read loop.
* Polls until the child exits (error/crash path), or until maxWaitMs elapses,
* Polls until the child exits (error/crash path), or until graceMs elapses,
* with exponential backoff — whichever comes first. Then adds a final grace
* delay so the TUI is ready for key input.
*
* Typical path: child never exits during the poll window, loop runs until
* maxWaitMs, then a 300ms grace is added for the render cycle to settle.
* graceMs, then a 300ms grace is added for the render cycle to settle.
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Docstring still mismatches the implementation — graceMs does not bound the poll loop.

The rewritten docstring conflates two distinct timeouts. In the implementation:

  • The poll loop is bounded by the hard-coded earlyExitMs = 1_000 (lines 21, 24), not graceMs.
  • graceMs is only the final sleep added once the poll loop completes without an early exit (lines 29–31), and it is configurable via the parameter (default 300ms), not a fixed 300ms.

So both phrases — "Polls until … graceMs elapses" and "loop runs until graceMs, then a 300ms grace is added" — are inaccurate. Given that doc accuracy is one of this PR's stated goals, worth tightening here.

📝 Suggested docstring
 /**
  * Wait for the spawned picker subprocess to reach its blocking read loop.
- * Polls until the child exits (error/crash path), or until graceMs elapses,
- * with exponential backoff — whichever comes first. Then adds a final grace
- * delay so the TUI is ready for key input.
+ * Polls (with exponential backoff) for up to ~1s to detect an early exit
+ * (error/crash path). If the child is still alive after that window, sleeps
+ * an additional `graceMs` so the TUI has time to reach its blocking read.
  *
- * Typical path: child never exits during the poll window, loop runs until
- * graceMs, then a 300ms grace is added for the render cycle to settle.
+ * Typical path: the child does not exit during the ~1s poll window, then
+ * `graceMs` (default 300ms) is added for the render cycle to settle.
  */
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Wait for the spawned picker subprocess to reach its blocking read loop.
* Polls until the child exits (error/crash path), or until maxWaitMs elapses,
* Polls until the child exits (error/crash path), or until graceMs elapses,
* with exponential backoff — whichever comes first. Then adds a final grace
* delay so the TUI is ready for key input.
*
* Typical path: child never exits during the poll window, loop runs until
* maxWaitMs, then a 300ms grace is added for the render cycle to settle.
* graceMs, then a 300ms grace is added for the render cycle to settle.
*/
/**
* Wait for the spawned picker subprocess to reach its blocking read loop.
* Polls (with exponential backoff) for up to ~1s to detect an early exit
* (error/crash path). If the child is still alive after that window, sleeps
* an additional `graceMs` so the TUI has time to reach its blocking read.
*
* Typical path: the child does not exit during the ~1s poll window, then
* `graceMs` (default 300ms) is added for the render cycle to settle.
*/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/ccaudit/src/__tests__/_test-helpers.ts` around lines 10 - 18, The
docstring is inaccurate: update it to state that the poll loop is bounded by the
hard-coded earlyExitMs (earlyExitMs = 1000) rather than graceMs, and that
graceMs is only the final configurable sleep added after the poll completes
(default 300ms), not the duration that the poll runs; reference the variables
earlyExitMs and graceMs in the docstring and remove the contradictory phrase
that the poll runs until graceMs then a 300ms grace is added.

Comment thread README.md Outdated
> `marketing/`, `testing/`, `sales/`, `integrations/`, `strategy/`,
> `project-management/`, `support/`, `paid-media/`, `spatial-computing/`,
> `examples/`, `scripts/`, `product/`, `specialized/`, `game-development/`,
> `examples/`, `scripts/`, `product/`, `specialized/`, `data-engineering/`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm DOMAIN_STOP_FOLDERS contains data-engineering to match README claim.
rg -nP -C2 'DOMAIN_STOP_FOLDERS' --type=ts
echo "--- searching for data-engineering literal ---"
rg -nP "['\"]data-engineering['\"]" --type=ts

Repository: fabio-dee/ccaudit

Length of output: 11153


🏁 Script executed:

# Read the complete DOMAIN_STOP_FOLDERS definition from stop-lists.ts
sed -n '54,85p' packages/internal/src/framework/stop-lists.ts

Repository: fabio-dee/ccaudit

Length of output: 852


🏁 Script executed:

# Read the README context around line 621 to understand the full claim
sed -n '610,630p' README.md

Repository: fabio-dee/ccaudit

Length of output: 1168


data-engineering/ is listed in README but missing from DOMAIN_STOP_FOLDERS.

The README (line 615–627) claims data-engineering/ is a domain folder that "ccaudit refuses to group," and explicitly states this is "enforced twice: once by gating folder-segment matches through the curated list only (Tier 1), and once by an explicit DOMAIN_STOP_FOLDERS list."

However, DOMAIN_STOP_FOLDERS in packages/internal/src/framework/stop-lists.ts (lines 54–71) contains 18 entries but does not include data-engineering. The test confirming the count and validating the expected list does not account for this entry either. This breaks the promised dual enforcement: Tier 2 will not prevent data-engineering/ from being auto-promoted to a framework if items cluster by prefix.

Add data-engineering to the DOMAIN_STOP_FOLDERS set and update the test count from 18 to 19.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 621, Add "data-engineering" to the DOMAIN_STOP_FOLDERS set
so the explicit Tier 2 stop-list matches the README's claim; update the
corresponding unit test that asserts the stop-list length (and expected entries)
from 18 to 19 to include the new "data-engineering" entry. Modify the
DOMAIN_STOP_FOLDERS constant and the test that validates its count/contents (the
test referencing DOMAIN_STOP_FOLDERS) so they both include "data-engineering" to
restore dual enforcement described in the README.

@coderabbitai coderabbitai 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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
README.md (2)

711-715: ⚠️ Potential issue | 🟡 Minor

Version mismatch: “Current package version: 1.5.0” vs release v1.5.1.

Near the end, the README lists:

  • Current release: v1.5.1 (Line 35)
  • Current package version: 1.5.0 (Line 713)

Unless that’s intentionally documenting something else, this is likely stale and confusing for users.

Consider updating the README version stanza to 1.5.1 (or clarifying why it remains 1.5.0).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 711 - 715, Update the README stanza that shows
"Current package version: **1.5.0**" to match the release v1.5.1 (or add a
clarifying note if the package intentionally remains at 1.5.0); locate the
"Current package version" text in the README (the block containing "Current
package version: **1.5.0**" and the referenced `apps/ccaudit/package.json` and
`apps/ccaudit/src/_version.ts`) and change the displayed version to **1.5.1**
(or append a brief explanation why it differs).

643-655: ⚠️ Potential issue | 🟠 Major

Fix doc/code contract mismatch: data-engineering/ is not in DOMAIN_STOP_FOLDERS; game-development/ is missing from the list.

The README lists data-engineering/ as enforced by the Tier 2 DOMAIN_STOP_FOLDERS constant, but the actual set at packages/internal/src/framework/stop-lists.ts:54 contains 18 entries excluding data-engineering/. Instead, the set includes game-development/, which is not mentioned in the README.

Update the README list to match the actual 18 entries in the code:

  • Remove: data-engineering/
  • Add: game-development/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 643 - 655, The README's list of domain stop folders
is out of sync with the code: update the README entry under "Domain folders are
NOT frameworks" so it matches the actual constant DOMAIN_STOP_FOLDERS used by
the framework logic — remove "data-engineering/" and add "game-development/" to
mirror the 18 entries defined in the code (refer to the DOMAIN_STOP_FOLDERS
constant) so documentation and implementation are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@README.md`:
- Around line 711-715: Update the README stanza that shows "Current package
version: **1.5.0**" to match the release v1.5.1 (or add a clarifying note if the
package intentionally remains at 1.5.0); locate the "Current package version"
text in the README (the block containing "Current package version: **1.5.0**"
and the referenced `apps/ccaudit/package.json` and
`apps/ccaudit/src/_version.ts`) and change the displayed version to **1.5.1**
(or append a brief explanation why it differs).
- Around line 643-655: The README's list of domain stop folders is out of sync
with the code: update the README entry under "Domain folders are NOT frameworks"
so it matches the actual constant DOMAIN_STOP_FOLDERS used by the framework
logic — remove "data-engineering/" and add "game-development/" to mirror the 18
entries defined in the code (refer to the DOMAIN_STOP_FOLDERS constant) so
documentation and implementation are consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ab26c31a-7a9d-4829-969a-1fdb7c7226c9

📥 Commits

Reviewing files that changed from the base of the PR and between 8c16009 and 32d6c5e.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • README.md
✅ Files skipped from review due to trivial changes (1)
  • CHANGELOG.md

…and bump package-version label to 1.5.1

Addresses CodeRabbit findings on PR #6:
- Stop-folder list cited data-engineering/ but DOMAIN_STOP_FOLDERS in
  packages/internal/src/framework/stop-lists.ts contains game-development/.
- Version stanza near footer still showed 1.5.0; release is 1.5.1.
@fabio-dee
fabio-dee merged commit 3c07a4e into main Apr 27, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant