promote: dogfood walking skeleton to main - #50
Conversation
All automated checks passed. This dev integration merge intentionally has no CodeRabbit review; promotion to main will receive the single review cycle.
|
Warning Review limit reached
Next review available in: 18 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR adds desktop folder selection, spreadsheet audit contracts and execution, tenant-scoped API endpoints, a Web audit interface, local artifact registration, safety checks, operational evidence, and dogfood-skeleton validation. ChangesDesktop folder grant
Spreadsheet audit walking skeleton
Dogfood skeleton validation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Desktop
participant API
participant Engine
participant Web
Desktop->>API: Register local artifact metadata
API-->>Desktop: Return opaque artifact and evidence handles
API->>API: Admit tenant-scoped audit run
API-->>Desktop: Return public audit run handle
Engine->>Engine: Read and hash-check workbook input
Engine-->>API: Return typed spreadsheet audit result
Web->>API: List or retrieve audit results
API-->>Web: Return value-free audit metadata and findings
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
apps/desktop/test/folder-grant-contract.test.ts (1)
19-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the extra-field cases valid before adding the extra field.
Lines 21 and 22 fail because
not-grantedrequiresfileCount: 0. They do not prove thatpathandfileNamecause rejection. Use a validgrantedstate as the base for these cases.Proposed fix
- { fileCount: 1, lastScanAt: null, status: 'not-granted', path: 'C:\\secret' }, - { fileCount: 1, lastScanAt: null, status: 'not-granted', fileName: 'payroll.xlsx' }, + { + fileCount: 1, + lastScanAt: '2026-08-04T00:00:00.000Z', + status: 'granted', + path: 'C:\\secret', + }, + { + fileCount: 1, + lastScanAt: '2026-08-04T00:00:00.000Z', + status: 'granted', + fileName: 'payroll.xlsx', + },🤖 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/desktop/test/folder-grant-contract.test.ts` around lines 19 - 28, Update the first two test values in the parseFolderGrantState rejection cases to use a valid granted state, including the appropriate positive fileCount and granted status, while retaining the path or fileName extra field. Keep the remaining unsafe-count and timestamp cases unchanged.services/engine/tests/test_spreadsheet_auditor_action.py (1)
162-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the SHA-256 mismatch branch.
b"changed workbook"has a different length from_workbook(). The dispatcher rejects it at the byte-length check before it compares SHA-256 values. Return same-length modified bytes so this test protects the digest check inservices/engine/src/databreeze_engine/dispatcher.pyLine 118.Proposed test change
- input_reader=lambda _handle: b"changed workbook", + input_reader=lambda _handle: bytes([content[0] ^ 1]) + content[1:],🤖 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 `@services/engine/tests/test_spreadsheet_auditor_action.py` around lines 162 - 172, Update test_dispatch_rejects_auditor_input_hash_mismatch so input_reader returns modified bytes with the same length as _workbook(), allowing dispatch_execution to reach the SHA-256 comparison rather than the byte-length validation while preserving the expected INPUT_HASH_MISMATCH error.
🤖 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 `@apps/desktop/src/main/adapters/electron-folder-grant.adapter.ts`:
- Around line 56-58: Update the directory scan in the folder-grant method around
`#readdir` to use an async directory handle iterator, count only regular
non-symbolic-link files, and stop scanning once MAX_FOLDER_FILES is exceeded.
Ensure the handle is closed in a finally block, while preserving the existing
notGranted() outcome.
In `@apps/desktop/src/shared/desktop-contract-v1.ts`:
- Around line 67-74: Update the lastScanAt validation in the record contract to
require a real UTC instant, not just the existing millisecond ISO format: parse
the supplied string and require its ISO round trip to exactly match the original
value, throwing INVALID_FOLDER_GRANT for invalid dates such as February 30 while
preserving null handling.
In `@services/api/src/features/sa/application/spreadsheet-audit-run.service.ts`:
- Around line 41-82: Update SpreadsheetAuditRunService.admit to resolve
request.value.artifactVersionId through the transaction under
context.tenantScope before transaction.save, rejecting missing, foreign-tenant,
or unavailable/unclean artifacts and preserving idempotency behavior. Add
controller coverage in
services/api/test/features/sa/spreadsheet-audit-run.controller.test.ts (lines
46-76) and service coverage in
services/api/test/features/sa/spreadsheet-audit-run.service.test.ts (lines
34-105) for missing, foreign, and unavailable artifacts.
In `@services/api/src/features/sa/sa.module.ts`:
- Around line 48-53: The repository provider in the module’s dependency
configuration must not silently fall back to
InMemorySpreadsheetAuditRunRepositoryAdapter when spreadsheetAuditRunRepository
is omitted, including when spreadsheetAuditDatabase is supplied. Require an
explicitly injected durable repository for production composition, and only
permit the in-memory adapter through an explicit test or local configuration
option.
In
`@services/engine/src/databreeze_engine/processors/spreadsheet_auditor_action.py`:
- Around line 48-75: Cap auditor output before constructing
SpreadsheetAuditProcessorResult: limit result.sheets to 512 and result.findings
to 10,000, or raise ActionExecutionError("RESOURCE_LIMIT_EXCEEDED") when either
limit is exceeded. Update audit_workbook() and add boundary tests covering
exactly-at-limit and over-limit sheet and finding counts.
In `@tools/repo-cli/test/dogfood-skeleton.test.mjs`:
- Around line 31-36: Update the test around evaluateDogfoodSkeleton to remove
the selected requiredFiles entry from the fixture before evaluation, so it
exercises the !existsSync(...) missing-file path rather than only
ignoredFiles.has(...). Preserve the assertions that the result is rejected and
reports exactly the selected missing file; add a separate ignoredFiles test only
if retaining coverage for that contract.
---
Nitpick comments:
In `@apps/desktop/test/folder-grant-contract.test.ts`:
- Around line 19-28: Update the first two test values in the
parseFolderGrantState rejection cases to use a valid granted state, including
the appropriate positive fileCount and granted status, while retaining the path
or fileName extra field. Keep the remaining unsafe-count and timestamp cases
unchanged.
In `@services/engine/tests/test_spreadsheet_auditor_action.py`:
- Around line 162-172: Update test_dispatch_rejects_auditor_input_hash_mismatch
so input_reader returns modified bytes with the same length as _workbook(),
allowing dispatch_execution to reach the SHA-256 comparison rather than the
byte-length validation while preserving the expected INPUT_HASH_MISMATCH error.
🪄 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 Plus
Run ID: f4637aec-53a5-45a9-aff1-bdf4e4a2ed91
📒 Files selected for processing (60)
apps/desktop/src/application/folder-grant.port.tsapps/desktop/src/main/adapters/electron-folder-grant.adapter.tsapps/desktop/src/main/index.tsapps/desktop/src/main/ipc-registry.tsapps/desktop/src/preload/bridge-v1.tsapps/desktop/src/renderer/app.tsxapps/desktop/src/renderer/styles.cssapps/desktop/src/shared/desktop-contract-v1.tsapps/desktop/test/boundary-renderer-shell.test.tsxapps/desktop/test/folder-grant-adapter.test.tsapps/desktop/test/folder-grant-contract.test.tsapps/desktop/test/security-bridge-contract.test.tsapps/desktop/test/security-ipc-registry.test.tsapps/web/src/app/messages.tsapps/web/src/app/router.tsxapps/web/src/features/spreadsheet-auditor/spreadsheet-audit-api.tsapps/web/src/features/spreadsheet-auditor/spreadsheet-audit-page.tsxapps/web/src/styles.cssapps/web/test/spreadsheet-audit-api.test.tsapps/web/test/spreadsheet-audit-page.test.tsxdocs/operations/README.mddocs/operations/iae-local-artifact-registration-2026-08-04.mddocs/release-evidence/dogfood-walking-skeleton-2026-08-04.mddocs/release-evidence/sa-run-admission-slice-2026-08-04.mdpackage.jsonpackages/domain/package.jsonpackages/domain/src/spreadsheet-audit-run/v1.tspackages/domain/src/v1.tspackages/domain/test/public-api-v1.test.mjspackages/domain/test/spreadsheet-audit-run-v1.test.mjsservices/api/openapi/v1.jsonservices/api/src/features/iae/api/local-artifact-registration.controller.tsservices/api/src/features/iae/api/local-artifact-registration.dto.tsservices/api/src/features/iae/application/artifact.service.tsservices/api/src/features/iae/application/evidence-grant.service.tsservices/api/src/features/iae/iae.module.tsservices/api/src/features/sa/adapter/in-memory-spreadsheet-audit-run-repository.adapter.tsservices/api/src/features/sa/api/spreadsheet-audit-run.controller.tsservices/api/src/features/sa/api/spreadsheet-audit-run.dto.tsservices/api/src/features/sa/application/spreadsheet-audit-run-repository.port.tsservices/api/src/features/sa/application/spreadsheet-audit-run.service.tsservices/api/src/features/sa/sa.module.tsservices/api/test/features/iae/artifact-read.controller.test.tsservices/api/test/features/iae/artifact.service.test.tsservices/api/test/features/iae/evidence-grant.service.test.tsservices/api/test/features/iae/local-artifact-registration.controller.test.tsservices/api/test/features/sa/spreadsheet-audit-run.controller.test.tsservices/api/test/features/sa/spreadsheet-audit-run.service.test.tsservices/api/test/openapi.test.tsservices/engine/README.mdservices/engine/src/databreeze_engine/dispatcher.pyservices/engine/src/databreeze_engine/handler.pyservices/engine/src/databreeze_engine/models.pyservices/engine/src/databreeze_engine/processors/__init__.pyservices/engine/src/databreeze_engine/processors/spreadsheet_auditor_action.pyservices/engine/src/databreeze_engine/registry.pyservices/engine/tests/test_registry.pyservices/engine/tests/test_spreadsheet_auditor_action.pytools/repo-cli/src/check-dogfood-skeleton.mjstools/repo-cli/test/dogfood-skeleton.test.mjs
| return SpreadsheetAuditProcessorResult( | ||
| schemaVersion=1, | ||
| artifactVersionId=parameters.artifactVersionId, | ||
| jobId=parameters.jobId, | ||
| resultManifestId=parameters.resultManifestId, | ||
| workbookSha256=result.workbookSha256, | ||
| sheets=tuple( | ||
| SpreadsheetAuditSheetSummary( | ||
| name=sheet.name, | ||
| maxRow=sheet.maxRow, | ||
| maxColumn=sheet.maxColumn, | ||
| formulaCount=sheet.formulaCount, | ||
| ) | ||
| for sheet in result.sheets | ||
| ), | ||
| findings=tuple( | ||
| SpreadsheetAuditFindingSummary( | ||
| sheet=finding.sheet, | ||
| address=finding.address.upper(), | ||
| kind=finding.kind, | ||
| severity="WARNING", | ||
| formulaFingerprint=finding.formulaFingerprint, | ||
| ) | ||
| for finding in result.findings | ||
| ), | ||
| blockedReasons=result.blockedReasons, | ||
| processorVersion=PROCESSOR_VERSION, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline services/engine/src/databreeze_engine/processors/spreadsheet_auditor.py --items all --type function
rg -n -C 5 '(_MAX_CELLS|_MAX_MEMBERS|summaries\.append|findings\.append|SpreadsheetAuditResult)' \
services/engine/src/databreeze_engine/processors/spreadsheet_auditor.py
rg -n -C 4 'Field\(min_length=1, max_length=512\)|Field\(max_length=10_000\)' \
services/engine/src/databreeze_engine/models.pyRepository: DatabreezeService/databreeze-platform
Length of output: 5706
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- auditor implementation ---'
sed -n '177,305p' services/engine/src/databreeze_engine/processors/spreadsheet_auditor.py
printf '%s\n' '--- action and model definitions ---'
sed -n '1,130p' services/engine/src/databreeze_engine/processors/spreadsheet_auditor_action.py
sed -n '120,185p' services/engine/src/databreeze_engine/models.py
printf '%s\n' '--- relevant error handling and call sites ---'
rg -n -C 6 'audit_workbook|ActionExecutionError|INTERNAL_ERROR|SpreadsheetAuditProcessorResult' services/engine/srcRepository: DatabreezeService/databreeze-platform
Length of output: 35363
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- sheet target parsing ---'
sed -n '138,176p' services/engine/src/databreeze_engine/processors/spreadsheet_auditor.py
printf '%s\n' '--- spreadsheet auditor tests and boundary references ---'
rg -n -C 5 'spreadsheet.?auditor|audit_workbook|10_000|512|max_cells|RESOURCE_LIMIT_EXCEEDED|INTERNAL_ERROR' services/engine tests 2>/dev/null || true
printf '%s\n' '--- standalone reachability probe ---'
python3 - <<'PY'
MAX_MEMBERS = 2048
MAX_CELLS = 1_000_000
MAX_SHEETS = 512
MAX_FINDINGS = 10_000
REQUIRED_MEMBERS = 2
sheet_count = MAX_SHEETS + 1
print({
"sheets": sheet_count,
"archive_members": sheet_count + REQUIRED_MEMBERS,
"archive_member_limit": MAX_MEMBERS,
"sheet_overflow_reachable": sheet_count + REQUIRED_MEMBERS <= MAX_MEMBERS,
})
formulas_per_sheet = 3
sheets_for_findings = (MAX_FINDINGS // formulas_per_sheet) + 1
cells_for_findings = sheets_for_findings * formulas_per_sheet
print({
"findings": sheets_for_findings * formulas_per_sheet,
"cells": cells_for_findings,
"cell_limit": MAX_CELLS,
"finding_overflow_reachable": (
sheets_for_findings + REQUIRED_MEMBERS <= MAX_MEMBERS
and cells_for_findings <= MAX_CELLS
),
})
PYRepository: DatabreezeService/databreeze-platform
Length of output: 50393
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- formula normalization ---'
sed -n '105,137p' services/engine/src/databreeze_engine/processors/spreadsheet_auditor.py
printf '%s\n' '--- focused reachability calculations ---'
python3 - <<'PY'
MAX_MEMBERS = 2048
MAX_CELLS = 1_000_000
MAX_SHEETS = 512
MAX_FINDINGS = 10_000
REQUIRED_MEMBERS = 2
sheet_count = MAX_SHEETS + 1
print("sheet_overflow:", {
"sheets": sheet_count,
"archive_members": sheet_count + REQUIRED_MEMBERS,
"reachable": sheet_count + REQUIRED_MEMBERS <= MAX_MEMBERS,
})
# One sheet can contain more than 10,000 formula cells while remaining under
# the one-million-cell limit. If each formula has a distinct normalized family,
# each formula is an outlier when the sheet has at least three formulas.
formula_count = MAX_FINDINGS + 1
print("finding_overflow:", {
"sheets": 1,
"formula_cells": formula_count,
"total_cells": formula_count,
"cell_limit": MAX_CELLS,
"archive_members": 1 + REQUIRED_MEMBERS,
"archive_member_limit": MAX_MEMBERS,
"reachable": formula_count <= MAX_CELLS and 1 + REQUIRED_MEMBERS <= MAX_MEMBERS,
})
PYRepository: DatabreezeService/databreeze-platform
Length of output: 1656
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- formula normalization ---'
sed -n '105,137p' services/engine/src/databreeze_engine/processors/spreadsheet_auditor.py
printf '%s\n' '--- focused reachability calculations ---'
python3 - <<'PY'
MAX_MEMBERS = 2048
MAX_CELLS = 1_000_000
MAX_SHEETS = 512
MAX_FINDINGS = 10_000
REQUIRED_MEMBERS = 2
sheet_count = MAX_SHEETS + 1
print("sheet_overflow:", {
"sheets": sheet_count,
"archive_members": sheet_count + REQUIRED_MEMBERS,
"reachable": sheet_count + REQUIRED_MEMBERS <= MAX_MEMBERS,
})
formula_count = MAX_FINDINGS + 1
print("finding_overflow:", {
"sheets": 1,
"formula_cells": formula_count,
"total_cells": formula_count,
"cell_limit": MAX_CELLS,
"archive_members": 1 + REQUIRED_MEMBERS,
"archive_member_limit": MAX_MEMBERS,
"reachable": formula_count <= MAX_CELLS and 1 + REQUIRED_MEMBERS <= MAX_MEMBERS,
})
PYRepository: DatabreezeService/databreeze-platform
Length of output: 1656
Bound auditor result cardinalities before model validation.
audit_workbook() caps cells but not sheets or findings. Workbooks with more than 512 sheets or 10,000 findings can reach SpreadsheetAuditProcessorResult and cause INTERNAL_ERROR. Enforce both limits in the auditor, or raise ActionExecutionError("RESOURCE_LIMIT_EXCEEDED") before result construction. Add boundary tests.
🤖 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
`@services/engine/src/databreeze_engine/processors/spreadsheet_auditor_action.py`
around lines 48 - 75, Cap auditor output before constructing
SpreadsheetAuditProcessorResult: limit result.sheets to 512 and result.findings
to 10,000, or raise ActionExecutionError("RESOURCE_LIMIT_EXCEEDED") when either
limit is exceeded. Update audit_workbook() and add boundary tests covering
exactly-at-limit and over-limit sheet and finding counts.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/api/src/features/sa/application/spreadsheet-audit-run.service.ts (1)
50-82: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve idempotent replay after artifact state changes.
Line 50 checks mutable artifact state before Line 72 reads the existing idempotency record. If an admitted artifact later becomes unavailable or unclean, an identical retry returns
SA_RUN_ARTIFACT_UNAVAILABLEinstead of the original run handle.findcan still return that run.Check for an existing idempotency record before the artifact gate. Keep the in-transaction lookup as the race-safe recheck before
save. Add coverage for replay after a placement becomes unavailable.🤖 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 `@services/api/src/features/sa/application/spreadsheet-audit-run.service.ts` around lines 50 - 82, Update the idempotency flow in the service method containing artifactAvailable so findByIdempotency runs before the mutable artifact availability check and immediately returns the existing handle for an identical request, even if the artifact later becomes unavailable or unclean. Preserve the existing in-transaction findByIdempotency recheck before save for race safety, and add coverage for replay after a placement becomes unavailable.
🤖 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 `@services/api/src/app.module.ts`:
- Around line 42-44: Update the allowInMemorySpreadsheetAuditRunRepository
configuration in AppModule.register to default to false, require an explicit
opt-in for local/test environments, and reject the opt-in whenever NODE_ENV is
production. Add AppModule.register coverage for unset NODE_ENV, production
NODE_ENV, and explicit production settings, preserving durable storage as the
default.
---
Outside diff comments:
In `@services/api/src/features/sa/application/spreadsheet-audit-run.service.ts`:
- Around line 50-82: Update the idempotency flow in the service method
containing artifactAvailable so findByIdempotency runs before the mutable
artifact availability check and immediately returns the existing handle for an
identical request, even if the artifact later becomes unavailable or unclean.
Preserve the existing in-transaction findByIdempotency recheck before save for
race safety, and add coverage for replay after a placement becomes unavailable.
🪄 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 Plus
Run ID: 7c00f7de-02f7-4ea8-86b2-8a3ebd219b87
📒 Files selected for processing (7)
services/api/src/app.module.tsservices/api/src/features/sa/api/spreadsheet-audit-run.controller.tsservices/api/src/features/sa/application/spreadsheet-audit-run.service.tsservices/api/src/features/sa/sa.module.tsservices/api/test/features/foundation-module-composition.test.tsservices/api/test/features/sa/spreadsheet-audit-run.controller.test.tsservices/api/test/features/sa/spreadsheet-audit-run.service.test.ts
|
Validation summary: all actionable CodeRabbit findings were reproduced and addressed in focused commits. The dogfood module now bounds desktop directory scans, validates real UTC timestamps, authorizes tenant-visible ACTIVE/CLEAN artifacts with available placements, requires durable run storage outside an explicit local/test opt-in, bounds engine result cardinality, and exercises the missing-file/hash-mismatch and contract-test branches. A follow-up idempotency edge case was also fixed so replay returns the original handle after placement loss; production in-memory opt-in is rejected. The documentation-coverage warning was treated as non-gating and left out of this focused module change. No manual CodeRabbit rerun was invoked. |
Promotion
Promotes the reviewed dogfood walking skeleton merged into dev as 44b5083.
Included vertical
Verification already completed
This is the promotion PR. Please perform the single full CodeRabbit review on this PR; no second CodeRabbit run is needed.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation