-
-
Notifications
You must be signed in to change notification settings - Fork 0
fix: resolve engine bugs, store desync, export formatting, and ESLint warnings #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| # smartsh!t — Pre-Launch Code Audit & Release Readiness Report | ||
|
|
||
| **Date:** July 25, 2026 | ||
| **Repository:** `Ocean82/smartshit` | ||
| **Branch:** `arena/019f98e3-smartshit` | ||
| **Status:** All code-level bugs and linter warnings fixed. V1 Release Checklist Gates **PASSED** (100%). | ||
|
|
||
| --- | ||
|
|
||
| ## 1. Executive Summary | ||
|
|
||
| A comprehensive audit was performed across the `smartsh!t` full-stack application (React 19 / Vite frontend + Express 5 Node backend + HyperFormula calculation engine + QuickJS sandbox + Ollama / Cloud LLM routing). | ||
|
|
||
| All code defects, calculation/logic bugs, state synchronization issues, and ESLint warnings fixable within the repository were identified, resolved, and verified through automated test suites and build gates. | ||
|
|
||
| Items requiring external environment or infrastructure configuration (API keys, DNS, production database hosting, GPU deployment for local LLM) have been documented in Section 4 for post-environment deployment. | ||
|
|
||
| --- | ||
|
|
||
| ## 2. Identified & Fixed Issues | ||
|
|
||
| ### A. Critical Logic & Engine Fixes | ||
|
|
||
| 1. **Pivot Table Aggregate Bug (`src/engine/spreadsheet.ts`)** | ||
| - **Issue:** When multiple value fields were configured in a pivot table (e.g., `SUM(Sales)` and `SUM(Profit)`), the engine pushed all raw values into a single array key (`aggKey`), causing aggregated sums/averages to mix and corrupt data across all value fields. | ||
| - **Fix:** Refactored `computePivotTable` to key aggregations by individual value field index (`rowKey||colKey||vfIdx`), ensuring isolated, mathematically precise aggregations per field. | ||
|
|
||
| 2. **HyperFormula & Zustand Store Desynchronization (`src/store/useStore.ts`)** | ||
| - **Issue:** Structural operations (`deleteRow`, `insertRow`, `deleteColumn`, `insertColumn`, and `deleteSelectedCells`) modified `sheet.cells` in the Zustand store but omitted re-synchronizing the underlying HyperFormula engine. Subsequent cell reads via `getComputedValue()` returned stale cached values from the wrong cell positions. | ||
| - **Fix:** Added `get().engine.loadWorkbook(get().workbook)` after structural row/col and cell deletion operations to guarantee 100% state synchronization. | ||
|
|
||
| 3. **Target Column Resolution Failure on Empty Columns (`src/agent/executor.ts`)** | ||
| - **Issue:** `resolveColumnIndex` checked `index <= maxCol` when resolving column letters (e.g., `"D"`). On sheets where data only existed up to column `"A"` (`maxCol = 0`), valid column letter targets were rejected with `"Could not find column D"`. | ||
| - **Fix:** Relaxed the bounds check to allow valid column indices (`0 <= index < 1000`), allowing operations targeting empty columns to succeed as expected. | ||
|
|
||
| 4. **Multiline CSV Export Corruption (`src/io/xlsx.ts`)** | ||
| - **Issue:** `exportSheetToCsv` only enclosed cells in double quotes if they contained commas or double quotes. Cells with newlines (`\n` or `\r`) were exported unquoted, violating RFC 4180 and corrupting exported CSV row structures. | ||
| - **Fix:** Added newline detection (`\n` and `\r`) to the CSV escape condition. | ||
|
|
||
| 5. **Imported Workbook Active Sheet Safety (`src/io/workbookJson.ts`)** | ||
| - **Issue:** Importing a corrupted or manually edited JSON package where `activeSheetId` did not match any sheet ID caused UI render errors. | ||
| - **Fix:** Added validation in `normalizeImportedWorkbook` to ensure `activeSheetId` is validated against valid sheet IDs, falling back to `validSheets[0].id`. | ||
|
|
||
| 6. **React Hook & ESLint Warnings Cleanup (25 Files, 48 Warnings -> 0 Warnings)** | ||
| - **Issue:** 48 ESLint warnings were flagged across frontend and backend codebase, including missing React `useEffect`/`useCallback`/`useMemo` dependencies in `App.tsx`, `SpreadsheetGrid.tsx`, `FindReplaceDialog.tsx`, and unused variables/imports across 22 other files. | ||
| - **Fix:** Resolved all 48 warnings. `npm run lint` now completes with 0 warnings and 0 errors. | ||
|
|
||
| --- | ||
|
|
||
| ## 3. Verification & Gate Results | ||
|
|
||
| All automated gates passed cleanly: | ||
|
|
||
| ``` | ||
| V1 Release Checklist | ||
|
|
||
| PASS - Frontend tests pass (38 test files, 369 tests passed) | ||
| PASS - Server tests pass (6 test files, 34 tests passed) | ||
| PASS - Frontend build succeeds (Vite singlefile production bundle built) | ||
| PASS - Server build succeeds (TypeScript compilation clean) | ||
| PASS - Import truncation guardrails are present | ||
| PASS - Preview-denial safety gate is present | ||
| PASS - Deterministic response telemetry is instrumented | ||
| PASS - LLM response telemetry is instrumented | ||
|
|
||
| ESLint Status: 0 warnings, 0 errors | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## 4. Environment & Deployment Findings (To Fix Later) | ||
|
|
||
| The following items cannot be configured inside this local development environment and must be set up in your production deployment pipeline: | ||
|
|
||
| ### 1. Cloud AI Provider API Keys | ||
| - **Description:** For cloud AI models (OpenRouter, Groq, HuggingFace), production API keys must be set in `server/.env`. | ||
| - **Action Required:** | ||
| - `OPENROUTER_API_KEY`: Required for OpenRouter failover. | ||
| - `GROQ_API_KEY`: Required for fast Groq inference. | ||
| - Set `LLM_PROVIDER_ORDER=openrouter,groq,ollama` in production environment variables. | ||
|
|
||
| ### 2. Ollama Local GPU Setup (Optional / Self-Hosted) | ||
| - **Description:** Local LLM execution requires a running Ollama server (`http://127.0.0.1:11434`) with the `smartsht` model loaded. | ||
| - **Action Required:** | ||
| - On your host/GPU server, run `npm run model:setup` (or `node server/scripts/setup-model.mjs`) to pull and build the Ollama model. | ||
|
|
||
| ### 3. Production Cloud Foundation (PostgreSQL & S3) | ||
| - **Description:** Cloud workbook saving, version history, and user authentication require PostgreSQL and S3 (or Cloudflare R2 / AWS S3). | ||
| - **Action Required:** | ||
| - Set `DATABASE_URL` in `server/.env` and execute migrations (`node server/scripts/run-migration.mjs`). | ||
| - Set `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, `AWS_REGION`, and `S3_BUCKET` in `server/.env`. | ||
|
|
||
| ### 4. Authentication (Clerk) | ||
| - **Description:** Clerk authentication JWT verification requires Clerk credentials. | ||
| - **Action Required:** | ||
| - Set `CLERK_SECRET_KEY` and `VITE_CLERK_PUBLISHABLE_KEY` in production environment variables. | ||
|
|
||
| ### 5. Stripe Billing Integration | ||
| - **Description:** Pro tier upgrades and checkout sessions require Stripe setup. | ||
| - **Action Required:** | ||
| - Set `STRIPE_SECRET_KEY`, `STRIPE_WEBHOOK_SECRET`, and `STRIPE_PRO_PRICE_ID` in `server/.env`. | ||
|
|
||
| ### 6. Licensing Resolution (HyperFormula GPLv3) | ||
| - **Description:** `README.md` and `docs/LICENSING.md` note that HyperFormula is used under its GPLv3 license option, which requires attribution/alignment with MIT license declarations. | ||
| - **Action Required:** Review `docs/LICENSING.md` before commercial SaaS distribution. | ||
|
|
||
| --- | ||
|
|
||
| ## 5. Conclusion | ||
|
|
||
| The application codebase is clean, well-tested, free of lint warnings, and mathematically consistent. Once the production environment variables and external cloud services listed in Section 4 are configured, the project is ready to ship to launch. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (typo): Possible typo or inconsistency in the Ollama model name (
smartsht).Elsewhere the project and repo use
smartsh!t/smartshit, sosmartshtlooks like it may be missing an "i". Ifsmartshtis the correct Ollama model name, consider adding a brief note to clarify this and prevent confusion.Fix in Cursor