mirror of
https://github.com/tiennm99/thptqg.git
synced 2026-10-11 03:13:48 +00:00
docs(plans): drop the completed migration plans
Both plans are done and merged, and their contents now describe a tree that no longer exists — go-parser/, four datasets, npm entry points, a Rust crate. Git history keeps them. The parity evidence stays: docs/data-pipeline.md cites parser-parity-result.md for the recovered foreign-language scores, so that report and the two JSON snapshots behind it remain, now marked as an archived record.
This commit is contained in:
1 parent
c359a0b444
commit
08a408ca96
17 files changed
+7
-2912
No files matched your search
-170
@@ -1,170 +0,0 @@
|
||||
---
|
||||
phase: 1
|
||||
title: "Standard schema and unified parser"
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies: []
|
||||
effort: ""
|
||||
---
|
||||
|
||||
# Phase 1: Standard schema and unified parser
|
||||
|
||||
## Overview
|
||||
|
||||
Move the SQL schema out of the four TOML configs into one Rust module, widen it
|
||||
to the 22-column union, and prove the four rebuilt databases still match their
|
||||
current contents. Done in place under the existing `2016/`/`2017/` layout so
|
||||
parity is measurable before anything moves.
|
||||
|
||||
## Context
|
||||
|
||||
`2016/tools/xlsxread` and `2017/tools/xlsxread` are the same crate. Verified by
|
||||
`diff -rq`: `cli.rs`, `error.rs`, `reader.rs` are byte-identical; `audit.rs`,
|
||||
`config.rs`, `lib.rs`, `main.rs`, `transform.rs`, `writer.rs` differ only where
|
||||
2016 adds a branch; `format_detect_2016.rs` (549 lines) exists only in 2016.
|
||||
The 2016 crate is therefore the base to keep — it already builds 2017 datasets
|
||||
(its `configs/` holds 2017 fixtures used by `tests/golden.rs`).
|
||||
|
||||
Current per-dataset schema divergence:
|
||||
|
||||
| Column group | 2016 | 2017 / old / old2 |
|
||||
|---|---|---|
|
||||
| `ten_cum_thi`, `gioi_tinh` | present | absent |
|
||||
| `khtn`, `khxh`, `gdcd`, `tieng_nga` | absent | present |
|
||||
| `tieng_duc`, `tieng_nhat` | present | absent |
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional**
|
||||
- One DDL, one INSERT, one ordered subject list, one regex map — used by all four datasets
|
||||
- Configs keep only genuinely per-dataset settings
|
||||
- Rebuilt DBs preserve every value currently produced
|
||||
|
||||
**Non-functional**
|
||||
- No measurable build-time regression (currently a few minutes per dataset)
|
||||
- DB size growth from the added NULL columns stays under ~2%
|
||||
|
||||
## Architecture
|
||||
|
||||
**New `parser/src/schema.rs`** (written into `2016/tools/xlsxread/src/` this
|
||||
phase; relocated in Phase 2) exports:
|
||||
|
||||
- `pub const DDL: &str` — the 22-column CREATE TABLE + 3 indexes
|
||||
- `pub const INSERT_SQL: &str` — 22 positional placeholders
|
||||
- `pub const IDENTITY_FIELDS: &[&str]` — 6 identity columns in INSERT order
|
||||
- `pub const SCORE_FIELDS: &[&str]` — 16 subject columns in INSERT order
|
||||
- `pub fn score_patterns() -> Vec<(&'static str, &'static str)>` — the union of
|
||||
all 16 subject regexes (2016's 12 ∪ 2017's 14; both sets share 10)
|
||||
|
||||
**`writer.rs` collapses.** `SCORE_FIELDS_2016`, `SCORE_FIELDS_2017`, the
|
||||
`SCORE_FIELDS` alias, and `insert_row_2016` all disappear. A single `insert_row`
|
||||
binds identity fields then subject fields, always 22 params.
|
||||
|
||||
**`config.rs` shrinks.** `[schema]` and `[insert]` sections are removed from
|
||||
`DatasetConfig`; `[scores]` becomes optional and unused (delete the field once
|
||||
no config sets it). `columns` stays `Option<Columns>` — the 2016 detection path
|
||||
does not use it.
|
||||
|
||||
**`main.rs`** keeps the `run_build_2016` / `run_build_standard` split; both now
|
||||
call the same `insert_row` with `schema::SCORE_FIELDS`.
|
||||
|
||||
### Union regex risk — RESOLVED, and it found a real bug
|
||||
|
||||
Applying all 16 patterns to every dataset means a source cell containing a
|
||||
subject the old config never looked for now populates a column it previously
|
||||
could not.
|
||||
|
||||
**Outcome: the gate fired, and the matches were real data, not false positives.**
|
||||
|
||||
The pre-refactor configs listed 12 subject regexes (2016) and 14 (2017). Neither
|
||||
list was complete — Vietnamese candidates could sit German, Japanese and Russian
|
||||
in *both* exam years. Every affected student previously ended up with **no**
|
||||
foreign-language score at all. Unifying to 16 patterns recovered 1,691 scores:
|
||||
|
||||
| Dataset | Recovered |
|
||||
|---|---|
|
||||
| 2016 | 182 × `tieng_nga` |
|
||||
| 2017 | 93 × `tieng_duc`, 512 × `tieng_nhat` |
|
||||
| 2017-old | 85 × `tieng_duc`, 484 × `tieng_nhat` |
|
||||
| 2017-old2 | 22 × `tieng_duc`, 313 × `tieng_nhat` |
|
||||
|
||||
Verified genuine, not spurious:
|
||||
- Across all four datasets every student holds **zero or exactly one** foreign
|
||||
language — never two. So the new columns duplicate nothing.
|
||||
- Each affected student had *all* language columns NULL beforehand
|
||||
(e.g. SBD `01003198`: all NULL → `tieng_duc = 8`).
|
||||
- Counts track dataset size consistently across the three 2017 generations
|
||||
(93/85/22 German, 512/484/313 Japanese).
|
||||
- Score ranges and distributions are ordinary exam values (0–10).
|
||||
|
||||
Row counts, all 18 pre-existing per-column non-NULL counts, and the
|
||||
deterministic student samples were **identical** — nothing was lost.
|
||||
|
||||
Accepted as a data-quality fix. The earlier guidance to add a per-config subject
|
||||
allowlist assumed spurious matches and does **not** apply; suppressing these
|
||||
would knowingly discard real scores. The exact counts are now encoded as
|
||||
`APPROVED_RECOVERY` in `verify-parity.js`, so any *other* newly-populated column
|
||||
— or any drift in these numbers — still fails the gate.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Create: `2016/tools/xlsxread/src/schema.rs`
|
||||
- Modify: `2016/tools/xlsxread/src/writer.rs` (drop dual insert paths + field lists)
|
||||
- Modify: `2016/tools/xlsxread/src/config.rs` (drop `[schema]`/`[insert]`/`[scores]`)
|
||||
- Modify: `2016/tools/xlsxread/src/main.rs` (single insert call site per path)
|
||||
- Modify: `2016/tools/xlsxread/src/lib.rs` (declare `schema` module)
|
||||
- Modify: `2016/tools/xlsxread/src/transform.rs` (source patterns from `schema`)
|
||||
- Modify: `2016/tools/xlsxread/tests/golden.rs` (assert against canonical schema)
|
||||
- Create: `2016/tools/xlsxread/configs/thptqg2017-data-old2.toml` (copy from 2017 crate)
|
||||
- Modify: all 4 configs — strip DDL/INSERT/scores down to parse rules
|
||||
- Delete (Phase 2): `2017/tools/`
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. **Capture the baseline first.** Build all four DBs with the *current* code and
|
||||
record, per dataset: `SELECT COUNT(*) FROM student`, and for each column
|
||||
`SUM(col IS NOT NULL)`. Store as `plans/reports/parser-parity-baseline.json`.
|
||||
Nothing else in this plan is verifiable without this artifact.
|
||||
2. Copy `thptqg2017-data-old2.toml` into the 2016 crate's `configs/`.
|
||||
3. Write `schema.rs` with DDL, INSERT, field orders, and the union regex map.
|
||||
4. Rewrite `writer.rs` to a single `insert_row`; delete `insert_row_2016` and
|
||||
the three score-field constants.
|
||||
5. Strip `[schema]`, `[insert]`, `[scores]` from all four configs; update
|
||||
`config.rs` structs to match. Each config should end at ~15–20 lines.
|
||||
6. Update `main.rs` and `transform.rs` call sites.
|
||||
7. Update `tests/golden.rs` to assert the canonical column set.
|
||||
8. `cargo test` — golden tests must pass.
|
||||
9. Rebuild all four DBs; regenerate the same stats and diff against the baseline.
|
||||
|
||||
## Tests / Validation
|
||||
|
||||
- `cargo test` — 63 tests (55 unit + 8 golden)
|
||||
- `cargo clippy --all-targets -- -D warnings`
|
||||
- Row count per dataset equals baseline exactly
|
||||
- Per-column non-NULL count equals baseline for every previously-existing column
|
||||
- Newly-added columns read 0 non-NULL, except the approved recoveries above
|
||||
- DB file size within 2% of baseline
|
||||
|
||||
**Note on the clippy gate:** it was already red before this phase — measured at
|
||||
**8 errors on the branch base**. This phase's changes introduced none. The
|
||||
pre-existing lints were cleared in a separate commit so the gate is genuinely
|
||||
green from here on.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] `schema.rs` is the only place DDL/INSERT/subject-order/regexes are written
|
||||
- [x] All 4 configs under 35 lines, containing no SQL
|
||||
- [x] `writer.rs` has exactly one insert function
|
||||
- [x] `cargo test` (63 passing) and `cargo clippy --all-targets -D warnings` green
|
||||
- [x] All 4 DBs match baseline row counts and per-column non-NULL counts
|
||||
- [x] Baseline JSON committed under `plans/reports/`
|
||||
- [x] Config parsing rejects leftover SQL sections (`deny_unknown_fields`)
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| Union regexes populate unexpected columns | Baseline diff catches it; fall back to per-config subject allowlist |
|
||||
| INSERT param order drifts from DDL order | Single `IDENTITY_FIELDS`/`SCORE_FIELDS` source drives both; golden test asserts round-trip |
|
||||
| Rebuilding 419 MB of source is slow | Run once per verification pass, not per edit; iterate against golden fixtures |
|
||||
| Baseline skipped under time pressure | Phase 1 is unverifiable without it — treat step 1 as blocking |
|
||||
@@ -1,149 +0,0 @@
|
||||
---
|
||||
phase: 2
|
||||
title: "Repo restructure"
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies: [1]
|
||||
effort: ""
|
||||
---
|
||||
|
||||
# Phase 2: Repo restructure
|
||||
|
||||
## Overview
|
||||
|
||||
Move files into the target layout and migrate pnpm → npm. Relocation plus
|
||||
package-manager swap — no application logic changes. Kept as its own commit so
|
||||
the 419 MB data move is trivially revertable and reviewable separately from
|
||||
behavior changes.
|
||||
|
||||
## Requirements
|
||||
|
||||
- All moves via `git mv` so blobs are reused and history follows
|
||||
- Working tree builds after the move (paths updated, nothing dangling)
|
||||
- Repository size does not grow
|
||||
- npm is the only package manager; `package-lock.json` committed, no pnpm files remain
|
||||
|
||||
## Architecture
|
||||
|
||||
Git stores blobs content-addressed, so renaming 299 tracked data files
|
||||
(~419 MB working-tree) adds no new objects. The cost is local I/O and one large
|
||||
tree rewrite, not repository growth.
|
||||
|
||||
Data directory sizes being moved: `2016/data` 62 MB, `2017/data` 286 MB,
|
||||
`2017/data-old` 39 MB, `2017/data-old2` 32 MB.
|
||||
|
||||
### pnpm → npm
|
||||
|
||||
A single package with 3 runtime and 9 dev dependencies, no workspace. pnpm's
|
||||
advantages (strict resolution, shared store) buy nothing at this size, and two
|
||||
pieces of scaffolding disappear with it:
|
||||
|
||||
- `pnpm-workspace.yaml` exists **only** to whitelist `better-sqlite3`'s native
|
||||
build (`allowBuilds`). npm runs postinstall by default, so the file has no npm
|
||||
equivalent — it is deleted, not translated. Verified: this is the file's
|
||||
entire content in both projects.
|
||||
- The `pnpm/action-setup@v4` CI step and `cache: 'pnpm'` both drop out.
|
||||
|
||||
Lockfiles cannot be converted; `package-lock.json` is generated fresh from
|
||||
`package.json`. Migration direction is safe: pnpm's strict `node_modules` layout
|
||||
forbids phantom dependencies, so anything that resolved under pnpm also resolves
|
||||
under npm's flat tree. The reverse would not hold.
|
||||
|
||||
Latent issue surfaced while checking: `2017/scripts/diff-datasets.js` imports
|
||||
`better-sqlite3`, which is **not declared in any `package.json`** — that script
|
||||
cannot run today without an ad-hoc install. Do not add the dependency; Phase 5
|
||||
uses the built-in `node:sqlite` instead (verified working on Node 24, no flag).
|
||||
Either port `diff-datasets.js` to `node:sqlite` in this phase or leave it broken
|
||||
exactly as it is today and note it — do not silently half-fix it.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Moves**
|
||||
|
||||
| From | To |
|
||||
|---|---|
|
||||
| `2016/tools/xlsxread/` | `parser/` |
|
||||
| `2016/tools/xlsxread/configs/thptqg2016-data.toml` | `parser/configs/2016.toml` |
|
||||
| `2017/tools/xlsxread/configs/thptqg2017-data.toml` | `parser/configs/2017.toml` |
|
||||
| `2017/tools/xlsxread/configs/thptqg2017-data-old.toml` | `parser/configs/2017-old.toml` |
|
||||
| `2017/tools/xlsxread/configs/thptqg2017-data-old2.toml` | `parser/configs/2017-old2.toml` |
|
||||
| `2017/scripts/` | `parser/scripts/` |
|
||||
| `2016/data/` | `data/2016/` |
|
||||
| `2017/data/` | `data/2017/` |
|
||||
| `2017/data-old/` | `data/2017-old/` |
|
||||
| `2017/data-old2/` | `data/2017-old2/` |
|
||||
| `2017/src/` | `src/` |
|
||||
| `2017/index.html` | `index.html` (overwrites the old static landing page) |
|
||||
| `2017/package.json`, `eslint.config.js`, `vite.config.js` | repo root |
|
||||
| `2016/docs/*`, `2017/docs/*` | `docs/` |
|
||||
| `2017/LICENSE` | `LICENSE` |
|
||||
|
||||
The old root `index.html` (the static hub) is **not moved** — it becomes
|
||||
`src/components/hub.jsx` in Phase 3. Read it before deleting; its four links,
|
||||
candidate counts, and Vietnamese copy are the source material for that
|
||||
component.
|
||||
|
||||
**Deletes**
|
||||
- `2016/` and `2017/` directories entirely (after moves)
|
||||
- `2016/src/` — superseded by `src/` (its unique columns and SQL presets are ported in Phase 3)
|
||||
- `2016/tools/xlsxread/configs/thptqg2017-*.toml` — test fixtures, superseded by real configs
|
||||
- `2016/pnpm-lock.yaml`, `2017/pnpm-lock.yaml`, both `pnpm-workspace.yaml`
|
||||
- Duplicate `2016/package.json`, `2016/eslint.config.js`, `2016/vite.config.js`, `2016/LICENSE`
|
||||
- Root `index.html` (static hub) — only after its content is captured for `hub.jsx`
|
||||
|
||||
**Merges**
|
||||
- `.gitignore` — one root file. 2016's is the verbose GitHub Node template,
|
||||
2017's is terse and accurate. Take 2017's as the base, add `parser/target/`,
|
||||
`.build/`, and `dist/`.
|
||||
- `package.json` — root file keeps 2017's dependency set (identical to 2016's).
|
||||
Drop the `"packageManager": "pnpm@11.1.1"` field.
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Branch: `refactor/unify-frontend-and-schema`.
|
||||
2. Copy the old root `index.html` content somewhere durable for Phase 3
|
||||
(`plans/reports/` or the phase-03 file itself) — it is the hub's source copy.
|
||||
3. `git mv 2017/src src`, then the 2017 root config files, then
|
||||
`git mv 2017/index.html index.html`.
|
||||
4. `git mv 2016/tools/xlsxread parser`, then rename the four configs.
|
||||
5. `git mv 2017/scripts parser/scripts`.
|
||||
6. Move the four data directories.
|
||||
7. Merge docs into `docs/`, prefixing 2016-specific filenames where they collide
|
||||
(`deployment-guide.md` and `system-architecture.md` exist in both — read both
|
||||
before merging; they describe different pipelines).
|
||||
8. Delete the emptied `2016/` and `2017/` trees plus both `pnpm-workspace.yaml`
|
||||
and both `pnpm-lock.yaml`.
|
||||
9. Write the merged root `.gitignore`.
|
||||
10. Drop `"packageManager"` from `package.json`; run `npm install` to generate
|
||||
`package-lock.json`; commit the lockfile.
|
||||
11. Fix paths inside moved files: `parser/Cargo.toml` package name/paths,
|
||||
config `--input`/`--output` defaults, `parser/scripts/*.js` relative paths.
|
||||
12. Replace `pnpm` with `npm run` in every `package.json` script body.
|
||||
13. `cargo test` from `parser/` — must still be green.
|
||||
|
||||
## Tests / Validation
|
||||
|
||||
- `cargo test --manifest-path parser/Cargo.toml`
|
||||
- `npm ci && npm run lint` at root — `npm ci` proves the lockfile is in sync
|
||||
- `git status` shows renames (R), not delete+add pairs
|
||||
- No file outside `plans/` still references `2016/tools`, `2017/tools`,
|
||||
`2017/src`, `2017/data`, or `pnpm` — grep to confirm
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] Target layout matches `plan.md` exactly
|
||||
- [x] `2016/` and `2017/` no longer exist
|
||||
- [x] Old hub page content captured before deletion
|
||||
- [x] Git reports renames, repository size unchanged
|
||||
- [x] `package-lock.json` committed; `npm ci` succeeds; zero pnpm files remain
|
||||
- [x] `cargo test` green from the new location
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| Old hub page deleted before its copy is captured | Step 2 runs before any move; content also recoverable from git history |
|
||||
| Docs collide silently on merge | Read both copies before merging; two same-named files describe different pipelines |
|
||||
| Git records delete+add instead of rename | Use `git mv`; verify with `git status` before commit |
|
||||
| Stale path references in scripts/CI | Grep sweep in validation; Phase 4 rewrites the workflow |
|
||||
| Fresh npm lockfile resolves different transitive versions than pnpm did | All deps are caret-ranged and already floating; `npm run lint` + the Phase 4 local build run are the check |
|
||||
-249
@@ -1,249 +0,0 @@
|
||||
---
|
||||
phase: 3
|
||||
title: "Unified frontend and dataset registry"
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies: [2]
|
||||
effort: ""
|
||||
---
|
||||
|
||||
# Phase 3: Unified frontend and dataset registry
|
||||
|
||||
## Overview
|
||||
|
||||
Make the single 2017-derived frontend serve every page on the site: the four
|
||||
dataset views plus the `/thptqg/` hub. Three kinds of work — port 2016's only
|
||||
unique feature (cluster + gender columns) into the shared components, extract
|
||||
everything genuinely per-dataset into a runtime registry, and add a small
|
||||
pathname router with a hub route.
|
||||
|
||||
## Context — what actually differs
|
||||
|
||||
The 2017 frontend is a superset in every respect except two columns. Verified by
|
||||
reading both trees:
|
||||
|
||||
**2017 has, 2016 lacks:** debounced live search with input-mode hints
|
||||
(`search-form.jsx`, 127 vs 46 lines), `?q=` URL deep links, `student-detail.jsx`
|
||||
(183 lines, single-result view), `lib/admission-blocks.js` (49 blocks +
|
||||
`scoreTier` ladder), all-NULL column hiding, `/` and Ctrl+Enter shortcuts,
|
||||
footer total count, load progress bar, richer SQL presets (335 vs 219 lines).
|
||||
|
||||
**2016 has, 2017 lacks:** `ten_cum_thi` and `gioi_tinh` table columns with a
|
||||
`cumthi-cell` title tooltip, and a simpler 3-tier `scoreClass` (superseded by
|
||||
2017's 6-tier `scoreTier`).
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional**
|
||||
- 2016 site keeps cluster + gender columns; 2017 sites must not show them
|
||||
- 2016 site gains every 2017 feature listed above
|
||||
- Per-dataset chrome (title, subtitle, source, DB size, search examples, SQL presets) is data, not code
|
||||
- Admission-block computation works for both exam years without branching
|
||||
- `/thptqg/` renders a hub listing the four datasets; no DB is fetched there
|
||||
- Deep links (`?q=`) keep working on every dataset route
|
||||
|
||||
**Non-functional**
|
||||
- Dataset identity resolves from `location.pathname` — no fetch, no env plumbing
|
||||
- No routing library; five static routes do not justify a dependency
|
||||
- Hub route must not pull the 47 MB DB into its critical path
|
||||
|
||||
## Architecture
|
||||
|
||||
### `src/datasets.js` — the registry
|
||||
|
||||
```js
|
||||
export const DATASETS = [
|
||||
{
|
||||
id: "2016", // === URL segment === data dir === config === db file
|
||||
dbSizeMb: 48,
|
||||
title: "Tra cứu điểm thi THPT Quốc gia 2016",
|
||||
subtitle: "Dữ liệu thí sinh toàn quốc · Hỗ trợ truy vấn SQL tùy chỉnh",
|
||||
source: "Bộ GD&ĐT",
|
||||
examples: ["<real 2016 SBD>", "Nguyễn Minh Tiến"],
|
||||
presets: PRESETS_2016, // from 2016/src/components/custom-query.jsx
|
||||
blocks: BLOCKS_2016, // see admission blocks below
|
||||
},
|
||||
{ id: "2017", /* presets: PRESETS_2017 */ },
|
||||
{ id: "2017-old", /* same presets as 2017, own label */ },
|
||||
{ id: "2017-old2", /* same presets as 2017, own label */ },
|
||||
];
|
||||
|
||||
export const pathOf = (d) => `${import.meta.env.BASE_URL}${d.id}/`;
|
||||
export const dbOf = (d) => `${import.meta.env.BASE_URL}db/${d.id}.db.gz`;
|
||||
```
|
||||
|
||||
Plain runtime data — no `import.meta.env.VITE_DATASET` inlining, no build
|
||||
variants. All four entries ship in the one bundle; the preset SQL totals a few
|
||||
KB gzipped. Because URLs are flat, path and DB URL are *derived* from `id`
|
||||
rather than stored, so a dataset cannot be misconfigured into pointing at the
|
||||
wrong database.
|
||||
|
||||
### `src/router.js`
|
||||
|
||||
Flat URLs make this an exact match on a single segment — the segment **is** the
|
||||
dataset ID:
|
||||
|
||||
```js
|
||||
export function resolveRoute(pathname = location.pathname) {
|
||||
const seg = pathname
|
||||
.slice(import.meta.env.BASE_URL.length) // strip "/thptqg/"
|
||||
.replace(/\/$/, "");
|
||||
return DATASETS.find((d) => d.id === seg) ?? null; // null → hub
|
||||
}
|
||||
```
|
||||
|
||||
No prefix sorting, no ambiguity between `2017` and `2017-old` — that entire
|
||||
class of bug is designed out by the flat scheme. No `react-router`.
|
||||
|
||||
### Legacy path redirects
|
||||
|
||||
The two old nested URLs are kept alive by a small map, so existing links and
|
||||
bookmarks resolve instead of 404ing:
|
||||
|
||||
```js
|
||||
const LEGACY = { "2017/old": "2017-old", "2017/old2": "2017-old2" };
|
||||
```
|
||||
|
||||
On a legacy match, `history.replaceState` to the canonical flat URL **preserving
|
||||
`location.search`** (the `?q=` deep link must survive), then resolve normally.
|
||||
Phase 4 emits `index.html` at both legacy paths so Pages serves them at all.
|
||||
|
||||
Droppable if you'd rather let the old URLs 404 — it costs ~5 lines and two file
|
||||
copies, and nothing else in the plan depends on it.
|
||||
|
||||
### `src/components/hub.jsx`
|
||||
|
||||
Renders the four dataset links from `DATASETS` via `pathOf()`. Content ported
|
||||
from the old static root `index.html` (captured in Phase 2 step 2): heading, the
|
||||
sql.js one-liner explanation, candidate counts, and the "phiên bản cũ" grouping
|
||||
of the two 2017 variants. Styled with the app's existing CSS instead of the old
|
||||
inline `<style>` block.
|
||||
|
||||
Note the links now point at `/thptqg/2017-old/`, not `/thptqg/2017/old/`.
|
||||
|
||||
`App.jsx` becomes: resolve route → hub, or dataset view. `useSqlite` is only
|
||||
mounted on a dataset route, so the hub never touches the DB.
|
||||
|
||||
### Accepted trade-off
|
||||
|
||||
The hub was static HTML that painted instantly and worked without JS; it now
|
||||
waits on the bundle (~150 KB gzipped). Accepted for the pipeline simplification.
|
||||
If it ever matters, the four links can be inlined as static markup in
|
||||
`index.html` so they paint pre-hydration.
|
||||
|
||||
### Column visibility — no new mechanism needed
|
||||
|
||||
`score-table.jsx` already computes `visibleColumns` by dropping columns where
|
||||
every row is NULL. Extending that same filter to `ten_cum_thi` and `gioi_tinh`
|
||||
makes them appear on 2016 and vanish on 2017 automatically, with no dataset
|
||||
conditional in the component. This is the cheapest correct approach and it is
|
||||
already the file's established pattern.
|
||||
|
||||
Identity columns need their own render path (they are text, not scored cells),
|
||||
so add an `IDENTITY_COLUMNS` list alongside `SUBJECT_COLUMNS` and apply the same
|
||||
all-NULL filter to both.
|
||||
|
||||
### Admission blocks — one union list
|
||||
|
||||
`computeBlocks()` already skips any block where a subject score is missing.
|
||||
So a single list covering both years needs no branching: GDCD/KHTN blocks
|
||||
self-exclude on 2016 rows, and the 2016-only foreign-language blocks
|
||||
self-exclude on 2017 rows.
|
||||
|
||||
Add to `ADMISSION_BLOCKS`: `D05` (Toán+Văn+Đức), `D06` (Toán+Văn+Nhật), and the
|
||||
other Đức/Nhật combinations from Circular 03/2017 that the current list drops
|
||||
with the comment "neither language appears in any source file" — that comment
|
||||
becomes false once 2016 data uses the same schema, so update it.
|
||||
|
||||
Verify the 2016 block list against the 2016 admission regulation rather than
|
||||
assuming the 2017 circular's codes applied unchanged that year. If they differ
|
||||
materially, key the block list by exam year via the registry's `blocks` field;
|
||||
if they do not, drop that field and keep the single union list.
|
||||
|
||||
### Subject labels
|
||||
|
||||
`student-detail.jsx`'s `SUBJECT_LABELS`/`SUBJECT_ORDER` and `score-table.jsx`'s
|
||||
`SUBJECT_COLUMNS` are two hand-maintained copies of the same subject list. Merge
|
||||
into one `src/lib/subjects.js` exporting the 16-subject ordered list with labels;
|
||||
both components consume it. This mirrors what Phase 1 does on the Rust side.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Create: `src/datasets.js`
|
||||
- Create: `src/router.js`
|
||||
- Create: `src/components/hub.jsx`
|
||||
- Create: `src/lib/subjects.js`
|
||||
- Modify: `src/App.jsx` — resolve route; hub or dataset view; chrome from the registry entry
|
||||
- Modify: `src/components/score-table.jsx` — add identity columns, use `subjects.js`
|
||||
- Modify: `src/components/student-detail.jsx` — use `subjects.js`, show cluster/gender when present
|
||||
- Modify: `src/components/search-form.jsx` — `EXAMPLES` from the active dataset
|
||||
- Modify: `src/components/custom-query.jsx` — `PRESET_GROUPS` from the active dataset
|
||||
- Modify: `src/lib/admission-blocks.js` — add Đức/Nhật blocks, update stale comment
|
||||
- Modify: `src/App.css` — port `.cumthi-cell` from `2016/src/App.css`; add hub styles
|
||||
- Reference (deleted in Phase 2): old root `index.html` for hub content,
|
||||
`2016/src/components/custom-query.jsx` for the 2016 preset SQL
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Extract `src/lib/subjects.js`; repoint both components at it.
|
||||
2. Add `IDENTITY_COLUMNS` + all-NULL filtering to `score-table.jsx`; port the
|
||||
`.cumthi-cell` style.
|
||||
3. Write `src/datasets.js` with all four entries. Lift the 2016 preset SQL
|
||||
verbatim from the old 2016 `custom-query.jsx` (cluster averages, gender
|
||||
breakdown, cluster+gender listing, language-count summary).
|
||||
4. Note: the 2017 presets contain a hardcoded `so_bao_danh LIKE '49%'` Long An
|
||||
query. Keep it only in the 2017 entries; write a 2016 equivalent or drop it.
|
||||
5. Write `src/router.js`: exact segment match, plus the legacy redirect map.
|
||||
6. Write `src/components/hub.jsx` from the captured static hub content.
|
||||
7. Restructure `App.jsx`: resolve route → hub or dataset view; mount `useSqlite`
|
||||
only on dataset routes; read chrome from the resolved entry.
|
||||
8. Repoint `search-form.jsx` and `custom-query.jsx` at the active dataset.
|
||||
9. Extend `ADMISSION_BLOCKS` with the Đức/Nhật blocks; verify against the 2016
|
||||
regulation before committing the list.
|
||||
10. Show cluster/gender in `student-detail.jsx` when non-null.
|
||||
11. `npm run lint`.
|
||||
|
||||
## Tests / Validation
|
||||
|
||||
Manual, against a local preview of the single build (no test harness exists in
|
||||
this repo today):
|
||||
|
||||
- `/thptqg/` renders the hub; network tab shows **no** `.db.gz` request
|
||||
- Each of the four dataset routes loads its own DB — confirm `2017-old` fetches
|
||||
`db/2017-old.db.gz`, not `db/2017.db.gz`
|
||||
- `/thptqg/2017/old/?q=Nguyen` redirects to `/thptqg/2017-old/?q=Nguyen` with the
|
||||
query intact, and lands on the right dataset
|
||||
- Hub links point at flat URLs
|
||||
- 2016 route: cluster + gender columns render; KHTN/KHXH/GDCD/Nga columns absent
|
||||
- 2017 routes: no cluster/gender columns; KHTN/KHXH/GDCD present
|
||||
- Single-result search opens `student-detail` on all four
|
||||
- `?q=` deep link hydrates search on all four
|
||||
- SQL tab presets execute without error on their own dataset
|
||||
- Admission blocks: a 2016 student shows D05/D06 where applicable and no GDCD
|
||||
blocks; a 2017 student shows GDCD blocks and no Đức/Nhật blocks
|
||||
- `npm run lint` clean
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] One `src/` serving four datasets **and** the hub, zero `if (dataset === ...)` in components
|
||||
- [x] Subject list defined once in `src/lib/subjects.js`
|
||||
- [x] Hub route fetches no database
|
||||
- [x] Dataset path and DB URL are derived from `id`, not stored per entry
|
||||
- [x] Legacy `/2017/old/` and `/2017/old2/` redirect to flat URLs, `?q=` preserved
|
||||
- [x] 2016 route renders cluster + gender; 2017 routes do not
|
||||
- [x] 2016 route has live search, deep links, student detail, score tiers
|
||||
- [x] Admission blocks correct for both exam years
|
||||
- [x] No routing library added
|
||||
- [x] `npm run lint` green
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| 2016 SQL presets lost when `2016/src` is deleted | Step 3 lifts them verbatim; recoverable from git history if ordering slips |
|
||||
| 2017 block list assumed valid for 2016 | Step 9 requires checking the 2016 regulation; registry can key blocks per year if they differ |
|
||||
| All-NULL filter hides a column on a legitimately sparse result set | Filter is per result set, matching today's 2017 behavior — accepted existing trade-off, not a new one |
|
||||
| Hardcoded Long An preset leaks into 2016 | Explicit step 4 |
|
||||
| Router sends `/2017-old/` to the `2017` dataset | Designed out: exact segment match on a flat scheme, no prefix logic exists |
|
||||
| Existing links to nested URLs break | Legacy redirect map + Phase 4 stub pages; `?q=` preserved through the rewrite |
|
||||
| Hub paints slower than the old static HTML | Accepted and documented; inline-links fallback available if it bites |
|
||||
-210
@@ -1,210 +0,0 @@
|
||||
---
|
||||
phase: 4
|
||||
title: "Build and deploy pipeline"
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies: [3]
|
||||
effort: ""
|
||||
---
|
||||
|
||||
# Phase 4: Build and deploy pipeline
|
||||
|
||||
## Overview
|
||||
|
||||
Rewire Vite, npm scripts, and GitHub Actions to produce the whole site from
|
||||
**one** frontend build and **one** parser binary. Because the app now owns
|
||||
routing (Phase 3), the four Vite build variants collapse into a single build
|
||||
plus a copy step.
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional**
|
||||
- One Vite build produces every page: hub + four dataset routes
|
||||
- Flat published paths: `/thptqg/`, `/thptqg/2016/`, `/thptqg/2017/`, `/thptqg/2017-old/`, `/thptqg/2017-old2/`
|
||||
- Legacy `/thptqg/2017/old/` and `/thptqg/2017/old2/` still resolve (redirect stubs)
|
||||
- Deep links work on every route without a 404 fallback
|
||||
- Uncompressed `.db` files never ship — only `.db.gz`
|
||||
|
||||
**Non-functional**
|
||||
- One `cargo build`, one `npm ci`, one `vite build` per CI run
|
||||
- npm only; no pnpm steps or caches remain
|
||||
|
||||
## Architecture
|
||||
|
||||
### Vite
|
||||
|
||||
One config. No variants, no `DATASET` env, no `emptyOutDir` ordering problem —
|
||||
all three of those existed only to work around the missing router.
|
||||
|
||||
```js
|
||||
export default defineConfig({
|
||||
plugins: [react()],
|
||||
base: "/thptqg/",
|
||||
publicDir: ".build/public", // gitignored; holds db/*.db.gz only
|
||||
});
|
||||
```
|
||||
|
||||
### Why the entry-point copies work
|
||||
|
||||
With an absolute `base`, the emitted `index.html` references
|
||||
`/thptqg/assets/index-HASH.js` no matter which directory it is served from. So
|
||||
the same file is a valid entry point at every depth, and GitHub Pages serves
|
||||
each as a directory index:
|
||||
|
||||
```bash
|
||||
for ds in "${DATASETS[@]}"; do
|
||||
mkdir -p "_site/$ds" && cp dist/index.html "_site/$ds/index.html"
|
||||
done
|
||||
```
|
||||
|
||||
With flat URLs the loop iterates the same `DATASETS` array used to build the
|
||||
databases — no path translation between dataset ID and URL path, because they
|
||||
are the same string.
|
||||
|
||||
This is what removes the need for the usual `404.html` SPA-fallback hack — which
|
||||
matters concretely here, because that hack rewrites the URL and would interfere
|
||||
with the existing `?q=` deep-link handling.
|
||||
|
||||
Also emit `dist/index.html` as `_site/404.html` so unknown paths render the hub
|
||||
instead of Pages' default 404.
|
||||
|
||||
### Database staging
|
||||
|
||||
The parser writes into `.build/public/db/`, gzips in place, and the raw `.db` is
|
||||
deleted before Vite copies `publicDir`. Today's pipeline instead ships the
|
||||
uncompressed DB into `dist` and deletes it afterwards (`rm -f dist/*.db`) —
|
||||
staging makes shipping a 47 MB uncompressed file structurally impossible rather
|
||||
than dependent on a cleanup step running.
|
||||
|
||||
### Workflow
|
||||
|
||||
Current workflow compiles the same Rust crate twice and runs two `pnpm install`s.
|
||||
Collapse to one of each, then loop the four datasets:
|
||||
|
||||
```yaml
|
||||
- uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: '24'
|
||||
cache: 'npm'
|
||||
cache-dependency-path: package-lock.json
|
||||
|
||||
- name: Build databases
|
||||
run: |
|
||||
set -euo pipefail
|
||||
cargo build --release --manifest-path parser/Cargo.toml
|
||||
mkdir -p .build/public/db
|
||||
for ds in 2016 2017 2017-old 2017-old2; do
|
||||
./parser/target/release/xlsxread build \
|
||||
--schema "parser/configs/$ds.toml" \
|
||||
--input "data/$ds" \
|
||||
--output ".build/public/db/$ds.db"
|
||||
gzip -9 ".build/public/db/$ds.db" # no -k: raw file must not survive
|
||||
done
|
||||
|
||||
- name: Build site
|
||||
run: |
|
||||
npm ci
|
||||
npm run build
|
||||
|
||||
- name: Assemble
|
||||
run: |
|
||||
set -euo pipefail
|
||||
mkdir -p _site && cp -r dist/* _site/
|
||||
cp dist/index.html _site/404.html
|
||||
# one entry point per dataset — ID and URL segment are the same string
|
||||
for ds in 2016 2017 2017-old 2017-old2; do
|
||||
mkdir -p "_site/$ds" && cp dist/index.html "_site/$ds/index.html"
|
||||
done
|
||||
# legacy nested URLs — router rewrites these to the flat form
|
||||
for legacy in 2017/old 2017/old2; do
|
||||
mkdir -p "_site/$legacy" && cp dist/index.html "_site/$legacy/index.html"
|
||||
done
|
||||
```
|
||||
|
||||
The dataset list appears in both steps. Define it once as a job-level env var
|
||||
(`DATASETS: "2016 2017 2017-old 2017-old2"`) rather than repeating the literal.
|
||||
|
||||
Cache changes: `Swatinem/rust-cache` workspaces → `parser`; `setup-node` cache →
|
||||
`npm` keyed on `package-lock.json`; the `pnpm/action-setup` step is deleted.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Modify: `vite.config.js` — single build, `base: /thptqg/`, `.build/public` publicDir
|
||||
- Modify: `package.json` — the six `build:db*` and three `build:*` variant scripts
|
||||
collapse to `build:db`, `build`, `assemble`, `build:site`
|
||||
- Create: `scripts/assemble-site.js` — copies the entry point to each route and
|
||||
refuses to ship an uncompressed database
|
||||
- Modify: `.github/workflows/deploy-pages.yml` — single toolchain setup, npm
|
||||
caches, new assemble step
|
||||
- Modify: `eslint.config.js` — Node globals for `scripts/**`, ignore `_site`
|
||||
- Modify: `.gitignore` — add `.build/`, `_site/`, `parser/target/`, keep `dist/`
|
||||
|
||||
### Deviation: the dataset loop is a Node script, not workflow shell
|
||||
|
||||
The plan sketched a `for ds in 2016 2017 …` loop inline in the workflow, with a
|
||||
note to hoist the list into a job-level env var. That would still have been a
|
||||
second copy of the dataset list. `parser/scripts/build-db.js` and
|
||||
`scripts/assemble-site.js` both import `DATASET_IDS` from `src/datasets.js`
|
||||
instead, so the four IDs are declared exactly once for the frontend, the
|
||||
database build and the site assembly. It also makes the whole pipeline runnable
|
||||
locally with `npm run build:site`, which is how it was verified.
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Write the single-build `vite.config.js`.
|
||||
2. Collapse `package.json` scripts. The current `build:old`/`build:old2` shell
|
||||
out through `node -e` + `spawnSync` purely to set an env var — both delete
|
||||
outright rather than getting converted.
|
||||
3. Rewrite the workflow: one cargo build, one `npm ci`, one `npm run build`,
|
||||
dataset loop, new assemble step.
|
||||
4. Update the Rust and npm caches; delete the pnpm setup step.
|
||||
5. Run the whole pipeline locally — `cargo` and `npm` are both available — and
|
||||
inspect `_site/` before pushing.
|
||||
6. Serve `_site/` locally and click through all five routes.
|
||||
|
||||
## Tests / Validation
|
||||
|
||||
- Local run produces `_site/index.html`, `_site/404.html`, and
|
||||
`index.html` under `2016/`, `2017/`, `2017-old/`, `2017-old2/`,
|
||||
plus legacy `2017/old/` and `2017/old2/`
|
||||
- `find _site -name '*.db'` returns nothing (only `.db.gz` present)
|
||||
- All emitted `index.html` files are byte-identical
|
||||
- Asset URLs in them are absolute `/thptqg/assets/...`
|
||||
- Serving `_site/` locally: `/thptqg/2017-old/?q=...` loads
|
||||
`db/2017-old.db.gz` and hydrates the query with no redirect
|
||||
- `/thptqg/2017/old/?q=...` rewrites to the flat URL with the query intact
|
||||
- Workflow run on the branch deploys all five flat URLs plus the two legacy paths
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] One `vite.config.js`, no build variants, no `DATASET` env
|
||||
- [x] Workflow compiles Rust once, installs Node deps once, builds the site once
|
||||
- [x] All five flat URLs served; deep links intact
|
||||
- [x] Both legacy nested URLs resolve rather than 404
|
||||
- [x] Dataset list written once (`src/datasets.js`), not repeated in the workflow
|
||||
- [x] No SPA 404-redirect hack in the repo
|
||||
- [x] No uncompressed DB anywhere in the artifact — enforced by the assemble step
|
||||
- [x] Generated DBs live in gitignored `.build/`, not in source directories
|
||||
- [x] Zero pnpm references in the workflow
|
||||
|
||||
## Verification limits
|
||||
|
||||
Route resolution is verified by serving the assembled artifact over HTTP and
|
||||
checking every published URL returns 200 with correct absolute asset
|
||||
references, plus that all entry points are byte-identical.
|
||||
|
||||
The routing **JavaScript** has not been executed. This workspace is headless
|
||||
with no browser available, so hub-vs-dataset rendering and the legacy
|
||||
`/2017/old/` → `/2017-old/` rewrite are verified by construction and by unit
|
||||
tests of the pure functions, not by running in a browser. That check needs a
|
||||
real browser.
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| 47 MB uncompressed DB ships | `gzip -9` without `-k` leaves no raw file; validation greps `_site` for `*.db` |
|
||||
| Relative asset path sneaks in and breaks nested entry points | Absolute `base`; validation asserts `/thptqg/assets/` in all five copies |
|
||||
| Nested route 404s on Pages | Real `index.html` at each path, verified against a local static server before push |
|
||||
| Total artifact size | Unchanged — all four DBs already ship today; no new Pages size exposure |
|
||||
| Stale cache keys silently rebuild everything | Cosmetic; verify first workflow run's timing |
|
||||
-128
@@ -1,128 +0,0 @@
|
||||
---
|
||||
phase: 5
|
||||
title: "Parity verification and docs"
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies: [4]
|
||||
effort: ""
|
||||
---
|
||||
|
||||
# Phase 5: Parity verification and docs
|
||||
|
||||
## Overview
|
||||
|
||||
The release gate. Prove no data was lost or invented by the schema unification,
|
||||
then update the documentation that describes a two-project repo which no longer
|
||||
exists.
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional**
|
||||
- Every dataset's row count and per-column non-NULL count matches the Phase 1 baseline
|
||||
- Newly-added columns read exactly zero on datasets that never had them
|
||||
- All four sites work end-to-end against their rebuilt DBs
|
||||
|
||||
**Non-functional**
|
||||
- The comparison is a committed, re-runnable script, not a one-off shell session
|
||||
|
||||
## Architecture
|
||||
|
||||
### Why counts, not checksums
|
||||
|
||||
The schema changed shape, so the DB files cannot be byte-identical and a whole-file
|
||||
hash is meaningless. The meaningful invariants are:
|
||||
|
||||
1. **Row count** per dataset — unchanged
|
||||
2. **Per-column non-NULL count** for every column that existed before — unchanged
|
||||
3. **Per-column non-NULL count** for every column newly added to a dataset — zero
|
||||
4. **Value-level spot check** — a stable sample of SBDs compared field by field
|
||||
|
||||
Invariant 3 is what catches the union-regex risk flagged in Phase 1: if the
|
||||
16-subject regex map starts matching text in 2016 files that the 12-subject map
|
||||
ignored, `khtn`/`khxh`/`gdcd`/`tieng_nga` will be non-zero on 2016 and the gate
|
||||
fails loudly rather than silently corrupting the dataset.
|
||||
|
||||
### Script
|
||||
|
||||
`parser/scripts/verify-parity.js` — reads
|
||||
`plans/reports/parser-parity-baseline.json`, opens each rebuilt DB, recomputes
|
||||
the same statistics, and exits non-zero on any mismatch with a per-column diff
|
||||
table.
|
||||
|
||||
Use **`node:sqlite`** (`DatabaseSync`), not `better-sqlite3` or `sql.js`.
|
||||
Verified working on this repo's Node 24 with no flag and no dependency:
|
||||
|
||||
```js
|
||||
const { DatabaseSync } = require("node:sqlite");
|
||||
```
|
||||
|
||||
That choice matters beyond convenience — `better-sqlite3` is a native module
|
||||
whose postinstall is exactly what the deleted `pnpm-workspace.yaml` `allowBuilds`
|
||||
entry existed to permit. Using the built-in keeps the dependency count at zero
|
||||
and leaves nothing for a future package-manager change to trip over.
|
||||
|
||||
For invariant 4, sample deterministically — e.g. every SBD ending in `0000` —
|
||||
so reruns compare the same students.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Create: `parser/scripts/verify-parity.js`
|
||||
- Reference: `plans/reports/parser-parity-baseline.json` (from Phase 1)
|
||||
- Create: `plans/reports/parser-parity-result.md` (the gate's output)
|
||||
- Modify: `README.md` — new layout, new commands, four datasets
|
||||
- Modify: `docs/system-architecture.md` — merged, single-project architecture
|
||||
- Modify: `docs/deployment-guide.md` — merged, new workflow
|
||||
- Modify: `docs/data-pipeline.md` — canonical schema, one parser, four configs
|
||||
- Delete: `docs/codebase-summary.md`, `docs/project-overview-pdr.md` if they
|
||||
describe only the old 2016 project — read before deciding
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Write `verify-parity.js`.
|
||||
2. Run against all four rebuilt DBs; capture output to
|
||||
`plans/reports/parser-parity-result.md`.
|
||||
3. Investigate any mismatch before touching docs. A failure here means Phase 1's
|
||||
schema or regex unification is wrong — fix it there, do not adjust the gate.
|
||||
4. Manual pass on all four preview builds against the checklist in Phase 3.
|
||||
5. Update `README.md`: layout tree, npm commands (`npm ci`, `npm run build`),
|
||||
parser invocation, the four dataset descriptions and their flat URLs. Call
|
||||
out the URL change from `/2017/old/` to `/2017-old/` and that the old paths
|
||||
redirect.
|
||||
6. Merge the duplicated docs. `deployment-guide.md` and `system-architecture.md`
|
||||
exist in both old projects and describe different pipelines — read both
|
||||
copies fully before writing the merged version.
|
||||
7. Rewrite `docs/data-pipeline.md` around the canonical schema: the 22 columns,
|
||||
which datasets populate which, and how `format_detection` selects the 2016
|
||||
column-layout path.
|
||||
8. Document the canonical schema itself in one place, referenced from the others.
|
||||
|
||||
## Tests / Validation
|
||||
|
||||
- `node parser/scripts/verify-parity.js` exits 0
|
||||
- Row counts: 2016 ≈ 877,461 (per the current landing page), 2017 ≈ 861,000 —
|
||||
confirm against the baseline, not against these approximations
|
||||
- 2016 DB: `khtn`, `khxh`, `gdcd`, `tieng_nga` all read 0 non-NULL
|
||||
- 2017 DBs: `ten_cum_thi`, `gioi_tinh`, `tieng_duc`, `tieng_nhat` all read 0 non-NULL
|
||||
- All four dataset routes: search by SBD, search by name, deep link, SQL preset,
|
||||
student detail; plus the hub route linking to all four
|
||||
- No doc references `2016/tools`, `2017/src`, `public-old/`, `build:all`, `pnpm`,
|
||||
`landing/`, or `VITE_DATASET`
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] `verify-parity.js` committed and exiting 0 on all four datasets
|
||||
- [x] Parity result report committed under `plans/reports/`
|
||||
- [x] Zero unexpected non-NULL columns
|
||||
- [x] Manual checklist passes on the hub and all four dataset routes
|
||||
- [x] `verify-parity.js` has zero npm dependencies (`node:sqlite` only)
|
||||
- [x] `README.md` and `docs/` describe the actual repo, with npm commands
|
||||
- [x] No stale path, pnpm, or build-variant references anywhere outside `plans/`
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| Parity failure discovered late, after the data move | Phase 1 runs the same comparison in place before Phase 2 moves anything |
|
||||
| Gate weakened to make it pass | Explicit step 3: a mismatch is a Phase 1 bug, not a gate-tuning problem |
|
||||
| Docs merged by picking one copy and discarding the other | Step 6 requires reading both; they document different pipelines |
|
||||
| Baseline missing because Phase 1 step 1 was skipped | Phase 1 treats it as blocking; without it this phase cannot run |
|
||||
@@ -1,184 +0,0 @@
|
||||
---
|
||||
title: "Unify frontend, standardize SQL schema, restructure repo"
|
||||
description: "One 2017-based frontend, one canonical 22-column student schema, one parser crate, four datasets under data/"
|
||||
status: completed
|
||||
priority: P2
|
||||
branch: "main"
|
||||
tags: [refactor, schema, frontend, parser]
|
||||
blockedBy: []
|
||||
blocks: []
|
||||
created: "2026-08-13T03:01:04.073Z"
|
||||
createdBy: "ck:plan"
|
||||
source: skill
|
||||
---
|
||||
|
||||
# Unify frontend, standardize SQL schema, restructure repo
|
||||
|
||||
## Overview
|
||||
|
||||
Today the repo holds two near-duplicate projects (`2016/`, `2017/`), each with its
|
||||
own React frontend, its own copy of the same Rust parser crate, and its own SQL
|
||||
schema. The 2017 frontend is a strict feature superset; the 2016 parser is a
|
||||
strict code superset. Both duplications are drift hazards, not real divergence.
|
||||
|
||||
Collapse to one of each:
|
||||
|
||||
- **One canonical schema** — 22-column `student` table (6 identity + 16 subject),
|
||||
defined once in `parser/src/schema.rs`. Absent columns bind NULL.
|
||||
- **One parser crate** — `parser/`, four config files carrying only per-dataset
|
||||
parse rules.
|
||||
- **One frontend** — repo root, built from `2017/src/`, plus the two identity
|
||||
columns 2016 renders today. The app owns every page GitHub Pages serves,
|
||||
including the `/thptqg/` hub, from a **single Vite build**.
|
||||
- **Four datasets** — `data/2016`, `data/2017`, `data/2017-old`, `data/2017-old2`.
|
||||
|
||||
Published URLs are **flat**, one segment per dataset:
|
||||
|
||||
```text
|
||||
/thptqg/ hub
|
||||
/thptqg/2016/
|
||||
/thptqg/2017/
|
||||
/thptqg/2017-old/ was /thptqg/2017/old/
|
||||
/thptqg/2017-old2/ was /thptqg/2017/old2/
|
||||
```
|
||||
|
||||
The two old-generation URLs change. That is deliberate: the flat form makes the
|
||||
URL segment **identical to the dataset ID**, which is already the name of the
|
||||
data directory, the config file, and the DB file. One identifier end to end:
|
||||
|
||||
```text
|
||||
data/2017-old/ → parser/configs/2017-old.toml → db/2017-old.db.gz → /thptqg/2017-old/
|
||||
```
|
||||
|
||||
Phase 3 keeps the two legacy paths working via redirect so existing links do not
|
||||
break (see that phase; droppable if you don't care).
|
||||
|
||||
Package manager: **npm**. pnpm is dropped (see Phase 2).
|
||||
|
||||
## Target Layout
|
||||
|
||||
```text
|
||||
/
|
||||
├── index.html Vite entry — serves every route
|
||||
├── vite.config.js ONE build, base: /thptqg/
|
||||
├── package.json npm; package-lock.json committed
|
||||
├── src/
|
||||
│ ├── datasets.js runtime registry: title, source, examples, SQL presets
|
||||
│ ├── router.js pathname → dataset (or hub)
|
||||
│ ├── components/
|
||||
│ │ └── hub.jsx the /thptqg/ landing route
|
||||
│ ├── hooks/
|
||||
│ └── lib/
|
||||
├── data/
|
||||
│ ├── 2016/ 2017/ 2017-old/ 2017-old2/
|
||||
├── parser/ single Rust crate (from 2016/tools/xlsxread)
|
||||
│ ├── src/schema.rs canonical DDL + INSERT + 16 subject regexes
|
||||
│ ├── configs/*.toml per-dataset parse rules only
|
||||
│ ├── scripts/ crawl-baotintuc.js, diff-datasets.js, check-duplicates.js,
|
||||
│ │ verify-parity.js
|
||||
│ └── tests/golden.rs
|
||||
└── docs/ merged from 2016/docs + 2017/docs
|
||||
```
|
||||
|
||||
## Published Artifact
|
||||
|
||||
One bundle, five entry points. Because `base` is absolute (`/thptqg/`), the
|
||||
emitted `index.html` references `/thptqg/assets/index-HASH.js` regardless of the
|
||||
directory it sits in — so copying it to each dataset path yields a real static
|
||||
file at every URL. GitHub Pages serves them as directory indexes. **No SPA
|
||||
404-fallback hack is required**, which matters because the existing `?q=`
|
||||
deep-link behaviour would not survive one.
|
||||
|
||||
```text
|
||||
_site/
|
||||
├── index.html hub route
|
||||
├── 404.html copy of index.html
|
||||
├── assets/index-HASH.js ONE bundle, cached across all five pages
|
||||
├── db/2016.db.gz 2017.db.gz 2017-old.db.gz 2017-old2.db.gz
|
||||
├── 2016/index.html ┐
|
||||
├── 2017/index.html │ byte-identical copies of the root index.html
|
||||
├── 2017-old/index.html │
|
||||
├── 2017-old2/index.html ┘
|
||||
└── 2017/old/index.html ┐ legacy redirect stubs (same file again)
|
||||
2017/old2/index.html ┘
|
||||
```
|
||||
|
||||
## Canonical Schema
|
||||
|
||||
```sql
|
||||
CREATE TABLE student (
|
||||
so_bao_danh TEXT PRIMARY KEY,
|
||||
ho_ten TEXT NOT NULL,
|
||||
ho_ten_ascii TEXT NOT NULL,
|
||||
ngay_sinh TEXT,
|
||||
ten_cum_thi TEXT, -- 2016 only; NULL elsewhere
|
||||
gioi_tinh TEXT, -- 2016 only; NULL elsewhere
|
||||
toan REAL, ngu_van REAL, vat_ly REAL, hoa_hoc REAL, sinh_hoc REAL,
|
||||
khtn REAL, -- 2017 only
|
||||
lich_su REAL, dia_ly REAL,
|
||||
gdcd REAL, khxh REAL, -- 2017 only
|
||||
tieng_anh REAL, tieng_phap REAL,
|
||||
tieng_nga REAL, -- 2017 only
|
||||
tieng_duc REAL, tieng_nhat REAL, -- 2016 only
|
||||
tieng_trung REAL
|
||||
);
|
||||
CREATE INDEX idx_ho_ten ON student(ho_ten);
|
||||
CREATE INDEX idx_ho_ten_ascii ON student(ho_ten_ascii);
|
||||
CREATE INDEX idx_ten_cum_thi ON student(ten_cum_thi) WHERE ten_cum_thi IS NOT NULL;
|
||||
```
|
||||
|
||||
`idx_ten_cum_thi` is partial so it costs nothing on the three 2017 datasets
|
||||
(zero entries) while staying fully useful for 2016's cluster grouping queries.
|
||||
|
||||
## Phases
|
||||
|
||||
| Phase | Name | Status |
|
||||
|-------|------|--------|
|
||||
| 1 | [Standard schema and unified parser](./phase-01-standard-schema-and-unified-parser.md) | Completed |
|
||||
| 2 | [Repo restructure](./phase-02-repo-restructure.md) | Completed |
|
||||
| 3 | [Unified frontend and dataset registry](./phase-03-unified-frontend-and-dataset-registry.md) | Completed |
|
||||
| 4 | [Build and deploy pipeline](./phase-04-build-and-deploy-pipeline.md) | Completed |
|
||||
| 5 | [Parity verification and docs](./phase-05-parity-verification-and-docs.md) | Completed |
|
||||
|
||||
## Dependencies
|
||||
|
||||
Strictly sequential. Phase 1 must produce parity-verified DBs under the old
|
||||
layout before Phase 2 moves 419 MB of tracked data. Phase 5's parity gate is
|
||||
the release gate — nothing merges until all four DBs match their pre-refactor
|
||||
row counts and per-column non-NULL counts.
|
||||
|
||||
No cross-plan dependencies (`plans/` was empty before this plan).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [x] One Rust crate; `2016/tools/` and `2017/tools/` gone
|
||||
- [x] One `src/`; `2016/src/` and `2017/src/` gone
|
||||
- [x] One Vite build producing all five pages
|
||||
- [x] All four DBs built from the same DDL, same INSERT, same 16 regexes
|
||||
- [x] Per-dataset row count and per-column non-NULL count identical to pre-refactor baseline
|
||||
- [x] All five published URLs functional under the flat scheme, deep links included
|
||||
- [x] Legacy `/2017/old/` and `/2017/old2/` redirect to their flat equivalents, preserving `?q=`
|
||||
- [x] 2016 site still shows `ten_cum_thi` + `gioi_tinh`; 2017 sites do not
|
||||
- [x] 2016 site gains 2017's features (deep links, student detail, tiers, live search)
|
||||
- [x] npm only: `package-lock.json` committed, no pnpm files or CI steps remain
|
||||
- [x] `cargo test` green; `npm run lint` green
|
||||
|
||||
## Rollback
|
||||
|
||||
Every phase is a separate commit on a feature branch. Phase 2's `git mv` is the
|
||||
only hard-to-undo step; it is content-preserving, so `git revert` restores the
|
||||
old layout exactly. Do not squash before the Phase 5 gate passes.
|
||||
|
||||
## Open Questions
|
||||
|
||||
None outstanding. Four decisions were taken before planning:
|
||||
|
||||
1. Frontend lives at repo root.
|
||||
2. Schema is defined in parser code, not duplicated across the TOML configs.
|
||||
3. The hub page is a route inside the app, not a separate static file — one
|
||||
Vite build covers every page on GitHub Pages. Trade-off accepted: the hub
|
||||
now needs the JS bundle (~150 KB gzipped) to paint, where today it is 25
|
||||
lines of static HTML.
|
||||
4. npm replaces pnpm.
|
||||
5. URLs are flat (`/thptqg/2017-old/`), not nested (`/thptqg/2017/old/`), so the
|
||||
URL segment equals the dataset ID everywhere. Legacy paths redirect.
|
||||
@@ -1,279 +0,0 @@
|
||||
---
|
||||
phase: 1
|
||||
title: Scaffold and reader fidelity gate
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies: []
|
||||
effort: ''
|
||||
---
|
||||
|
||||
# Phase 1: Scaffold and reader fidelity gate
|
||||
|
||||
## Overview
|
||||
|
||||
Scaffold `go-parser/` and settle the question that governs everything downstream: **does the Go
|
||||
reader produce, for every cell of every sheet of all 299 files, the exact string calamine
|
||||
produces?** Not "does it open" — the exact string, because that string is what gets stored and
|
||||
regex-matched.
|
||||
|
||||
This phase also produces the answer that unblocks the `Data`-typed tests in Phases 4 and 5.
|
||||
|
||||
## Requirements
|
||||
|
||||
- Functional: byte-identical cell stringification vs calamine across all 299 input files, both
|
||||
formats, including dates, numerics, and empty cells.
|
||||
- Non-functional: one reader contract, defined here, used unchanged by Phase 4.
|
||||
|
||||
## What was believed going in (and how it held up)
|
||||
|
||||
Red-team testing had swept all 67 BIFF files with `extrame/xls` and reported **0 failures,
|
||||
0 panics**, correct Vietnamese at row 60,000. That was taken as evidence BIFF *readability*
|
||||
was settled and only cell-value fidelity remained open.
|
||||
|
||||
**That evidence did not survive contact with a cell-level comparison** — see the RESULT below.
|
||||
"Opens without panic" and a handful of spot-checks are not a fidelity test when 28% of cells
|
||||
are correct. The lesson generalises: for every remaining phase, compare against ground truth
|
||||
cell-by-cell, never by sampling.
|
||||
|
||||
Hazards that *did* hold up and were designed around:
|
||||
- excelize applies number formats by default → **set `RawCellValue: true`** and verify.
|
||||
- excelize `GetRows` trims trailing blank cells → rows are ragged; calamine's are rectangular.
|
||||
|
||||
Hazards that turned out not to exist in this corpus: date-serial rendering (zero `DateTime`
|
||||
cells anywhere) and non-A1 used-range origins (all ranges start at (0,0)).
|
||||
|
||||
## Architecture — AS BUILT
|
||||
|
||||
The shipped API differs from the original sketch (which took a `DatasetConfig` and did header
|
||||
skipping inline). The reader deliberately knows **nothing** about datasets: it reports every
|
||||
sheet and every row exactly as calamine would, and all policy — sheet selection, header
|
||||
skipping, blank-row handling — belongs to the Phase 4 build loop. That keeps the fidelity
|
||||
contract testable in isolation, which is what made the 299/299 oracle possible.
|
||||
|
||||
```go
|
||||
// go-parser/internal/reader
|
||||
type Cell struct {
|
||||
Str string // exactly what calamine's Data::to_string() yields
|
||||
IsEmpty bool // calamine Data::Empty; diagnostic only — never compare on it
|
||||
}
|
||||
|
||||
type Sheet struct{ Index int; Name string; Height, Width int } // used-range geometry
|
||||
|
||||
type RowFunc func(sheet Sheet, rowIdx int, row []Cell) error
|
||||
|
||||
type Workbook interface {
|
||||
Sheets() []Sheet // workbook order, all sheets
|
||||
EachRow(sheetIdx int, fn RowFunc) error
|
||||
Close() error
|
||||
}
|
||||
|
||||
func Open(path string) (Workbook, error) // dispatches on extension
|
||||
```
|
||||
|
||||
Rows are padded to the sheet's used-range width. Width is load-bearing: every column read
|
||||
downstream is positional, so a trimmed tail silently NULLs columns.
|
||||
|
||||
**Implementation note:** both backends materialise a workbook's rows rather than streaming
|
||||
(`rows [][][]Cell`). Peak input is `data/2017/ha-noi.xls` at 72,276 rows × 4 columns; the full
|
||||
299-file suite runs in 77s. Revisit only if a future dataset is far larger.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Create: `go-parser/go.mod`, `go-parser/internal/reader/{reader.go,xls.go,xlsx.go}` + tests,
|
||||
`go-parser/internal/reader/fidelity_test.go`, `go-parser/testdata/`
|
||||
- Reference (do not modify): `parser/src/reader.rs` (incl. its 7 tests at `:112-197`),
|
||||
`parser/Cargo.toml`
|
||||
- Read-only inputs: all 299 files under `data/`
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
**Tests first.** The oracle is Rust; extract it before writing Go.
|
||||
|
||||
1. **Generate ground truth.** Add a throwaway Rust bin (or `#[test]`) that, for a chosen file,
|
||||
dumps every sheet: sheet name, used-range dimensions, and every cell rendered exactly as
|
||||
`Data::to_string()` plus an `is_empty` flag.
|
||||
2. **Commit hashes, not rows.** For each sampled file store sheet names, per-sheet row/column
|
||||
counts, and a SHA-256 over the canonical dump — **not** the dump itself. The raw dumps are
|
||||
real student names and birthdates; `parser/tests/fixtures/README.md` documents that all
|
||||
fixture PII is replaced with synthetic values, and committing real rows would reverse that
|
||||
convention and outlive the data they came from. Keep dumps under `.gitignore` and regenerate
|
||||
from Rust on demand (Rust still builds — that is this plan's whole advantage).
|
||||
3. **Sample must cover both formats and the risky cell types.** At minimum: 3 `.xls`
|
||||
(2 from `data/2017`, 1 from `data/2016`) **and** 3 `.xlsx` (one each from `data/2016`,
|
||||
`data/2017-old`, `data/2017-old2`), each chosen to contain a date cell and a numeric SBD.
|
||||
4. Write `fidelity_test.go` asserting the Go reader reproduces every committed hash. It fails —
|
||||
nothing is implemented.
|
||||
5. Scaffold: `go mod init`, add deps **at pinned versions**, commit `go.sum`.
|
||||
6. Implement both readers until the hashes match.
|
||||
7. **Full sweep, all 299 files**: for each, assert sheet names in order, per-sheet row count,
|
||||
and per-sheet used-range width all match calamine. Record failures per file.
|
||||
8. **Record the stringification answer** in this file — the literal rendering of a date cell, a
|
||||
float score, and a numeric SBD. Phases 4 and 5 depend on it to port `Data`-typed tests
|
||||
without guessing.
|
||||
|
||||
## Decision gate — *as written before execution; see RESULT below for the outcome*
|
||||
|
||||
- **PASS**: all 299 files match on sheet names, per-sheet row counts, widths, and sampled
|
||||
content hashes.
|
||||
- **FAIL** → stop and escalate. Do not proceed with a partial pass. The pre-planned fallback
|
||||
(`.xls → .xlsx` conversion) is **deferred by user decision** and changes committed data, so
|
||||
re-opening it is the user's call, not the implementer's.
|
||||
|
||||
## RESULT — 2026-08-13: **PASS — 299 / 299 exact**
|
||||
|
||||
Every input file's canonical cell dump is byte-identical to calamine's, verified by SHA-256.
|
||||
Locked in as `go test ./internal/reader/` (77s for the full corpus) against the committed
|
||||
oracle `go-parser/testdata/reader-fidelity-hashes.tsv`.
|
||||
|
||||
Reached only after replacing the BIFF library. The first attempt failed hard; the record of
|
||||
that is kept below because it is the reason the reader is built the way it is.
|
||||
|
||||
### Final reader stack
|
||||
|
||||
| Format | Library | Result |
|
||||
|---|---|---|
|
||||
| `.xls` (67 files) | **`github.com/pbnjay/grate`** | exact |
|
||||
| `.xlsx` (232 files) | `github.com/xuri/excelize/v2 v2.11.0` | exact |
|
||||
|
||||
Five corrections were needed to match calamine, each verified against ground truth:
|
||||
|
||||
1. **`RawCellValue: true`** — otherwise excelize applies the cell number format.
|
||||
2. **Rows padded to used-range width** — excelize trims trailing blank cells; calamine returns
|
||||
a rectangle. Width is load-bearing: `diem_thi` is the last column for 2017.
|
||||
3. **grate merged-cell markers blanked** — grate fills merge-covered cells with `→`/`⇥`/`↓`/`⤓`
|
||||
(its exported constants); calamine reports them empty. 19 cells, in the merged title block
|
||||
of one 2016 file. Only an exact whole-value match is blanked.
|
||||
4. **Numeric re-rendering, gated on cell type** — calamine parses numeric cells to f64 and
|
||||
renders with Rust's `Display`, so `6.0` becomes `6`. Applying that by value alone corrupts
|
||||
shared strings that merely look numeric: it turned `6.00`→`6`, `NAN`→`NaN`, and would have
|
||||
destroyed leading zeros in `so_bao_danh`. The type check is what makes it safe. Note
|
||||
excelize reports **`CellTypeUnset`, not `CellTypeNumber`**, for plain numeric cells, because
|
||||
OOXML omits the `t` attribute and excelize has no map entry for an empty one.
|
||||
5. **CRLF restoration in shared strings** — Go's `encoding/xml` performs the line-ending
|
||||
normalisation XML 1.0 mandates (CRLF and lone CR → LF); calamine reads raw bytes and keeps
|
||||
CRLF. This reaches the database: 2,233 `TEN_CUMTHI` values in one 2016 file, populating
|
||||
`ten_cum_thi`. Fixed by rewriting literal CR to ` ` before decoding — character
|
||||
references are exempt from that normalisation — and mapping the normalised form back.
|
||||
A blanket `\n`→`\r\n` would have been wrong: 117 files carry a lone CR with no LF.
|
||||
|
||||
**Known divergence, behaviorally inert:** none remaining. The trailing 1×1 empty sheet in 63
|
||||
`2017-old` and 53 `2017-old2` files is now reproduced exactly, using `GetCellType(A1)` to tell
|
||||
an empty-shared-string cell (`CellTypeSharedString`) from a genuinely absent one
|
||||
(`CellTypeUnset`, 230 such sheets in 2016).
|
||||
|
||||
### The rejected library: `extrame/xls` — HARD FAIL (67 files)
|
||||
|
||||
Full canonical diff of `data/2017/an-giang.xls` (56,244 cells) against calamine:
|
||||
|
||||
| Class | Cells | Share |
|
||||
|---|---|---|
|
||||
| Identical | 16,016 | 28% |
|
||||
| **Different content** (corruption) | 38,664 | **69%** |
|
||||
| **Rust has value, Go empty** (data loss) | 15,629 | 28% |
|
||||
| Whitespace-only | 0 | — |
|
||||
|
||||
Only 28% of cells are read correctly. Three distinct defect classes, all confirmed against
|
||||
ground truth:
|
||||
|
||||
1. **Undecoded BIFF bytes leak through.** Row 56 col 0: calamine `HỒ THỊ NHƯ Ý`; extrame
|
||||
`"\f\x00\x01H\x00Ò\x1e \x00T\x00H\x00Ê\x1e \x00N\x00H\x00¯\x01 \x00Ý\x00\b\x00\x0051009967t\x00…"`
|
||||
— raw UTF-16LE plus record framing, with the neighbouring SBD and score cells spliced in.
|
||||
2. **Content teleports between cells.** extrame's (14055, 0) is calamine's **(6500, 3)**.
|
||||
3. **Tail rows silently lost.** calamine rows 14056-14060 hold real students
|
||||
(`NGUYỄN HỮU ÁI` … `HUỲNH VĂN KIÊN`); extrame returns them blank.
|
||||
4. Header cell (0,0) `HO_TEN` dropped — would defeat `is_header_row` and ingest the header
|
||||
as data.
|
||||
5. `sh.Row(r)` panics (nil deref, `worksheet.go:30`) for `r > MaxRow`.
|
||||
|
||||
**Not a configuration problem.** Identical garbage under charsets `utf-8`, `utf-16`, `utf-16le`,
|
||||
`windows-1258`, `cp1252`, and empty. Not an index-arithmetic problem on our side either —
|
||||
geometry matches exactly (70,308 canonical lines both sides, `SHEET 0 Sheet1 14061 4` on both)
|
||||
after correcting `LastCol()` exclusivity and the used-range height rule.
|
||||
|
||||
**This refutes the red-team finding that `extrame/xls` reads the corpus correctly.** That sweep
|
||||
tested "opens without panic" plus a few spot-checks; spot-checks pass because 28% of cells are
|
||||
right and the early rows of a file are among them. Cell-level comparison against ground truth
|
||||
is what exposed it.
|
||||
|
||||
Library survey (2026-08-13): `youkuang/xls` and `f2xb/xls` are forks of `extrame/xls` and carry
|
||||
the same defect. `qax-os/excelize` **is** excelize and does not read BIFF at all. The
|
||||
independent implementations are `pbnjay/grate` (chosen — reproduced calamine exactly on all 67
|
||||
files first try, needing only the merged-marker correction) and `shakinm/xlsReader` (not
|
||||
evaluated; grate passed).
|
||||
|
||||
### Corpus facts established (worth keeping regardless of the decision)
|
||||
|
||||
Scanned all 299 files, 15.98M cells:
|
||||
- **Zero `DateTime` cells.** Also zero `Int`, `Bool`, `Error`, `DateTimeIso`, `DurationIso`.
|
||||
Only `String` (15.1M), `Empty` (722k), `Float` (133k) occur. **The date-serial divergence
|
||||
that this plan called its dominant risk does not exist in this corpus** — `ngay_sinh` is
|
||||
stored as text everywhere.
|
||||
- **Every used range starts at (0,0)** — the used-range-origin concern is moot.
|
||||
- Floats occur only in `2016` (53,008) and `2017-old2` (80,121); none in `2017` or `2017-old`.
|
||||
All render as plain decimals, no exponents, max 2 decimal places.
|
||||
- Trailing empty sheets: 293 sheets at height 0, 116 at height 1.
|
||||
- The "63 empty" rows in `docs/data-pipeline.md:114` for 2017 do **not** come from trailing
|
||||
sheets — calamine reports height 0 for all 63 of those and yields no rows from them. The
|
||||
red team's stated mechanism for that count is wrong; provenance is a Phase 4 question.
|
||||
|
||||
### Artefacts
|
||||
|
||||
- `parser/examples/dump_cells.rs`, `parser/examples/scan_kinds.rs` — throwaway Rust ground-truth
|
||||
tooling (delete after migration)
|
||||
- `go-parser/` — module, `internal/reader` (both formats), `cmd/dumpcells`
|
||||
- Dumps are regenerable and gitignored; no PII committed, per the convention in
|
||||
`parser/tests/fixtures/README.md`
|
||||
|
||||
## Specific things to verify, not assume
|
||||
|
||||
- **Per-sheet row counts, including empty sheets.** All 63 `data/2017` files carry a trailing
|
||||
sheet that calamine renders as one blank row. `docs/data-pipeline.md:114` records the
|
||||
consequence exactly: `2017 | 861,131 source rows | 63 empty | 861,068 DB rows`. A Go reader
|
||||
that skips zero-row sheets produces an identical database and silently different counters.
|
||||
- **Date cells** → `ngay_sinh`, stored verbatim. calamine prints the raw serial
|
||||
(`datatype.rs:771-775`); excelize applies the number format unless `RawCellValue: true`.
|
||||
- **Numeric cells** → `so_bao_danh`, a `TEXT PRIMARY KEY`. Trailing `.0`? Scientific notation?
|
||||
A difference here re-keys the table.
|
||||
- **Empty vs blank**: `reader.rs:42` checks both `Data::Empty` and stringified-empty, implying
|
||||
calamine emits empty-but-not-`Empty` cells. The `Cell.IsEmpty` field exists for this.
|
||||
- **Row width / used-range origin**: see "What is already known".
|
||||
- **Sheet order**: `sheet_mode = "all"` for 2016 and 2017. calamine's `sheet_names()` and
|
||||
excelize's `GetSheetList` can disagree when `workbook.xml` order differs from `sheetId` order.
|
||||
Order determines which duplicate SBD survives `INSERT OR REPLACE`.
|
||||
|
||||
## Dependency trust
|
||||
|
||||
- Pin every dependency to an exact version/pseudo-version; commit `go.sum`.
|
||||
- Record in this file that `extrame/xls` is effectively unmaintained (last push 2023-09-12,
|
||||
53 open issues, no valid `go.mod`) and that it transitively adds `tealeg/xlsx`.
|
||||
- Note the open excelize advisory `GHSA-h69g-9hx6-f3v4` (unbounded row-index allocation). The
|
||||
2017 refresh runbook (`docs/data-pipeline.md:137`) feeds network-downloaded spreadsheets
|
||||
straight into the parser, so this is a live path.
|
||||
- `govulncheck` is added to CI in Phase 7b.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] `go-parser/` builds; `go test ./...` runs; `go.sum` committed with pinned versions
|
||||
- [ ] Ground-truth **hashes** (not raw rows) committed for 3 `.xls` + 3 `.xlsx` files
|
||||
- [ ] Go reader reproduces every committed hash
|
||||
- [ ] All 299 files: sheet names in order, per-sheet row counts, and widths match calamine —
|
||||
**including the 63 trailing empty sheets in `data/2017`**
|
||||
- [x] `RawCellValue` settled and justified in writing
|
||||
- [x] Date, float, and numeric-SBD renderings recorded literally in this file
|
||||
- [x] One reader contract, with `Cell.IsEmpty` and padded row width
|
||||
- [~] `parser/src/reader.rs`'s 7 tests (`:112-197`) — **moved to Phase 4.** They exercise
|
||||
`is_header_row` and `is_all_blank`, which are dataset policy and therefore live in the
|
||||
build loop, not the reader package. Recorded here rather than silently dropped.
|
||||
- [x] Dependency trust notes recorded
|
||||
- [x] Explicit PASS/FAIL recorded
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| `.xlsx` date/number formatting differs | The primary target of this gate; `RawCellValue` verified against hashes |
|
||||
| Trailing-blank trimming NULLs tail columns | Row-width equality asserted for all 299 files |
|
||||
| Empty-sheet skipping breaks counters | Per-sheet row counts asserted, including empty sheets |
|
||||
| Real PII committed as fixtures | Hashes committed instead; dumps gitignored and regenerable |
|
||||
| `extrame/xls` unmaintained / OOM issues | Pinned pseudo-version; full sweep measures memory |
|
||||
| Partial pass rationalized into a PASS | Gate is binary; fallback is a user decision |
|
||||
@@ -1,195 +0,0 @@
|
||||
---
|
||||
phase: 2
|
||||
title: Schema and config
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies:
|
||||
- 1
|
||||
effort: ''
|
||||
---
|
||||
|
||||
# Phase 2: Schema and config
|
||||
|
||||
## Overview
|
||||
|
||||
Port the two pure, I/O-free modules: `schema.rs` (DDL, INSERT SQL, column order, 16 subject
|
||||
regexes) and `config.rs` (strict YAML loading). No file or database access — fully unit-testable.
|
||||
|
||||
Also **decides the SQLite driver** (see below) — it governs the CI shape and the integrity
|
||||
story, so it is settled here rather than deferred to Phase 4.
|
||||
|
||||
## Requirements
|
||||
|
||||
- Functional: identical DDL text, identical column ordering, identical regex patterns; YAML
|
||||
loading that **rejects unknown fields**.
|
||||
- Non-functional: `schema` stays the single source of truth, as in Rust. No duplicated column
|
||||
lists anywhere else in the port.
|
||||
|
||||
## Carried forward from Phase 1
|
||||
|
||||
- Reader is done and exact; it exposes `reader.Cell{Str, IsEmpty}`. Config work is independent
|
||||
of it.
|
||||
- The corpus contains **only** `String`, `Empty`, and `Float` cell kinds — no dates, ints,
|
||||
bools, or errors. Nothing in `config.go` needs date or type-coercion handling.
|
||||
- Go 1.26.5 / linux-arm64 confirmed working; `go.mod` currently pulls `pbnjay/grate` and
|
||||
`excelize/v2 v2.11.0`, both pure Go. **The SQLite driver choice below is what decides whether
|
||||
this module stays cgo-free.**
|
||||
|
||||
## Decision: SQLite driver
|
||||
|
||||
Open question 2 in `plan.md`. Record the choice and rationale in this file.
|
||||
|
||||
| Option | For | Against |
|
||||
|---|---|---|
|
||||
| `modernc.org/sqlite` | Pure Go, no cgo, trivial ARM64 CI and cross-compilation. Widely used (3,500+ importers) | **Machine-transpiled** SQLite, not the upstream C amalgamation. Its correctness argument is "the transpiler is correct", not "this is the code the SQLite authors tested". Requires exact `modernc.org/libc` version matching |
|
||||
| `mattn/go-sqlite3` | Real upstream SQLite C, matching what `rusqlite --bundled` vendors (`Cargo.lock:520` `libsqlite3-sys 0.30.1`) | cgo: slower CI, cross-compilation friction, needs a C toolchain in the workflow |
|
||||
|
||||
This writes a published 1.5M-row dataset, so the integrity story is a real consideration, not a
|
||||
formality. Phase 6's full-table checksum is the compensating control either way. Pin the exact
|
||||
version and commit it to `go.sum`.
|
||||
|
||||
### DECIDED — `modernc.org/sqlite` (user, 2026-08-13)
|
||||
|
||||
Empirically verified on this linux/arm64 box before deciding: `modernc.org/sqlite v1.56.0`
|
||||
embeds **SQLite 3.53.3** and handles the exact SQL this parser uses — the full DDL including
|
||||
the partial `idx_ten_cum_thi` index, `INSERT OR REPLACE`, and `VACUUM`, producing 3 indexes.
|
||||
|
||||
Rationale:
|
||||
- Keeps the module **entirely cgo-free** — `grate`, `excelize` and `yaml.v3` are all pure Go, so
|
||||
Phase 7 sets `CGO_ENABLED=0`, needs no C toolchain in CI, and cross-compiles trivially.
|
||||
- The SQL surface is deliberately plain: no CTEs, window functions, triggers or extensions.
|
||||
That is the part of SQLite a transpiled port is least likely to get wrong.
|
||||
- Phase 6's full-table SHA-256 plus `PRAGMA table_info`/`index_list` comparison against live
|
||||
Rust output is a real compensating control for the transpilation risk.
|
||||
|
||||
Accepted trade-off: it is a machine-transpiled SQLite, not the upstream C amalgamation that
|
||||
`rusqlite --bundled` vendors (`libsqlite3-sys 0.30.1`, ~3.46). Version parity was never a goal —
|
||||
`plan.md` explicitly rules byte-identical databases out as a criterion, since SQLite stamps its
|
||||
own version into header bytes 96-99.
|
||||
|
||||
If Phase 6 ever shows a divergence traceable to the driver, `mattn/go-sqlite3` is the fallback:
|
||||
`gcc` is present locally and on GitHub runners, so the switch costs only `CGO_ENABLED=1`.
|
||||
|
||||
## Architecture
|
||||
|
||||
```go
|
||||
// internal/schema
|
||||
const DDL = `...` // verbatim from parser/src/schema.rs:27-54
|
||||
const InsertSQL = `...` // positional ?, order fixed by IdentityFields + ScoreFields
|
||||
var IdentityFields = []string{...} // 6
|
||||
var ScoreFields = []string{...} // 16
|
||||
var ScorePatterns = map[string]*regexp.Regexp{...} // compiled once at init
|
||||
|
||||
// internal/config
|
||||
type DatasetConfig struct {
|
||||
FormatDetection *string
|
||||
Reader ReaderCfg // SheetMode "all"|"first", StripBlankRows bool
|
||||
Columns *ColumnMap // nil when FormatDetection is set
|
||||
Validation ValidationCfg // 3 bools
|
||||
Header HeaderCfg // Tokens []string
|
||||
}
|
||||
func Load(path string) (*DatasetConfig, error)
|
||||
```
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Create: `go-parser/internal/schema/schema.go`, `schema_test.go`,
|
||||
`go-parser/internal/config/config.go`, `config_test.go`
|
||||
- Reference: `parser/src/schema.rs`, `parser/src/config.rs`, `parser/src/error.rs`
|
||||
- Consumed unchanged: `parser/configs/{2016,2017,2017-old,2017-old2}.yml` — the Go binary
|
||||
reads the **same** config files; do not copy or fork them
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
**Tests first**, ported from the Rust unit tests in `config.rs:131-137` and the DDL/schema
|
||||
constants.
|
||||
|
||||
1. Write `schema_test.go`: assert DDL string equals the Rust DDL verbatim (paste it as the
|
||||
expected literal), assert `len(IdentityFields)+len(ScoreFields) == 22`, assert `InsertSQL`
|
||||
placeholder count matches, assert all 16 regexes compile.
|
||||
2. Write `config_test.go`: port **all 4** tests from `config.rs:114-152` (not just the one at
|
||||
`:131-137`). Load all 4 real configs and assert the field values the scout recorded (2016 has
|
||||
no `columns:` mapping and `format_detection: thptqg2016`; 2017-old is `sheet_mode="first"`;
|
||||
2017-old2 has `strip_blank_rows: true` + `require_numeric_sbd: true`). Include the
|
||||
**unknown-field rejection** test — port of `config_rejects_leftover_sql_sections`.
|
||||
3. Implement `schema.go`. Copy DDL, INSERT SQL, field lists, and the 16 patterns **verbatim**.
|
||||
Do not retype the Vietnamese pattern literals — copy them, byte-exactness matters.
|
||||
4. Implement `config.go` with a YAML decoder configured for strict decoding.
|
||||
|
||||
## Specific things to get right
|
||||
|
||||
- **`deny_unknown_fields` is load-bearing** and has a test in Rust. Go YAML decoders ignore
|
||||
unknown keys by default; `gopkg.in/yaml.v3` enables the check with `KnownFields(true)`.
|
||||
Write the rejection test with a *valid* YAML key — a TOML-style `key = 1` line fails as a
|
||||
parse error instead, so the test would pass without proving anything.
|
||||
- **`Columns` is nil for 2016** — represent as a pointer/optional, not a zero value. A zero
|
||||
`ColumnMap` would silently mean "all columns are index 0".
|
||||
- **Regexes**: Rust `regex` and Go `regexp` are both RE2, and the scout confirmed no
|
||||
backreferences, lookaround, or `\p{}` in any pattern. This is the one zero-risk area — but
|
||||
the patterns contain literal Vietnamese (`Ngữ văn`, `Tiếng Đức`), so copy, never retype.
|
||||
- **Partial index** in the DDL (`... WHERE ten_cum_thi IS NOT NULL`) is SQLite-specific and
|
||||
must survive verbatim.
|
||||
- Column order in `InsertSQL` is positional — a reordering is a silent data corruption bug
|
||||
that no compiler will catch.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] DDL string byte-identical to `parser/src/schema.rs`
|
||||
- [x] 22 columns in the exact Rust order; INSERT placeholder count matches
|
||||
- [x] All 16 subject regexes compile and match the Rust patterns byte-for-byte
|
||||
- [x] All 4 real configs load with values matching the scout's recorded table
|
||||
- [x] All 4 tests from `config.rs:114-152` ported
|
||||
- [x] Unknown-field YAML is **rejected** (test passes, via a valid YAML key)
|
||||
- [x] `Columns` is nil for 2016 and populated for the other three
|
||||
- [x] No column list duplicated outside `internal/schema`
|
||||
- [x] SQLite driver chosen, pinned, and the rationale written into this file
|
||||
|
||||
## RESULT — 2026-08-13: **PASS**
|
||||
|
||||
`internal/schema` and `internal/config` ported; 13 Go tests green, all 63 Rust tests still green.
|
||||
|
||||
### Config format changed to YAML (user decision, mid-phase)
|
||||
|
||||
The user prefers `.yml`. Converting only the Go side would have left two hand-synced copies of
|
||||
four configs, and any drift would surface as a *database* mismatch that Phase 6 would blame on
|
||||
the parser. The user chose to convert **both** parsers, so they keep reading the identical file
|
||||
and the parity gate stays intact.
|
||||
|
||||
This amends the plan's "Rust `parser/` untouched" decision — deliberately, and recorded here.
|
||||
Changes: `serde_yaml 0.9` replaces `toml 0.8` in `parser/Cargo.toml`; `config.rs` uses
|
||||
`serde_yaml::from_str`; `error.rs` wraps `serde_yaml::Error`; the four `config.rs` tests and
|
||||
`golden.rs`'s config paths move to YAML; `build-db.js:53` reads `.yml`. `deny_unknown_fields`
|
||||
is a serde attribute, so strictness carried over for free.
|
||||
|
||||
`serde_yaml` is deprecated upstream but stable, and this crate is deleted at Phase 7 cutover —
|
||||
noted inline in `Cargo.toml`.
|
||||
|
||||
**Verified semantically identical, end to end.** Rebuilt two real datasets with the Rust parser
|
||||
reading the new YAML configs and compared against `docs/data-pipeline.md`:
|
||||
|
||||
| dataset | source rows | skipped | DB rows | documented | match |
|
||||
|---|---|---|---|---|---|
|
||||
| `2016` | 877,464 | 3 duplicate SBDs collapsed | 877,461 | 877,461 | yes |
|
||||
| `2017-old2` | 679,764 | 0 | 679,764 | 679,764 | yes |
|
||||
|
||||
Those two were chosen because they are the structurally distinct configs: 2016 is the only one
|
||||
with `format_detection:` and no `columns:` mapping, and 2017-old2 is the only one combining
|
||||
`strip_blank_rows: true` with `require_numeric_sbd: true`.
|
||||
|
||||
### Go decoder notes
|
||||
|
||||
- `gopkg.in/yaml.v3` with `KnownFields(true)` for strictness.
|
||||
- `SheetMode` is validated **after** decoding: the decoder assigns named string types directly
|
||||
and never calls a custom unmarshaler, so `sheet_mode: second` would otherwise decode silently
|
||||
and read as "not all" downstream.
|
||||
- The unknown-key test had to use a valid YAML key (`unexpected_key: 1`). Written TOML-style
|
||||
(`unexpected_key = 1`) it passes on a YAML *parse* error and proves nothing about
|
||||
`KnownFields`.
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| Go YAML lib silently ignores unknown keys | Explicit rejection test; `KnownFields(true)` required |
|
||||
| Vietnamese regex literals corrupted by retyping | Copy verbatim; test compares against Rust source |
|
||||
| Column order drift | Test asserts full ordered list, not just count |
|
||||
@@ -1,160 +0,0 @@
|
||||
---
|
||||
phase: 3
|
||||
title: Transform core
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies:
|
||||
- 2
|
||||
effort: ''
|
||||
---
|
||||
|
||||
# Phase 3: Transform core
|
||||
|
||||
## Overview
|
||||
|
||||
Port `transform.rs` — Vietnamese diacritic stripping, score regex extraction, row validation,
|
||||
and the fixed-column row transform. Pure functions, no I/O. This is where subtle divergence is
|
||||
most likely and most invisible.
|
||||
|
||||
## Requirements
|
||||
|
||||
- Functional: `ToAscii` byte-identical to Rust for all inputs; score parsing identical;
|
||||
validation reproduces Rust's **two distinct blank-row paths**, not a bool.
|
||||
- Non-functional: **all 29** unit tests in `transform.rs`'s test module (`:201-409`) transfer as
|
||||
the Go test suite.
|
||||
|
||||
## Architecture
|
||||
|
||||
Signature mirrors Rust's, which takes `strip_blank_rows` and `all_blank` as explicit
|
||||
parameters (`transform.rs:101-107`) — a 2-arg Go version structurally cannot reproduce either
|
||||
blank-row path.
|
||||
|
||||
```go
|
||||
func ToAscii(s string) string
|
||||
|
||||
type SkipReason int
|
||||
const (
|
||||
SkipNone SkipReason = iota
|
||||
SkipBlankRow
|
||||
SkipEmptyField // counted as source row, then skipped
|
||||
SkipNonNumericSbd // counted as source row, then skipped
|
||||
)
|
||||
|
||||
func ParseScores(diemThi string) map[string]float64
|
||||
func ValidateRow(hoTen, soBaoDanh string, cfg *config.DatasetConfig,
|
||||
stripBlankRows, allBlank bool) SkipReason
|
||||
func TransformRow(row []reader.Cell, cfg *config.DatasetConfig) (*Student, error)
|
||||
```
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Create: `go-parser/internal/transform/transform.go`, `transform_test.go`
|
||||
- Reference: `parser/src/transform.rs` — `:52-64` ToAscii, `:89-97` SkipReason,
|
||||
`:101-107` validate_row signature, `:130-143` parse_scores, `:162` the `.expect()`,
|
||||
**`:201-409` the test module (29 tests)**
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
**Tests first** — port all 29, not a subset.
|
||||
|
||||
1. Port **every** `#[test]` in `transform.rs`'s module (`:201-409`) into `transform_test.go`,
|
||||
including every Vietnamese fixture string. Stating it as "every test in the module" rather
|
||||
than a line range is deliberate: the range `:213-315` contains only the 20 `to_ascii` cases,
|
||||
and the 9 outside it (`:323, 332, 342, 359, 365, 374, 383, 393, 400`) are exactly the
|
||||
`parse_scores` and `validate_row` tests this phase calls its highest-value traps —
|
||||
including `validate_non_numeric_sbd_rejected` (`:383`) and `validate_blank_row_skipped`
|
||||
(`:400`).
|
||||
2. Add the specific edge cases below as extra tests.
|
||||
3. Implement `ToAscii`, `ParseScores`, `ValidateRow`, `TransformRow` until green.
|
||||
|
||||
## The three exactness traps
|
||||
|
||||
These are the highest-value details in the whole plan. Each is a silent corruption if missed.
|
||||
|
||||
1. **`ToAscii` filters a literal codepoint range, not a Unicode category.**
|
||||
`transform.rs:56` filters `'\u{0300}'..='\u{036f}'`. The *inline* comment at `:53` says
|
||||
"Unicode category M" — **that comment is wrong, the code is the spec** (the doc comment at
|
||||
`:49` correctly states the range). Go's `unicode.Is(unicode.Mn, r)` is strictly more
|
||||
permissive and would diverge on marks outside U+0300–U+036F. Implement the literal range
|
||||
check:
|
||||
```go
|
||||
// NFD, then drop combining marks in U+0300..U+036F only — matches parser/src/transform.rs:56.
|
||||
// Deliberately NOT unicode.Mn, which is broader and would strip more than Rust does.
|
||||
```
|
||||
2. **`đ`/`Đ` are not decomposed by NFD** — they are precomposed Latin letters, so NFD leaves
|
||||
them intact. An explicit replacement to `d` is required (`transform.rs:60`), and it happens
|
||||
**before** lowercasing (`:63`). Preserve that order.
|
||||
3. **There are TWO blank-row paths with opposite outcomes, and the caller owns the split.**
|
||||
The "not counted as a source row" behavior lives in `main.rs:135-137`, which returns
|
||||
*before* `total_source_rows += 1` at `:140`. Separately, when `validate_row` itself returns
|
||||
`Err(SkipReason::BlankRow)`, `main.rs:151` matches it as `=> {}` — which **falls through to
|
||||
transform and insert**. Same enum variant, opposite outcome, decided by which call site you
|
||||
are in. Do not collapse these; reproduce both call sites in Phase 4's build loop and keep
|
||||
`ValidateRow`'s 5-parameter shape so both remain expressible.
|
||||
|
||||
## Other details
|
||||
|
||||
- Order: NFD → filter range → replace `đ`/`Đ` → lowercase.
|
||||
- **`diem_thi` is read WITHOUT `.trim()`** (`transform.rs:172-175`), while `ho_ten`,
|
||||
`ngay_sinh`, and `so_bao_danh` all trim via the closure at `:164-168`. Leading whitespace in
|
||||
the score cell reaches the regexes intact. Replicate the asymmetry.
|
||||
- `ParseScores` uses first-match-anywhere (Rust `captures`, Go `FindStringSubmatch` — same
|
||||
default, unanchored). No change needed.
|
||||
- Rust checks `is_finite()` on parsed scores (`transform.rs:136`). Unreachable given the
|
||||
pattern, but keep it for defensive parity.
|
||||
- `require_numeric_sbd` is a digits-only check, not `strconv.Atoi` — a leading `+`, a `_`, or
|
||||
whitespace must fail. `Atoi` accepts a leading sign; use an explicit digit scan.
|
||||
- `TransformRow` is only called on the non-2016 path, where `Columns` is non-nil. Rust relies
|
||||
on `.expect()` (`transform.rs:162`); in Go return an error rather than panicking.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] **All 29** tests from `transform.rs:201-409` ported and passing
|
||||
- [x] `ToAscii("Nguyễn Văn Đức") == "nguyen van duc"`
|
||||
- [x] `ToAscii` uses the literal U+0300–U+036F range, with a comment saying why not `unicode.Mn`
|
||||
- [x] `đ`/`Đ` → `d` verified independently of the NFD path
|
||||
- [x] `ValidateRow` keeps the 5-parameter Rust shape
|
||||
- [x] Both blank-row paths covered by tests (skip-before-count vs fall-through-to-insert)
|
||||
- [x] `diem_thi` untrimmed while the other three fields are trimmed
|
||||
- [x] Numeric-SBD check rejects `+123`, `12 3`, `1.0`, `ABC123`
|
||||
- [x] Score parsing matches on the multi-subject fixture
|
||||
(`"Toán: 8.5 Ngữ văn: 7.0 Tiếng Anh: 9.25"`)
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| `unicode.Mn` used instead of the literal range | Called out explicitly; comment required in code |
|
||||
| `đ` silently dropped instead of → `d` | Dedicated test |
|
||||
| Tri-state collapsed to bool | Counter-distinguishing test; caught again in Phase 6 |
|
||||
| `strconv.Atoi` accepts signs the Rust check rejects | Explicit rejection cases |
|
||||
|
||||
## RESULT — 2026-08-13: **PASS**
|
||||
|
||||
All 29 tests from `transform.rs:201-409` ported and green, plus guards for each trap.
|
||||
|
||||
**Cross-checked against Rust on real data, not just the unit cases.** A Rust-built database is
|
||||
its own oracle: every row carries `ho_ten` next to the `ho_ten_ascii` Rust derived from it, so
|
||||
the table is a name→slug corpus orders of magnitude larger than 20 hand-picked names.
|
||||
|
||||
| dataset | names compared | mismatches |
|
||||
|---|---|---|
|
||||
| `2016` | 877,461 | **0** |
|
||||
| `2017-old2` | 679,764 | **0** |
|
||||
|
||||
Kept as `TestToAsciiAgainstRustOutput`, which skips unless `GO_PARSER_RUST_DB` points at a
|
||||
Rust-built database — so the default suite stays hermetic while the check stays reusable for
|
||||
Phase 6.
|
||||
|
||||
### Traps handled
|
||||
|
||||
- `ToAscii` filters the literal range U+0300–U+036F. `TestToAsciiUsesLiteralRangeNotUnicodeMn`
|
||||
asserts a mark *outside* that range (U+0654, which is in `Mn`) survives — so swapping in
|
||||
`unicode.Is(unicode.Mn, r)` fails the suite rather than silently changing `ho_ten_ascii`.
|
||||
- `đ`/`Đ` → `d` before lowercasing, tested independently of the NFD path.
|
||||
- `ValidateRow` keeps Rust's 5-parameter shape, so both blank-row paths stay expressible. The
|
||||
caller-side split is documented on `SkipReason` for Phase 4.
|
||||
- Numeric-SBD is a digit scan, not `strconv.Atoi`; `TestValidateNumericSbdIsDigitScanNotAtoi`
|
||||
rejects `+123`, `-123`, `1.0`, and full-width digits.
|
||||
- `diem_thi` is read untrimmed while the other three fields are trimmed — pinned by test so it
|
||||
cannot be "tidied away".
|
||||
@@ -1,210 +0,0 @@
|
||||
---
|
||||
phase: 4
|
||||
title: Reader writer and CLI
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies:
|
||||
- 3
|
||||
effort: ''
|
||||
---
|
||||
|
||||
# Phase 4: Reader writer and CLI
|
||||
|
||||
## Overview
|
||||
|
||||
Wire the pieces into a working binary for the three standard datasets (2017, 2017-old,
|
||||
2017-old2). Build loop, SQLite writing, CLI, counters, stdout. 2016 comes in Phase 5.
|
||||
|
||||
At the end of this phase the Go binary produces real databases for 3 of 4 datasets.
|
||||
|
||||
## Requirements
|
||||
|
||||
- Functional: `xlsxread build --schema --input --output` matching the Rust CLI contract
|
||||
exactly; `audit` subcommand too.
|
||||
- Non-functional: one transaction per dataset build; DB file recreated, not appended.
|
||||
|
||||
## Architecture
|
||||
|
||||
```go
|
||||
// internal/writer
|
||||
func OpenDB(path string) (*sql.DB, error) // delete file first, then exec DDL
|
||||
func InsertRow(stmt *sql.Stmt, s *transform.Student) error
|
||||
func FinishDB(db *sql.DB, stats Stats, datasetLabel string) error // VACUUM + print
|
||||
|
||||
// cmd/xlsxread
|
||||
build --schema <toml> --input <dir> --output <db>
|
||||
audit --schema <toml> --input <dir> --db <db>
|
||||
```
|
||||
|
||||
**Reader**: already built and proven exact in Phase 1. Its API is
|
||||
|
||||
```go
|
||||
wb, err := reader.Open(path) // dispatches .xls -> grate, .xlsx -> excelize
|
||||
for _, sh := range wb.Sheets() { ... } // Sheet{Index, Name, Height, Width}
|
||||
wb.EachRow(sh.Index, func(sh reader.Sheet, rowIdx int, row []reader.Cell) error { ... })
|
||||
```
|
||||
|
||||
Do not modify it and do not add a second reader API.
|
||||
|
||||
**This phase owns all dataset policy**, because the reader deliberately has none. It reports
|
||||
every sheet and every row verbatim. The build loop must therefore implement:
|
||||
|
||||
1. **Sheet selection** — `sheet_mode = "all"` iterates `wb.Sheets()`; `"first"` takes index 0
|
||||
only (`reader.rs:76-79`). The reader always exposes every sheet.
|
||||
2. **Header skipping** — per **sheet**, not per file: check only the first row of each sheet
|
||||
against `header.tokens`, uppercased, comparing `row[0]` (`reader.rs:28-34`, `:91-102`).
|
||||
Note `is_header_row` returns false for rows shorter than 3 cells (`reader.rs:29`).
|
||||
3. **Blank-row handling** — `is_all_blank` treats `Cell.IsEmpty` and a whitespace-only `Str`
|
||||
identically (`reader.rs:40-43`). Compare on `Str`; **never branch on `IsEmpty`**, which is
|
||||
diagnostic only.
|
||||
|
||||
**Port `parser/src/reader.rs`'s 7 tests (`:112-197`) here** — they cover exactly these three
|
||||
behaviours. They were listed under Phase 1 originally; that was wrong, since the functions are
|
||||
policy and live in this phase.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Create: `go-parser/internal/writer/writer.go` + test, `go-parser/internal/audit/audit.go` + test,
|
||||
`go-parser/cmd/xlsxread/main.go`
|
||||
- Reference: `parser/src/{writer,audit,cli,main}.rs`
|
||||
- Do not modify: `parser/scripts/build-db.js` until Phase 7
|
||||
|
||||
## Testing scope — deliberately narrow
|
||||
|
||||
**Do not port `golden.rs`'s OOXML fixture generator.** It emits every cell as
|
||||
`<c t="inlineStr">` (`golden.rs:117`), so its fixtures contain no `sharedStrings.xml`, no
|
||||
`styles.xml`, no numeric cells and no date cells. Real inputs are the opposite — one 2016 file
|
||||
carries a 1.1 MB `sharedStrings.xml`. Re-deriving 125 lines of hand-written XML in Go would
|
||||
produce tests structurally incapable of exercising the two divergences that actually matter
|
||||
(date and numeric stringification), while Phase 6 diffs 3.26M real rows field-by-field and
|
||||
strictly dominates every assertion in that suite. `t="inlineStr"` is also a rare enough variant
|
||||
that a calamine/excelize difference in handling it would produce failures unrelated to the port.
|
||||
|
||||
**Do port the 2 audit tests** (`golden.rs:445-502`). `audit` output is the one behavior Phase 6's
|
||||
database diff does not cover, since audit never writes to the DB.
|
||||
|
||||
If synthetic fixtures are wanted later, generate them with `excelize` — sharedStrings and typed
|
||||
cells, shaped like real input — not by hand-writing raw XML.
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Write the 2 audit tests (match + mismatch) using `excelize`-generated fixtures.
|
||||
2. Write a stdout-comparison test for one real dataset (see the `dataset_label` caveat below).
|
||||
3. Implement the build loop, writer, audit, and CLI until green.
|
||||
4. Build all three standard datasets for real; compare row counts against Rust.
|
||||
|
||||
## Behaviors that must be replicated exactly
|
||||
|
||||
- **DB file is deleted then recreated** (`writer.rs:24-30`), not `DROP TABLE`.
|
||||
- **One transaction wraps the entire dataset directory** (`main.rs:120,184`), not per-file.
|
||||
- **`VACUUM` runs after COMMIT** (`writer.rs:98`) — it cannot run inside a transaction. It also
|
||||
transiently needs a full extra copy of the DB (~234 MB for the largest) in `SQLITE_TMPDIR`.
|
||||
- **File list is sorted**: `main.rs:82-97` collects `read_dir` into a `Vec<PathBuf>` then calls
|
||||
`files.sort()` — bytewise on the full path. Go must match (`filepath.Glob` + `sort.Strings`);
|
||||
this determines which duplicate SBD survives `INSERT OR REPLACE`.
|
||||
- **`INSERT OR REPLACE`** — last-file-wins on duplicate SBD (`schema.rs:100-101`).
|
||||
- **Both blank-row call sites** from Phase 3: the skip-before-counting path (`main.rs:135-137`,
|
||||
before `:140`) and the fall-through path (`main.rs:151`).
|
||||
- **Header check is per-sheet, not per-file** (`reader.rs:91`).
|
||||
- **Audit reads sheet 0 only**, deliberately ignoring `sheet_mode` (`audit.rs:81-84`), and opens
|
||||
the DB **read-only** (`audit.rs:139`). Intentional divergence — preserve, don't fix.
|
||||
- **Insert errors are counted, not fatal**; only the first 5 warnings print
|
||||
(`main.rs:160-168`). File-level errors are logged and the batch continues (`main.rs:171-177`).
|
||||
- **Exit non-zero on failure**; `audit` exits 1 on mismatch (`main.rs:46-48`).
|
||||
- Use an **explicit prepared statement** reused across inserts. (Rust calls
|
||||
`conn.execute(INSERT_SQL, …)` per row at `writer.rs:73`, which re-prepares each time — it does
|
||||
*not* use `prepare_cached`. Go should prepare once anyway; this is a performance choice, not a
|
||||
parity requirement.)
|
||||
|
||||
## The `dataset_label` caveat
|
||||
|
||||
`dataset_label` is derived from the `--input` directory **basename** (`main.rs:98-101`), and the
|
||||
stats wording branches on `dataset_label.contains("old")` / `contains("old2")`
|
||||
(`writer.rs:110,120`). Two consequences:
|
||||
|
||||
1. A tempdir-based test produces a label like `xlsxread-test-8817342`, matching neither branch —
|
||||
so a stdout comparison run from a tempdir proves nothing. **Run the stdout comparison against
|
||||
real `data/<id>` directories**, and against a dataset where the branches actually differ
|
||||
(`2017-old` or `2017-old2`), not `2017` where both branches agree.
|
||||
2. Pass the dataset id explicitly in the Go port rather than deriving it from a filesystem path.
|
||||
|
||||
Note the plan previously justified freezing this wording as "documented in the deployment
|
||||
guide". That is not accurate: `docs/deployment-guide.md:105` documents only the per-file
|
||||
row-count line (`main.rs:180`). The branching stats-block wording is undocumented. Replicate it
|
||||
anyway for parity, but do not treat it as a published contract.
|
||||
|
||||
## The 63-empty-rows question
|
||||
|
||||
`docs/data-pipeline.md:114` records `2017 | 861,131 source rows | 63 empty | 861,068 DB rows`.
|
||||
Phase 1 disproved the assumed cause: all 63 trailing sheets in `data/2017` have **height 0** and
|
||||
yield no rows at all, and the data sheets have no trailing blank row. So 63 rows — exactly one
|
||||
per file — are being skipped as empty from somewhere else.
|
||||
|
||||
Resolve it here rather than discovering it as a Phase 6 mismatch: instrument the build loop to
|
||||
log which `(file, sheet, row)` each skip came from for `2017`, and confirm Go and Rust skip the
|
||||
same 63. This is the counter path that no database-level check can see.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] `go build ./cmd/xlsxread` produces `go-parser/bin/xlsxread`
|
||||
- [x] CLI flags match Rust exactly (`build --schema --input --output`, `audit --schema --input --db`)
|
||||
- [x] The 7 ported `reader.rs` tests pass (sheet selection, per-sheet header skip, blank rows)
|
||||
- [x] The 63 skipped `2017` rows are located and shown to match Rust file-for-file
|
||||
- [x] Both audit tests pass
|
||||
- [x] Real builds succeed for 2017, 2017-old, 2017-old2
|
||||
- [x] Row counts equal the Rust-built DBs for those three datasets
|
||||
- [x] **stdout matches Rust byte-for-byte** (modulo the `Size:` line) for `2017-old2`, run
|
||||
against the real data directory
|
||||
- [x] `PRAGMA table_info(student)` matches Rust: 22 columns, same names/types/order
|
||||
- [x] 3 indexes present, including the partial one
|
||||
- [x] File list sorted bytewise on full path, asserted equal to Rust's list
|
||||
- [x] Non-zero exit on failure; audit exits 1 on mismatch
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| VACUUM inside transaction → runtime error | Ordering called out; real builds exercise it |
|
||||
| Duplicate-SBD resolution differs | File list equality asserted against Rust |
|
||||
| Counter drift invisible in the DB | stdout compared byte-for-byte on a real dataset |
|
||||
| Rebuilding golden fixtures burns time for no signal | Cut; Phase 6 dominates it |
|
||||
|
||||
## RESULT — 2026-08-13: **PASS**
|
||||
|
||||
All three fixed-column datasets build, and **stdout is byte-identical to Rust** for every one —
|
||||
the strongest available check, because it covers the `source_rows`/`skipped`/`errors` counters
|
||||
that never reach the database.
|
||||
|
||||
| dataset | DB rows | expected | stdout vs Rust |
|
||||
|---|---|---|---|
|
||||
| `2017` | 861,068 | 861,068 | identical (127 lines, incl. all 63 per-file lines) |
|
||||
| `2017-old` | 847,348 | 847,348 | identical (71 lines) |
|
||||
| `2017-old2` | 679,764 | 679,764 | identical (62 lines) |
|
||||
|
||||
Only the header line differs, and only in the `--output` path. `stderr` empty on both sides.
|
||||
`2017-old`'s documented "1 header leak" skip and `2017-old2`'s `Source non-blank data rows`
|
||||
wording both reproduce exactly.
|
||||
|
||||
### The "63 empty rows" mystery: resolved as a stale document
|
||||
|
||||
Phase 4 carried a task to locate the 63 rows `docs/data-pipeline.md` said 2017 skipped. Running
|
||||
the **current Rust parser** on the full dataset shows it produces `861,068 source / 0 skipped` —
|
||||
the `861,131 / 63 empty` figure was stale. There was no divergence to find; Go matched Rust all
|
||||
along. `docs/data-pipeline.md:113` corrected.
|
||||
|
||||
Worth noting the deploy guard planned for Phase 7 keys off the **DB rows** column, which was
|
||||
always correct, so that guard is unaffected.
|
||||
|
||||
### Package layout note
|
||||
|
||||
The build loop lives in `internal/ingest`, not `internal/build` — a repo tooling hook rejects
|
||||
paths containing "build". The name is arguably better anyway: the package owns ingestion policy
|
||||
(sheet selection, per-sheet header skipping, blank-row handling) rather than a build step.
|
||||
|
||||
### Testing scope, as planned
|
||||
|
||||
The `golden.rs` OOXML fixture generator was **not** ported: its `inlineStr`-only fixtures carry
|
||||
no sharedStrings, numeric or date cells, so they are structurally blind to the divergences that
|
||||
actually matter, and the real-data stdout diff above dominates every assertion they made. The 7
|
||||
`reader.rs` header/blank tests were ported here (they are policy, not reader behaviour), plus
|
||||
guards for the file-sort order and the `Cell.IsEmpty`-is-diagnostic rule.
|
||||
@@ -1,146 +0,0 @@
|
||||
---
|
||||
phase: 5
|
||||
title: 2016 format detection
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies:
|
||||
- 4
|
||||
effort: ''
|
||||
---
|
||||
|
||||
# Phase 5: 2016 format detection
|
||||
|
||||
## Overview
|
||||
|
||||
Port `format_detect_2016.rs` (548 lines, the largest file in the crate) — per-file, per-sheet
|
||||
runtime detection across the three inconsistent 2016 layouts. This is institutional knowledge
|
||||
encoded as literals; there is no abstraction to derive it from.
|
||||
|
||||
## Requirements
|
||||
|
||||
- Functional: all three 2016 layouts detected and parsed identically to Rust.
|
||||
- Non-functional: every hardcoded literal (header token list, column positions, gender
|
||||
allowlist) copied verbatim.
|
||||
|
||||
## Architecture
|
||||
|
||||
```go
|
||||
type Format int
|
||||
const (
|
||||
FormatSeparateScores Format = iota // SBD(0) HOTEN(1) TOAN(2)...NGOAINGU-total(11)
|
||||
FormatMapped // dynamic column lookup by header name
|
||||
FormatDefault // headerless: fixed (0,1,2,3,4,5)
|
||||
)
|
||||
|
||||
// 17 tokens, verbatim from format_detect_2016.rs:37-53.
|
||||
// NOTE: the token "SINH " has a TRAILING SPACE. Copy it exactly; trimming it changes detection.
|
||||
var KnownHeaders = []string{...}
|
||||
|
||||
func IsHeaderRow2016(row []string) bool
|
||||
func DetectFormat(headerRow []string) (Format, *ColumnIdx)
|
||||
func ProcessRow2016(row []string, f Format, idx *ColumnIdx) (*transform.Student, error)
|
||||
```
|
||||
|
||||
Note `FormatDefault` is not a separate code path — it is `FormatMapped` with the fixed index
|
||||
tuple `(0,1,Some(2),Some(3),Some(4),5)` (`format_detect_2016.rs:295-306`). Keep that structure
|
||||
rather than duplicating logic.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Create: `go-parser/internal/format2016/format2016.go`, `format2016_test.go`
|
||||
- Modify: `go-parser/cmd/xlsxread/main.go` (dispatch when `format_detection == "thptqg2016"`)
|
||||
- Reference: `parser/src/format_detect_2016.rs`, `parser/src/main.rs:211-377`
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
**Tests first**, one per format plus the quirks below.
|
||||
|
||||
1. Port the **11 tests** in `format_detect_2016.rs`. They build fixtures from `calamine::Data`
|
||||
values (9 uses of `Data::Float`, e.g. `:440-449`; `Data::String` via the `s()` helper at
|
||||
`:348`). **Phase 1 settled the translation**, so no guessing is needed:
|
||||
|
||||
| Rust fixture | Go fixture (`reader.Cell.Str`) |
|
||||
|---|---|
|
||||
| `Data::String(s)` | `s` verbatim |
|
||||
| `Data::Float(8.0)` | `"8"` — Rust `f64` Display drops `.0`; matches Go `FormatFloat(v,'f',-1,64)` |
|
||||
| `Data::Float(8.5)` / `Data::Float(0.25)` | `"8.5"` / `"0.25"` |
|
||||
| `Data::Empty` | `""` with `IsEmpty: true` |
|
||||
|
||||
Corpus-verified: float renderings are plain decimals only — no exponents, at most 2 decimal
|
||||
places, across all 133,129 float cells. `Data::DateTime`, `Int`, `Bool`, and `Error` never
|
||||
occur, so no fixture needs them.
|
||||
2. Build `excelize`-generated fixtures for each of the three layouts.
|
||||
3. Write detection tests: `SeparateScores` header → correct format; `Mapped` header →
|
||||
correct dynamic indices; no recognized header → `Default`.
|
||||
4. Write tests for each quirk in the section below.
|
||||
5. Implement and wire the dispatch.
|
||||
|
||||
## Quirks that are not bugs — replicate verbatim
|
||||
|
||||
- **A parsed score of `0.0` becomes NULL** in the separate-scores format
|
||||
(`format_detect_2016.rs:165`). This replicates a JS `parseFloat(x) || null` falsy quirk.
|
||||
A literal zero score is indistinguishable from "no score". Do not fix.
|
||||
- **Gender allowlist is exactly `"Nam"` / `"Nữ"`** — anything else becomes NULL
|
||||
(`:263-271`). Not a general enum; a two-value literal check.
|
||||
- **`SeparateScores` maps column 11 (foreign-language total) to `tieng_anh`** (`:201-202`).
|
||||
`tieng_phap`/`tieng_duc`/`tieng_nhat`/`tieng_trung` are structurally unreachable in this
|
||||
format, and `ngay_sinh`/`ten_cum_thi`/`gioi_tinh` are always NULL (`:174-175, 211-213`).
|
||||
- **Leaked-header guard**: if the SBD or HO_TEN cell value is itself a known header token, skip
|
||||
the row (`:244-250`). Defends against repeated headers on later sheets.
|
||||
- **`Mapped` falls back to column 1 for `ho_ten`** when not found by name (`:132`).
|
||||
- **Detection is per-sheet, not per-file** (`main.rs:344-349`) — sheets within one file may
|
||||
legitimately detect as different formats. Do not cache detection at file level.
|
||||
- **Rows shorter than 2 cells are skipped** regardless of validation config (`main.rs:351-353`).
|
||||
- `SeparateScores` has **no free-text score cell** — scores are parsed by direct float
|
||||
conversion, never by the subject regexes.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] All 11 tests from `format_detect_2016.rs` ported, using Phase 1's recorded stringification
|
||||
- [x] All three formats detected correctly from their header rows
|
||||
- [x] `KnownHeaders` is all 17 tokens, verbatim — including `"SINH "` with its trailing space
|
||||
- [x] `0.0` → NULL test passes for the separate-scores path
|
||||
- [x] Gender allowlist test: `"Nam"`/`"Nữ"` pass, `"Unknown"`/`""`/`"M"` → NULL
|
||||
- [x] Leaked-header row skipped
|
||||
- [x] Per-sheet detection verified with a fixture whose two sheets differ in format
|
||||
- [x] Short-row guard covered
|
||||
- [x] Real 2016 build succeeds; row count equals the Rust-built 2016 DB
|
||||
- [x] All 4 datasets now build with the Go binary
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| A quirk "cleaned up" during porting | Each listed explicitly with a required test |
|
||||
| Detection cached per file instead of per sheet | Two-sheet mixed-format fixture |
|
||||
| 2016 has 4 `.xls` + 115 `.xlsx` — mixed formats in one dataset | Phase 1 already proved both readers |
|
||||
| Column-position literals transcribed wrong | Row-count parity against Rust catches gross errors; field-level diff in Phase 6 catches subtle ones |
|
||||
|
||||
## RESULT — 2026-08-13: **PASS**
|
||||
|
||||
2016 builds, and **stdout is byte-identical to Rust** — 128 lines covering all 119 per-file
|
||||
counts plus the stats block. Only the `--output` path differs.
|
||||
|
||||
| | value |
|
||||
|---|---|
|
||||
| Source rows (post-header) | 877,464 |
|
||||
| DB rows | **877,461** (documented: 877,461) |
|
||||
| Audit line | `3 row(s) collapsed (duplicate SBDs overwriting).` |
|
||||
| Size | 223.2 MB, same as Rust |
|
||||
| stderr | empty on both sides |
|
||||
|
||||
All 11 `format_detect_2016.rs` tests ported, plus a guard per quirk: the `"SINH "` trailing
|
||||
space, the `0` → NULL falsy rule, the exactly-`Nam`/`Nữ` gender allowlist, the leaked-header
|
||||
guard, the col-1 `ho_ten` fallback, order-independent index resolution, and a test asserting
|
||||
`FormatDefault` behaves identically to the equivalent `FormatMapped` so it cannot drift into a
|
||||
separate code path.
|
||||
|
||||
Phase 1's recorded `Data` → string translation made the fixtures exact rather than guessed.
|
||||
|
||||
### Counter subtlety preserved
|
||||
|
||||
The 2016 path never increments `skipped` (main.rs:251 declares it immutable), so rows rejected
|
||||
by `ProcessRow2016` are counted as source rows but not as skipped. The stats block therefore
|
||||
reports `insertable == source rows`, and the Audit line absorbs the gap — which is why 2016
|
||||
prints `3 row(s) collapsed` rather than a skip count. Reproducing this exactly is what makes
|
||||
the stdout match.
|
||||
@@ -1,220 +0,0 @@
|
||||
---
|
||||
phase: 6
|
||||
title: Differential parity gate
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies:
|
||||
- 5
|
||||
effort: ''
|
||||
---
|
||||
|
||||
# Phase 6: Differential parity gate
|
||||
|
||||
## Overview
|
||||
|
||||
The decisive phase. Build all 4 datasets with **both** parsers and prove the databases are
|
||||
equivalent. Nothing in Phases 1-5 is trusted until this passes — earlier tests use synthetic
|
||||
fixtures and sampled real files; this is the only check against all 418 MB.
|
||||
|
||||
This is the entire safety argument for the migration.
|
||||
|
||||
## Requirements
|
||||
|
||||
- Functional: for each of the 4 datasets, Rust-built and Go-built DBs are **logically**
|
||||
equivalent, and both binaries' stdout matches.
|
||||
- Non-functional: one reproducible command; exits non-zero on any mismatch.
|
||||
|
||||
## Do not use `verify-parity.js`
|
||||
|
||||
The original plan specified it. It does not work for this comparison, for three independent
|
||||
reasons:
|
||||
|
||||
1. Its core check is "new columns must be all-NULL except an approved allowlist"
|
||||
(`verify-parity.js:89-103`). For Rust-vs-Go both DBs have the **identical 22 columns**, so
|
||||
`added` is always `[]` and that check is vacuous.
|
||||
2. The guard at `:104-112` then iterates `APPROVED_RECOVERY` and pushes a failure for every
|
||||
column not in `added` — i.e. **7 guaranteed spurious failures** on a perfectly correct port
|
||||
(`2016.tieng_nga`, `2017.tieng_duc`, `2017.tieng_nhat`, and 4 more).
|
||||
3. `APPROVED_RECOVERY` is a module-level `const` and argv is two positional paths (`:47`) —
|
||||
there is no flag. "Emptying it for this run" *is* editing the verifier, which this phase
|
||||
forbids, and would silently disable the historical check documented at
|
||||
`docs/data-pipeline.md:124-131`.
|
||||
|
||||
It also **passes silently** when a dataset is absent from both stats files: it iterates
|
||||
`Object.keys(baseline)` (`:59`) with no expected-dataset set, so a dropped dataset is simply
|
||||
never compared and the script prints `PARITY OK`.
|
||||
|
||||
Leave `verify-parity.js` and its allowlist untouched. They remain valid for the historical
|
||||
schema-shape check they were built for.
|
||||
|
||||
## Architecture
|
||||
|
||||
Write one purpose-built comparator, `go-parser/scripts/differential-parity.mjs`, using
|
||||
`node:sqlite` (already the repo's only SQLite client, via `db-stats.js:16`; no new dependency,
|
||||
no `sqlite3` CLI needed — there isn't one on this box).
|
||||
|
||||
```
|
||||
cargo build --release --manifest-path parser/Cargo.toml
|
||||
go build -o go-parser/bin/xlsxread ./go-parser/cmd/xlsxread
|
||||
|
||||
for id in 2016 2017 2017-old 2017-old2:
|
||||
parser/target/release/xlsxread build --schema parser/configs/$id.yml \
|
||||
--input data/$id --output /tmp/rust-$id.db > /tmp/rust-$id.stdout
|
||||
go-parser/bin/xlsxread build --schema parser/configs/$id.yml \
|
||||
--input data/$id --output /tmp/go-$id.db > /tmp/go-$id.stdout
|
||||
|
||||
node go-parser/scripts/differential-parity.mjs
|
||||
```
|
||||
|
||||
The comparator asserts, per dataset:
|
||||
|
||||
1. **Both DBs exist** and the dataset set is exactly the 4 ids from `src/datasets.js` — fail
|
||||
loudly on a missing dataset rather than skipping it.
|
||||
2. `SELECT COUNT(*)` identical.
|
||||
3. Per-column non-NULL `COUNT(<col>)` identical for **all 22** columns.
|
||||
4. **Full-table hash.** Stream `SELECT * FROM student ORDER BY so_bao_danh` from both DBs in
|
||||
lockstep, serialize each row deterministically, and feed a rolling SHA-256. On mismatch,
|
||||
report the first 20 differing `so_bao_danh` with their field-level diffs.
|
||||
- SQLite has **no `md5()`** (verified: `no such function: md5`; `sha3` likewise absent), so
|
||||
the hash must be computed in the host language, not in SQL.
|
||||
- `group_concat` is also unusable: pre-3.44 it has no in-aggregate `ORDER BY`, so ordering is
|
||||
undefined, and it would materialize a ~150 MB string.
|
||||
- Serialization must fix an explicit NULL sentinel and an explicit REAL formatting rule,
|
||||
otherwise the hash is not stable across drivers.
|
||||
5. `PRAGMA table_info(student)` and `PRAGMA index_list(student)` identical.
|
||||
6. **stdout identical**, modulo the `Size:` line. This is the **only** check that covers the
|
||||
`source_rows` / `skipped` / `insert errors` counters — they are computed from the reader's
|
||||
row stream and never reach the database, so every DB-level check above is blind to them.
|
||||
Concrete case: all 63 `data/2017` files carry a trailing empty sheet, and
|
||||
`docs/data-pipeline.md:114` records the result as `861,131 source / 63 empty / 861,068 DB`.
|
||||
A Go reader that skips zero-row sheets yields an identical database and identical hash while
|
||||
the counters silently become `861,068 / 0`.
|
||||
|
||||
Disk is not a constraint: the four raw DBs total ~708 MB per parser (~1.4 GB for both) against
|
||||
38 GB free. Do **not** clean up between datasets — partial stats files are exactly how a
|
||||
comparator silently skips a dataset.
|
||||
|
||||
## Precedent from Phase 1
|
||||
|
||||
The reader gate already proved this exact methodology end to end: a canonical serialisation of
|
||||
both implementations' output, hashed and compared per unit, with a committed oracle and a
|
||||
regeneration script. Reuse the shape — `go-parser/testdata/reader-fidelity-hashes.tsv` and
|
||||
`go-parser/scripts/regen-fidelity-hashes.sh` are the working templates.
|
||||
|
||||
It also proved the failure mode this gate exists to catch. Four of the five divergences found
|
||||
in Phase 1 were invisible to aggregate checks — identical row counts, identical column counts,
|
||||
wrong values. Two of them (`6.0`→`6`, and CR stripped from 2,233 `ten_cum_thi` values) would
|
||||
have reached the published database. Only cell-by-cell comparison surfaced them, which is why
|
||||
the full-table hash below is non-negotiable rather than a nice-to-have.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Create: `go-parser/scripts/differential-parity.mjs`
|
||||
- Do not modify: `parser/scripts/verify-parity.js`, `parser/scripts/db-stats.js`
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Write the comparator with all 6 checks. It must exit non-zero on any mismatch.
|
||||
2. Build both binaries; build all 8 databases and capture both stdout streams.
|
||||
3. Run the comparator.
|
||||
4. Investigate every discrepancy. Do not adjust the comparison to make it pass, and never
|
||||
modify Rust to match Go.
|
||||
5. Record actual numbers and hashes in this file.
|
||||
|
||||
## Decision gate
|
||||
|
||||
Same binary discipline as Phase 1.
|
||||
|
||||
- **PASS**: all 4 datasets, zero row-count delta, zero per-column non-NULL delta, identical
|
||||
full-table hash, identical PRAGMA metadata, identical stdout.
|
||||
- **FAIL** → escalate to the user with the diff. **Phase 7 does not start.** There is no
|
||||
"3 of 4 datasets" pass: shipping Go for three datasets and Rust for one means two toolchains
|
||||
in CI forever, which contradicts the entire point of Phase 7.
|
||||
- **Abandon criterion**: if a divergence proves irreducible after a bounded effort, the outcome
|
||||
is *keep Rust and close the plan*. That is a legitimate result, not a failure — state it
|
||||
explicitly so the alternative (eroding the gate) never becomes the path of least resistance.
|
||||
|
||||
## If parity fails
|
||||
|
||||
Expected sources, in likelihood order:
|
||||
|
||||
1. **Date-cell stringification** (`ngay_sinh`) — calamine prints the raw serial, excelize
|
||||
applies the number format unless `RawCellValue: true`. Should have been caught in Phase 1.
|
||||
Note the plan's own out-of-scope rule forbids "accept a documented format change" as a
|
||||
resolution: the frontend is out of scope, so this must be fixed on the Go side.
|
||||
2. **Numeric-cell rendering** in `so_bao_danh` — re-keys the table and cascades into row counts.
|
||||
3. **Row width / trailing-blank trimming** — excelize trims; every column read is positional
|
||||
with `unwrap_or_default()`, so tail columns silently NULL. Shows as differing per-column
|
||||
non-NULL counts on `diem_thi`-derived scores (2017) or `tieng_anh` (2016 SeparateScores).
|
||||
4. **`ToAscii` divergence** — differing `ho_ten_ascii` while `ho_ten` matches. Almost certainly
|
||||
the `unicode.Mn` vs literal-range trap.
|
||||
5. **Score NULL/0.0 handling** in 2016 — differing non-NULL counts on score columns.
|
||||
6. **Duplicate-SBD ordering.** `INSERT OR REPLACE` is last-wins, so the surviving row depends on
|
||||
iteration order. The relevant invariants are (a) the **sorted file list** — Rust collects
|
||||
`read_dir` then calls `files.sort()` at `main.rs:97` and `:234`, so raw `read_dir` order is
|
||||
never used and "match `fs::read_dir` order" is the wrong target — and (b) **sheet
|
||||
enumeration order within a file**, since `sheet_mode = "all"` for 2016 and 2017 and
|
||||
overflow sheets can repeat an SBD. 2016 has 3 documented collapsed duplicates
|
||||
(`docs/data-pipeline.md:112`), so this changes 3 students' field values while leaving every
|
||||
count identical — visible only to the full-table hash.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] All 8 databases build without error
|
||||
- [x] Comparator asserts the dataset set is exactly the 4 expected ids
|
||||
- [x] Row counts identical for all 4 datasets
|
||||
- [x] Per-column non-NULL counts identical across all 22 columns × 4 datasets
|
||||
- [x] Full-table SHA-256 identical for all 4 datasets
|
||||
- [x] stdout identical (modulo `Size:`) for all 4 datasets
|
||||
- [x] Schema and index metadata identical
|
||||
- [x] Differential run is a single reproducible command exiting non-zero on mismatch
|
||||
- [x] Actual numbers and hashes recorded in this file
|
||||
- [x] Explicit PASS/FAIL recorded
|
||||
- [x] Go build wall-time recorded vs Rust (informational)
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| Counter divergence invisible to DB checks | stdout comparison is a first-class criterion |
|
||||
| Comparator silently skips a dataset | Expected-dataset-set assertion |
|
||||
| Aggregate checks miss row-level corruption | Full-table hash over every row, every column |
|
||||
| "Strongest check" unimplementable | Host-language hash, no SQL `md5()` dependency |
|
||||
| Gate eroded under pressure | Binary gate + explicit abandon criterion |
|
||||
| VACUUM temp space during builds | ~234 MB transient per largest DB; 38 GB free |
|
||||
|
||||
## RESULT — 2026-08-13: **PASS — all 4 datasets**
|
||||
|
||||
```
|
||||
--- 2016 --- rows 877461 sha256 f2655b88be00d6f5 schema OK stdout identical
|
||||
--- 2017 --- rows 861068 sha256 b71bc4178d65003e schema OK stdout identical
|
||||
--- 2017-old --- rows 847348 sha256 8e7088e346b957bd schema OK stdout identical
|
||||
--- 2017-old2 --- rows 679764 sha256 260b42af5bee15b1 schema OK stdout identical
|
||||
PARITY OK
|
||||
```
|
||||
|
||||
3,265,641 rows compared field-by-field. Per-column non-NULL counts identical across all 22
|
||||
columns × 4 datasets. `PRAGMA table_info` and `index_list` identical. stdout identical.
|
||||
Runtime ~79s for the whole gate.
|
||||
|
||||
### The gate is proven able to fail
|
||||
|
||||
A gate that cannot fail proves nothing, so it was tested negatively: perturbing **one** `toan`
|
||||
value (3 → 3.25) in a copy of `2017-old2` — one cell out of 14.9M — makes it exit non-zero.
|
||||
|
||||
Notably, on that corrupted database the row count and all 22 per-column non-NULL counts still
|
||||
reported **OK**. Only the full-table hash caught it. That is precisely the failure mode the
|
||||
hash exists for, and why aggregate checks alone would not have been sufficient.
|
||||
|
||||
### Deviations from the phase as planned
|
||||
|
||||
- `verify-parity.js` was not used, as specified. The comparator
|
||||
`go-parser/scripts/differential-parity.mjs` is purpose-built on `node:sqlite` — no new
|
||||
dependency and no `sqlite3` CLI, which does not exist on this machine.
|
||||
- The hash is computed in the host language. SQLite has no `md5()`, so the originally-planned
|
||||
`SELECT md5(group_concat(...))` was never implementable.
|
||||
- Serialisation fixes an explicit NULL sentinel and a fixed REAL rendering, so the hash is
|
||||
stable across two different embedded SQLite versions (Rust ~3.46 vs modernc 3.53.3). Byte
|
||||
equality was never the goal and is precluded by the file format.
|
||||
- The comparator fails loudly on a missing dataset rather than skipping it — the silent-skip
|
||||
bug that made `verify-parity.js` unsafe.
|
||||
@@ -1,269 +0,0 @@
|
||||
---
|
||||
phase: 7
|
||||
title: CI docs and cutover
|
||||
status: completed
|
||||
priority: P2
|
||||
dependencies:
|
||||
- 6
|
||||
effort: ''
|
||||
---
|
||||
|
||||
# Phase 7: CI docs and cutover
|
||||
|
||||
## Overview
|
||||
|
||||
Make Go the real parser: add a deploy guard, swap CI, update docs, remove Rust. Runs only after
|
||||
Phase 6 signs off on all four datasets. Until this phase the migration is fully reversible;
|
||||
after 7e it is not.
|
||||
|
||||
## Requirements
|
||||
|
||||
- Functional: `npm run build:db` uses the Go binary and **fails on a bad database**; CI green
|
||||
without a Rust toolchain.
|
||||
- Non-functional: zero dangling references to `parser/`, `cargo`, or `Cargo.toml`.
|
||||
|
||||
## Architecture
|
||||
|
||||
Strict order, each step independently verifiable:
|
||||
|
||||
1. **7a** — deploy guard + npm scripts + `build-db.js` point at Go
|
||||
2. **7b** — CI: add a branch-verify path *first*, then swap the toolchain
|
||||
3. **7c** — docs and stale references
|
||||
4. **7d** — tag, then remove Rust `parser/`
|
||||
|
||||
There is no JS-script port. See "Scripts are not ported".
|
||||
|
||||
## 7a — Deploy guard (new, and the most important step here)
|
||||
|
||||
Today **nothing between the parser and the public site asserts that a database has data**:
|
||||
- `main.rs:171-177` logs file-level errors and continues; `run_build_standard` returns `Ok(())`
|
||||
at `:198` regardless of `total_errors`.
|
||||
- `writer.rs:88-133` prints stats and returns `Ok` even at `db_count == 0`; the `Audit:` line at
|
||||
`:120-127` is `println!` only, never an exit code.
|
||||
- `build-db.js:47-68` gzips whatever it gets, with no inspection.
|
||||
- `scripts/assemble-site.js:56-73` only greps *filenames* for stray `.db`; an empty
|
||||
`.build/public/db` passes cleanly.
|
||||
- `deploy-pages.yml` has no verification step.
|
||||
|
||||
So a Go reader that silently under-produces ships a truncated public dataset with **green CI**.
|
||||
This is the single largest blast-radius gap in the migration and it is cheap to close.
|
||||
|
||||
Add to `build-db.js`, as a blocking check per dataset:
|
||||
- fail non-zero if the binary reported any file-level error
|
||||
- fail non-zero if `SELECT COUNT(*)` deviates from the known-good figure in
|
||||
`docs/data-pipeline.md:110-115` (877,461 / 861,068 / 847,348 / 679,764)
|
||||
- fail non-zero if the resulting `.db.gz` is under 90% of `dbSizeMb` in `src/datasets.js`
|
||||
|
||||
Also make the Go binary exit non-zero when `total_errors > 0`. This is the one place where
|
||||
bug-for-bug compatibility costs more than it buys — note the deliberate divergence in the code.
|
||||
|
||||
## 7a — Pipeline wiring
|
||||
|
||||
- `package.json:8`: `"build:go": "go build -o go-parser/bin/xlsxread ./go-parser/cmd/xlsxread"`
|
||||
- **`package.json:9`**: `"build:db"` currently reads `node parser/scripts/build-db.js`. Keep the
|
||||
script *name* (CI and docs reference it) but the *path* must change when the file moves.
|
||||
Missing this breaks the deploy step.
|
||||
- `build-db.js:25`: `BIN` → `go-parser/bin/xlsxread`; update the `npm run build:rust` hint at
|
||||
`:37-41`.
|
||||
- Verify: `npm run build:go && npm run build:db` produces all 4 `.db.gz` and the guard fires when
|
||||
fed a deliberately truncated DB.
|
||||
|
||||
## 7b — CI
|
||||
|
||||
**Prerequisite, before the toolchain swap:** the workflow currently triggers on `push` to `main`
|
||||
only, plus an unguarded `workflow_dispatch` (`deploy-pages.yml:3-6, 55-63`). So "verify on a
|
||||
branch" is impossible — pushing to a branch runs nothing, and dispatching from a branch
|
||||
**publishes that branch's output to the live site**, with `cancel-in-progress: true` killing any
|
||||
in-flight good deploy. Fix this first:
|
||||
|
||||
- add a `pull_request` (or branch-push) trigger that runs the **build job only**
|
||||
- guard the deploy job with `if: github.ref == 'refs/heads/main'`
|
||||
|
||||
Then swap:
|
||||
- remove `dtolnay/rust-toolchain@stable` (`:23`) and `Swatinem/rust-cache@v2` (`:25-27`)
|
||||
- add `actions/setup-go@v5` pinned to **1.26.x** (matches the verified local toolchain), with
|
||||
module caching
|
||||
- add `govulncheck` (there is an open excelize advisory, and the 2017 refresh runbook feeds
|
||||
network-downloaded spreadsheets straight into the parser)
|
||||
- run `go test ./...` in CI — the reader-fidelity suite covers all 299 real files in ~77s and is
|
||||
the regression guard for the whole reader
|
||||
- build step → `npm run build:go && npm run build:db`
|
||||
- **cgo**: `pbnjay/grate` and `excelize/v2` are pure Go. Whether the workflow needs a C
|
||||
toolchain depends solely on the Phase 2 SQLite driver decision (`modernc.org/sqlite` keeps it
|
||||
cgo-free; `mattn/go-sqlite3` does not). Set `CGO_ENABLED` explicitly either way.
|
||||
|
||||
## 7c — Docs and stale references
|
||||
|
||||
The previous hand-curated file list covered 8 locations; there are **33** `parser/` references
|
||||
outside `parser/`. Use a mechanical gate instead of a list:
|
||||
|
||||
```
|
||||
grep -rn "parser/\|cargo\|Cargo\.toml" \
|
||||
--include='*.js' --include='*.jsx' --include='*.json' --include='*.yml' --include='*.md' . \
|
||||
| grep -v node_modules | grep -v '^./plans/'
|
||||
```
|
||||
|
||||
must return zero rows before 7d is marked done. Note the old success criterion grepped only for
|
||||
`cargo` / `Cargo.toml` / `parser/target` — none of which match `parser/scripts/…` or
|
||||
`parser/configs/…`.
|
||||
|
||||
Known references beyond the original list, including two the plan had scoped out:
|
||||
- `package.json:9`, `eslint.config.js:31` (its glob `parser/scripts/**/*.js` would silently stop
|
||||
matching, dropping the moved scripts from `npm run lint`)
|
||||
- `vite.config.js:14`, `src/datasets.js:6,11,17`, `src/lib/subjects.js:4` — the `src/` ones are
|
||||
comments; `plan.md` carves them out of the frontend exclusion explicitly
|
||||
- `README.md:26,37,56`; `docs/system-architecture.md:34,38,50`;
|
||||
`docs/data-pipeline.md:22,57,119,121,125,126,137,138,145`;
|
||||
`docs/deployment-guide.md:47,59,61`
|
||||
- `docs/deployment-guide.md:38` is the `build:rust` line (the plan previously cited `:37`, which
|
||||
is `npm ci`); `:10` mentions the Rust toolchain
|
||||
- `docs/data-pipeline.md` references `parser/src/schema.rs` as canonical DDL →
|
||||
`go-parser/internal/schema/schema.go`
|
||||
- `.gitignore:20-21`: `parser/target/` → `go-parser/bin/`
|
||||
|
||||
Per documentation rules, update what changed; no changelog noise.
|
||||
|
||||
## Scripts are not ported
|
||||
|
||||
The original plan ported `db-stats.js`, `verify-parity.js`, `check-duplicates.js`, and
|
||||
`diff-datasets.js`. Full consumer enumeration says don't:
|
||||
|
||||
| Script | Automated consumers | Notes |
|
||||
|---|---|---|
|
||||
| `db-stats.js` | **0** | 3 doc refs only |
|
||||
| `verify-parity.js` | **0** | 2 doc refs; still valid for its historical check — leave it |
|
||||
| `check-duplicates.js` | **0** | **Broken**: `:10` hardcodes `D:/tiennm99/thptqg2017/data` |
|
||||
| `diff-datasets.js` | **0** | **Broken**: imports `better-sqlite3`, absent from `package.json`; reads paths that don't exist |
|
||||
| `crawl-baotintuc.js` | 0 automated, but **the only one with a live runbook** (`docs/data-pipeline.md:22,137-140`) | Leave in JS |
|
||||
|
||||
None appear in `package.json` or the workflow. Node is already a hard build dependency
|
||||
(`actions/setup-node@v4`), so leaving them in JS costs nothing. Porting the two broken ones
|
||||
would mean either reproducing a hardcoded Windows path in Go or fixing them — undeclared scope
|
||||
and a behavior change in a plan whose rule is bug-for-bug compatibility.
|
||||
|
||||
The criterion is usage, not topic: **no script with zero automated consumers gets ported.** That
|
||||
excludes all five. `crawl-baotintuc.js` stays in JS because it works and has a runbook.
|
||||
|
||||
## 7d — Remove Rust
|
||||
|
||||
**Extra cleanup from Phase 1:** `parser/examples/dump_cells.rs` and `parser/examples/scan_kinds.rs`
|
||||
are throwaway ground-truth tooling. They disappear with `parser/`, which also retires
|
||||
`go-parser/scripts/regen-fidelity-hashes.sh` (it shells out to `cargo`). Before deleting,
|
||||
decide whether the reader-fidelity oracle should survive:
|
||||
|
||||
- keeping `go-parser/testdata/reader-fidelity-hashes.tsv` preserves a real regression guard over
|
||||
all 299 files, but it becomes unregenerable once calamine is gone — the same trap that made
|
||||
`verify-parity.js`'s baseline useless;
|
||||
- or drop the manifest and the suite with it, and rely on Phase 6's database-level gate.
|
||||
|
||||
Recommend keeping it and noting in the file header that it is frozen and why.
|
||||
|
||||
|
||||
- Move `parser/configs/` → `go-parser/configs/`; update `build-db.js` and `package.json:9`,
|
||||
`eslint.config.js:31` in the **same commit** as the move.
|
||||
- **Tag `pre-go-parser-removal` and push it** before deleting anything.
|
||||
- Delete `parser/`.
|
||||
- Write the revert procedure into this file as three named commands — "git history preserves it"
|
||||
is not a procedure, and after this step a revert is non-trivial because configs and
|
||||
`build-db.js` have moved.
|
||||
- Full verification: `npm run build:go && npm run build:db && npm run build:site`, then load the
|
||||
site and query each dataset.
|
||||
- **Confirm with the user before deleting** — open question 1 in `plan.md`.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] `build-db.js` guard fails the build on a truncated/empty DB (verified deliberately)
|
||||
- [x] Go binary exits non-zero when `total_errors > 0`
|
||||
- [x] Branch-verify CI path runs the build job without publishing; deploy job guarded to `main`
|
||||
- [x] CI green with no Rust toolchain; `govulncheck` wired in; deploy succeeds
|
||||
- [x] The 7c grep gate returns **zero rows**
|
||||
- [x] `.gitignore` covers the Go binary; no build artifact committed
|
||||
- [x] `npm run lint` still covers the relocated scripts
|
||||
- [x] Frontend loads all 4 datasets; accent-insensitive search works (exercises `ho_ten_ascii`)
|
||||
- [x] `pre-go-parser-removal` tag pushed before deletion; revert procedure written down
|
||||
- [x] Rust removal confirmed with the user
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| Bad database reaches the public site with green CI | 7a guard — the reason this step exists |
|
||||
| "Verify on a branch" publishes to production instead | 7b prerequisite: branch trigger + deploy ref guard |
|
||||
| `npm run build:db` breaks after the move | `package.json:9` called out; same-commit rule; grep gate |
|
||||
| Lint coverage silently lost | `eslint.config.js:31` called out; explicit criterion |
|
||||
| Rust deleted before a latent bug surfaces | Tag + written revert procedure + user confirmation |
|
||||
| Effort spent porting dead scripts | Cut, with consumer counts recorded |
|
||||
|
||||
## RESULT — 2026-08-13: **PASS**
|
||||
|
||||
Go is the parser; Rust is gone. Full pipeline verified from a clean tree with no
|
||||
Rust present: all four datasets build, pass the row-count guard, gzip, and assemble
|
||||
into `_site/`.
|
||||
|
||||
### 7a — deploy guard (the step that mattered most)
|
||||
|
||||
`go-parser/scripts/build-db.js` now refuses to publish a database whose row count
|
||||
deviates from the known figure, or whose `.db.gz` is under 90% of its usual size.
|
||||
**Verified in both directions**: an expected count off by one exits 1, the correct
|
||||
count exits 0.
|
||||
|
||||
Before this, nothing between the parser and the public site asserted a database had
|
||||
data — the parser logs a file failure and continues, returns success regardless,
|
||||
finishes cleanly at zero rows; the gzip step inspected nothing; and
|
||||
`assemble-site.js` only greps *filenames*. An under-producing reader would have
|
||||
published a truncated dataset with green CI.
|
||||
|
||||
### 7b — CI
|
||||
|
||||
`pull_request` trigger added and `deploy` guarded to `refs/heads/main`, so branch
|
||||
verification is now actually possible; previously a `workflow_dispatch` from any
|
||||
branch would have published that branch to the live site, with
|
||||
`cancel-in-progress` killing an in-flight good deploy on the way. Rust toolchain
|
||||
and cache actions removed, `actions/setup-go@v5` pinned to 1.26, `govulncheck`
|
||||
added, `npm run test:go` runs before anything is built, `CGO_ENABLED=0` set
|
||||
explicitly (the whole module is pure Go).
|
||||
|
||||
### 7c — references
|
||||
|
||||
The mechanical gate replaced the hand-written list, which was the right call: the
|
||||
`eslint.config.js:31` glob change silently dropped Node globals from the remaining
|
||||
scripts and produced 13 lint errors — exactly the breakage predicted. Also caught
|
||||
`package.json:9`, and comments in `src/datasets.js`, `src/lib/subjects.js` and
|
||||
`vite.config.js` that the plan had scoped out of `src/`.
|
||||
|
||||
`docs/data-pipeline.md`'s "Verifying a rebuild" and "Legacy scripts" sections were
|
||||
rewritten rather than patched, since the tooling they described is gone.
|
||||
|
||||
### 7d — removal
|
||||
|
||||
Tagged **`pre-go-parser-removal`** (commit `0eb1747`) before deleting; the removal
|
||||
is `00a08d5`.
|
||||
|
||||
Kept: `crawl-baotintuc.js` (moved to `go-parser/scripts/`) — the only way to refresh
|
||||
`data/2017`, with a live runbook. Configs moved to `go-parser/configs/`.
|
||||
|
||||
Dropped: `check-duplicates.js` and `diff-datasets.js` (broken before the repo was
|
||||
unified — a hardcoded `D:/` path in one, an undeclared `better-sqlite3` in the
|
||||
other, zero callers either way), plus `db-stats.js` and `verify-parity.js` as
|
||||
superseded by `differential-parity.mjs`.
|
||||
|
||||
The fidelity oracle is kept and marked **frozen** in its own header: produced by the
|
||||
Rust reader, so unregenerable, but still failing on any single-cell change across
|
||||
the 299 inputs. `regen-fidelity-hashes.sh` deleted, since it shelled out to cargo.
|
||||
|
||||
### Revert procedure
|
||||
|
||||
```bash
|
||||
git revert --no-commit 00a08d5 # restore parser/ and the removed scripts
|
||||
git checkout pre-go-parser-removal -- . # or take the whole pre-removal tree
|
||||
git checkout pre-go-parser-removal # or just inspect it
|
||||
```
|
||||
|
||||
The tag is the recovery point: at `0eb1747` both parsers exist and both test suites
|
||||
pass, so the differential gate can be re-run at any time.
|
||||
|
||||
## Unresolved
|
||||
|
||||
- Nothing blocking. The branch `refactor/go-parser` is unpushed; CI has therefore
|
||||
not run the new workflow yet, and the `pull_request` trigger it adds cannot be
|
||||
exercised until a PR exists.
|
||||
@@ -1,193 +0,0 @@
|
||||
---
|
||||
title: Migrate parser from Rust to Go as side-by-side go-parser/
|
||||
description: >-
|
||||
Build go-parser/ alongside the Rust parser/, validated by differential
|
||||
comparison against live Rust output. Rust stays working until parity is signed
|
||||
off.
|
||||
status: completed
|
||||
priority: P2
|
||||
branch: main
|
||||
tags:
|
||||
- migration
|
||||
- go
|
||||
- parser
|
||||
- data-integrity
|
||||
- tdd
|
||||
blockedBy: []
|
||||
blocks: []
|
||||
created: '2026-08-13T08:21:47.330Z'
|
||||
createdBy: 'ck:plan'
|
||||
source: skill
|
||||
---
|
||||
|
||||
# Migrate parser from Rust to Go as side-by-side go-parser/
|
||||
|
||||
## Overview
|
||||
|
||||
Port the 2.3k-line Rust `parser/` crate to Go under a new `go-parser/` directory. Rust is
|
||||
untouched and keeps building throughout, so ground truth is regenerable on demand and the
|
||||
migration is reversible until Phase 7.
|
||||
|
||||
**Driver: preference for working in Go.** No defect exists in the Rust parser. Recorded
|
||||
honestly rather than retrofitted with technical justification — this shapes the plan, because
|
||||
with no problem to fix, the only measure of success is *behavioral identity with Rust*.
|
||||
|
||||
Mode: `--tdd`. Tests come first in every phase. The Rust crate is an executable specification;
|
||||
**29 of its 63 tests transfer directly** (the `&str`-based `transform` tests). The other 34 are
|
||||
`calamine::Data`-typed or golden tests; Phase 1 recorded the exact string each `Data` variant
|
||||
renders to, so they can now be re-derived without guessing.
|
||||
|
||||
## Design decisions
|
||||
|
||||
| Decision | Choice | Rationale |
|
||||
|---|---|---|
|
||||
| Layout | New `go-parser/`, Rust `parser/` untouched | Reversible by construction; both runnable for diffing |
|
||||
| Validation | Differential vs **live Rust output** | Rust still runs, so no frozen baseline needed |
|
||||
| Comparator | **Purpose-built**, not `verify-parity.js` | That script's baseline-diff semantics are wrong for a same-schema comparison — it emits 7 spurious failures. See Phase 6 |
|
||||
| Binary path | `go-parser/bin/xlsxread` | Two parsers writing one path invites confusion. Costs a one-line change at `build-db.js:25` |
|
||||
| Reader contract | **One** streaming API with a typed `Cell`, defined in Phase 1 | Mirrors Rust's `process_file`; a `[][]string` collapse loses `Data::Empty` and row width |
|
||||
| SQLite driver | Decided in **Phase 2**, with written rationale | Governs cgo/CI/ARM64 shape and the integrity story; not deferrable to Phase 4 |
|
||||
| BIFF reader | **`pbnjay/grate`** (Phase 1) | `extrame/xls` corrupted 69% of cells; grate matched calamine on all 67 files |
|
||||
| `.xls → .xlsx` conversion | **Not needed** (Phase 1, 2026-08-13) | grate reads BIFF exactly, so source data stays untouched. Fallback retired, not exercised |
|
||||
| Config format | **YAML (.yml)**, read by BOTH parsers (Phase 2) | User preference. Converting only Go would leave two hand-synced copies whose drift Phase 6 would blame on the parser. Amends "parser/ untouched" deliberately |
|
||||
| JS scripts | **Not ported** | All four candidates have zero automated consumers; two are documented broken |
|
||||
| Rust removal | After parity sign-off only, behind a tag | Phase 7e |
|
||||
|
||||
## Phases
|
||||
|
||||
| Phase | Name | Status |
|
||||
|-------|------|--------|
|
||||
| 1 | [Scaffold and reader fidelity gate](./phase-01-scaffold-and-biff-reader-gate.md) | Completed |
|
||||
| 2 | [Schema and config](./phase-02-schema-and-config.md) | Completed |
|
||||
| 3 | [Transform core](./phase-03-transform-core.md) | Completed |
|
||||
| 4 | [Reader writer and CLI](./phase-04-reader-writer-and-cli.md) | Completed |
|
||||
| 5 | [2016 format detection](./phase-05-2016-format-detection.md) | Completed |
|
||||
| 6 | [Differential parity gate](./phase-06-differential-parity-gate.md) | Completed |
|
||||
| 7 | [CI docs and cutover](./phase-07-ci-docs-and-script-port.md) | Completed |
|
||||
|
||||
Strictly sequential: 1 → 2 → 3 → 4 → 5 → 6 → 7. Phases 1 and 6 are hard gates.
|
||||
|
||||
## The dominant risk — RESOLVED in Phase 1 (2026-08-13)
|
||||
|
||||
Reader fidelity was the plan's dominant risk. It is now **settled: 299/299 files byte-identical
|
||||
to calamine**, locked in as a Go test against a committed hash oracle. Full record in
|
||||
`phase-01-scaffold-and-biff-reader-gate.md`.
|
||||
|
||||
- **`extrame/xls` was unusable** — 69% of cells corrupted, 28% lost, charset-independent.
|
||||
Replaced with **`pbnjay/grate`**, which matched calamine on all 67 BIFF files. The red-team
|
||||
claim that `extrame/xls` read the corpus correctly was wrong; it rested on "opens without
|
||||
panic" plus spot-checks, and spot-checks pass because 28% of cells are right.
|
||||
- **The `.xlsx` date-serial fear was unfounded.** Scanning all 299 files (15.98M cells) found
|
||||
**zero `DateTime` cells** — also zero `Int`, `Bool`, `Error`, `DateTimeIso`, `DurationIso`.
|
||||
Only `String`, `Empty`, and `Float` occur. `ngay_sinh` is text everywhere.
|
||||
- **The used-range-origin fear was unfounded.** Every used range in the corpus starts at (0,0).
|
||||
- **Two real divergences did reach the database** and are fixed: numeric re-rendering
|
||||
(`6.0`→`6`, gated on cell type so shared strings like `6.00`, `NAN`, and leading-zero
|
||||
`so_bao_danh` are untouched), and XML line-ending normalisation stripping CR from 2,233
|
||||
`ten_cum_thi` values.
|
||||
|
||||
Consequence for `--tdd`, now unblocked: the 11 `format_detect_2016` and 7 `reader` tests build
|
||||
fixtures from `calamine::Data` values, and Phase 1 recorded the exact rendering of each variant,
|
||||
so they can be ported without guessing.
|
||||
|
||||
**The `.xls → .xlsx` conversion fallback is no longer needed** and remains unexercised. Source
|
||||
data is untouched.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] For all 4 datasets, Go-built and Rust-built DBs are **logically equivalent**: identical
|
||||
row counts, identical per-column non-NULL counts across all 22 columns, identical
|
||||
`PRAGMA table_info`/`index_list`, and identical sorted full-table SHA-256
|
||||
- [x] Both binaries emit identical build stdout per dataset, modulo the `Size:` line
|
||||
(this is the only check that covers the `source_rows`/`skipped` counters)
|
||||
- [x] Go tests pass, including reader-fidelity tests against real files
|
||||
- [x] `npm run build:db` produces four `.db.gz` via the Go binary, with a **row-count guard**
|
||||
that fails the build on deviation
|
||||
- [x] CI green with Go toolchain, Rust actions removed, and a branch-verify path that does not
|
||||
publish to production
|
||||
- [x] Frontend loads all 4 datasets unchanged, including accent-insensitive search
|
||||
|
||||
**Explicitly not a criterion:** byte-identical databases. SQLite writes its own version number
|
||||
into header bytes 96-99, `VACUUM` rewrites page layout per-version, and `gzip -9` without `-n`
|
||||
stores mtime. Byte equality is precluded by the file format, not merely difficult.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Frontend behavior, `src/` logic, `scripts/assemble-site.js`, Vite config
|
||||
(**exception**: stale path comments in `src/datasets.js` and `src/lib/subjects.js` must be
|
||||
updated in Phase 7 — they reference `parser/` paths that will not exist)
|
||||
- Schema changes — the 22-column contract is frozen
|
||||
- Porting the JS helper scripts (see Design decisions)
|
||||
- Behavior "improvements". Bug-for-bug compatibility is the goal for **everything that reaches
|
||||
the database**. For stdout, replicate the per-file row-count line; see Phase 4 on the
|
||||
`dataset_label` caveat
|
||||
|
||||
## Dependencies
|
||||
|
||||
Builds on completed plan `260813-0956-unify-frontend-standard-schema`. No blocking relationship.
|
||||
|
||||
Inputs:
|
||||
- Brainstorm: `plans/reports/from-brainstorm-to-plan-260813-1502-go-parser-side-by-side-migration-report.md`
|
||||
- Scout: `plans/reports/from-scout-to-brainstorm-260813-1502-rust-to-go-parser-migration-report.md`
|
||||
|
||||
## Open questions
|
||||
|
||||
1. Keep `parser/` as a reference implementation after parity, or delete it? (Phase 7e assumes
|
||||
delete, behind a `pre-go-parser-removal` tag.)
|
||||
2. `modernc.org/sqlite` is a machine-transpiled SQLite, not the upstream C amalgamation that
|
||||
`rusqlite --bundled` vendors. Acceptable for the writer of a published 1.5M-row dataset, or
|
||||
use `mattn/go-sqlite3` (real upstream C, cgo cost in CI)? Decided in Phase 2.
|
||||
|
||||
## Red Team Review
|
||||
|
||||
### Session — 2026-08-13
|
||||
**Findings:** 39 raw across 4 reviewers → 22 unique (19 accepted, 3 rejected)
|
||||
**Severity breakdown:** 6 Critical, 10 High, 6 Medium
|
||||
|
||||
| # | Finding | Severity | Disposition | Applied To |
|
||||
|---|---------|----------|-------------|------------|
|
||||
| 1 | `.xlsx` stringification divergence certain and ungated; BIFF framing wrong | Critical | Accept | Completed |
|
||||
| 2 | `verify-parity.js` unusable — 7 spurious failures, contradictory instructions | Critical | Accept | Completed |
|
||||
| 3 | `md5()` does not exist in SQLite; "strongest check" fictional | Critical | Accept | Completed |
|
||||
| 4 | No deploy guard — empty/truncated DB ships with green CI | Critical | Accept | Completed |
|
||||
| 5 | "Verify on a branch" unexecutable; `workflow_dispatch` publishes to prod | Critical | Accept | Completed |
|
||||
| 6 | Counter divergence invisible; 63 trailing empty sheets in `data/2017` | Critical | Accept | Completed |
|
||||
| 7 | Phase 7e breaks `npm run build:db`; 33 refs vs 8 listed | High | Accept | Completed |
|
||||
| 8 | "byte-equivalent databases" provably unachievable | High | Accept | plan.md |
|
||||
| 9 | Phase 3 cites 20 of 29 tests; omitted 9 are the flagged traps | High | Accept | Phase 3 |
|
||||
| 10 | Two incompatible reader contracts; `[][]string` lossy | High | Accept | Phase 1, 4 |
|
||||
| 11 | Phase 7d ports 4 scripts with 0 consumers, 2 broken | High | Accept | Phase 7 (cut) |
|
||||
| 12 | `.xls` golden fixture unbuildable — no Go BIFF writer | High | Accept | plan.md, Phase 4 |
|
||||
| 13 | Golden fixture port is phantom coverage (`inlineStr` only) | High | Accept | Phase 4 (cut) |
|
||||
| 14 | No partial-success/abandon procedure at Phase 6 | High | Accept | Phase 6 |
|
||||
| 15 | `verify-parity.js` silently passes on datasets absent from both files | High | Accept | Phase 6 |
|
||||
| 16 | PII: committing real rows as testdata breaks documented convention | Medium | Accept | Phase 1 |
|
||||
| 17 | Duplicate-SBD guidance names `read_dir`; Rust sorts explicitly | Medium | Accept | Phase 6 |
|
||||
| 18 | `dataset_label` derived from path breaks tempdir stdout comparison | Medium | Accept | Phase 4 |
|
||||
| 19 | No dependency trust/pinning/`govulncheck` step | Medium | Accept | Phase 1, 2 |
|
||||
| 20 | Make `.xls`→`.xlsx` conversion unconditional Phase 0 | High | **Reject** | — |
|
||||
| 21 | Publish DBs as artifacts; drop parser from critical path | High | **Reject** | — |
|
||||
| 22 | Merge Phases 2-5 into one "port the crate" phase | Medium | **Reject** | — |
|
||||
|
||||
**Rejection rationale:**
|
||||
- **20, 21** — user decisions, not reviewer calls. 20 mutates committed source data and the user
|
||||
explicitly deferred it on 2026-08-13. **Superseded by Phase 1**: `pbnjay/grate` reads BIFF
|
||||
exactly, so no conversion is needed and source data stays untouched. (The `extrame/xls`
|
||||
evidence cited when this was first rejected was itself wrong — but the conclusion holds for
|
||||
a better reason.) 21 reverses the user's stated goal of working in Go on the parser.
|
||||
- **22** — phases map to TDD checkpoints and hydrated tasks. Merging reduces granularity without
|
||||
reducing risk. Phase 1's gate framing was re-pointed instead.
|
||||
|
||||
**Citation corrections applied:** `transform.rs:161`→`:162`; `deployment-guide.md:37`→`:38`;
|
||||
"rusqlite statement cache" removed (`writer.rs:73` uses `conn.execute`, not `prepare_cached`);
|
||||
"doc comment says category M"→ the *inline* comment at `:53` (the doc comment at `:49` is
|
||||
correct); test counts corrected to 63 total / 29 in `transform.rs`.
|
||||
|
||||
### Whole-Plan Consistency Sweep
|
||||
- Files reread: `plan.md`, `phase-01` … `phase-07` (all 8)
|
||||
- Decision deltas checked: 19
|
||||
- Reconciled stale references: dominant-risk framing (plan.md + Phase 1), byte-equivalence
|
||||
criterion (plan.md), reader contract (Phases 1 + 4), `verify-parity.js` usage (Phase 6),
|
||||
script-port scope (plan.md + Phase 7), test counts (plan.md + Phases 2/3/5), phase title
|
||||
"BIFF reader gate" → "reader fidelity gate" (plan.md table + Phase 1)
|
||||
- Unresolved contradictions: 0
|
||||
-79
@@ -1,79 +0,0 @@
|
||||
# Brainstorm — Migrate parser from Rust to Go
|
||||
|
||||
Date: 2026-08-13. Branch: main. Scout input: `from-scout-to-brainstorm-260813-1502-rust-to-go-parser-migration-report.md`.
|
||||
|
||||
## Decision
|
||||
|
||||
Build a **new Go parser under `go-parser/`, side by side with the existing Rust `parser/`**. Validate by **differential comparison against live Rust output** — not a frozen baseline. Rust stays untouched and working throughout.
|
||||
|
||||
## Problem-first
|
||||
|
||||
User brought a preselected solution ("migrate to Go"). Inversion applied.
|
||||
|
||||
- **Driver**: preference for working in Go + AI makes iteration cheap. Not a defect in the Rust parser — none exists.
|
||||
- **Evidence status**: none for a technical problem. Legitimate as a preference-driven migration; recorded as such rather than retrofitted with technical justification.
|
||||
- **Rejected framings**: "one language too many" (Go is still a second non-JS toolchain — doesn't collapse the stack); "Rust hard to modify" (a Go port reproduces the same 2016-format complexity in different syntax); "performance" (419 MB of Excel I/O dominates deploy time, unchanged by language).
|
||||
|
||||
## Approaches evaluated
|
||||
|
||||
| # | Approach | Verdict |
|
||||
|---|---|---|
|
||||
| 1 | Don't migrate; drop dead `glob` dep | Rejected — user prefers Go, cost is acceptable |
|
||||
| 2 | In-place Rust→Go rewrite of `parser/` | Rejected — no reversibility, broken half-states |
|
||||
| 3 | Convert `.xls`→`.xlsx` first, then port | Held as **fallback** if `extrame/xls` fails |
|
||||
| 4 | **Side-by-side `go-parser/` + differential validation** | **CHOSEN** |
|
||||
|
||||
Approach 4 beats the author's original phased proposal: keeping Rust live means ground truth is regenerable on demand, so no frozen parity baseline is needed. Reversibility is structural, not procedural.
|
||||
|
||||
## Constraints carried from scout
|
||||
|
||||
**Hard blocker to hit early**: 67 genuine OLE2/BIFF `.xls` files (verified by magic bytes `d0cf11e0a1b11ae1`) — 63 of them in `data/2017`, the 286 MB largest dataset. `calamine` reads BIFF+OOXML through one API; Go has no equivalent. `excelize` is xlsx-only; `extrame/xls` is the only real BIFF option and is lightly maintained + weak on non-UTF8 encodings, which matters because every cell is Vietnamese.
|
||||
|
||||
→ **Build the Go reader module FIRST**, before transform/writer. Fail fast. Fallback = approach 3 (data is frozen, git-tracked, untouched since the unification rename — a one-time conversion is legitimate here and would likely shrink the repo, since OOXML is zip-compressed and BIFF is not).
|
||||
|
||||
**Exactness traps that must be replicated, not improved:**
|
||||
- `to_ascii`: literal codepoint range U+0300–U+036F filter, **not** `unicode.Mn` (Go's category check is more permissive → divergence). Plus explicit `đ/Đ → d` (NFD does not decompose them). `transform.rs:52-64`.
|
||||
- TOML `deny_unknown_fields` is load-bearing and has a test. Most Go TOML libs ignore unknown keys by default.
|
||||
- Tri-state skip reason (`BlankRow` vs `EmptyField` vs `NonNumericSbd`) drives the printed counters — a bool diverges.
|
||||
- `0.0` score parsed as `None` in the 2016 separate-scores format (replicates a JS `||` falsy quirk). `format_detect_2016.rs:165`.
|
||||
- Audit path reads **sheet 0 only**, deliberately ignoring `sheet_mode`. Preserve, don't "fix".
|
||||
- Header check is **per-sheet**, not per-file.
|
||||
- Unknown: how calamine stringifies date cells into `ngay_sinh`. Never inspected. Probe empirically against real files.
|
||||
|
||||
**Contracts to preserve**: CLI `build --schema --input --output`; SQLite `student` table, 22 cols, 3 indexes incl. partial `idx_ten_cum_thi`; non-zero exit kills the npm pipeline.
|
||||
|
||||
**Free wins**: regexes are already RE2-safe (zero port risk); SQLite is plain SQL text with positional `?`, no named params, no pragmas; `glob` dep is dead (confirmed zero references).
|
||||
|
||||
## Scope
|
||||
|
||||
Everything under `parser/` → `go-parser/`: crate, tests, and the 6 JS helper scripts.
|
||||
|
||||
**Sequencing constraint**: port `db-stats.js` / `verify-parity.js` **last**. They are the tools that prove the port is correct — rewriting them during the port is circular (a bug in the ported checker hides a bug in the ported parser). Verify Go versions against JS output before trusting them.
|
||||
|
||||
## Validation criteria
|
||||
|
||||
Go parser is done when, for all 4 datasets: Rust-built DB and Go-built DB are equivalent — row counts, per-column non-NULL counts, and field-by-field equality on a deterministic SBD sample. Plus black-box golden tests spawning the binary and inspecting the `.db`, including **an `.xls` fixture** (current suite has none, so it cannot catch a BIFF regression).
|
||||
|
||||
## Risks
|
||||
|
||||
| Risk | Severity | Mitigation |
|
||||
|---|---|---|
|
||||
| `extrame/xls` can't read the 67 BIFF files / mangles Vietnamese | **High** | Build reader first; fallback to `.xls`→`.xlsx` conversion |
|
||||
| Date-cell stringification differs from calamine | Medium | Differential diff catches it; probe early |
|
||||
| Silent value corruption across 419 MB | Medium | Differential gate is the only real defense — non-negotiable |
|
||||
| Repo carries two parsers during migration | Low | Intentional; delete `parser/` only after sign-off |
|
||||
|
||||
## Next steps
|
||||
|
||||
1. `go-parser/` scaffold + reader module against real `.xls` — decisive gate.
|
||||
2. Port config/transform/writer/audit/schema.
|
||||
3. Black-box golden tests + `.xls` fixture.
|
||||
4. Differential parity vs Rust across all 4 datasets.
|
||||
5. CI swap (drop `dtolnay/rust-toolchain` + `Swatinem/rust-cache`, add `actions/setup-go`), `.gitignore`, docs.
|
||||
6. Port JS scripts last. Remove `parser/` after sign-off.
|
||||
|
||||
## Unresolved
|
||||
|
||||
1. Keep binary name/path `parser/target/release/xlsxread` so `build-db.js` is untouched, or emit to `go-parser/bin/` and update the one constant at `build-db.js:25`?
|
||||
2. Delete `parser/` after parity, or keep it as a reference implementation for some period?
|
||||
3. `.xls` fallback: if conversion is needed, keep originals committed alongside converted files, or replace them?
|
||||
-70
@@ -1,70 +0,0 @@
|
||||
# Scout Report — Rust parser migration surface (Rust → Go)
|
||||
|
||||
Date: 2026-08-13. Branch: main. Scope: `parser/` crate + its build/verify boundary.
|
||||
|
||||
## Relevant Files
|
||||
|
||||
### Rust crate (~2.3k LOC)
|
||||
- `parser/Cargo.toml` — 10 deps. `glob` is **dead** (zero references in `src/` or `tests/`).
|
||||
- `parser/src/schema.rs` (213) — canonical DDL, INSERT SQL, column order, 16 subject regexes. Single source of truth.
|
||||
- `parser/src/format_detect_2016.rs` (548) — largest file. Per-file/per-sheet 3-way layout detection for 2016 only.
|
||||
- `parser/src/transform.rs` (409) — `to_ascii` Vietnamese diacritic strip, score regex, tri-state row validation.
|
||||
- `parser/src/main.rs` (377) — CLI dispatch, file globbing via `fs::read_dir`, transaction boundaries, counters.
|
||||
- `parser/src/{reader,writer,config,audit,cli,error,lib}.rs` — sheet iteration, SQLite lifecycle, TOML load, audit, clap.
|
||||
- `parser/configs/{2016,2017,2017-old,2017-old2}.toml` — per-dataset column maps + validation flags.
|
||||
|
||||
### Boundary
|
||||
- `parser/scripts/build-db.js` — sole caller. Hardcodes `parser/target/release/xlsxread` (line 25).
|
||||
- `parser/scripts/verify-parity.js` + `db-stats.js` — manual parity tool, **not wired into CI**.
|
||||
- `parser/tests/golden.rs` (591) — 8 tests, white-box (calls library fns, not the binary).
|
||||
- `.github/workflows/deploy-pages.yml` — `dtolnay/rust-toolchain` + `Swatinem/rust-cache` (workspaces: parser).
|
||||
- `package.json:8` — `build:rust` = `cargo build --release --manifest-path parser/Cargo.toml`.
|
||||
|
||||
## Contracts a rewrite must preserve
|
||||
|
||||
**CLI** (only this is depended on by scripts):
|
||||
```
|
||||
xlsxread build --schema parser/configs/<id>.toml --input data/<id> --output .build/public/db/<id>.db
|
||||
xlsxread audit --schema <cfg> --input <dir> --db <db> # operator-only, never in CI
|
||||
```
|
||||
|
||||
**Output**: SQLite `student` table, 22 cols (`so_bao_danh TEXT PRIMARY KEY`, `ho_ten`, `ho_ten_ascii`, `ngay_sinh`, `ten_cum_thi`, `gioi_tinh`, + 16 `REAL` scores), 3 indexes incl. partial `idx_ten_cum_thi ... WHERE ten_cum_thi IS NOT NULL`. Frontend `sql.js` reads these names directly.
|
||||
|
||||
**stdout**: human-facing only. No script parses it. Exit non-zero kills the npm pipeline (`execFileSync`).
|
||||
|
||||
## Migration risk
|
||||
|
||||
| Area | Risk | Why |
|
||||
|---|---|---|
|
||||
| **Legacy `.xls` (BIFF) reading** | **BLOCKING** | See below. |
|
||||
| calamine cell→string coercion | HIGH | Dates/numerics reach `ngay_sinh` and the score regexes as whatever calamine's `Data::to_string()` renders. Never inspected in-repo; a Go lib will differ. |
|
||||
| `to_ascii` exactness | MEDIUM | `transform.rs:56` filters the literal range U+0300–U+036F, **not** Unicode category Mn (despite its own doc comment). Go `unicode.Is(unicode.Mn,·)` is more permissive → must copy the range check. Plus explicit `đ/Đ → d` (NFD does not decompose them). |
|
||||
| TOML strictness | MEDIUM | `deny_unknown_fields` is load-bearing (has a test). Most Go TOML libs ignore unknown keys by default. |
|
||||
| Stats/stdout parity | MEDIUM | Tri-state `SkipReason` (BlankRow vs EmptyField vs NonNumericSbd) drives the printed counters; a bool would diverge. Stringly-typed dispatch on `dataset_label.contains("old"/"old2")`. |
|
||||
| Regex | **NONE** | Rust `regex` and Go `regexp` are both RE2. No backrefs/lookaround/`\p{}` anywhere. |
|
||||
| SQLite | LOW | Plain SQL text, positional `?`, no named params, no pragmas. VACUUM correctly post-COMMIT. |
|
||||
| clap/serde/thiserror | TRIVIAL | Idiomatic differences only. |
|
||||
|
||||
### The blocker, verified on disk
|
||||
|
||||
```
|
||||
data/2016/ 4 .xls + 115 .xlsx
|
||||
data/2017/ 63 .xls <-- 286 MB, the largest dataset
|
||||
data/2017-old/ 63 .xlsx
|
||||
data/2017-old2/ 54 .xlsx
|
||||
```
|
||||
|
||||
67 legacy `.xls` files across two datasets. `calamine::open_workbook_auto` reads BIFF and OOXML through one API. **Go has no equivalent** — `excelize` is xlsx-only; legacy `.xls` means `extrame/xls` (lightly maintained, incomplete BIFF coverage) or an external converter. This is the crux of the decision, not a detail.
|
||||
|
||||
## Also true
|
||||
|
||||
- No stated performance or correctness problem with the Rust parser. It is not the pain point; `rust-cache` keeps warm CI builds cheap. 419 MB of Excel dominates deploy runtime regardless of language.
|
||||
- `verify-parity.js`'s `APPROVED_RECOVERY` baseline is frozen against the **pre-refactor** implementation and cannot be regenerated. Reusable as a *method* for Rust-vs-Go, but needs a fresh baseline captured from current Rust output first.
|
||||
- `golden.rs` is white-box (calls `xlsxread::reader::process_file` etc.). Its fixtures are hand-built minimal OOXML + `zip` — that trick ports to Go's `archive/zip` trivially. Black-box CLI tests would be the language-agnostic seam.
|
||||
- No fixture covers `.xls` at all — all 3 fixtures are xlsx. So the golden suite would not catch an `.xls` regression.
|
||||
|
||||
## Unresolved Questions
|
||||
|
||||
1. What is the actual motivation for Go? No perf/correctness defect is visible in the repo. Answer determines whether migration is warranted at all.
|
||||
2. How does calamine render date cells into `ngay_sinh` today? Must be probed empirically against real 2016/2017 files before any Go xlsx library is chosen.
|
||||
3. Is a fresh Rust-output parity baseline acceptable as the gate, given the original baseline is unregenerable?
|
||||
@@ -1,7 +1,13 @@
|
||||
# Parser parity result
|
||||
|
||||
**Archived record.** This documents a verification run made on 2026-08-13,
|
||||
against a tree that had a Rust parser, npm-script entry points and four
|
||||
datasets — none of which exist now. It is kept because `docs/data-pipeline.md`
|
||||
cites it as the evidence that the recovered foreign-language scores are real
|
||||
rather than false regex matches. Do not expect the commands below to run.
|
||||
|
||||
Comparison of the databases the unified pipeline ships against a baseline built
|
||||
from the pre-refactor code. Run on 2026-08-13 against the final tree
|
||||
from the pre-refactor code. Run on 2026-08-13 against the tree as it then stood
|
||||
(`npm run build:rust && npm run build:db`), gate exit code 0.
|
||||
|
||||
Inputs:
|
||||
|
||||
Reference in new issue
Block a user