merge main into development

[mikib0]

Summary by CodeRabbit

• New Features

  • Added comprehensive coverage ratcheting system for quality gates.
  • Enhanced ETL pipeline with improved validation, error handling, and admin endpoints for catalog re-ingestion and reconciliation.
  • Added support for JSONL catalog uploads alongside CSV format.
  • Catalog items now support unknown weight values.
  • Added structured logging and observability enhancements.

• Chores

  • Bumped all packages to version 2.0.27.
  • Migrated testing documentation and updated CI workflows.

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
✅ Deployment successful! View logs	packrat-api	2964e6b	Jun 01 2026, 03:12 AM

[coderabbitai]

Walkthrough

Introduces Cloudflare Workflows-based catalog ETL with chunking, JSONL/CSV parsing, admin retry/repair/reconcile/audit routes, structured logger and Sentry wiring, invalid log retention sweep with cron, schema/migration updates, CI coverage ratchet and baseline updater, test/doc updates, and UI/type changes for nullable item weights.

Changes

Workflow ETL, retention, and coverage gates

Layer / File(s)	Summary
Workflow entrypoints and admin routes packages/api/src/index.ts, packages/api/src/routes/catalog/index.ts, packages/api/src/routes/admin/analytics/catalog.ts	Exports Sentry-instrumented CatalogEtlWorkflow, adds cron-based invalid-log sweep, and implements ETL engine switch (workflow/queue) with admin reingest, reconcile, and audit endpoints.

Sequence Diagram(s)

sequenceDiagram
  participant Admin
  participant API
  participant Workflow
  participant R2
  participant DB
  Admin->>API: POST /admin/catalog/etl?engine=workflow
  API->>R2: head/get → chunk plan
  API->>Workflow: create(instanceId, params)
  loop per chunk
    Workflow->>R2: ranged get
    Workflow->>DB: upsert valid items, log invalid
  end
  Workflow->>DB: write totals, mark completed
  Admin->>API: POST /admin/catalog/etl/:id/reconcile
  API->>R2: stream parse CSV
  API->>DB: update verified counts

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• PackRat-AI/PackRat#2462 — Prior Cloudflare Workflows-based ETL migration touching the same workflow, chunker, and admin paths.
• PackRat-AI/PackRat#2476 — Adds ETL job fields used here: failure/verification metrics in admin schemas and routes.
• PackRat-AI/PackRat#2481 — Adjusts catalog schema/tests for nullable weight, matching the nullable weight changes in this PR.

Suggested reviewers

• Isthisanmol
• andrew-bierman

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch main

⚔️ Resolve merge conflicts

• Resolve merge conflict in branch main

[coderabbitai]

Actionable comments posted: 23

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/coverage.yml:
- Line 109: Several checkout steps in the CI workflow (the actions/checkout@v6
usages in the coverage, ratchet, and scripts-tests jobs) are leaving credentials
persisted; update each of those checkout steps to include persist-credentials:
false so they perform read-only checkouts and reduce supply-chain risk, while
leaving the bump-baseline job's checkout unchanged because it needs push
credentials.
- Around line 109-280: Replace all mutable action tags in the workflow with the
provided commit SHAs: change every occurrence of actions/checkout@v6 →
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd, oven-sh/setup-bun@v2
→ oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6,
davelosert/vitest-coverage-report-action@v2 →
davelosert/vitest-coverage-report-action@02f3c2e641286b7fa308cd3e430783103ce6103b,
actions/upload-artifact@v4 →
actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02,
actions/download-artifact@v4 →
actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093, and
stefanzweifel/git-auto-commit-action@v6 →
stefanzweifel/git-auto-commit-action@778341af668090896ca464160c2def5d1d1a3eb0;
ensure you update every use of the symbols (each uses: line) in the file so no
mutable tags remain.
- Around line 36-38: Move the workflow-level permissions into each job: remove
the top-level "permissions: contents: write" and "pull-requests: write" and
instead add "permissions: contents: write" only to the bump-baseline job (and
guard that job to run on main), add "permissions: pull-requests: write" only to
the coverage job, and leave scripts-tests and ratchet jobs with no write
permissions (or with minimal default read-only permissions). Update the
bump-baseline and coverage job definitions to include these job-scoped
permissions so only those jobs have the elevated rights.

In `@apps/web/components/screens/packs-screen.tsx`:
- Line 635: The current payload line "weight: item.weight ?? 0" silently
converts null weights to 0; change behavior so items with item.weight === null
are not sent to the backend and the UI prevents adding them. Locate the place
building the pack-item payload (the line containing "weight: item.weight ?? 0"
inside the add/submit handler such as the function that adds a catalog item to a
pack) and instead short-circuit when item.weight === null (do not call the API)
and surface a UI state: disable the corresponding add button and show a tooltip
or label like "Weight required" (update the item's button/row rendering to check
item.weight === null and set disabled + explanatory text); if you prefer
prompting, replace the short-circuit with a modal/inline prompt that collects a
valid weight before constructing the payload.

In `@docs/audits/2026-05-16-etl-audit.md`:
- Around line 11-32: The fenced architecture diagram in
docs/audits/2026-05-16-etl-audit.md is missing a language identifier which fails
linting and renders less predictably; update the opening code fence for the ETL
diagram (the triple-backtick block that begins with "POST /api/catalog/etl ...")
to include a language tag such as text or mermaid (e.g., change ``` to ```text)
so the diagram is properly highlighted and the linter is satisfied.

In `@docs/plans/2026-05-19-001-fix-etl-pipeline-audit-remediation-plan.md`:
- Around line 1-10: This plan file (title "fix: ETL pipeline audit remediation",
field supersededBy:
docs/plans/2026-05-20-001-fix-etl-pipeline-workflows-migration-plan.md) is
marked superseded and should be relocated out of the active plans directory into
your archive/superseded area; move the file, keep the frontmatter (status:
superseded, supersededBy, supersededReason, date) intact, and update any plan
index/TOC or links that list active plans so this file is no longer shown as
current.

In `@packages/analytics/test/core/spec-parser.test.ts`:
- Around line 202-235: Update the tests to assert that user-controlled strings
are properly SQL-escaped and that sortBy is validated: in the getProductSpecs
test (using parser.getProductSpecs with query "tent's") assert
runAndReadAll.mock.calls[0][0] includes the escaped form "tent''s"
(SQLFragments.escapeSql behavior), and in the filterProducts test assert the
generated SQL contains "women''s" for gender and that ORDER BY uses an
allowed/whitelisted identifier (or assert the exact safe token produced) instead
of arbitrary interpolation; if filterProducts currently interpolates sortBy
directly, add or assert a runtime whitelist/validation for sortBy values before
interpolation to prevent SQL injection. Ensure you reference
parser.getProductSpecs, parser.filterProducts, SQLFragments.escapeSql and
runAndReadAll in the changes.

In `@packages/api/src/routes/admin/analytics/catalog.ts`:
- Around line 717-723: The catch block that handles ETL reconciliation failures
currently only console.errors and returns status(500); update it to use the
project's Sentry helpers: call the Sentry helper (e.g., captureException or the
repo-specific sentryCapture) with the caught error, add contextual data (job id,
CSV/R2 keys, and request metadata) via withScope or setExtras, and add a
breadcrumb indicating the failure source before returning status(500) with the
same payload; also ensure the Sentry helper is imported at top of the file and
avoid exposing sensitive data in extras.
- Around line 906-909: The catch block that currently does
console.error('Catalog audit error:', error) and returns status(500, { error:
'Failed to generate catalog audit', code: 'AUDIT_ERROR' }) needs Sentry
instrumentation: replace the console.error with using the Sentry helpers
(captureException and adding context) so the error is reported to Sentry with
relevant context (e.g., operation = "catalog_audit" and the AUDIT_ERROR code and
any request/user identifiers), then continue to return the same status(500, ...)
response; update the catch in the catalog audit handler to call
Sentry.captureException(error) (and optionally Sentry.setContext or
Sentry.setTag) before returning to ensure the error is recorded.
- Around line 135-147: The insert into etlJobs for the new job incorrectly sets
supersededByJobId to originalJobId (reversing the relationship); instead, remove
supersededByJobId/supersededAt from the new job insert (keep id=newJobId,
status='running', workflowInstanceId, etc.), and after the workflow is
successfully created/update persisted, update the existing original job row
(identified by originalJobId) to set supersededByJobId = newJobId and
supersededAt = new Date(); use the etlJobs table and the newJobId/originalJobId
identifiers to locate the records and perform the update.
- Around line 167-174: The catch block that logs ETL failures (the catch
handling "ETL ${mode} error") must be instrumented with Sentry: import the
Sentry helper functions (e.g., captureException, captureMessage, addBreadcrumb)
at the top of the file, then inside this catch call addBreadcrumb with context
about mode and any relevant variables before external ops, call
captureException(error) (and optionally captureMessage with a human-readable
string like `ETL ${mode} error`) so the error is sent to Sentry, and keep the
existing console.error and returned error payload; update the block that
currently returns the 500 payload in admin/analytics/catalog.ts accordingly.

In `@packages/api/src/routes/catalog/index.ts`:
- Around line 368-376: The catch block after calling env.ETL_WORKFLOW.create
should call captureApiException(err, { extra: { jobId, instanceId, params } })
before marking the job failed and rethrowing; update the catch in the route
handler to invoke captureApiException with the caught error and relevant context
(jobId, instanceId, params) and then proceed to await db.update(etlJobs)... and
throw err so Sentry captures the failure for env.ETL_WORKFLOW.create.

In `@packages/api/src/services/etl/processCatalogEtl.ts`:
- Around line 85-184: The JSONL branch in useJsonl (looping over
streamToText(r2Object.body), JSON.parse, mapJsonRowToItem,
validator.validateItem, and batch flushes via
processValidItemsBatch/processLogsBatch) lacks Sentry instrumentation; add
apiAddBreadcrumb calls at the start of the R2 stream processing, before each
batch flush, and before key async operations, and call captureApiException in
every catch block (the JSON.parse catch and the final lastLine parse catch) to
record errors; ensure breadcrumbs include context (jobId, rowIndex, batch sizes)
and that captureApiException is invoked with the caught error and contextual
info whenever pushing to invalidItemsBatch or when
processValidItemsBatch/processLogsBatch throws.
- Around line 88-106: The variable firstLineSkipped is initialized true which
makes the subsequent "discard partial row" branch never run; either remove the
dead-code variable and the if-block that skips the first line inside the
for-await loop (the firstLineSkipped flag and the branch that does "continue; //
discard partial row") if JSONL chunks never require skipping, or change the
initialization to false and adopt the same chunk-boundary behavior as the CSV
path (see skipPartialRow logic used later) so non-first chunks correctly drop a
partial row; update streamToText loop logic and tests accordingly to reflect the
chosen behavior.

In `@packages/api/src/services/etl/processValidItemsBatch.ts`:
- Around line 83-89: The DB update that increments
etlJobs.totalEmbeddingFailures should be moved out of the finally block and into
the catch block so failures are only counted when an exception occurs: in the
catch handler of processValidItemsBatch (where you currently handle embedding
errors), call createDbClient(env) and run the update on etlJobs (using
eq(etlJobs.id, jobId)) to set totalEmbeddingFailures =
COALESCE(totalEmbeddingFailures, 0) + items.length, then remove that update from
the finally block so successful runs do not increment the counter.

In `@packages/api/src/utils/__tests__/auth.test.ts`:
- Around line 67-71: Add two unit tests for timingSafeEqual: one that asserts it
returns true when both inputs are identical (e.g., pass { a: 'test-api-key', b:
'test-api-key' } to timingSafeEqual and expect true), and one that asserts it
returns false for same-length differing strings (e.g., two strings of equal
length but different characters, pass { a: 'test-api-xxxx', b: 'test-api-yyyy' }
and expect false); place these alongside the existing test in
packages/api/src/utils/__tests__/auth.test.ts and use the timingSafeEqual
function name to locate where to add the new it blocks.

In `@packages/api/src/utils/embeddingHelper.ts`:
- Around line 24-29: The variants mapping currently filters falsy values only in
the scalar branch, causing empty entries like "Color: Black, , " to appear;
update the normalization in the mapping over item.variants so you first coerce
v.values into an array (e.g., ensure an array regardless of scalar or array
input) and then apply .filter(Boolean) to remove empty/falsy entries before
joining; change the logic around v.values and v.attribute in the map used for
embeddings (and apply the same fix to the analogous block at the other
occurrence) so both branches normalize then filter consistently.

In `@packages/api/src/utils/json-utils.ts`:
- Around line 123-133: Add a clarifying comment above the numeric-weight branch
to explain why we require rawWeight > 0 (so zero intentionally falls through and
item.weight remains unset) and note that the string branch mirrors the same
positive-only validation; reference the variables/constructs involved
(rawWeight, parseWeight, item.weight, WeightUnitSchema) so maintainers
understand the intent of the isNumber(rawWeight) && rawWeight > 0 check and that
zero is an explicit "ignore" case.
- Around line 136-157: The code unsafely casts raw arrays (rawVariants,
rawLinks, rawReviews, rawQas) to NewCatalogItem types without validating inner
shapes; update the parsing in json-utils.ts to perform runtime
validation/filtering of each array before assigning to item. For each symbol
(rawVariants -> item.variants, rawLinks -> item.links, rawReviews ->
item.reviews, rawQas -> item.qas) iterate the array, ensure each entry has the
required fields and correct types (e.g., variant objects have expected keys like
color/size, link objects have url as string, etc.), skip or reject invalid
entries (or return a validation error) and only then assign the
filtered/validated array to the corresponding item property; alternatively, call
a reusable validator function (e.g.,
validateVariant/validateLink/validateReview/validateQa) to centralize checks.

In `@packages/api/src/workflows/catalog-etl-workflow.ts`:
- Around line 126-140: The firstLineSkipped dead-code should be removed: delete
the let firstLineSkipped = true declaration and remove the conditional block
that checks if (!firstLineSkipped) { firstLineSkipped = true; continue; } inside
the loop in the catalog ETL workflow (in
packages/api/src/workflows/catalog-etl-workflow.ts, locate the for await (const
text of streamToText(obj.body)) loop and the inner for (const line of lines)
block); also remove any other references to firstLineSkipped later in the file
(e.g., the reference mentioned around the later lines) so the chunk-processing
logic simply trims/continues on empty lines without the unreachable skip logic.

In `@packages/api/test/catalog.test.ts`:
- Around line 57-65: The test currently queries
apiWithAuth(`/catalog?sort=${sort}`) and searches data.items for seededItem,
which is brittle if pagination moves the item off page 1; update the request to
be deterministic by adding an explicit limit parameter (e.g., &limit=100) or add
a filter query that targets a unique property of seededItem (e.g.,
&name=<uniqueName>), then call expectJsonResponse(res, ['items']) and assert
against the returned items for seededItem.id; keep references to seededItem,
apiWithAuth, and expectJsonResponse so reviewers can locate the change.

In `@packages/api/test/db-schema-etl.test.ts`:
- Around line 41-63: The test only asserts two of the eight columns added by
Migration 0047; update the test in db-schema-etl.test.ts to either document that
it is intentionally a smoke test or add assertions for the remaining columns
(`verified_at`, `verified_row_count`, `source_etag`, `source_last_modified`,
`superseded_by_job_id`, `superseded_at`) using the existing helpers
`describeColumns('etl_jobs')` to check data_type/is_nullable/column_default as
appropriate, and add an assertion that `describeIndexes('etl_jobs')` includes
the `superseded_by_idx` index (in addition to the existing
`etl_jobs_workflow_instance_id_idx` check). Ensure each new assertion mirrors
the style of the existing tests for `workflow_instance_id` and
`total_embedding_failures`.

In `@scripts/lint/coverage-ratchet.ts`:
- Around line 172-207: renderReport currently hard-codes a 0.05 threshold when
showing metric improvements which can differ from the actual epsilon used by
compareWorkspace; update renderReport(signature) to accept an epsilon parameter
and use that instead of 0.05 when deciding which improvements to print, then
update the caller that invokes renderReport (the call that builds the final
report after compareWorkspace runs) to pass through the same epsilon value used
during comparison so displayed improvement thresholds match actual logic.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 38756c05-1ec0-4509-a7ca-8dc6d8db8c86

📥 Commits

Reviewing files that changed from the base of the PR and between cb9f64d and 2964e6b.

📒 Files selected for processing (98)

• .github/workflows/coverage.yml
• .github/workflows/eas-update.yml
• .github/workflows/unit-tests.yml
• CLAUDE.md
• README.md
• TESTING.md
• apps/admin/package.json
• apps/expo/app.config.ts
• apps/expo/features/catalog/types.ts
• apps/expo/lib/utils/__tests__/getRelativeTime.test.ts
• apps/expo/package.json
• apps/guides/package.json
• apps/landing/package.json
• apps/trails/package.json
• apps/web/components/screens/catalog-screen.tsx
• apps/web/components/screens/packs-screen.tsx
• apps/web/lib/types.ts
• copilot-instructions.md
• coverage-baselines.json
• docs/audits/2026-05-16-etl-audit.md
• docs/plans/2026-05-19-001-chore-coverage-ratchet-and-quality-gates-plan.md
• docs/plans/2026-05-19-001-fix-etl-pipeline-audit-remediation-plan.md
• docs/plans/2026-05-20-001-fix-etl-pipeline-workflows-migration-plan.md
• docs/runbooks/etl-pipeline.md
• docs/solutions/ui-bugs/android-textinput-keyboard-focus-loss.md
• docs/testing.md
• package.json
• packages/analytics/package.json
• packages/analytics/test/core/cache-metadata.test.ts
• packages/analytics/test/core/spec-parser.test.ts
• packages/api-client/package.json
• packages/api/container_src/package.json
• packages/api/drizzle.config.ts
• packages/api/drizzle/0047_clear_monster_badoon.sql
• packages/api/drizzle/meta/0047_snapshot.json
• packages/api/drizzle/meta/_journal.json
• packages/api/package.json
• packages/api/src/__test-stubs__/cloudflare-workers.ts
• packages/api/src/db/schema.ts
• packages/api/src/index.ts
• packages/api/src/routes/admin/analytics/catalog.ts
• packages/api/src/routes/catalog/__tests__/instanceId.test.ts
• packages/api/src/routes/catalog/index.ts
• packages/api/src/services/catalogService.ts
• packages/api/src/services/etl/CatalogItemValidator.ts
• packages/api/src/services/etl/__tests__/CatalogItemValidator.test.ts
• packages/api/src/services/etl/processCatalogEtl.ts
• packages/api/src/services/etl/processLogsBatch.ts
• packages/api/src/services/etl/processValidItemsBatch.ts
• packages/api/src/services/imageDetectionService.ts
• packages/api/src/services/retention/__tests__/invalidLogRetention.test.ts
• packages/api/src/services/retention/invalidLogRetention.ts
• packages/api/src/utils/__tests__/auth.test.ts
• packages/api/src/utils/__tests__/csv-utils.test.ts
• packages/api/src/utils/__tests__/embeddingHelper.test.ts
• packages/api/src/utils/__tests__/json-utils.test.ts
• packages/api/src/utils/__tests__/logger.test.ts
• packages/api/src/utils/__tests__/sentry.test.ts
• packages/api/src/utils/embeddingHelper.ts
• packages/api/src/utils/env-validation.ts
• packages/api/src/utils/json-utils.ts
• packages/api/src/utils/logger.ts
• packages/api/src/workflows/catalog-etl-workflow.ts
• packages/api/src/workflows/shared/__tests__/chunk-csv-for-r2.test.ts
• packages/api/src/workflows/shared/chunkCsvForR2.ts
• packages/api/test/catalog.test.ts
• packages/api/test/db-schema-etl.test.ts
• packages/api/vitest.unit.config.ts
• packages/api/wrangler.jsonc
• packages/app/package.json
• packages/checks/package.json
• packages/cli/package.json
• packages/config/package.json
• packages/constants/package.json
• packages/db/package.json
• packages/db/src/schema.ts
• packages/env/package.json
• packages/guards/package.json
• packages/mcp/package.json
• packages/mcp/src/__tests__/client.test.ts
• packages/osm-db/package.json
• packages/osm-import/package.json
• packages/overpass/package.json
• packages/overpass/src/client.test.ts
• packages/schemas/package.json
• packages/schemas/src/admin.ts
• packages/schemas/src/catalog.ts
• packages/types/package.json
• packages/ui/package.json
• packages/units/package.json
• packages/web-ui/package.json
• scripts/lint/__tests__/coverage-ratchet.test.ts
• scripts/lint/__tests__/no-weak-assertions.test.ts
• scripts/lint/coverage-baseline-update.ts
• scripts/lint/coverage-ratchet.ts
• scripts/lint/no-owned-max-params.ts
• scripts/lint/no-weak-assertions.ts
• scripts/vitest.config.ts

💤 Files with no reviewable changes (2)

• TESTING.md
• .github/workflows/unit-tests.yml

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Scope permissions to jobs, not workflow-level.

Workflow-level contents: write and pull-requests: write grant excessive permissions to all jobs. Only bump-baseline needs contents: write (and only on main), and only coverage needs pull-requests: write (for PR comments). The scripts-tests and ratchet jobs don't need write permissions at all.

🔒 Recommended fix: move permissions to job level

-permissions:
-  contents: write       # for baseline auto-commit on main
-  pull-requests: write  # for vitest-coverage-report-action comments
+permissions:
+  contents: read

 jobs:
   coverage:
     name: Coverage (${{ matrix.name }})
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      pull-requests: write  # for vitest-coverage-report-action
     strategy:

   ratchet:
     name: Coverage Ratchet
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     needs: coverage

   bump-baseline:
     name: Bump Coverage Baselines
     runs-on: ubuntu-latest
+    permissions:
+      contents: write  # for git-auto-commit-action
     needs: ratchet

   scripts-tests:
     name: Scripts Tests
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     steps:

As per coding guidelines: minimize permissions scope to reduce supply-chain attack surface.

📝 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

	permissions:
	contents: write # for baseline auto-commit on main
	pull-requests: write # for vitest-coverage-report-action comments
	permissions:
	contents: read
	jobs:
	coverage:
	name: Coverage (${{ matrix.name }})
	runs-on: ubuntu-latest
	permissions:
	contents: read
	pull-requests: write # for vitest-coverage-report-action
	strategy:

🧰 Tools

🪛 zizmor (1.25.2)

[error] 37-37: overly broad permissions (excessive-permissions): contents: write is overly broad at the workflow level

(excessive-permissions)

---

[error] 38-38: overly broad permissions (excessive-permissions): pull-requests: write is overly broad at the workflow level

(excessive-permissions)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/coverage.yml around lines 36 - 38, Move the workflow-level
permissions into each job: remove the top-level "permissions: contents: write"
and "pull-requests: write" and instead add "permissions: contents: write" only
to the bump-baseline job (and guard that job to run on main), add "permissions:
pull-requests: write" only to the coverage job, and leave scripts-tests and
ratchet jobs with no write permissions (or with minimal default read-only
permissions). Update the bump-baseline and coverage job definitions to include
these job-scoped permissions so only those jobs have the elevated rights.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add persist-credentials: false to read-only checkouts.

Three checkout actions in jobs that don't push changes should disable credential persistence to minimize supply-chain risk.

🔐 Recommended fix

       - uses: actions/checkout@v6
+        with:
+          persist-credentials: false

Apply to the checkouts in coverage, ratchet, and scripts-tests jobs. The bump-baseline job's checkout (line 212) correctly preserves credentials since it needs to push.

Also applies to: 150-150, 271-271

🧰 Tools

🪛 zizmor (1.25.2)

[warning] 109-109: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

---

[error] 109-109: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/coverage.yml at line 109, Several checkout steps in the CI
workflow (the actions/checkout@v6 usages in the coverage, ratchet, and
scripts-tests jobs) are leaving credentials persisted; update each of those
checkout steps to include persist-credentials: false so they perform read-only
checkouts and reduce supply-chain risk, while leaving the bump-baseline job's
checkout unchanged because it needs push credentials.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch latest commit SHAs for the action versions currently in use

echo "actions/checkout@v6:"
gh api repos/actions/checkout/commits/refs/tags/v6 --jq '.sha'

echo "oven-sh/setup-bun@v2:"
gh api repos/oven-sh/setup-bun/commits/refs/tags/v2 --jq '.sha'

echo "davelosert/vitest-coverage-report-action@v2:"
gh api repos/davelosert/vitest-coverage-report-action/commits/refs/tags/v2 --jq '.sha'

echo "actions/upload-artifact@v4:"
gh api repos/actions/upload-artifact/commits/refs/tags/v4 --jq '.sha'

echo "actions/download-artifact@v4:"
gh api repos/actions/download-artifact/commits/refs/tags/v4 --jq '.sha'

echo "stefanzweifel/git-auto-commit-action@v6:"
gh api repos/stefanzweifel/git-auto-commit-action/commits/refs/tags/v6 --jq '.sha'

Repository: PackRat-AI/PackRat

Length of output: 495

---

Pin GitHub Actions in .github/workflows/coverage.yml to commit SHAs.

All uses: entries reference mutable tags; pin them to the following SHAs (update every occurrence):

• actions/checkout@v6 → actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
• oven-sh/setup-bun@v2 → oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6
• davelosert/vitest-coverage-report-action@v2 → davelosert/vitest-coverage-report-action@02f3c2e641286b7fa308cd3e430783103ce6103b
• actions/upload-artifact@v4 → actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02
• actions/download-artifact@v4 → actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093
• stefanzweifel/git-auto-commit-action@v6 → stefanzweifel/git-auto-commit-action@778341af668090896ca464160c2def5d1d1a3eb0

🧰 Tools

🪛 zizmor (1.25.2)

[warning] 109-109: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

---

[warning] 150-150: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

---

[warning] 212-215: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

---

[warning] 271-271: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

---

[warning] 121-121: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

---

[error] 109-109: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 111-111: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 125-125: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 135-135: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 150-150: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 152-152: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 162-162: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 212-212: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 217-217: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 227-227: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 260-260: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 271-271: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[error] 272-272: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

---

[info] 260-260: action functionality is already included by the runner (superfluous-actions): use git add, git commit, and git push in a script step

(superfluous-actions)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/coverage.yml around lines 109 - 280, Replace all mutable
action tags in the workflow with the provided commit SHAs: change every
occurrence of actions/checkout@v6 →
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd, oven-sh/setup-bun@v2
→ oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6,
davelosert/vitest-coverage-report-action@v2 →
davelosert/vitest-coverage-report-action@02f3c2e641286b7fa308cd3e430783103ce6103b,
actions/upload-artifact@v4 →
actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02,
actions/download-artifact@v4 →
actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093, and
stefanzweifel/git-auto-commit-action@v6 →
stefanzweifel/git-auto-commit-action@778341af668090896ca464160c2def5d1d1a3eb0;
ensure you update every use of the symbols (each uses: line) in the file so no
mutable tags remain.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Defaulting null weight to 0 creates poor UX.

When a catalog item has null weight, this code sends weight: 0 to the backend. This will either fail validation (if pack items require positive weight) or create a pack item with 0g weight.

Either case provides poor user experience - the user won't understand why the item couldn't be added, or they'll have incorrect data in their pack.

Consider one of these alternatives:

• Disable the item button and show "Weight required" if item.weight === null
• Show a prompt for the user to enter weight before adding
• Display a clear error message explaining the issue

Proposed fix to disable items without weight

 <button
   type="button"
   key={item.id}
+  disabled={item.weight === null}
   onClick={() => {
+    if (item.weight === null) return;
     addItem.mutate(
       {
         packId,
         body: {
           name: item.name,
-          weight: item.weight ?? 0,
+          weight: item.weight,
           weightUnit: 'g',
           catalogItemId: item.id,
         },
       },
       { onSuccess: onClose },
     );
   }}
-  className="w-full rounded-xl bg-card border border-border p-3 text-left hover:border-primary/50 transition-colors"
+  className={cn(
+    "w-full rounded-xl bg-card border border-border p-3 text-left transition-colors",
+    item.weight === null 
+      ? "opacity-50 cursor-not-allowed"
+      : "hover:border-primary/50"
+  )}
 >

And update the display to show why it's disabled:

 <div className="text-right">
   <p className="text-sm font-semibold">
     {item.weight === null ? 'Unknown' : fw(item.weight)}
   </p>
+  {item.weight === null && (
+    <p className="text-xs text-destructive">Weight required</p>
+  )}
   {item.price && <p className="text-xs text-muted-foreground">${item.price}</p>}
 </div>

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/components/screens/packs-screen.tsx` at line 635, The current
payload line "weight: item.weight ?? 0" silently converts null weights to 0;
change behavior so items with item.weight === null are not sent to the backend
and the UI prevents adding them. Locate the place building the pack-item payload
(the line containing "weight: item.weight ?? 0" inside the add/submit handler
such as the function that adds a catalog item to a pack) and instead
short-circuit when item.weight === null (do not call the API) and surface a UI
state: disable the corresponding add button and show a tooltip or label like
"Weight required" (update the item's button/row rendering to check item.weight
=== null and set disabled + explanatory text); if you prefer prompting, replace
the short-circuit with a modal/inline prompt that collects a valid weight before
constructing the payload.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Add language identifier to code fence.

The architecture diagram code fence should specify a language identifier (e.g., text or mermaid) for better markdown rendering and to satisfy linting rules.

-```
+```text
 POST /api/catalog/etl            ── api-key auth
    │  body: { filename, chunks[], source, scraperRevision }

🧰 Tools

🪛 markdownlint-cli2 (0.22.1)

[warning] 11-11: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/audits/2026-05-16-etl-audit.md` around lines 11 - 32, The fenced
architecture diagram in docs/audits/2026-05-16-etl-audit.md is missing a
language identifier which fails linting and renders less predictably; update the
opening code fence for the ETL diagram (the triple-backtick block that begins
with "POST /api/catalog/etl ...") to include a language tag such as text or
mermaid (e.g., change ``` to ```text) so the diagram is properly highlighted and
the linter is satisfied.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Unsafe type assertions on array fields.

Lines 138, 144, 150, 156 use as NewCatalogItem['variants'] (and similar) to cast raw arrays without validating their internal structure. If the JSONL source contains malformed objects in these arrays, downstream code expecting { color: string } for variants or { url: string } for links could crash on missing/wrong-typed fields.

Either add runtime validation for these arrays (check object shapes, required fields) or document that the validator is responsible for rejecting malformed structures in a later stage.

🛡️ Example validation for variants

 const rawVariants = obj.variants;
 if (Array.isArray(rawVariants)) {
-  item.variants = rawVariants as NewCatalogItem['variants'];
+  // Validate each variant has expected shape before accepting
+  const validVariants = rawVariants.filter((v): v is Record<string, unknown> => 
+    isObject(v) && Object.keys(v).length > 0
+  );
+  if (validVariants.length > 0) {
+    item.variants = validVariants as NewCatalogItem['variants'];
+  }
 }

Apply similar filtering to links, reviews, and qas.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/api/src/utils/json-utils.ts` around lines 136 - 157, The code
unsafely casts raw arrays (rawVariants, rawLinks, rawReviews, rawQas) to
NewCatalogItem types without validating inner shapes; update the parsing in
json-utils.ts to perform runtime validation/filtering of each array before
assigning to item. For each symbol (rawVariants -> item.variants, rawLinks ->
item.links, rawReviews -> item.reviews, rawQas -> item.qas) iterate the array,
ensure each entry has the required fields and correct types (e.g., variant
objects have expected keys like color/size, link objects have url as string,
etc.), skip or reject invalid entries (or return a validation error) and only
then assign the filtered/validated array to the corresponding item property;
alternatively, call a reusable validator function (e.g.,
validateVariant/validateLink/validateReview/validateQa) to centralize checks.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Remove dead code: firstLineSkipped logic is unreachable.

firstLineSkipped is initialized to true (line 126), so the condition if (!firstLineSkipped) (line 137) can never execute. This entire block is unreachable dead code.

The comment on lines 123-124 confirms: "The chunker snaps boundaries to newlines, so every chunk starts at a clean line boundary — no partial first-line skip needed for any chunk." The firstLineSkipped logic should be removed entirely.

🧹 Proposed fix

     let buffer = '';
-    let firstLineSkipped = true;
 
     for await (const text of streamToText(obj.body)) {
       buffer += text;
       const lines = buffer.split('\n');
       buffer = lines.pop() ?? '';
 
       for (const line of lines) {
         const trimmed = line.trim();
         if (!trimmed) continue;
 
-        if (!firstLineSkipped) {
-          firstLineSkipped = true;
-          continue; // discard partial row at chunk boundary
-        }
-
         if (rowIndex % 100 === 0) {

Then remove the reference on line 196:

     // Flush remaining buffer line (last line without trailing newline)
     const lastLine = buffer.trim();
-    if (lastLine && firstLineSkipped) {
+    if (lastLine) {
       try {

📝 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

	let firstLineSkipped = true;
	for await (const text of streamToText(obj.body)) {
	buffer += text;
	const lines = buffer.split('\n');
	buffer = lines.pop() ?? '';
	for (const line of lines) {
	const trimmed = line.trim();
	if (!trimmed) continue;
	if (!firstLineSkipped) {
	firstLineSkipped = true;
	continue; // discard partial row at chunk boundary
	}
	let buffer = '';
	for await (const text of streamToText(obj.body)) {
	buffer += text;
	const lines = buffer.split('\n');
	buffer = lines.pop() ?? '';
	for (const line of lines) {
	const trimmed = line.trim();
	if (!trimmed) continue;
	if (rowIndex % 100 === 0) {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/api/src/workflows/catalog-etl-workflow.ts` around lines 126 - 140,
The firstLineSkipped dead-code should be removed: delete the let
firstLineSkipped = true declaration and remove the conditional block that checks
if (!firstLineSkipped) { firstLineSkipped = true; continue; } inside the loop in
the catalog ETL workflow (in packages/api/src/workflows/catalog-etl-workflow.ts,
locate the for await (const text of streamToText(obj.body)) loop and the inner
for (const line of lines) block); also remove any other references to
firstLineSkipped later in the file (e.g., the reference mentioned around the
later lines) so the chunk-processing logic simply trims/continues on empty lines
without the unreachable skip logic.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make this assertion independent of the first page.

This only searches data.items from the default paginated response. If other seeded rows exist, the new item can fall off page 1 and this test flakes even though null weights are returned correctly. Pass an explicit limit or filter the request by a unique field from the seeded row before asserting.

As per coding guidelines, "Tests must be deterministic — mock all external services and clocks."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/api/test/catalog.test.ts` around lines 57 - 65, The test currently
queries apiWithAuth(`/catalog?sort=${sort}`) and searches data.items for
seededItem, which is brittle if pagination moves the item off page 1; update the
request to be deterministic by adding an explicit limit parameter (e.g.,
&limit=100) or add a filter query that targets a unique property of seededItem
(e.g., &name=<uniqueName>), then call expectJsonResponse(res, ['items']) and
assert against the returned items for seededItem.id; keep references to
seededItem, apiWithAuth, and expectJsonResponse so reviewers can locate the
change.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Test coverage is partial — only 2 of 8 new columns checked.

Migration 0047 adds workflow_instance_id, total_embedding_failures, verified_at, verified_row_count, source_etag, source_last_modified, superseded_by_job_id, and superseded_at. This test only verifies the first two.

If this is intentional (smoke test for critical columns), document that choice. Otherwise, consider adding assertions for the remaining columns and the superseded_by_idx index.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/api/test/db-schema-etl.test.ts` around lines 41 - 63, The test only
asserts two of the eight columns added by Migration 0047; update the test in
db-schema-etl.test.ts to either document that it is intentionally a smoke test
or add assertions for the remaining columns (`verified_at`,
`verified_row_count`, `source_etag`, `source_last_modified`,
`superseded_by_job_id`, `superseded_at`) using the existing helpers
`describeColumns('etl_jobs')` to check data_type/is_nullable/column_default as
appropriate, and add an assertion that `describeIndexes('etl_jobs')` includes
the `superseded_by_idx` index (in addition to the existing
`etl_jobs_workflow_instance_id_idx` check). Ensure each new assertion mirrors
the style of the existing tests for `workflow_instance_id` and
`total_embedding_failures`.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Pass epsilon parameter to renderReport for consistent improvement thresholds.

Line 184 hard-codes 0.05 for improvement display, but the actual comparison in compareWorkspace uses the configurable epsilon. If the baseline file sets _epsilon: 0.1, improvements will be detected with the 0.1 threshold but displayed with a 0.05 threshold, causing inconsistency.

🔧 Proposed fix

-function renderReport(report: RatchetReport): string {
+function renderReport(report: RatchetReport, epsilon: number): string {
   const lines: string[] = [];
   const DIVIDER = '─'.repeat(60);
   lines.push('\nCoverage Ratchet');
   lines.push(DIVIDER);
   for (const c of report.checks) {
     if (c.status === 'ok') {
       lines.push(`✅  ${c.workspace} — baseline met`);
     } else if (c.status === 'improvement') {
       lines.push(`📈  ${c.workspace} — coverage improved (CI on main will bump baseline)`);
       if (c.before && c.after) {
         for (const m of METRICS) {
-          if (c.after[m] - c.before[m] > 0.05) {
+          if (c.after[m] - c.before[m] > epsilon) {
             lines.push(`     ${m}: ${fmtPct(c.before[m])} → ${fmtPct(c.after[m])}`);
           }
         }
       }

And update the call site at line 230:

-  console.log(renderReport(report));
+  console.log(renderReport(report, epsilon));

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/lint/coverage-ratchet.ts` around lines 172 - 207, renderReport
currently hard-codes a 0.05 threshold when showing metric improvements which can
differ from the actual epsilon used by compareWorkspace; update
renderReport(signature) to accept an epsilon parameter and use that instead of
0.05 when deciding which improvements to print, then update the caller that
invokes renderReport (the call that builds the final report after
compareWorkspace runs) to pass through the same epsilon value used during
comparison so displayed improvement thresholds match actual logic.