Release: Rebuild csvjson.com as a minimal modern converter - #155
Conversation
…ersion service (#156) * chore: scaffold vite react spa Vite + React 18 + TypeScript (strict) + Tailwind v4 + shadcn/ui foundation (components.json, cn util, theme variables) with vitest and eslint flat config. Sits in app/ alongside the PHP app until cutover. * chore: add ci workflow GitHub Actions from zero: lint + typecheck + vitest + production build on every PR (and pushes to the release branch). Node 20, npm ci with lockfile cache. * feat: conversion service with typed options Typed csvToJson/jsonToJsonCsv wrappers over csvjson-csv2json@5.0.6 and csvjson-json2csv@1.0.3 with .d.ts shims (packages are untyped CommonJS). Wrapper strips UTF-8 BOM, normalizes the packages' thrown strings into real Errors, and maps non-tabular JSON to the spec's 'I need an array of objects' message. Minify is exposed as toJsonString(), matching the legacy tool's serialization behavior (js/src/csv2json.js) — csv2json has no minify parse option. Conversion logic is imported, not reimplemented. * test: round-trip + options + edge-case fixtures 23 vitest fixtures over csvToJson/jsonToJsonCsv covering spec criteria 1, 2, 11: round-trip fidelity (quoted fields, embedded commas/newlines/quotes, TSV, semicolons, unicode), per-option behavior, and edge cases (ragged rows, duplicate headers, empty cells stay empty strings, BOM, CRLF/mixed endings, non-tabular JSON error). * fix: normalize csvToJson errors and scope CI token permissions Addresses obvious/code-review findings: csvToJson now rethrows the packages' plain-string throws as real Errors (same contract as jsonToJsonCsv), the error-contract test asserts Error instances explicitly, ci.yml gains a least-privilege permissions block (contents: read), and the build step runs vite build alone since the typecheck step already runs tsc -b. --------- Co-authored-by: Obvious <obvious@obvious.ai>
* feat: static-serving front controller Replace the CodeIgniter bootstrap with a minimal PHP shim that serves the built SPA from app/dist: / and built assets (with fingerprint-aware cache headers), plus /img/favicon.ico. Everything else is rewritten to the front controller by .htaccess. Sitemap and robots regenerate to list / only. * feat: 301 redirect map Permanent 301s home for every retired tool URL — /csv2json, /json2csv, /json_validator, /json_beautifier, /sql2json, /csvjson2json, /datajanitor and all sub-routes, plus the /dataclean alias. Legacy /<tool>/<32-hex-id> permalinks are not redirected: they pass through to the SPA shell so the router can hydrate them read-only from S3. /csv2json/instrument and /<tool>/upload return 410 Gone — uploads are client-side FileReader now and the telemetry write is deleted. CI gains a shim job: php -l plus curl checks covering every row of the redirect table, permalink passthrough, 410s, 404s, and the root serving the SPA shell (spec criterion 7). * chore: delete legacy controllers, views, and telemetry Remove the entire legacy stack the static shim replaces: all tool controllers and views (including /build, the banner and CTA chrome, the ad views, and Typekit), the CodeIgniter framework, the vendored jQuery/Underscore/Backbone/Backgrid/Bootstrap/CodeMirror-5 stacks and legacy stylesheets, the Csv2json::instrument() MySQL write, MY_Controller::save(), and the S3 write library wiring. The S3 bucket and its legacy objects are untouched — the browser reads them directly. CI also gains a grep gate asserting zero hits for promotional or telemetry terms across the shipped frontend and PHP shim (spec criterion 8), and stale ignore rules for deleted folders are dropped. * fix: address review — delete unreferenced promo images, test dist-asset serving Two review findings from the automated review: - The remnant gate never scanned image assets, and img/ still shipped the Flatfile/Obvious ad artwork (logomarks, sponsor logo, banner background, ad PNGs) orphaned by the view teardown. All unreferenced images are deleted — favicon.ico is the only survivor, the one file the shim serves — and the CI gate now also fails on surviving img/ filenames matching flatfile|obvious|_ad|sponsor. - serve_dist_file had zero test coverage despite carrying the traversal refusal and cache-control logic. verify-shim.sh now fixtures a hashed asset and asserts 200 + immutable Cache-Control on it, 404 on a ..%2f traversal attempt, and 404 on a missing dist path. --------- Co-authored-by: Obvious <obvious@obvious.ai>
…158) * feat: converter shell with split pane and seam switch Full-viewport SplitPane with a flush 1px seam (no gutter), drag resize clamped 20-80%, double-click reset to 50/50, and vertical stacking below 768px where the seam becomes a horizontal drag strip. Floating circular DividerSwitch centered on the seam flips CSV<->JSON and swaps pane roles: the last valid output becomes the new input; an error output leaves the input untouched. Conversion stays in src/lib — components are presentational and prop-driven. Includes the jsdom testing setup and component tests for seam clamping, reset, flip rules, and output rendering. * feat: topbar with theme toggle csvjson wordmark, empty chat-promo slot reserved for the promo task, and a dark/light toggle persisted to localStorage behind try/catch with OS-preference fallback. * feat: options bar Direction-conditional bottom bar: CSV->JSON shows separator (auto/ forced), parse numbers, parse JSON, transpose, hash; JSON->CSV shows separator and flatten; minify appears when applicable. Prerendered info-icon hints keep the SEO initial-DOM rule. * fix: sync npm lockfile with testing dependencies package.json gained the jsdom/testing-library devDependencies but the lockfile was regenerated with a different package manager, leaving @testing-library/user-event absent from package-lock.json — npm ci failed on CI. Regenerated with npm install; verified with a clean rm -rf node_modules && npm ci, then lint/typecheck/tests/build. * fix: address review findings — flip race, seam input, theme flash High: handleFlip used the debounced memoized result, which lags input by the debounce window (1s for >2MB pastes) — flipping inside that window could wipe a freshly typed input. The handler now converts the current input synchronously; regression test flips without waiting for the debounce. Medium: - OutputView memoizes tokens, wraps in React.memo, and skips highlighting above 256KB (multi-MB outputs render plain but stay usable). - SplitPane seam drag now uses Pointer Events (touch devices can resize; touch-action: none) and the separator is keyboard-operable — arrows ±1% (shift ±5), Home/End to the clamps, Enter/space resets, tabIndex + aria-label. - index.html applies the saved/OS theme before first paint — no dark-mode white flash. Theme persists only on explicit toggle, not when an OS-derived theme is applied on mount. Also: hint buttons get aria-expanded + aria-describedby linkage, and the large-file notice copy now matches the actual mechanism. Tests updated to pointer events; added flip-within-debounce and keyboard-resize regression tests. 35/35 green. * fix: end seam drag on pointercancel (palm rejection, gesture takeover) --------- Co-authored-by: Obvious <obvious@obvious.ai>
* feat: chat promo with provider constant and deep links "Chat with this data in…" in the top bar — the single surviving promotion (spec art_K61RhTbt, "Chat with this data in…" section). - CHAT_PROMO_PROVIDERS: one configurable constant (Obvious, ChatGPT, Claude — Codex omitted, no documented web prefill mechanism; re-add is a one-line append). - Static prompt templates: data inlined at ≤ 8000 chars; larger data goes to the clipboard with the paste-it-next prompt and a toast. - Deep links: Claude claude.ai/new?q= (documented prefill), ChatGPT chatgpt.com/?q= (best-effort, may auto-submit — clipboard fallback covers it); prompt always copied as the prefill fallback. - Chips disabled while the input is empty; enabled the moment any valid input exists. - Clipboard has an execCommand fallback; every successful copy toasts. Verified: 54/54 tests (template purity, ~8k branch at both edges, disabled-when-empty, per-provider URL construction, fallback copy), lint + tsc + vite build green. * fix: open provider synchronously within the click gesture Bot review (obvious-autobuild-staging[bot]) on #159: - High: window.open after await loses the Safari user gesture — popups silently blocked. Open synchronously in the gesture; toast only after the copy promise settles; new toast when copy and open both fail. - Suggestion: title tooltip moved to a wrapper span with pointer-events-none on the disabled button, so the empty-state hint is actually reachable. 55/55 tests, lint + tsc + build green. --------- Co-authored-by: Obvious <obvious@obvious.ai>
… states (#160) * feat: csv dense table with virtualization Dense full-bleed CSV table renderer with sticky header, compact rows, monospace numerics, and TanStack Virtual so 10k+ rows render without frame freeze. Delimiter sniffing (comma/semicolon/tab) and row/col utilities live in lib/csvTable. * feat: json codemirror editor CodeMirror 6 JSON editor with lint for the input side and a read-only highlighted view for the output side. Replaces the scaffold's OutputView/highlight implementation. * feat: file upload and pane states Dropzone empty state ('Drag & drop, paste, or browse' + Try an example) only while the input is empty, whole-pane drag-over highlight with inline non-text rejection, FileReader upload (file picker + drop, zero network) with filename and row/col counts in pane headers, TSV auto-detection alongside comma/semicolon, inline errors with position that keep the last valid conversion visible, >2MB debounce stretch with the bottom-bar hint, raw-text toggle on the CSV input side, and copy/clear/download actions — CSV downloads carry a UTF-8 BOM. * fix: address bot review — dropzone activation, paste-on-load, gate hardening - Dropzone: click + Enter/Space open the browse dialog (was a dead target); holds focus while empty so a fresh-load Ctrl+V reaches the pane's paste handler instead of document.body. - clipboard.ts: restore the try/catch around the execCommand fallback (a throw became an unhandled rejection). - files.ts: accept known text extensions regardless of MIME — older Excel saves put application/vnd.ms-excel on .csv files. - numericColumns: per-column sawCell flag; an all-empty column is no longer monospace just because another column has data. - JsonCodeMirror: skip JSON.parse lint above 512KB — cost exceeds the diagnostics on multi-MB docs. Tests: dropzone click/Enter/Space open the picker, paste lands on a fresh load, and .csv with a non-text MIME is accepted. * fix: container keydown no longer swallows nested-button activation The dropzone's onKeyDown matched event.key only, so Enter/Space bubbling from 'Try an example' hit the container guard, and its preventDefault() cancelled the child button's native activation — keyboard users tabbing to the button got the file picker instead of the sample. The handler now returns early when event.target !== event.currentTarget; a test pins the nested-button path. --------- Co-authored-by: Obvious <obvious@obvious.ai>
) * feat: legacy permalink route and hydration service Route /:tool/:id in the SPA: legacy /<tool>/<32-hex-id> URLs passed through by the PHP front controller now hydrate in the client. A read-only fetchLegacyPermalink(id) GETs data/<id> straight from the production S3 bucket (build-time VITE_S3_DATA_URL, cors mode) and a single element-id → converter-state mapping table populates the converter in the matching direction. The URL is never rewritten and nothing is ever written back; CI gains a zero-tolerance no-write-API check (spec: Old share links keep resolving — read-only). * test: hydrate and failure states Component tests for hydrate/not-found/failure-with-retry states and the read-only contract (URL never rewritten, plain GET with cors mode, no calls without a permalink id), unit tests for path parsing, shape matching, and fetch failure classes, plus a manual E2E script that hydrates a real 2019 bucket object through the unmocked code paths. * fix: gate hydration by tool, honest unsupported notice, runnable E2E Address the automated review (1 blocker, 1 high, 1 medium): - parsePermalinkPath now returns {tool, id} and hydration is gated to converter-shaped tools (csv2json, json2csv, csvjson2json). A sql2json save's `json: true` format radio can no longer masquerade as input: only a non-empty string counts as saved input, so a boolean radio can never hydrate an empty json2csv converter. Fixtures built from the real legacy view ids (sql2json, json_beautifier, json_validator) pin the routing. - Objects that exist but no longer map (Data Janitor sessions, any unmappable shape) get a distinct "can't be shown in the new converter" notice — "doesn't exist (or was deleted)" is reserved for genuine 403/404. - The env read goes through an optional chain with trailing-slash normalization, so permalink.ts loads in non-Vite runtimes and the committed E2E script runs as-is under tsx (inert globalThis shim deleted). Unit fixtures pin the sql2json/json_beautifier/ json_validator view shapes. --------- Co-authored-by: Obvious <obvious@obvious.ai>
…red data (#162) * feat: prerender single route at build The SPA route is rendered to static HTML at build time (scripts/ prerender.mjs boots Vite in middleware mode, renderToStrings the App, and writes it into dist/index.html) — a build step, not SSR. The deployed index.html now carries the title, an sr-only H1 naming both directions, meta description, OG/Twitter tags, and canonical to /, so crawlers index the page without executing JavaScript. CI runs npm run build (which ends with the prerender) and then curls the served build, grepping for each on-page target; any miss fails the run (spec criterion 10). * feat: faq accordion and option hints Adds the collapsed-by-default "How it works & FAQ" accordion below the converter (how it works, what the options do, TSV vs CSV, and privacy — conversion never leaves the browser), plus info-icon hints on the non-obvious options (parse numbers, parse JSON, transpose, hash output, minify, flatten). Every answer and hint is rendered into the DOM at load; clicks only toggle visibility via the hidden attribute, so the prerendered shell carries the full copy for crawlers and nothing is mounted on click (spec: SEO disclosure pattern). FAQ copy lives in one module the accordion renders from. Tests pin the initial-DOM rule for both the FAQ and the option hints. * chore: structured data SoftwareApplication JSON-LD in the document head names the app and both conversion directions; the FAQ block carries FAQPage markup derived from the same items the accordion renders, so the machine-readable questions can never drift from the visible copy. Both are rendered unconditionally, so the prerendered HTML ships them regardless of UI state. CI greps for both blocks in the built index.html. * fix: neutralize the pre-mount input-loss window; FAQ aria linkage The prerendered shell is real markup a user can see before the bundle loads, and React's fresh mount discards #root's contents — anything typed during that window was silently wiped. An inline script now sets inert on #root before first paint and main.tsx lifts it right after createRoot(...).render(...), so the shell is never interactive while its state is disposable (review: Medium reliability). Also links the FAQ toggle to its region (useId + aria-controls), matching the OptionHint convention (review suggestion). * fix: guard window read so the prerender survives #161's permalink hydration parsePermalinkPath(window.location.pathname) ran during render and blew up the build-time prerender ("window is not defined") after the rebase onto #161. Guard with the codebase's standard typeof-window check — the shell renders the default converter state server-side; the hook already accepts null and only fetches in a client effect. --------- Co-authored-by: obvious-autobuild-staging[bot] <262703071+obvious-autobuild-staging[bot]@users.noreply.github.com>
…utton The seam's onPointerDown called setPointerCapture() unconditionally — including when the gesture started on the floating ⇄ switch, retargeting the click to the seam so the direction switch never fired on real mouse input (jsdom's synthetic click cannot reproduce pointer capture, so the component tests missed it; the Playwright walkthrough caught it). The seam now ignores pointerdowns that originate on the switch, and a regression test pins the behavior.
…bundle csvjson-json2csv@1.0.3 assigns `keyValues = []` without a declaration — an implicit global that Node tolerates in sloppy mode but that throws `ReferenceError: keyValues is not defined` in a strict-mode ESM bundle, i.e. in the shipped browser build (the legacy site loaded the same code as a classic script, so it never hit this; caught by the Playwright walkthrough, invisible to jsdom). Fix is a one-token mechanical patch applied by app/scripts/patch-json2csv.mjs as a postinstall step so CI's fresh `npm ci` gets it too. No conversion logic changes.
Scripted trusted-input walkthrough of every state in the spec's states table (empty, drag-over, ready, raw editing, direction flip, invalid input, dark mode, narrow viewport, permalink hydration, FAQ) against the PHP shim serving the production build — 18/18 checks green. Both browser-only bugs above were caught by this walkthrough and are invisible to jsdom. verification-screenshots/ holds the 12 evidence captures referenced by docs/verification-report.md.
Spec default pending David's host-access call: the production build is committed (app/dist), so the host needs only a git pull — Apache's PHP front controller serves it unchanged. The built bundle also carries the keyValues fix, so the deployed artifact is current.
![obvious-autobuild-staging[bot]](https://avatars.githubusercontent.com/u/31830338?s=60&v=4){kind=small}
There was a problem hiding this comment.
Obvious Code Review
Verdict: COMMENT (2 Medium findings, 2 Suggestions — no Blocker/High)
Release promotion of the reviewed-clean stack #156–#162 plus a 5-commit delta. Verified: release head app code is identical to #162's clean-pass head plus exactly the two fix commits; the committed app/dist bundle contains both post-stack fixes (keyValues patch and split-pane pointer guard); asset paths are absolute so permalink URLs load correctly; the teardown ↔ shim seam (favicon, sitemap, robots, 301 map) is coherent; no credentials ship with the SPA.
Medium
.github/workflows/ci.yml:40— CI never verifies the committedapp/dist(the deploy transport) matches a fresh build; a future source edit with a forgotten rebuild ships stale silently.app/scripts/states-walkthrough.mjs:10— output path hardcoded to a machine-specific absolute path; the script is non-portable off this sandbox.
Suggestions
app/src/components/SplitPane.tsx:131— production gesture logic keyed on a test-onlydata-testidattribute; use a semantic attribute the component owns..github/workflows/ci.yml:6— push CI triggers only on the release branch; direct pushes to master after merge run no CI.
| run: npm test | ||
|
|
||
| - name: Production build | ||
| run: npm run build # typecheck step already runs tsc -b; build ends with the prerender step |
There was a problem hiding this comment.
Medium · reliability — No CI gate that the committed dist matches a fresh build. This PR commits app/dist as the default deploy transport (98d9012), but this step builds into the working tree and verify-seo.sh validates that fresh build — the committed app/dist is never compared. Failure mode: a future PR edits app/src, CI goes green (lint/tsc/tests/build all validate the fresh build, not the committed artifact), the author forgets to re-commit dist/, and the deploy ships the stale bundle silently. Add a step after the build: git diff --exit-code app/dist (or fail on git status --porcelain app/dist output). Vite's content-hashed filenames make drift mechanically detectable. Run it in this ci job before verify-shim.sh writes its stand-in app/dist/index.html.
There was a problem hiding this comment.
Fixed in 5a149c6. The ci job now hashes app/dist with sha256sum before the build regenerates it and diffs the two manifests after — drift in file set, name (Vite content hashes), or bytes fails the job with an ::error pointing at npm run build + commit. Snapshot runs before Production build, well before verify-shim.sh can write anything. The guard proved itself immediately: it flagged the committed dist as stale after the SplitPane selector change, so the rebuilt bundle is committed (756f4f0).
| import { mkdirSync } from "node:fs"; | ||
|
|
||
| const BASE = process.env.BASE_URL ?? "http://127.0.0.1:8899"; | ||
| const OUT = "/home/user/work/csvjson-app/verification-screenshots"; |
There was a problem hiding this comment.
Medium · reliability — Walkthrough output path is a hardcoded machine-specific absolute path. OUT = "/home/user/work/csvjson-app/verification-screenshots" is the repo sandbox's path, committed to the repo. On any other checkout location (CI, another machine) the script either crashes in mkdirSync (permission denied outside $HOME) or litters an unrelated absolute directory. BASE is correctly env-overridable; OUT is not. Derive it from the script location, matching the pattern already used in patch-json2csv.mjs: const OUT = fileURLToPath(new URL("../../verification-screenshots", import.meta.url));
There was a problem hiding this comment.
Fixed in d04d82b — OUT is now derived from the script's own location (path.resolve(dirname(fileURLToPath(import.meta.url)), "../..") + verification-screenshots), matching the pattern in patch-json2csv.mjs. Verified locally from the sandbox checkout.
The seam's pointer-capture guard keyed on the switch's test-only data-testid attribute; production gesture logic should not depend on a test-only attribute. The component now skips pointer capture for any pointerdown originating on a button element — a semantic guarantee the component owns, and the existing regression test still pins it.
The output path was a machine-specific absolute path; derive verification-screenshots/ from the script's own location so the walkthrough runs from any checkout.
Two guards for the committed-artifact deploy transport: - hash app/dist before the build regenerates it and diff after, so a forgotten rebuild cannot ship stale frontend code; - run CI on pushes to master too, so post-merge pushes stay covered. Triggered locally: the guard correctly flagged the committed dist as stale after the SplitPane selector change; fresh dist is committed in the next commit.
Carries the SplitPane semantic-guard change; verified byte-identical to a fresh build and walkthrough 18/18.
|
All four findings addressed on release/rebuild-csvjson-spa and pushed (c02d448, d04d82b, 5a149c6, 756f4f0):
The staleness guard caught the refreshed-bundle drift on this very push cycle, so the regenerated app/dist is committed too (756f4f0). Walkthrough re-run: 18/18. Promotion merge stays with David. |
There was a problem hiding this comment.
Obvious Code Review — Pass 2
Verdict: COMMENT (1 new Medium finding — no Blocker/High)
All four Pass 1 findings verified resolved:
- dist freshness gate — snapshot-before-build / diff-after-build sha256 comparison, correctly ordered and isolated from
verify-shim.sh's stand-in overwrite; ran on this push and passed (committed dist is byte-identical to a fresh build). - walkthrough output path — now resolved repo-root-relative from
import.meta.url; correct. - seam gesture guard —
closest("button")instead of the test-only testid; the seam subtree contains only the switch button, so behavior is unchanged. - push CI on master — branch added to the trigger.
Also verified: old hashed assets fully removed from the tree, dist/index.html references only the new hashes, and the new bundle carries the guard.
New finding
app/src/index.css:1(withapp/dist/assets/index-ChiiLHFC.css) — the rebuild grew the stylesheet 75% (18,460 → 32,314 bytes) with utility classes the app never uses; root cause is Tailwind auto source detection scanning the whole repo, so non-app text (CI workflow, script comments) now leaks into production CSS.
| @@ -0,0 +1,62 @@ | |||
| @import "tailwindcss"; | |||
There was a problem hiding this comment.
Medium · performance — Rebuilt CSS ships ~75% unused utility bloat; Tailwind source scan is unconstrained. The dist rebuild (756f4f0) changed the stylesheet from 18,460 → 32,314 bytes even though no CSS-affecting source changed (the only TSX delta was a 1-line guard swap). Spot-check of 18 added utility families: 17 have zero occurrences in app/src or the prerendered markup (.transform, .filter, .blur, .grayscale, .isolate, .not-sr-only, .table-caption, .table-row-group, .stacked-fractions, .oldstyle-nums, .touch-pinch-zoom, .inline-grid, .flow-root, .list-item, .rounded-tl, .divide-y, .backdrop-blur). Root cause: bare @import "tailwindcss" here means Tailwind v4 auto source detection scans the whole repository — CI workflow text, script comments, tsconfig (isolatedModules) — and emits utilities for candidate tokens found in non-app files. Editing .github/workflows/ci.yml or app/scripts/*.mjs changed the shipped stylesheet; any text edit anywhere in the repo can silently change production CSS, and the new dist-freshness gate will lock that bloat in as "correct". Fix: constrain the scan explicitly — @source "../src/**/*.{ts,tsx}"; @source "../index.html"; — which shrinks the stylesheet back and makes the freshness gate detect real drift instead of repo-text noise.
There was a problem hiding this comment.
Fixed in 8a435c4. The import is now @import "tailwindcss" source(none); with explicit @source "./**/*.{ts,tsx}" (relative to src, so it covers the app components and their tests) plus the entry HTML — stylesheet is back to exactly 18,460 bytes (same content hash as the pre-bloat copy), 111/111 tests green, walkthrough re-run 18/18 against the rebuilt bundle. Editing CI workflows or scripts can no longer change shipped CSS, so the freshness gate now detects real drift only.
Bare auto source detection scanned the whole repository, so text in CI workflows, script comments, and tsconfig leaked unused utilities into production CSS (18,460 -> 32,314 bytes on the last rebuild). Constrain it explicitly with source(none) + @source for src and the entry HTML: the stylesheet is back to 18,460 bytes and editing non-app files can no longer change shipped CSS, so the dist-freshness gate detects real drift only.
|
New finding addressed in 8a435c4: the Tailwind import is now |
There was a problem hiding this comment.
Obvious Code Review — Pass 3
No findings — clean pass.
The Pass 2 finding (unused CSS utility bloat from the unconstrained Tailwind source scan) is resolved and decisively verified:
8a435c4constrains the scan inapp/src/index.css—@import "tailwindcss" source(none)plus explicit@sourcefor./**/*.{ts,tsx}and../index.html.- The rebuilt stylesheet (
index-BO5hHe75.css) is byte-identical to the original 18,460-byte output (the original content-hash filename returned) — the constrained scan reproduces exactly the app's real usage, zero bloat. - JS blob unchanged (identical git blob hash; the filename rehash is a chunk-graph naming artifact, not a content change).
dist/index.htmlreferences only the new hashed names; the stale asset is fully removed from the tree. - CI on that head — including the new dist-freshness gate — fully green. With the scan constrained, the gate now detects real drift rather than repo-text noise.
The follow-up push (4ae40b0) is a walkthrough screenshot refresh for the rebuilt bundle — PNG binaries only, no code.
All findings raised across this PR's three passes (2 Medium + 2 Suggestions, then 1 Medium) are resolved and verified. Zero open findings.
Product decision (David): the "Chat with this data in…" promo is parked for a future release. Surgically removes the PR #159 surface: ChatPromo component + tests, chatPromo lib (providers, prompt templates, 8k inline/clipboard handoff, deep links, toasts), the App composition, and the walkthrough's chat-chip check (18 → 17). The TopBar keeps its wordmark and theme toggle; its slot stays as a neutral, intentionally empty extension point for the future re-add. Grep-verified: no chat/ChatGPT/Claude references remain in app/src, app/index.html, scripts, or the prerendered markup.
17/17 checks against the promo-free production build.
Criterion 13 marked cut per David's direction — a product decision, not a verification failure. Counts updated: 91/91 tests (11 files), 17/17 walkthrough checks, 15329-char prerender.
There was a problem hiding this comment.
Obvious Code Review — Pass 4
No findings — clean pass.
This push cuts the chat promo for v1 (documented product decision in docs/verification-report.md, criterion 13). The removal is complete and verified:
- No dead code:
ChatPromo.tsx, its test suite,chatPromo.ts, and its tests all deleted;App.tsxusage removed. Zero chat-promo references remain in the tree outside the intentional decision record. - TopBar generalized cleanly:
chatPromoprop → genericslotprop with a comment documenting the intentionally-empty extension point for the future re-add. - Prerender rebuilt correctly: the promo block is gone from
dist/index.html; the JSON-LD / SEO surface is untouched (the promo was never in structured data). Stale hashed assets fully removed. - CSS shrank 18,460 → 17,645 bytes as promo-exclusive utilities dropped — the Pass-3
@sourceconstraint held; no scan bloat reintroduced. - Verification artifacts updated honestly: walkthrough 18/18 → 17/17 with all screenshots regenerated, tests 111/111 (13 files) → 91/91 (11 files), prerender size restated with pre-cut values for traceability.
- CI fully green on this head — including the dist-freshness gate, which byte-verified the committed promo-free dist against a fresh build.
All findings raised across this PR's four passes (2 Medium + 2 Suggestions, then 1 Medium) are resolved and verified. Zero open findings.
|
Thank you — glad the constrained Tailwind scan checked out byte-identical. Note this thread pre-dates the chat-promo cut: since then the promo has been removed for v1 (a849543) and the stylesheet shrank further to 17,645 bytes as promo-exclusive utilities dropped. See the Pass 4 review for the verification of that change. |
|
Thank you — all four deliverables are on release/rebuild-csvjson-spa at 2031376 (a849543 removal, a3178fd dist rebuild, 22a457e refreshed screenshots, 2031376 docs decision record). 91/91 tests, lint/typecheck clean, 17/17 walkthrough checks against the promo-free build, committed dist verified byte-identical to a fresh build by the CI guard. CI is green on both runs. Promotion merge remains David's call. |
…oots the rebuilt SPA (#164) * feat(deploy): Procfile + composer stub for Heroku cutover Root Procfile boots heroku-php-apache2 at the default docroot (app root); the committed .htaccess rewrites non-file requests to index.php, which serves app/dist and applies the legacy 301 map. composer.json pins the PHP runtime to ~8.3.0 and is required for PHP buildpack detection; matching platform-only composer.lock generated with composer update --no-install --ignore-platform-reqs. docs/deploy.md is the transition runbook (buildpack detection, cutover checklist after PR #155 merges, immutable-release rollback, DNS/SSL untouched at Cloudflare). Handoff section in docs/verification-report.md now points at it. Shim verified: php -l clean on 8.4, full verify-shim.sh redirect table green locally. * fix(security): deny docroot internals in the default Heroku docroot With the default docroot = repo root, Apache serves every committed real file unless denied. Extend the .htaccess deny list to repo docs, verification screenshots, dotfiles, build/dep metadata, and editor project files (403), mirror the same list in the front controller so the dev server 404s them identically, pin both behaviors in verify-shim.sh, and state in the runbook that the deny list is the only gate on docroot files. * docs(deploy): SEO monitoring checkpoints Post-cutover SEO monitoring section: Search Console baseline export is a blocking pre-cutover artifact (step 0, the only before/after evidence); week-2/week-4 checkpoints against the baseline with the specific check that legacy tool URLs show as 'Page is redirected' (not errors) in coverage; sustained >20% drop on core-tool queries at week 4 triggers a revisit, with dedicated landing pages as the recovery play — undoing the redirects is explicitly not the mitigation; satellite-tool query traffic (sql to json, data janitor) is expected to drop per the locked product decision. --------- Co-authored-by: Obvious <obvious@obvious.ai>
There was a problem hiding this comment.
Obvious Code Review — Pass 5 (final)
Verdict: COMMENT — no findings; clean pass. PR merged as 78c3834; review closed out.
Head: 2ca3a7e — the delta since Pass 4 is exactly the squash-merge of PR #164 (Procfile + composer deploy config, docroot deny list, deploy docs), whose content is byte-identical to #164's reviewed-clean final head (3 passes there, including the resolved-and-verified docroot exposure fix). CI fully green on this head (both ci and shim jobs).
Final ledger across 5 passes: 2 Medium + 2 Suggestions (Pass 1, all resolved and verified in Pass 2) → 1 Medium performance (Pass 2, resolved and byte-verified in Pass 3) → clean (Pass 3) → clean (Pass 4, chat-promo cut) → clean (Pass 5). Zero open findings at merge.
Post-merge recommendations issued in the workspace.
2 participants
Release: Rebuild csvjson.com as a minimal modern converter
Rebuild csvjson.com (FlatFilers/csvjson-app, PHP/CodeIgniter) as a single-purpose, minimal, modern converter: static Vite + React + TS + Tailwind + shadcn/ui SPA, all conversion client-side, thin PHP shim (static serving + 301 map), read-only hydration of legacy S3 permalinks, full legacy teardown (controllers, views, promos, ads, telemetry), and a prerendered single route for SEO. Sharing and the chat promo are cut for v1 (promo removed post-verification per product decision; parked for a future release).
Branch:
release/rebuild-csvjson-spaSpec: approved Blueprint (art_K61RhTbt) · Research: art_XBXAvnl3 · Mockup: art_m9v3z0Rv
Progress: 9/9 PRs merged — ready for review
Verification: docs/verification-report.md (all 12 spec criteria with evidence) · Playwright walkthrough 18/18 · full report in the description below
Completed Work
web: heroku-php-apache2), composer.json/lock (PHP ~8.3) for buildpack detection, docs/deploy.md transition runbook, docroot deny hardeningMerged PRs
web: heroku-php-apache2), composer.json (~8.3.0) + platform-only composer.lock for buildpack detection, docs/deploy.md transition runbook (buildpack detection, cutover checklist, immutable-release rollback, post-cutover SEO monitoring with blocking Search Console baseline + week-2/4 checkpoints), .htaccess/index.php docroot deny list (docs/, screenshots, dotfiles, composer files no longer publicly served), verify-shim.sh extendedIntegrated Verification (this PR)
All 12 spec acceptance criteria verified with evidence on the integrated branch (docs/verification-report.md):
Two browser-only bugs found during verification and fixed on this PR (both invisible to jsdom): (1) the seam's setPointerCapture swallowed ⇄ switch clicks on real mouse input; (2) csvjson-json2csv@1.0.3's implicit global
keyValuescrashed JSON→CSV in the strict-mode ESM bundle — fixed via a one-token postinstall patch (app/scripts/patch-json2csv.mjs), no conversion logic changed.Cutover (after this PR merges)
The deploy path is documented in docs/deploy.md (PR #164): the existing Heroku app picks the release branch up via Procfile (
web: heroku-php-apache2) + composer buildpack detection — no infra changes; Cloudflare DNS/SSL untouched. Cutover checklist, immutable-release rollback, and post-cutover SEO monitoring (week-2/week-4 Search Console checkpoints) are in the runbook.Pre-launch items for David
🔗 Obvious Project · 🧵 Obvious Thread