From 933adf70c9572bbbaf90ef4e5b42a21d7630c681 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Fri, 14 Aug 2026 09:40:41 +0700 Subject: [PATCH] refactor: comments state current behavior, not project history Comments across the tree justified the code by pointing at a Rust implementation that is no longer in the repository, citing files and line numbers (config.rs:132, schema.rs:26-54, reader.rs:42) that cannot be opened, plus crates and datasets that are equally gone. A reader could not check any of it. Every invariant those comments carried is kept and restated so it stands on its own: the bytewise sort that decides which row survives a duplicate exam number, the literal U+0300..U+036F range that must match the site's toAscii, the trailing space in "SINH ", the BIFF and shared-string corrections, the VACUUM-after-COMMIT rule, the deploy-from-main guard. The reader's contract is now anchored to the frozen oracle in parser/testdata, which still exists and is still checked, rather than to the tool that originally produced it. TestDDLMatchesRust becomes TestDDLIsFrozen: it compares against a copy of the DDL inside the test and never read schema.rs, so both the name and the failure message were misleading. ToAscii no longer claims the d-replacement must precede lowercasing. Both cases map to 'd' and ToLower runs last, so the order has no effect. --- .github/workflows/deploy-pages.yml | 13 ++-- assembler/internal/site/site.go | 2 +- assembler/internal/verify/verify.go | 5 +- crawler/internal/fetch/fetch.go | 9 +-- crawler/internal/sources/source_2016.go | 7 +- datasets.json | 25 +++---- parser/cmd/dumpcells/main.go | 22 +++--- parser/cmd/xlsxread/main.go | 11 ++- parser/internal/audit/audit.go | 20 +++-- parser/internal/config/config.go | 20 +++-- parser/internal/config/config_test.go | 23 +++--- parser/internal/ingest/detect2016.go | 63 +++++++--------- parser/internal/ingest/detect2016_test.go | 27 +++---- parser/internal/ingest/ingest.go | 63 +++++++--------- parser/internal/ingest/ingest_test.go | 20 ++--- parser/internal/reader/fidelity_test.go | 29 +++---- parser/internal/reader/reader.go | 29 +++---- parser/internal/reader/xls.go | 22 +++--- parser/internal/reader/xlsx.go | 27 +++---- parser/internal/reader/xlsx_fixups.go | 9 ++- parser/internal/schema/schema.go | 29 ++++--- parser/internal/schema/schema_test.go | 27 +++---- parser/internal/sqlitedb/sqlitedb.go | 13 ++-- .../transform/ascii_crosscheck_test.go | 15 ++-- parser/internal/transform/transform.go | 75 ++++++++----------- parser/internal/transform/transform_test.go | 38 ++++------ parser/internal/writer/writer.go | 27 +++---- parser/testdata/reader-fidelity-hashes.tsv | 19 +++-- web/eslint.config.js | 4 +- web/src/App.css | 11 +-- web/src/App.jsx | 10 +-- web/src/components/custom-query.jsx | 10 ++- web/src/components/hub.jsx | 8 +- web/src/components/score-table.jsx | 7 +- web/src/components/student-detail.jsx | 4 +- web/src/datasets.js | 33 ++++---- web/src/hooks/use-sqlite.js | 12 ++- web/src/lib/admission-blocks.js | 30 ++++---- web/src/lib/query-mode.js | 12 +-- web/src/lib/sql-presets.js | 16 ++-- web/src/lib/subjects.js | 22 +++--- web/src/router.js | 12 +-- web/vite.config.js | 27 +++---- 43 files changed, 392 insertions(+), 515 deletions(-) diff --git a/.github/workflows/deploy-pages.yml b/.github/workflows/deploy-pages.yml index ef85e05..c983db2 100644 --- a/.github/workflows/deploy-pages.yml +++ b/.github/workflows/deploy-pages.yml @@ -3,10 +3,8 @@ name: Deploy to GitHub Pages on: push: branches: [main] - # Pull requests run the build job only. Before this, the workflow triggered on - # main-push and workflow_dispatch alone, so "verify on a branch first" was not - # actually possible: pushing to a branch ran nothing, and dispatching from one - # published that branch straight to the live site. + # Pull requests run the build job only: the deploy job is guarded to main, so + # a branch can be verified end to end without touching the live site. pull_request: workflow_dispatch: @@ -92,9 +90,10 @@ jobs: path: _site deploy: - # Guarded to main. Without this, a workflow_dispatch from any branch would - # publish that branch's output to the live site, and concurrency - # cancel-in-progress would kill an in-flight good deploy on the way. + # Deploy only from main. pull_request and workflow_dispatch both run on + # other branches, and publishing one would put that branch's output on the + # live site while concurrency cancel-in-progress killed an in-flight good + # deploy on the way. if: github.ref == 'refs/heads/main' needs: build runs-on: ubuntu-latest diff --git a/assembler/internal/site/site.go b/assembler/internal/site/site.go index 75d2344..4c1bef6 100644 --- a/assembler/internal/site/site.go +++ b/assembler/internal/site/site.go @@ -47,7 +47,7 @@ func DefaultPaths(root string) Paths { // BuildWeb runs the Vite build. // // Shelling out to npm is not a wart: Vite is a Node tool, and web/ is the only -// npm project left in the repository. This stage owns the sequencing, not the +// npm project in the repository. This stage owns the sequencing, not the // bundling. func BuildWeb(p Paths) error { cmd := exec.Command("npm", "run", "build") diff --git a/assembler/internal/verify/verify.go b/assembler/internal/verify/verify.go index 76c0892..3674889 100644 --- a/assembler/internal/verify/verify.go +++ b/assembler/internal/verify/verify.go @@ -5,9 +5,8 @@ // out — but a change in transform or writer logic can alter what is in those // rows while both of those still pass. This is what catches that. // -// It was written as the gate for the Rust-to-Go parser migration and works for -// any two builds: before and after a refactor, or a re-crawl against the -// databases already published. +// It works on any two builds: either side of a refactor, or a re-crawl against +// the databases already published. package verify import ( diff --git a/crawler/internal/fetch/fetch.go b/crawler/internal/fetch/fetch.go index 292f72e..3ae9ff6 100644 --- a/crawler/internal/fetch/fetch.go +++ b/crawler/internal/fetch/fetch.go @@ -51,12 +51,11 @@ type Result struct { type Options struct { // Concurrency is the number of files downloaded at once. Concurrency int - // Timeout bounds each individual request, headers and body together. - // The original JS crawler had no timeout, so one stalled connection could - // hang the whole run indefinitely. + // Timeout bounds each individual request, headers and body together, so a + // single stalled connection cannot hang the whole run. Timeout time.Duration - // Headers are sent with every request. The CDN this was written against - // rejects requests without a browser User-Agent and a matching Referer. + // Headers are sent with every request. The 2017 CDN rejects requests + // without a browser User-Agent and a matching Referer. Headers map[string]string // OnResult, if set, is called once per completed item. Calls are // serialised, so it does not need its own locking, but they arrive in diff --git a/crawler/internal/sources/source_2016.go b/crawler/internal/sources/source_2016.go index 2f64094..36ac631 100644 --- a/crawler/internal/sources/source_2016.go +++ b/crawler/internal/sources/source_2016.go @@ -9,8 +9,7 @@ import ( // source2016 fetches the 2016 dataset: one spreadsheet per exam cluster // (cụm thi), 4 .xls and 115 .xlsx. // -// The article is a school site that aggregated every cluster's file. It is -// still online, and its links are read at crawl time like any other source. +// The article is a school site that aggregated every cluster's file. // // Dest keeps the server's own filename verbatim — a 32-hex content hash, the // cluster slug, then a millisecond timestamp. Two reasons not to prettify it: @@ -19,8 +18,8 @@ import ( // which row survives a duplicate exam number. That is live here, not // hypothetical: 877,464 source rows collapse to 877,461, so three rows' // contents depend on this ordering. -// - parser/testdata/reader-fidelity-hashes.tsv is keyed by full path, and -// it is frozen — it was produced by the Rust reader, which no longer exists. +// - parser/testdata/reader-fidelity-hashes.tsv is keyed by full path and is +// frozen, so a renamed file no longer matches its oracle entry. // // Unlike 2017 this needs no transliteration: the name comes from the URL, so it // is exact by construction rather than derived from a label. diff --git a/datasets.json b/datasets.json index aab7660..b91387b 100644 --- a/datasets.json +++ b/datasets.json @@ -1,20 +1,19 @@ { "_comment": [ - "The dataset registry: the one place every stage agrees on what exists.", + "The dataset registry: the one place every pipeline stage agrees on what", + "exists. JSON because the Go stages and the Vite app both read it.", "", - " crawler/ fills data//", - " parser/ reads data// with configs/.yml, writes .db", - " assembler/ verifies, compresses and publishes it as db/.db.gz", - " web/ serves it at /thptqg//", + " id the identifier used end to end, from data// to /thptqg//", + " expectedRows exact row count of the built database, not an estimate. The", + " inputs are frozen exam results, so a deviation of even one row", + " means something changed unintentionally and the assembler", + " refuses to publish.", + " dbSizeMb usual size of the gzipped database. Required and non-zero:", + " the assembler rejects a build that comes out far smaller, and", + " the web app shows it while the download runs.", "", - "It is JSON rather than a module because Go and the Vite app both read it,", - "and JSON is the only format both parse without a dependency. Presentation", - "(titles, labels, SQL presets) stays in web/src/datasets.js keyed by id;", - "that file fails loudly if the two lists disagree.", - "", - "expectedRows is exact, not approximate. The inputs are frozen historical", - "exam results, so a deviation of even one row means something changed that", - "nobody intended, and the assembler refuses to publish." + "Presentation (titles, labels, SQL presets) lives in web/src/datasets.js keyed", + "by id; that file throws at load if the two lists disagree." ], "datasets": [ { diff --git a/parser/cmd/dumpcells/main.go b/parser/cmd/dumpcells/main.go index 9430adc..00fce4d 100644 --- a/parser/cmd/dumpcells/main.go +++ b/parser/cmd/dumpcells/main.go @@ -1,16 +1,15 @@ // Command dumpcells emits the canonical cell rendering of a spreadsheet. // -// It exists to diagnose a reader-fidelity failure. That suite compares a -// SHA-256 per input file against a frozen oracle, so a mismatch says which file -// changed but not which cell; this prints the stream the hash is taken over, so -// two runs can be diffed. It was originally written to compare against the -// Rust/calamine ground truth from parser/examples/dump_cells.rs, which was -// removed with the Rust tree — the hashes it produced are what remains. +// It exists to diagnose a TestReaderFidelity failure. That suite compares a +// SHA-256 per input file against the frozen oracle, so a mismatch names the +// file but not the cell; this prints the stream the hash is taken over — plus a +// leading FILE line, which the hash excludes — so a dump from before and after +// a reader change can be diffed line by line. // -// The canonical stream carries geometry and rendered cell values only. The -// calamine Data variant is deliberately excluded: Data::Empty and -// Data::String("") both render "" and both count as blank everywhere -// downstream, so the distinction cannot affect the database. +// The canonical stream carries geometry and rendered cell values only. Whether +// a cell was absent or an empty string is deliberately excluded: both render "" +// and both count as blank everywhere downstream, so the distinction cannot +// affect the database. // // Usage: dumpcells [out-file] package main @@ -24,7 +23,8 @@ import ( "github.com/tiennm99/thptqg/parser/internal/reader" ) -// escape mirrors the Rust dumper so field separators can never break the format. +// escape hides the field and record separators so a cell value can never break +// the format. Must stay in step with the fidelity test's escaping. func escape(s string) string { var b strings.Builder b.Grow(len(s)) diff --git a/parser/cmd/xlsxread/main.go b/parser/cmd/xlsxread/main.go index 332c899..78a5cac 100644 --- a/parser/cmd/xlsxread/main.go +++ b/parser/cmd/xlsxread/main.go @@ -1,8 +1,7 @@ // Command xlsxread reads .xls/.xlsx files and builds SQLite databases for the // thptqg datasets. // -// The CLI contract is fixed by the assembler, which drives this binary, and it -// matched the Rust binary it replaced exactly: +// The CLI contract is fixed by the assembler, which drives this binary: // // xlsxread build --schema --input --output // xlsxread audit --schema --input --db @@ -67,8 +66,8 @@ func runBuild(args []string) { fatalf("Failed to load config: %v", err) } - // The 2016 dataset selects its column layout per file at runtime; every other - // dataset uses the fixed columns: mapping (main.rs:63-67). + // The 2016 dataset selects its column layout per sheet at runtime; every + // other dataset uses the fixed columns: mapping from its config. if cfg.FormatDetection != nil && *cfg.FormatDetection == "thptqg2016" { if err := ingest.Detect2016(cfg, *inputDir, *outputPath); err != nil { fatalf("%v", err) @@ -102,8 +101,8 @@ func runAudit(args []string) { } audit.PrintReport(res) - // A mismatch is a non-zero exit even though the audit itself succeeded - // (main.rs:46-48) — CI treats it as a failure signal. + // A mismatch is a non-zero exit even though the audit itself succeeded — + // CI treats it as a failure signal. if !res.Matched { os.Exit(1) } diff --git a/parser/internal/audit/audit.go b/parser/internal/audit/audit.go index 7185ef5..496d979 100644 --- a/parser/internal/audit/audit.go +++ b/parser/internal/audit/audit.go @@ -1,13 +1,12 @@ // Package audit compares distinct SBDs in the source spreadsheets against the -// row count in a built database — a port of parser/src/audit.rs. +// row count in a built database. // -// Two deliberate divergences from the build path are preserved, both inherited -// from audit-row-counts.js: +// Two divergences from the build path are deliberate: // -// - only .xlsx files are considered (audit.rs:53-59) -// - only sheet 0 is read, regardless of sheet_mode (audit.rs:81-84) +// - only .xlsx files are considered +// - only sheet 0 is read, regardless of sheet_mode // -// These are intentional, not oversights. Do not "fix" them. +// They are intentional, not oversights. Do not "fix" them. package audit import ( @@ -56,8 +55,8 @@ func Run(inputDir, dbPath string, cfg *config.DatasetConfig) (*Result, error) { res := &Result{} seen := make(map[string]struct{}) - // The audit uses positional defaults for format-detection configs, mirroring - // audit-row-counts.js's fixed column assumption (audit.rs:104-111). + // Format-detection configs carry no Columns block, so the audit falls back + // to fixed positional columns for them. hoTenCol, sbdCol := 1, 0 if cfg.Columns != nil { hoTenCol, sbdCol = cfg.Columns.HoTen, cfg.Columns.SoBaoDanh @@ -108,8 +107,7 @@ func Run(inputDir, dbPath string, cfg *config.DatasetConfig) (*Result, error) { } } - // Opened read-only: the audit must never mutate the database it inspects - // (audit.rs:139). + // Opened read-only: the audit must never mutate the database it inspects. db, err := sql.Open(sqlitedb.DriverName, "file:"+dbPath+"?mode=ro") if err != nil { return nil, fmt.Errorf("open db %s: %w", dbPath, err) @@ -124,7 +122,7 @@ func Run(inputDir, dbPath string, cfg *config.DatasetConfig) (*Result, error) { return res, nil } -// PrintReport mirrors audit-row-counts.js:54-62 exactly. +// PrintReport writes the source-versus-database comparison block. func PrintReport(r *Result) { fmt.Println("=== Source vs DB ===") fmt.Printf("Source: total data rows across all files: %d\n", r.TotalDataRows) diff --git a/parser/internal/config/config.go b/parser/internal/config/config.go index e4749d1..f645fb5 100644 --- a/parser/internal/config/config.go +++ b/parser/internal/config/config.go @@ -1,8 +1,8 @@ -// Package config loads per-dataset parse rules — a port of parser/src/config.rs. +// Package config loads per-dataset parse rules. // // The config deliberately carries no SQL. The table shape, the INSERT and the // subject regexes are identical for every dataset and live in internal/schema; -// keeping them here meant one copy of the DDL per dataset, which is how the +// keeping them here means one copy of the DDL per dataset, which is how the // 2016 and 2017 schemas drifted apart. package config @@ -25,7 +25,7 @@ const ( SheetModeFirst SheetMode = "first" ) -// valid reports whether m is one of the two values the Rust enum accepts. +// valid reports whether m is one of the two accepted sheet modes. // // Checked after decoding rather than via a custom unmarshaler: the decoder // assigns named string types directly, so a typo would otherwise decode @@ -52,7 +52,7 @@ type DatasetConfig struct { type ReaderCfg struct { SheetMode SheetMode `yaml:"sheet_mode"` // StripBlankRows skips rows where every cell is empty before counting them - // as source rows (a 2017-old2 quirk). + // as source rows. StripBlankRows bool `yaml:"strip_blank_rows"` } @@ -66,8 +66,7 @@ type ColumnMap struct { } type ValidationCfg struct { - // RequireNumericSbd mirrors build-database-old.js / -old2.js, which require - // so_bao_danh to match ^\d+$. + // RequireNumericSbd skips rows whose so_bao_danh is not all ASCII digits. RequireNumericSbd bool `yaml:"require_numeric_sbd"` RequireNonemptyName bool `yaml:"require_nonempty_name"` RequireNonemptySbd bool `yaml:"require_nonempty_sbd"` @@ -80,11 +79,10 @@ type HeaderCfg struct { // Parse decodes a config, rejecting any key the struct does not declare. // -// Strictness is load-bearing and has its own test. Rust gets it from serde's -// deny_unknown_fields; yaml.v3 needs KnownFields(true) explicitly, and Go YAML -// decoders ignore unknown keys by default. Without it a leftover `schema:` -// mapping would look effective while internal/schema actually drove the build — -// the drift that produced two divergent schemas before the unification. +// Strictness is load-bearing and has its own test. YAML decoders ignore unknown +// keys by default, so KnownFields(true) has to be set explicitly. Without it a +// leftover `schema:` mapping would look effective while internal/schema actually +// drove the build — the drift that produced two divergent schemas. func Parse(src []byte) (*DatasetConfig, error) { var cfg DatasetConfig dec := yaml.NewDecoder(bytes.NewReader(src)) diff --git a/parser/internal/config/config_test.go b/parser/internal/config/config_test.go index 9e7e195..baa5a84 100644 --- a/parser/internal/config/config_test.go +++ b/parser/internal/config/config_test.go @@ -6,7 +6,7 @@ import ( "testing" ) -// sampleYAML mirrors SAMPLE_YAML in parser/src/config.rs. +// sampleYAML is a fixed-column (2017-style) config with every section populated. const sampleYAML = ` reader: sheet_mode: all @@ -27,7 +27,6 @@ header: tokens: ["HO_TEN", "HỌ TÊN", "STT"] ` -// TestConfigRoundTrip ports config_round_trip (config.rs:115). func TestConfigRoundTrip(t *testing.T) { cfg, err := Parse([]byte(sampleYAML)) if err != nil { @@ -62,12 +61,10 @@ func TestConfigRoundTrip(t *testing.T) { } } -// TestConfigRejectsLeftoverSQLSections ports config_rejects_leftover_sql_sections -// (config.rs:132). This is the load-bearing one: Rust gets the behaviour free -// from serde's deny_unknown_fields, whereas Go YAML decoders ignore unknown keys -// unless KnownFields(true) is set. Without it a stale schema: mapping would look effective while -// internal/schema silently drove the build — exactly the drift that produced two -// divergent schemas before the unification. +// TestConfigRejectsLeftoverSQLSections is the load-bearing one: YAML decoders +// ignore unknown keys unless KnownFields(true) is set, and without it a stale +// schema: mapping would look effective while internal/schema silently drove the +// build — exactly the drift that produced two divergent schemas. func TestConfigRejectsLeftoverSQLSections(t *testing.T) { withDDL := sampleYAML + "\nschema:\n ddl: \"CREATE TABLE student (so_bao_danh TEXT);\"\n" if _, err := Parse([]byte(withDDL)); err == nil { @@ -84,7 +81,6 @@ func TestConfigRejectsUnknownScalarKey(t *testing.T) { } } -// TestConfigFirstSheetMode ports config_first_sheet_mode (config.rs:140). func TestConfigFirstSheetMode(t *testing.T) { src := []byte(replaceAll(sampleYAML, "sheet_mode: all", "sheet_mode: first")) cfg, err := Parse(src) @@ -96,7 +92,7 @@ func TestConfigFirstSheetMode(t *testing.T) { } } -// TestConfigRejectsUnknownSheetMode: the Rust enum accepts only "all"/"first", +// TestConfigRejectsUnknownSheetMode: sheet_mode accepts only "all" or "first", // so anything else must fail rather than defaulting. func TestConfigRejectsUnknownSheetMode(t *testing.T) { src := []byte(replaceAll(sampleYAML, "sheet_mode: all", "sheet_mode: second")) @@ -105,8 +101,7 @@ func TestConfigRejectsUnknownSheetMode(t *testing.T) { } } -// TestConfigFormatDetectionField ports config_format_detection_field -// (config.rs:147): a 2016-style config has no columns: mapping at all. +// TestConfigFormatDetectionField: a 2016-style config has no columns: mapping at all. func TestConfigFormatDetectionField(t *testing.T) { const src = ` format_detection: thptqg2016 @@ -135,8 +130,8 @@ header: } } -// TestLoadRealConfigs loads the shipped configs and asserts the per-dataset -// differences recorded during scouting. +// TestLoadRealConfigs loads the shipped configs and pins the per-dataset +// differences between them. func TestLoadRealConfigs(t *testing.T) { root := repoRoot(t) want := map[string]struct { diff --git a/parser/internal/ingest/detect2016.go b/parser/internal/ingest/detect2016.go index 371f128..ff6f2d7 100644 --- a/parser/internal/ingest/detect2016.go +++ b/parser/internal/ingest/detect2016.go @@ -14,16 +14,14 @@ import ( ) // The 2016 dataset's 119 files were produced by inconsistent tooling and use -// three different column layouts, chosen per sheet at runtime. This is a port of -// parser/src/format_detect_2016.rs — institutional knowledge encoded as -// literals, with no abstraction to derive it from, so everything here is copied -// verbatim rather than rationalised. +// three different column layouts, chosen per sheet at runtime. The literals +// below are institutional knowledge about those files, with no rule to derive +// them from — extend them, but do not tidy them into something more regular. -// KnownHeaders are the upper-cased first-cell values that identify a header row -// (format_detect_2016.rs:36-54, mirroring KNOWN_HEADERS in build-database.js). +// KnownHeaders are the upper-cased first-cell values that identify a header row. // -// NOTE "SINH " carries a TRAILING SPACE, exactly as in the Rust source. Trimming -// it would change which rows are recognised as headers. +// NOTE "SINH " carries a TRAILING SPACE. Trimming it would change which rows +// are recognised as headers. var KnownHeaders = []string{ "SOBAODANH", "SBD", @@ -53,8 +51,7 @@ func isKnownHeader(s string) bool { return false } -// IsHeaderRow2016 reports whether row[0] is a known header token -// (format_detect_2016.rs:58-64). +// IsHeaderRow2016 reports whether row[0] is a known header token. // // The guard here is len < 2, not the len < 3 used by the 2017 header check. func IsHeaderRow2016(row []reader.Cell) bool { @@ -89,8 +86,7 @@ type Format struct { DiemThi int } -// defaultFormat is FormatMapped with the fixed positional indices -// (format_detect_2016.rs:295-306). +// defaultFormat is FormatMapped with the fixed positional indices. func defaultFormat() Format { two, three, four := 2, 3, 4 return Format{ @@ -99,8 +95,7 @@ func defaultFormat() Format { } } -// DetectFormat inspects a header row and decides which layout applies -// (format_detect_2016.rs:95-145). +// DetectFormat inspects a header row and decides which layout applies. func DetectFormat(headerRow []reader.Cell) Format { cols := make([]string, len(headerRow)) for i, c := range headerRow { @@ -150,10 +145,9 @@ func DetectFormat(headerRow []reader.Cell) Format { // parseFloatCell parses a per-subject score cell. // -// A parsed 0.0 becomes "no score" — this replicates JavaScript's -// `parseFloat(row[N]) || null`, where 0 is falsy (format_detect_2016.rs:165). -// It means a genuine zero is indistinguishable from a blank. Not obviously -// correct, but it is the shipped behaviour and the published data depends on it. +// A parsed 0.0 becomes "no score", so a genuine zero is indistinguishable from +// a blank. Not obviously correct, but it is the shipped behaviour and the +// published data depends on it. func parseFloatCell(row []reader.Cell, idx int) (float64, bool) { s := cellAt(row, idx) if s == "" { @@ -166,8 +160,7 @@ func parseFloatCell(row []reader.Cell, idx int) (float64, bool) { return v, true } -// processSeparateScoresRow handles the fixed 12-column layout -// (format_detect_2016.rs:176-216): +// processSeparateScoresRow handles the fixed 12-column layout: // // 0=SBD 1=HOTEN 2=TOAN 3=VAN 4=LY 5=HOA 6=SINH 7=SU 8=DIA // 9=NGOAINGUTN 10=NGOAINGUTL 11=NGOAINGU(total -> tieng_anh) @@ -202,8 +195,7 @@ func processSeparateScoresRow(row []reader.Cell) *transform.ParsedRow { } } -// processMappedRow handles header-derived column indices -// (format_detect_2016.rs:226-289). +// processMappedRow handles header-derived column indices. func processMappedRow(row []reader.Cell, f Format) *transform.ParsedRow { sbd := cellAt(row, f.Sbd) hoTen := cellAt(row, f.HoTen) @@ -212,7 +204,7 @@ func processMappedRow(row []reader.Cell, f Format) *transform.ParsedRow { } // Leaked-header guard: a row whose SBD or name cell is itself a header token - // is a repeated header, not data (format_detect_2016.rs:244-250). + // is a repeated header, not data. if isKnownHeader(strings.ToUpper(sbd)) || isKnownHeader(strings.ToUpper(hoTen)) { return nil } @@ -228,7 +220,7 @@ func processMappedRow(row []reader.Cell, f Format) *transform.ParsedRow { } // Gender is a two-value allowlist, not a general enum: anything other than - // exactly "Nam" or "Nữ" becomes nil (format_detect_2016.rs:263-271). + // exactly "Nam" or "Nữ" becomes nil. var gioiTinh *string if f.GioiTinh != nil { if s := cellAt(row, *f.GioiTinh); s == "Nam" || s == "Nữ" { @@ -236,7 +228,7 @@ func processMappedRow(row []reader.Cell, f Format) *transform.ParsedRow { } } - // Read untrimmed, matching format_detect_2016.rs:273-276. + // The free-text score cell is read untrimmed, unlike every other cell here. diemThi := "" if f.DiemThi >= 0 && f.DiemThi < len(row) { diemThi = row[f.DiemThi].Str @@ -264,8 +256,7 @@ func ProcessRow2016(row []reader.Cell, f Format) *transform.ParsedRow { return processMappedRow(row, f) } -// Detect2016 ingests the 2016 dataset — the port of run_build_2016 and -// process_file_2016 (main.rs:211-377). +// Detect2016 ingests the 2016 dataset. func Detect2016(cfg *config.DatasetConfig, inputDir, outputPath string) error { files, err := InputFiles(inputDir) if err != nil { @@ -290,7 +281,7 @@ func Detect2016(cfg *config.DatasetConfig, inputDir, outputPath string) error { return err } - var st writer.Stats // Skipped stays 0 on this path, matching main.rs:251 + var st writer.Stats // Skipped stays 0 on this path for _, file := range files { base := filepath.Base(file) fileRows, err := processFile2016(file, cfg, ins, base, &st) @@ -314,9 +305,9 @@ func Detect2016(cfg *config.DatasetConfig, inputDir, outputPath string) error { // processFile2016 detects the layout per SHEET and processes that sheet's rows. // -// Detection is per sheet, not per file (main.rs:344-349): within one workbook, -// sheet 2 may legitimately detect differently from sheet 1, which matters for -// the province files that overflow past Excel's row cap. +// Detection is per sheet, not per file: within one workbook, sheet 2 may +// legitimately detect differently from sheet 1, which matters for the province +// files that overflow past Excel's row cap. func processFile2016(path string, cfg *config.DatasetConfig, ins *writer.Inserter, base string, st *writer.Stats) (uint64, error) { wb, err := reader.Open(path) if err != nil { @@ -353,8 +344,8 @@ func processFile2016(path string, cfg *config.DatasetConfig, ins *writer.Inserte } for _, row := range rows[startIdx:] { - // Rows shorter than 2 cells are dropped BEFORE the counter - // (main.rs:351-353), so they never appear in the source total. + // Rows shorter than 2 cells are dropped BEFORE the counter, so they + // never appear in the source total. if len(row) < 2 { continue } @@ -363,9 +354,9 @@ func processFile2016(path string, cfg *config.DatasetConfig, ins *writer.Inserte parsed := ProcessRow2016(row, f) if parsed == nil { // Empty or invalid. Note this is NOT counted as skipped — the - // 2016 path leaves that counter at zero (main.rs:251), so the - // stats block reports insertable == source rows and the Audit - // line absorbs the difference. + // 2016 path leaves that counter at zero, so the stats block + // reports insertable == source rows and the Audit line absorbs + // the difference. continue } if err := ins.Insert(parsed); err != nil { diff --git a/parser/internal/ingest/detect2016_test.go b/parser/internal/ingest/detect2016_test.go index 93c3b48..7b7baa2 100644 --- a/parser/internal/ingest/detect2016_test.go +++ b/parser/internal/ingest/detect2016_test.go @@ -6,12 +6,9 @@ import ( "github.com/tiennm99/thptqg/parser/internal/reader" ) -// Ports the 11 tests in parser/src/format_detect_2016.rs, plus a guard per quirk. -// -// Phase 1 settled the Data -> string translation these fixtures need, so no -// guessing: Data::String(s) is s verbatim, Data::Float(8.0) renders "8" (Rust's -// f64 Display drops the .0, matching Go's FormatFloat(v,'f',-1,64)), -// Data::Float(8.5) renders "8.5", and Data::Empty is "" with IsEmpty true. +// Fixtures use the reader's rendering: a string cell is its text verbatim, a +// numeric cell is its shortest round-tripping form (8.0 is "8", 8.5 is "8.5"), +// and a cell with no value is "" with IsEmpty set. // --- header detection --- @@ -81,8 +78,7 @@ func TestDetectFormatMappedIsOrderIndependent(t *testing.T) { } } -// TestDetectFormatMappedHoTenFallback ports the col-1 fallback -// (format_detect_2016.rs:132). +// TestDetectFormatMappedHoTenFallback covers the col-1 fallback for ho_ten. func TestDetectFormatMappedHoTenFallback(t *testing.T) { f := DetectFormat(cells("SBD", "SOMETHING", "DIEM_THI")) if f.Kind != FormatMapped { @@ -137,8 +133,8 @@ func TestProcessSeparateScoresRow(t *testing.T) { } } -// TestZeroScoreBecomesNull pins the JS falsy quirk: parseFloat(x) || null means -// a literal 0 is indistinguishable from "no score" (format_detect_2016.rs:165). +// TestZeroScoreBecomesNull pins the quirk that a literal 0 is indistinguishable +// from "no score". func TestZeroScoreBecomesNull(t *testing.T) { row := cells("1000", "A", "0", "0.0", "1", "", "", "", "", "", "", "") got := ProcessRow2016(row, Format{Kind: FormatSeparateScores}) @@ -156,7 +152,7 @@ func TestZeroScoreBecomesNull(t *testing.T) { } } -// TestGenderAllowlist ports format_detect_2016.rs:263-271 — exactly two values. +// TestGenderAllowlist: exactly two accepted values, matched case-sensitively. func TestGenderAllowlist(t *testing.T) { f := defaultFormat() for in, want := range map[string]string{"Nam": "Nam", "Nữ": "Nữ"} { @@ -178,7 +174,8 @@ func TestGenderAllowlist(t *testing.T) { } } -// TestLeakedHeaderRowSkipped ports format_detect_2016.rs:244-250. +// TestLeakedHeaderRowSkipped: a header repeated inside the data must not become +// a student. func TestLeakedHeaderRowSkipped(t *testing.T) { f := defaultFormat() if ProcessRow2016(cells("SBD", "HO_TEN", "", "", "", ""), f) != nil { @@ -222,8 +219,8 @@ func TestMappedRowPopulates2016OnlyColumns(t *testing.T) { } } -// TestDefaultIsMappedWithFixedIndices: FormatDefault must not be a separate code -// path (format_detect_2016.rs:295-306). +// TestDefaultIsMappedWithFixedIndices: FormatDefault must not become a separate +// code path. func TestDefaultIsMappedWithFixedIndices(t *testing.T) { row := cells("123", "A", "01/01/2000", "Cluster", "Nữ", "Toán: 5") viaDefault := ProcessRow2016(row, defaultFormat()) @@ -244,7 +241,7 @@ func TestDefaultIsMappedWithFixedIndices(t *testing.T) { } // TestShortRowGuardIsSeparateFromValidation documents that rows under 2 cells -// are dropped by the loop before the counter (main.rs:351-353), not here. +// are dropped by the ingest loop before the counter, not by row processing. func TestShortRowGuardIsSeparateFromValidation(t *testing.T) { if got := ProcessRow2016([]reader.Cell{{Str: "123"}}, defaultFormat()); got != nil { t.Error("a 1-cell row has no name and must be rejected") diff --git a/parser/internal/ingest/ingest.go b/parser/internal/ingest/ingest.go index 5da024f..b64ea00 100644 --- a/parser/internal/ingest/ingest.go +++ b/parser/internal/ingest/ingest.go @@ -2,9 +2,7 @@ // headers, which are blank, and how rows are counted. // // The reader deliberately has none of this — it reports every sheet and every -// row exactly as calamine would, which is what made its fidelity independently -// testable. This package ports the build loop in parser/src/main.rs plus the -// header/blank helpers in parser/src/reader.rs. +// row verbatim, which is what keeps its fidelity independently testable. package ingest import ( @@ -21,10 +19,10 @@ import ( ) // IsHeaderRow reports whether row is a header, by matching its uppercased first -// cell against the configured tokens (reader.rs:28-34). +// cell against the configured tokens. // -// Rows shorter than 3 cells are never headers (reader.rs:29) — a 1- or 2-cell -// row is a stray fragment, not a real header. +// Rows shorter than 3 cells are never headers — a 1- or 2-cell row is a stray +// fragment, not a real header. func IsHeaderRow(row []reader.Cell, tokens []string) bool { if len(row) < 3 { return false @@ -38,13 +36,11 @@ func IsHeaderRow(row []reader.Cell, tokens []string) bool { return false } -// IsAllBlank reports whether every cell is empty or whitespace-only -// (reader.rs:40-43). +// IsAllBlank reports whether every cell is empty or whitespace-only. // -// Compares on Str only. Cell.IsEmpty is diagnostic: calamine distinguishes -// Data::Empty from an empty string cell, but both render "" and both count as -// blank here, so branching on the flag would invent a distinction the Rust -// original never acts on. +// Compares on Str only. Cell.IsEmpty is diagnostic: an absent cell and an empty +// string cell both render "" and both count as blank here, so branching on the +// flag would invent a distinction nothing downstream acts on. func IsAllBlank(row []reader.Cell) bool { for _, c := range row { if strings.TrimSpace(c.Str) != "" { @@ -54,11 +50,11 @@ func IsAllBlank(row []reader.Cell) bool { return true } -// InputFiles lists a dataset directory's spreadsheets, sorted. +// InputFiles lists a dataset directory's spreadsheets, bytewise-sorted on the +// full path. // // The sort is load-bearing, not cosmetic: INSERT OR REPLACE is last-wins, so -// file order decides which row survives a duplicate SBD. Rust collects read_dir -// then calls files.sort() (main.rs:82-97) — a bytewise sort on the full path. +// file order decides which row survives a duplicate SBD. func InputFiles(dir string) ([]string, error) { entries, err := os.ReadDir(dir) if err != nil { @@ -78,8 +74,7 @@ func InputFiles(dir string) ([]string, error) { return out, nil } -// DatasetLabel derives the stats-wording key from the input directory basename, -// matching main.rs:98-101. +// DatasetLabel derives the build-log label from the input directory basename. func DatasetLabel(inputDir string) string { base := filepath.Base(strings.TrimRight(inputDir, string(filepath.Separator))) if base == "" || base == "." || base == string(filepath.Separator) { @@ -92,11 +87,11 @@ func DatasetLabel(inputDir string) string { type RowFn func(sheetIdx int, row []reader.Cell) // ProcessFile applies sheet selection and per-sheet header skipping, invoking fn -// for every remaining row — the port of reader.rs:54-105. +// for every remaining row. // -// The header check is per SHEET, not per file: first_row resets inside the sheet -// loop (reader.rs:91), so a workbook whose second sheet repeats the header has -// it skipped there too. +// The header check is per SHEET, not per file: firstRow resets inside the sheet +// loop, so a workbook whose second sheet repeats the header has it skipped +// there too. func ProcessFile(path string, cfg *config.DatasetConfig, fn RowFn) error { wb, err := reader.Open(path) if err != nil { @@ -131,8 +126,7 @@ func ProcessFile(path string, cfg *config.DatasetConfig, fn RowFn) error { return nil } -// Standard runs the fixed-column path for the 2017-family datasets — the port of -// run_build_standard (main.rs:74-199). +// Standard runs the fixed-column path for the 2017-family datasets. func Standard(cfg *config.DatasetConfig, inputDir, outputPath string) error { files, err := InputFiles(inputDir) if err != nil { @@ -149,7 +143,7 @@ func Standard(cfg *config.DatasetConfig, inputDir, outputPath string) error { stripBlank := cfg.Reader.StripBlankRows - // One transaction spans the whole dataset directory (main.rs:120,184). + // One transaction spans the whole dataset directory. tx, err := db.Begin() if err != nil { return fmt.Errorf("begin: %w", err) @@ -167,8 +161,8 @@ func Standard(cfg *config.DatasetConfig, inputDir, outputPath string) error { procErr := ProcessFile(file, cfg, func(_ int, row []reader.Cell) { allBlank := IsAllBlank(row) - // 2017-old2: blank rows drop out BEFORE the source-row counter - // (main.rs:134-137, ahead of the increment at :140). + // Blank rows drop out BEFORE the source-row counter, so they never + // reach the source total. if stripBlank && allBlank { return } @@ -182,10 +176,10 @@ func Standard(cfg *config.DatasetConfig, inputDir, outputPath string) error { switch transform.ValidateRow(hoTen, soBaoDanh, &cfg.Validation, stripBlank, allBlank) { case transform.SkipBlankRow: - // Falls through to transform and insert, matching main.rs:150. - // Unreachable here: BlankRow requires stripBlank && allBlank, - // which returned above. Kept so the two call sites with opposite - // outcomes stay visibly distinct. + // Falls through to transform and insert. Unreachable here: + // BlankRow requires stripBlank && allBlank, which returned + // above. Kept so the two call sites with opposite outcomes stay + // visibly distinct. case transform.SkipNone: // proceed default: @@ -200,7 +194,7 @@ func Standard(cfg *config.DatasetConfig, inputDir, outputPath string) error { } if err := ins.Insert(parsed); err != nil { fileErrors++ - // Only the first five insert warnings print (main.rs:164). + // Only the first five insert warnings print. if st.Errors+fileErrors <= 5 { fmt.Fprintf(os.Stderr, " [warn] %s: %v\n", base, err) } @@ -209,8 +203,8 @@ func Standard(cfg *config.DatasetConfig, inputDir, outputPath string) error { fileRows++ }) if procErr != nil { - // A file that cannot be read is logged and counted, never fatal - // (main.rs:171-177) — one corrupt file must not abandon the batch. + // A file that cannot be read is logged and counted, never fatal — + // one corrupt file must not abandon the batch. fmt.Fprintf(os.Stderr, " [error] %s: %v\n", base, procErr) fileErrors++ } @@ -232,8 +226,7 @@ func Standard(cfg *config.DatasetConfig, inputDir, outputPath string) error { return writer.Finish(db, outputPath, st) } -// cellAt returns the trimmed cell at idx, or "" when out of range — the -// unwrap_or_default() behaviour of transform.rs:163-167. +// cellAt returns the trimmed cell at idx, or "" when idx is out of range. func cellAt(row []reader.Cell, idx int) string { if idx < 0 || idx >= len(row) { return "" diff --git a/parser/internal/ingest/ingest_test.go b/parser/internal/ingest/ingest_test.go index 22a4e06..470ceb9 100644 --- a/parser/internal/ingest/ingest_test.go +++ b/parser/internal/ingest/ingest_test.go @@ -8,10 +8,6 @@ import ( "github.com/tiennm99/thptqg/parser/internal/reader" ) -// Ports the 7 tests in parser/src/reader.rs:116-197. They were listed under -// Phase 1 originally, which was wrong: they exercise header and blank-row -// policy, which lives here rather than in the reader package. - func cells(vals ...string) []reader.Cell { out := make([]reader.Cell, len(vals)) for i, v := range vals { @@ -22,50 +18,45 @@ func cells(vals ...string) []reader.Cell { var stdTokens = []string{"HO_TEN", "HỌ TÊN", "STT"} -// header_detects_ho_ten (reader.rs:128) func TestHeaderDetectsHoTen(t *testing.T) { if !IsHeaderRow(cells("HO_TEN", "NGAY_SINH", "SBD"), stdTokens) { t.Error("HO_TEN header not detected") } } -// header_detects_stt (reader.rs:139) func TestHeaderDetectsStt(t *testing.T) { if !IsHeaderRow(cells("STT", "B", "C"), stdTokens) { t.Error("STT header not detected") } } -// header_detects_ho_ten_unicode (reader.rs:150) func TestHeaderDetectsHoTenUnicode(t *testing.T) { if !IsHeaderRow(cells("HỌ TÊN", "B", "C"), stdTokens) { t.Error("HỌ TÊN header not detected") } } -// header_rejects_data_row (reader.rs:161) func TestHeaderRejectsDataRow(t *testing.T) { if IsHeaderRow(cells("Nguyen Van A", "01/01/2000", "12345678"), stdTokens) { t.Error("data row wrongly detected as header") } } -// header_rejects_short_row (reader.rs:172) — the <3 cell guard. +// TestHeaderRejectsShortRow covers the <3-cell guard. func TestHeaderRejectsShortRow(t *testing.T) { if IsHeaderRow(cells("HO_TEN", ""), stdTokens) { t.Error("a 2-cell row must never be a header, even with a matching token") } } -// header_case_insensitive (reader.rs:179) func TestHeaderCaseInsensitive(t *testing.T) { if !IsHeaderRow(cells("ho_ten", "B", "C"), stdTokens) { t.Error("lowercase header not detected") } } -// blank_row_detection (reader.rs:190) — note the third cell is an empty *string* -// cell, not Data::Empty, and must still count as blank. +// TestBlankRowDetection: the third cell is an empty *string* cell rather than an +// absent one, and must still count as blank. func TestBlankRowDetection(t *testing.T) { blank := []reader.Cell{{IsEmpty: true}, {IsEmpty: true}, {Str: ""}} if !IsAllBlank(blank) { @@ -77,14 +68,13 @@ func TestBlankRowDetection(t *testing.T) { } // TestIsAllBlankIgnoresIsEmptyFlag pins the rule that blankness is decided by -// the rendered string, never by Cell.IsEmpty — calamine emits empty-but-not-Empty -// cells, and treating the flag as authoritative would diverge. +// the rendered string, never by Cell.IsEmpty, which is diagnostic only. func TestIsAllBlankIgnoresIsEmptyFlag(t *testing.T) { // Content present but IsEmpty wrongly set: still not blank. if IsAllBlank([]reader.Cell{{Str: "x", IsEmpty: true}}) { t.Error("a cell with content must not be blank regardless of IsEmpty") } - // Whitespace only: blank, matching Rust's trim().is_empty(). + // Whitespace only: blank. if !IsAllBlank([]reader.Cell{{Str: " "}, {Str: "\t"}}) { t.Error("whitespace-only cells should be blank") } diff --git a/parser/internal/reader/fidelity_test.go b/parser/internal/reader/fidelity_test.go index 44851ae..b7eedcf 100644 --- a/parser/internal/reader/fidelity_test.go +++ b/parser/internal/reader/fidelity_test.go @@ -13,19 +13,20 @@ import ( "github.com/tiennm99/thptqg/parser/internal/reader" ) -// TestReaderFidelity asserts the Go reader reproduces calamine byte-for-byte on -// every real input file. +// TestReaderFidelity asserts the reader reproduces the reference rendering +// byte-for-byte on every real input file. // -// The oracle is a committed SHA-256 per file over a canonical cell dump. The -// dumps themselves are real student names and birthdates, so only the hashes are -// committed — regenerate the dumps from the Rust side on demand -// (parser/examples/dump_cells.rs), which is possible because parser/ still -// builds. +// The oracle is parser/testdata/reader-fidelity-hashes.tsv: one SHA-256 per +// file over a canonical cell dump. The dumps are real student names and +// birthdates, so only the hashes are committed. The manifest is FROZEN — it is +// the only guard on reader output, so a mismatch is a reader bug until proven +// otherwise, never a cue to regenerate the hashes. +// +// The canonical form carries geometry and rendered cell values only. Whether a +// cell was absent or an empty string is excluded on purpose: both render "" and +// both count as blank in IsAllBlank and transform, so the distinction cannot +// reach the database. // -// The canonical form carries geometry and rendered cell values. The calamine -// Data variant is excluded on purpose: Data::Empty and Data::String("") both -// render "" and both count as blank in is_all_blank and transform, so the -// distinction cannot reach the database. // Runs by default so CI and `go test ./...` keep the full guarantee; skipped // under -short, which is how to iterate without paying ~77s to re-read 418 MB. func TestReaderFidelity(t *testing.T) { @@ -65,7 +66,7 @@ func TestReaderFidelity(t *testing.T) { t.Fatalf("hash %s: %v", rel, err) } if got != want { - t.Errorf("cell dump diverges from calamine\n want %s\n got %s", want, got) + t.Errorf("cell dump diverges from the frozen oracle\n want %s\n got %s", want, got) } }) } @@ -77,8 +78,8 @@ func TestReaderFidelity(t *testing.T) { } } -// canonicalHash renders one file in the canonical form and hashes it. Kept -// byte-identical to the awk canonicalisation used to build the manifest. +// canonicalHash renders one file in the canonical form and hashes it. The byte +// format is fixed: any change to it invalidates every hash in the manifest. func canonicalHash(path string) (string, error) { wb, err := reader.Open(path) if err != nil { diff --git a/parser/internal/reader/reader.go b/parser/internal/reader/reader.go index fd1a1eb..2c67a4d 100644 --- a/parser/internal/reader/reader.go +++ b/parser/internal/reader/reader.go @@ -1,6 +1,9 @@ -// Package reader wraps the two spreadsheet libraries behind one streaming -// contract, mirroring parser/src/reader.rs (which wraps calamine's -// open_workbook_auto for both formats). +// Package reader wraps the two spreadsheet libraries behind one contract. +// +// Cell rendering and sheet geometry are pinned by the frozen fidelity oracle in +// parser/testdata/reader-fidelity-hashes.tsv. "The reference" below means that +// pinned rendering: whenever an underlying library disagrees with it, this +// package corrects the library, never the oracle. // // The *contract* is row-at-a-time: callers receive one row per callback and // never hold the workbook. The two implementations do not honour that in @@ -16,21 +19,19 @@ import ( "strings" ) -// Cell is one spreadsheet cell rendered the way calamine's Data::to_string() -// renders it. +// Cell is one spreadsheet cell rendered as reference text. // -// IsEmpty tracks calamine's Data::Empty variant separately from a string cell -// that happens to be empty. Rust distinguishes them at reader.rs:42, but both -// render "" and both count as blank in is_all_blank and in transform, so the -// flag is diagnostic only — never compare on it. +// IsEmpty marks a cell with no value, as opposed to a string cell that happens +// to be empty. Both render "" and both count as blank in IsAllBlank and in +// transform, so the flag is diagnostic only — never compare on it. type Cell struct { Str string IsEmpty bool } // Sheet carries a sheet's identity and geometry. Height and Width describe the -// used range, matching calamine's Range::height()/width(); every used range in -// the corpus starts at (0,0), verified across every input file. +// reference used range; every used range in the corpus starts at (0,0), +// verified across every input file. type Sheet struct { Index int Name string @@ -40,8 +41,8 @@ type Sheet struct { // RowFunc receives each row of each sheet. Rows are padded to the sheet's used // width — width is load-bearing because every column read downstream is -// positional with an unwrap_or_default() equivalent, so a short row silently -// NULLs its tail columns. +// positional and falls back to "" when the index is out of range, so a short +// row silently NULLs its tail columns. type RowFunc func(sheet Sheet, rowIdx int, row []Cell) error // Workbook is one opened spreadsheet. @@ -53,7 +54,7 @@ type Workbook interface { Close() error } -// Open dispatches on file extension, mirroring calamine's open_workbook_auto. +// Open dispatches on file extension. func Open(path string) (Workbook, error) { switch strings.ToLower(filepath.Ext(path)) { case ".xls": diff --git a/parser/internal/reader/xls.go b/parser/internal/reader/xls.go index 8f15c06..ca623e8 100644 --- a/parser/internal/reader/xls.go +++ b/parser/internal/reader/xls.go @@ -10,18 +10,16 @@ import ( // xlsWorkbook reads legacy BIFF through pbnjay/grate. // -// grate replaced extrame/xls, which was measured against calamine ground truth -// and found to corrupt 69% of cells and drop a further 28%: undecoded UTF-16LE -// and BIFF record framing leaked into cell values, content moved between rows -// and columns, and tail rows came back blank. That was charset-independent and -// unfixable from the outside. grate reproduces calamine exactly on the same -// files. +// Do not swap grate back for extrame/xls: on this corpus it corrupted 69% of +// cells and dropped a further 28% — undecoded UTF-16LE and BIFF record framing +// leaked into cell values, content moved between rows and columns, and tail +// rows came back blank. Charset-independent, and unfixable from the outside. // // The one normalisation grate needs is trailing blank rows: it yields rows past -// the end of calamine's used range (one for a populated sheet, two for an empty -// one), so trailing all-blank rows are trimmed. Note this is the opposite of -// the xlsx path, where excelize already trims and calamine keeps a 1x1 empty -// range — in both cases the rule is "match calamine's used range". +// the end of the reference used range (one for a populated sheet, two for an +// empty one), so trailing all-blank rows are trimmed. Note this is the opposite +// of the xlsx path, where excelize already trims and the reference keeps a 1x1 +// empty range — in both cases the rule is "match the reference used range". type xlsWorkbook struct { sheets []Sheet rows [][][]Cell @@ -56,7 +54,7 @@ func openXLS(path string) (Workbook, error) { raw = append(raw, cp) } - // Trim to calamine's used range. + // Trim to the reference used range. height := len(raw) for height > 0 && rowAllBlank(raw[height-1]) { height-- @@ -87,7 +85,7 @@ func openXLS(path string) (Workbook, error) { // demergeMarker blanks grate's merged-cell continuation markers. // -// grate fills the cells covered by a merge with sentinel runes; calamine +// grate fills the cells covered by a merge with sentinel runes; the reference // reports them as empty. In this corpus they occur only in the merged title // block of the 2016 spreadsheets (19 cells in rows 0-2 of one file). Only an // exact whole-value match is blanked, so a real cell that merely contains an diff --git a/parser/internal/reader/xlsx.go b/parser/internal/reader/xlsx.go index 399901c..895160d 100644 --- a/parser/internal/reader/xlsx.go +++ b/parser/internal/reader/xlsx.go @@ -8,12 +8,12 @@ import ( // xlsxWorkbook reads OOXML through excelize. // -// Two excelize behaviours must be corrected to match calamine: +// Two excelize behaviours must be corrected to match the reference: // -// 1. GetRows applies the cell number format by default, while calamine renders -// the underlying value. RawCellValue: true disables that. -// 2. GetRows trims trailing blank cells, so rows are ragged; calamine returns a -// rectangular used range. Rows are padded back out to the sheet width. +// 1. GetRows applies the cell number format by default, while the reference +// renders the underlying value. RawCellValue: true disables that. +// 2. GetRows trims trailing blank cells, so rows are ragged; the reference used +// range is rectangular. Rows are padded back out to the sheet width. type xlsxWorkbook struct { f *excelize.File sheets []Sheet @@ -35,15 +35,16 @@ func openXLSX(path string) (Workbook, error) { return nil, fmt.Errorf("excelize GetRows %s/%s: %w", path, name, err) } - // Do NOT trim trailing blank rows: calamine's used range keeps them, and - // excelize's GetRows already drops trailing fully-empty rows itself. + // Do NOT trim trailing blank rows: the reference used range keeps them, + // and excelize's GetRows already drops trailing fully-empty rows itself. // - // One correction is needed. 63 sheets in 2017-old and 53 in 2017-old2 - // hold a single empty shared-string cell at A1; calamine reports those - // as a 1x1 range, while GetRows returns nothing. A genuinely empty sheet - // (230 of them in 2016) is height 0 on both sides. GetCellType tells the - // two apart: the empty-shared-string cell exists in the XML and types as - // CellTypeSharedString, an absent cell types as CellTypeUnset. + // One correction is needed. A sheet holding a single empty + // shared-string cell at A1 is a 1x1 range to the reference, while + // GetRows returns nothing for it. A genuinely empty sheet (230 of them + // in 2016) is height 0 either way. + // GetCellType tells the two apart: the empty-shared-string cell exists + // in the XML and types as CellTypeSharedString, an absent cell types as + // CellTypeUnset. if len(raw) == 0 { if t, terr := f.GetCellType(name, "A1"); terr == nil && t != excelize.CellTypeUnset { raw = [][]string{{""}} diff --git a/parser/internal/reader/xlsx_fixups.go b/parser/internal/reader/xlsx_fixups.go index d66bc25..38f8fcd 100644 --- a/parser/internal/reader/xlsx_fixups.go +++ b/parser/internal/reader/xlsx_fixups.go @@ -10,10 +10,11 @@ import ( "github.com/xuri/excelize/v2" ) -// normalizeNumeric reproduces calamine's rendering of a numeric cell. +// normalizeNumeric reproduces the reference rendering of a numeric cell. // -// calamine parses a numeric cell to f64 and renders it with Rust's f64 Display, -// so the stored literal "6.0" becomes "6". RawCellValue hands back the literal. +// The reference parses a numeric cell to a float and prints its shortest +// round-tripping form, so the stored literal "6.0" becomes "6". RawCellValue +// hands back the literal instead. // // The cell type must be consulted, not guessed: "01063476", "6.00" and "NAN" are // all shared strings that survive ParseFloat, and renumbering them would drop a @@ -54,7 +55,7 @@ func normalizeNumeric(f *excelize.File, sheet string, col, row int, v string) st // // Go's encoding/xml performs the line-ending normalisation the XML 1.0 spec // mandates (CRLF and lone CR both become LF), so excelize returns "a\nb" where -// calamine — which reads the raw bytes — returns "a\r\nb". That difference +// the reference — which reads the raw bytes — returns "a\r\nb". That difference // reaches the database: in one 2016 file it affects 2,233 TEN_CUMTHI values, // which populate the ten_cum_thi column. // diff --git a/parser/internal/schema/schema.go b/parser/internal/schema/schema.go index fd6487e..a5c09ca 100644 --- a/parser/internal/schema/schema.go +++ b/parser/internal/schema/schema.go @@ -1,5 +1,5 @@ // Package schema is the single source of truth for the SQL shape of every -// dataset — a direct port of parser/src/schema.rs. +// dataset. // // Every dataset writes into the same 22-column student table. Columns a dataset has no data for bind NULL. // @@ -9,20 +9,22 @@ // khtn, khxh, gdcd, tieng_nga -> 2017 datasets only // everything else -> both // -// Before this consolidation the DDL, the INSERT and the subject regexes were -// duplicated across four TOML configs, which is how the 2016 and 2017 schemas -// drifted apart. The configs now carry only per-dataset parse rules. +// The DDL, the INSERT and the subject regexes belong here and nowhere else. One +// copy per dataset is what let the 2016 and 2017 schemas drift apart; the +// per-dataset configs carry only parse rules. package schema import "regexp" // DDL is executed verbatim after the output database is (re)created. // -// idx_ten_cum_thi is partial, so it holds zero entries on the three 2017 -// datasets — where the column is always NULL — while staying useful for the -// 2016 cluster-grouping queries. Partial indexes are SQLite-specific. +// idx_ten_cum_thi is partial, so it holds zero entries on the 2017 dataset — +// where the column is always NULL — while staying useful for the 2016 +// cluster-grouping queries. Partial indexes are SQLite-specific. // -// Byte-identical to parser/src/schema.rs:26-54; TestDDLMatchesRust enforces it. +// This text is frozen: it decides the shape of every database the parser +// produces. TestDDLIsFrozen holds an independent copy so any edit has to be +// deliberate. const DDL = ` CREATE TABLE student ( so_bao_danh TEXT PRIMARY KEY, @@ -103,15 +105,12 @@ VALUES ` // scorePatternSources holds the regex per subject, applied to the DIEM_THI cell -// text. Copied verbatim from parser/src/schema.rs:126-143 — the literals contain -// Vietnamese text and must never be retyped. +// text. The literals contain Vietnamese subject names exactly as they appear in +// the source files — copy them, never retype them. // // Every pattern runs against every dataset. A subject absent from a given exam // year simply never matches and stays NULL: 2016 files contain no "KHTN:" or // "Tiếng Nga:" tokens, and 2017 files contain no "Tiếng Đức:" or "Tiếng Nhật:". -// -// Go's regexp and Rust's regex crate are both RE2, and these patterns use no -// backreferences, lookaround or Unicode classes, so they port with zero risk. var scorePatternSources = map[string]string{ "toan": `Toán:\s*(\d+(?:\.\d+)?)`, "ngu_van": `Ngữ văn:\s*(\d+(?:\.\d+)?)`, @@ -131,9 +130,7 @@ var scorePatternSources = map[string]string{ "tieng_trung": `Tiếng Trung:\s*(\d+(?:\.\d+)?)`, } -// ScorePatterns holds the compiled subject regexes, compiled once at init. -// Rust compiles them once per run in CompiledPatterns::new; a package-level map -// is the equivalent for a single-threaded CLI. +// ScorePatterns holds the subject regexes, compiled once at package init. var ScorePatterns = func() map[string]*regexp.Regexp { out := make(map[string]*regexp.Regexp, len(scorePatternSources)) for field, src := range scorePatternSources { diff --git a/parser/internal/schema/schema_test.go b/parser/internal/schema/schema_test.go index f657e8c..d54f72b 100644 --- a/parser/internal/schema/schema_test.go +++ b/parser/internal/schema/schema_test.go @@ -5,11 +5,9 @@ import ( "testing" ) -// Ports the four tests in parser/src/schema.rs:149-213. Their purpose is to stop -// the DDL, the INSERT column list and the field-order constants from drifting -// apart — a drift that silently lands values in the wrong columns. +// These tests stop the DDL, the INSERT column list and the field-order constants +// from drifting apart — a drift that silently lands values in the wrong columns. -// TestInsertMatchesFieldOrder ports insert_matches_field_order (schema.rs:156). func TestInsertMatchesFieldOrder(t *testing.T) { if ParamCount != 22 { t.Errorf("ParamCount = %d, want 22", ParamCount) @@ -41,8 +39,6 @@ func TestInsertMatchesFieldOrder(t *testing.T) { } } -// TestScorePatternsCoverScoreFields ports score_patterns_cover_score_fields -// (schema.rs:183). func TestScorePatternsCoverScoreFields(t *testing.T) { if len(ScorePatterns) != len(ScoreFields) { t.Fatalf("%d patterns for %d score columns", len(ScorePatterns), len(ScoreFields)) @@ -63,7 +59,6 @@ func TestScorePatternsCoverScoreFields(t *testing.T) { } } -// TestDDLColumnsMatchInsert ports ddl_columns_match_insert (schema.rs:201). func TestDDLColumnsMatchInsert(t *testing.T) { for _, field := range append(append([]string{}, IdentityFields...), ScoreFields...) { if !strings.Contains(DDL, field) { @@ -72,9 +67,9 @@ func TestDDLColumnsMatchInsert(t *testing.T) { } } -// TestScorePatternsCompile ports score_patterns_compile (schema.rs:208). -// Compilation happens in the package initialiser, so reaching this point already -// proves it; the explicit checks guard against an empty or partial table. +// TestScorePatternsCompile: compilation happens in the package initialiser, so +// reaching this point already proves it; the explicit checks guard against an +// empty or partial table. func TestScorePatternsCompile(t *testing.T) { for field, re := range ScorePatterns { if re == nil { @@ -83,10 +78,12 @@ func TestScorePatternsCompile(t *testing.T) { } } -// TestDDLMatchesRust asserts the DDL is byte-identical to parser/src/schema.rs. -// Anything less and the two parsers can produce structurally different databases -// while every row-level check still passes. -func TestDDLMatchesRust(t *testing.T) { +// TestDDLIsFrozen compares DDL against an independent copy of the exact text, +// down to the byte. It catches column, type and index changes that every +// row-level check would still pass — a database can be structurally different +// and look fine one row at a time. Update the copy below only when the schema +// change is intended. +func TestDDLIsFrozen(t *testing.T) { const want = ` CREATE TABLE student ( so_bao_danh TEXT PRIMARY KEY, @@ -117,7 +114,7 @@ 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; ` if DDL != want { - t.Errorf("DDL diverges from parser/src/schema.rs:26-54\n--- got ---\n%s\n--- want ---\n%s", DDL, want) + t.Errorf("DDL changed\n--- got ---\n%s\n--- want ---\n%s", DDL, want) } } diff --git a/parser/internal/sqlitedb/sqlitedb.go b/parser/internal/sqlitedb/sqlitedb.go index 98ed4d6..401af14 100644 --- a/parser/internal/sqlitedb/sqlitedb.go +++ b/parser/internal/sqlitedb/sqlitedb.go @@ -5,17 +5,16 @@ // cgo-free — grate, excelize and yaml.v3 are pure Go too, so CI can compile with // CGO_ENABLED=0 and no C toolchain. // -// It is a machine-transpiled SQLite rather than the upstream C amalgamation that -// Rust's rusqlite --bundled vendors (libsqlite3-sys 0.30.1). Two things make that -// acceptable: this parser uses only plain SQL — no CTEs, window functions, -// triggers or extensions — and the differential gate compares a full-table -// SHA-256 plus PRAGMA table_info/index_list against live Rust output. +// It is a machine-transpiled SQLite, not the upstream C amalgamation, which is +// acceptable only because the parser sticks to plain SQL: no CTEs, window +// functions, triggers or extensions. Adding any of those means re-verifying the +// driver. // // Verified on linux/arm64 with v1.56.0 (SQLite 3.53.3): the full DDL including // the partial idx_ten_cum_thi index, INSERT OR REPLACE, and VACUUM. // -// Fallback if the differential gate ever implicates the driver: mattn/go-sqlite3 -// is upstream C at the cost of cgo. +// Fallback if the driver is ever implicated: mattn/go-sqlite3 is upstream C at +// the cost of cgo. package sqlitedb // Registers "sqlite" with database/sql. diff --git a/parser/internal/transform/ascii_crosscheck_test.go b/parser/internal/transform/ascii_crosscheck_test.go index 7ab5279..38a15af 100644 --- a/parser/internal/transform/ascii_crosscheck_test.go +++ b/parser/internal/transform/ascii_crosscheck_test.go @@ -9,17 +9,18 @@ import ( "github.com/tiennm99/thptqg/parser/internal/transform" ) -// TestToAsciiAgainstRustOutput cross-checks ToAscii against Rust on real data. +// TestToAsciiAgainstRustOutput cross-checks ToAscii against a reference database +// on real data. // -// A Rust-built database is its own oracle: every row carries ho_ten alongside -// the ho_ten_ascii that Rust derived from it, so the whole table is a -// name -> expected-slug corpus far broader than the 20 hand-picked unit cases. -// -// Point it at a Rust-built database: +// Such a database is its own oracle: every row carries ho_ten alongside the +// ho_ten_ascii derived from it, so the whole table is a name -> expected-slug +// corpus far broader than the 20 hand-picked unit cases. // // GO_PARSER_RUST_DB=/tmp/rust-2016.db go test ./internal/transform/ // -// Skips when unset, so the default suite stays hermetic. +// Skips when unset, so the default suite stays hermetic. The reference parser is +// gone from the tree, so no new oracle database can be produced and this always +// skips today. func TestToAsciiAgainstRustOutput(t *testing.T) { path := os.Getenv("GO_PARSER_RUST_DB") if path == "" { diff --git a/parser/internal/transform/transform.go b/parser/internal/transform/transform.go index 92af37b..a6d01bd 100644 --- a/parser/internal/transform/transform.go +++ b/parser/internal/transform/transform.go @@ -1,9 +1,5 @@ // Package transform performs row transformation: ASCII normalisation, score -// regex parsing, and validation — a port of parser/src/transform.rs. -// -// ToAscii replicates build-lib.js toAscii exactly: -// -// str.normalize("NFD").replace(/[̀-ͯ]/g,"").replace(/đ/gi,"d").toLowerCase() +// regex parsing, and validation. package transform import ( @@ -26,14 +22,11 @@ import ( // 3. Replace đ/Đ with d (NFD does not decompose them) // 4. Lowercase // -// Step 2 filters a LITERAL CODEPOINT RANGE, not a Unicode category. The inline -// comment at transform.rs:53 says "Unicode category M", but the code at :56 is -// the specification and it checks '\u{0300}'..='\u{036f}'. unicode.Is(unicode.Mn, r) -// is strictly broader and would strip marks Rust keeps, silently changing -// ho_ten_ascii — the column the site's accent-insensitive search runs on. -// -// Step order matters and mirrors transform.rs:54-63: the đ/Đ replacement happens -// before lowercasing. +// Step 2 filters a LITERAL CODEPOINT RANGE, not a Unicode category. +// unicode.Is(unicode.Mn, r) is strictly broader and would strip marks this keeps, +// silently changing ho_ten_ascii — the column the site's accent-insensitive +// search runs on. That search normalises the query with the same four steps in +// web/src/App.jsx toAscii; the two must produce identical output or lookups miss. func ToAscii(s string) string { if s == "" { return "" @@ -44,7 +37,7 @@ func ToAscii(s string) string { b.Grow(len(decomposed)) for _, r := range decomposed { if r >= 0x0300 && r <= 0x036F { - continue // combining mark, in the range Rust drops + continue // combining mark inside the dropped range } switch r { case 'đ', 'Đ': @@ -73,23 +66,22 @@ type ParsedRow struct { // SkipReason says why a row was skipped, or SkipNone when it passed. // -// The distinction is load-bearing for the printed counters, and the two -// non-blank reasons are counted as source rows while BlankRow is not — but note -// that split lives in the CALLER, not here. parser/src/main.rs has two call -// sites with opposite outcomes for BlankRow: at :135-137 it returns before the -// counter at :140, while at :151 it matches Err(BlankRow) => {} and falls -// through to transform and insert. The build loop must reproduce both. +// The distinction is load-bearing for the printed counters: the two non-blank +// reasons count as source rows while SkipBlankRow does not. That split lives in +// the CALLER, not here — the build loop drops blank rows before the source-row +// counter and, at the later switch, lets SkipBlankRow fall through to transform +// and insert. Both call sites have to stay as they are. type SkipReason int const ( SkipNone SkipReason = iota - // SkipBlankRow: row is fully blank (2017-old2 only, checked before the - // source-row counter). + // SkipBlankRow: row is fully blank, and strip_blank_rows is on. Checked + // before the source-row counter. SkipBlankRow // SkipEmptyField: so_bao_danh or ho_ten empty/missing. SkipEmptyField - // SkipNonNumericSbd: so_bao_danh contains non-digit characters - // (2017-old / 2017-old2 guard). + // SkipNonNumericSbd: so_bao_danh contains non-digit characters, and + // require_numeric_sbd is on. SkipNonNumericSbd ) @@ -109,10 +101,10 @@ func (s SkipReason) String() string { // ValidateRow checks a row against the dataset's validation rules. // -// Signature mirrors transform.rs:101-107, taking stripBlankRows and allBlank -// explicitly; a shorter signature could not express both blank-row paths. +// stripBlankRows and allBlank are passed explicitly because a shorter signature +// could not express both blank-row paths. func ValidateRow(hoTen, soBaoDanh string, cfg *config.ValidationCfg, stripBlankRows, allBlank bool) SkipReason { - // 2017-old2: skip fully blank rows BEFORE counting source rows. + // Skip fully blank rows BEFORE counting source rows. if stripBlankRows && allBlank { return SkipBlankRow } @@ -128,11 +120,11 @@ func ValidateRow(hoTen, soBaoDanh string, cfg *config.ValidationCfg, stripBlankR return SkipNone } -// allASCIIDigits mirrors Rust's chars().all(|c| c.is_ascii_digit()). +// allASCIIDigits reports whether every byte of s is an ASCII digit. // // Deliberately not strconv.Atoi: Atoi accepts a leading sign, so "+123" would -// pass a check Rust rejects. Empty input returns true, matching Rust's all() on -// an empty iterator — the empty case is caught earlier by RequireNonemptySbd. +// pass a check that must reject it. Empty input returns true; the empty case is +// caught earlier by RequireNonemptySbd. func allASCIIDigits(s string) bool { for i := 0; i < len(s); i++ { if s[i] < '0' || s[i] > '9' { @@ -146,7 +138,7 @@ func allASCIIDigits(s string) bool { // // Every one of the 16 patterns runs against every dataset; a subject absent from // a given exam year never matches and stays NULL. Matching is unanchored -// first-match, like Rust's Regex::captures. +// first-match. func ParseScores(diemThi string) map[string]float64 { out := make(map[string]float64) if diemThi == "" { @@ -161,8 +153,8 @@ func ParseScores(diemThi string) map[string]float64 { if err != nil { continue } - // Unreachable given the pattern shape, but kept for parity with - // transform.rs:136 (is_finite). + // Unreachable given the pattern shape; kept so a widened pattern can + // never write Inf or NaN into a score column. if math.IsInf(v, 0) || math.IsNaN(v) { continue } @@ -172,21 +164,20 @@ func ParseScores(diemThi string) map[string]float64 { } // ErrNoColumns is returned when the fixed-column path is used on a config that -// has no columns: mapping. Rust panics here via .expect() (transform.rs:162); -// returning an error is the Go-idiomatic equivalent and is unreachable in -// practice, since only the non-2016 path calls this. +// has no columns: mapping. Unreachable in practice, since only the non-2016 path +// calls TransformRow. var ErrNoColumns = errors.New("transform: config has no columns mapping") // TransformRow extracts one row into a ParsedRow using fixed column indices, -// the 2017-family path. 2016 uses runtime format detection instead. +// the 2017 path. 2016 uses runtime format detection instead. func TransformRow(raw []reader.Cell, cfg *config.DatasetConfig) (*ParsedRow, error) { cols := cfg.Columns if cols == nil { return nil, ErrNoColumns } - // Trimmed accessor, mirroring the closure at transform.rs:163-167. Out-of-range - // indices yield "" rather than an error, matching unwrap_or_default(). + // Trimmed accessor. Out-of-range indices yield "" rather than an error, so a + // short row produces empty fields instead of failing. get := func(idx int) string { if idx < 0 || idx >= len(raw) { return "" @@ -198,9 +189,9 @@ func TransformRow(raw []reader.Cell, cfg *config.DatasetConfig) (*ParsedRow, err ngaySinh := get(cols.NgaySinh) soBaoDanh := get(cols.SoBaoDanh) - // diem_thi is read WITHOUT trimming (transform.rs:172-175), unlike the three - // fields above. Harmless because the score patterns are unanchored, but it is - // the shipped behaviour — do not "tidy" it. + // diem_thi is read WITHOUT trimming, unlike the three fields above. Harmless + // because the score patterns are unanchored, but it is the shipped behaviour + // — do not "tidy" it. diemThi := "" if cols.DiemThi >= 0 && cols.DiemThi < len(raw) { diemThi = raw[cols.DiemThi].Str diff --git a/parser/internal/transform/transform_test.go b/parser/internal/transform/transform_test.go index 0e2e5e2..4ad7618 100644 --- a/parser/internal/transform/transform_test.go +++ b/parser/internal/transform/transform_test.go @@ -7,12 +7,7 @@ import ( "github.com/tiennm99/thptqg/parser/internal/reader" ) -// Ports every test in parser/src/transform.rs's test module (:201-409) — 29 in -// total, not the 20 in the :213-315 range, which covers only ToAscii. The nine -// outside that range are the ParseScores and ValidateRow cases, i.e. exactly the -// behaviours this package's traps concern. - -// --- ToAscii: the 20 cases at transform.rs:213-315 --- +// --- ToAscii --- func TestToAscii(t *testing.T) { cases := []struct{ name, in, want string }{ @@ -47,10 +42,9 @@ func TestToAscii(t *testing.T) { } // TestToAsciiUsesLiteralRangeNotUnicodeMn guards the highest-value trap in this -// package. transform.rs:56 filters the literal range U+0300..U+036F; the inline -// comment at :53 calls it "Unicode category M", but the code is the spec. -// unicode.Mn is strictly broader, so using it would strip marks Rust keeps. -// U+0654 (ARABIC HAMZA ABOVE) is in Mn but outside the range: Rust keeps it. +// package: ToAscii drops the literal range U+0300..U+036F, not Unicode category +// Mn, which is strictly broader. U+0654 (ARABIC HAMZA ABOVE) is in Mn but outside +// the range, so it must survive. func TestToAsciiUsesLiteralRangeNotUnicodeMn(t *testing.T) { const in = "aٔb" if got := ToAscii(in); got != in { @@ -76,7 +70,7 @@ func TestToAsciiDStrokeIndependentOfNFD(t *testing.T) { } } -// --- ParseScores: transform.rs:323, :332, :342 --- +// --- ParseScores --- func TestParseScoresSingle(t *testing.T) { s := ParseScores("Toán: 8.5") @@ -115,7 +109,7 @@ func TestParseScoresRealCellShape(t *testing.T) { } } -// --- ValidateRow: transform.rs:359, :365, :374, :383, :393, :400 --- +// --- ValidateRow --- func defaultValidation() *config.ValidationCfg { return &config.ValidationCfg{ @@ -165,9 +159,8 @@ func TestValidateBlankRowSkipped(t *testing.T) { } } -// TestValidateNumericSbdIsDigitScanNotAtoi: Rust uses chars().all(is_ascii_digit), -// which strconv.Atoi does not reproduce — Atoi accepts a leading sign, and would -// wrongly admit "+123". +// TestValidateNumericSbdIsDigitScanNotAtoi: the check is a digit scan, not +// strconv.Atoi — Atoi accepts a leading sign and would wrongly admit "+123". func TestValidateNumericSbdIsDigitScanNotAtoi(t *testing.T) { v := defaultValidation() v.RequireNumericSbd = true @@ -179,7 +172,7 @@ func TestValidateNumericSbdIsDigitScanNotAtoi(t *testing.T) { } // TestValidateBlankRowOnlyWhenStripEnabled: with strip_blank_rows false, an -// all-blank row falls through to the empty-field checks instead (transform.rs:109). +// all-blank row falls through to the empty-field checks instead. func TestValidateBlankRowOnlyWhenStripEnabled(t *testing.T) { if r := ValidateRow("", "", defaultValidation(), false, true); r != SkipEmptyField { t.Errorf("got %v, want SkipEmptyField when strip_blank_rows is off", r) @@ -227,7 +220,6 @@ func TestTransformRow(t *testing.T) { } } -// TestTransformRowEmptyNgaySinhBecomesNil ports transform.rs:179-183. func TestTransformRowEmptyNgaySinhBecomesNil(t *testing.T) { got, err := TransformRow(cells("A", "", "1", ""), fixedColumnCfg()) if err != nil { @@ -238,9 +230,8 @@ func TestTransformRowEmptyNgaySinhBecomesNil(t *testing.T) { } } -// TestTransformRowShortRowYieldsEmptyFields ports the unwrap_or_default() -// behaviour at transform.rs:163-176: a row shorter than the configured indices -// yields empty strings rather than an error. +// TestTransformRowShortRowYieldsEmptyFields: a row shorter than the configured +// indices yields empty strings rather than an error. func TestTransformRowShortRowYieldsEmptyFields(t *testing.T) { got, err := TransformRow(cells("OnlyName"), fixedColumnCfg()) if err != nil { @@ -252,8 +243,8 @@ func TestTransformRowShortRowYieldsEmptyFields(t *testing.T) { } // TestTransformRowDiemThiIsNotTrimmed pins an asymmetry that is easy to -// "tidy away": ho_ten, ngay_sinh and so_bao_danh are trimmed through the closure -// at transform.rs:164-168, but diem_thi is read raw at :172-175. +// "tidy away": ho_ten, ngay_sinh and so_bao_danh are trimmed, but diem_thi is +// read raw. func TestTransformRowDiemThiIsNotTrimmed(t *testing.T) { row := cells(" A ", " 01/01/2000 ", " 123 ", " Toán: 5 ") got, err := TransformRow(row, fixedColumnCfg()) @@ -273,8 +264,7 @@ func TestTransformRowDiemThiIsNotTrimmed(t *testing.T) { } // TestTransformRowRequiresColumns: the fixed-column path is only reachable when -// the config has a columns: mapping. Rust panics via .expect() (transform.rs:162); -// Go returns an error instead. +// the config has a columns: mapping, and must error rather than guess otherwise. func TestTransformRowRequiresColumns(t *testing.T) { cfg := &config.DatasetConfig{Validation: *defaultValidation()} if _, err := TransformRow(cells("A", "B", "C", "D"), cfg); err == nil { diff --git a/parser/internal/writer/writer.go b/parser/internal/writer/writer.go index a82ad07..339918e 100644 --- a/parser/internal/writer/writer.go +++ b/parser/internal/writer/writer.go @@ -1,12 +1,12 @@ // Package writer handles SQLite output: DDL setup, INSERT OR REPLACE, VACUUM -// and the stats block — a port of parser/src/writer.rs. +// and the stats block. // // Every dataset writes the same canonical table (internal/schema), so there is // exactly one insert path. Columns a dataset carries no data for bind NULL. // -// The stats lines are reproduced verbatim because they are the operator-facing -// output of the build, and docs/deployment-guide.md points at the per-file row -// counts for troubleshooting. +// The stats lines are the operator-facing output of the build, and +// docs/deployment-guide.md points at the per-file row counts for +// troubleshooting — do not reword them casually. package writer import ( @@ -23,9 +23,9 @@ import ( // OpenDB deletes any existing database at dbPath, recreates it, and executes the // canonical DDL. // -// Deleting the file rather than issuing DROP TABLE mirrors build-lib.js:54 via -// writer.rs:24-30. A consequence worth knowing: a concurrent reader sees the file -// vanish mid-rebuild rather than a transactional swap. +// Deleting the file rather than issuing DROP TABLE has a consequence worth +// knowing: a concurrent reader sees the file vanish mid-rebuild rather than a +// transactional swap. func OpenDB(dbPath string) (*sql.DB, error) { if _, err := os.Stat(dbPath); err == nil { if err := os.Remove(dbPath); err != nil { @@ -49,11 +49,8 @@ func OpenDB(dbPath string) (*sql.DB, error) { return db, nil } -// Inserter wraps a prepared INSERT statement. -// -// Rust calls conn.execute(INSERT_SQL, ...) per row (writer.rs:73), re-preparing -// each time. Preparing once is a performance choice, not a parity requirement — -// the SQL and its bindings are identical either way. +// Inserter wraps the canonical INSERT, prepared once per transaction rather +// than per row. type Inserter struct{ stmt *sql.Stmt } // Prepare compiles the canonical INSERT against tx. @@ -116,12 +113,6 @@ type Stats struct { // Finish runs VACUUM and prints the stats block. // // VACUUM must run AFTER the transaction commits — SQLite refuses it inside one. -// -// The Rust original varied the wording by dataset, keying off the input -// directory's basename (main.rs:98-101) — one phrasing for the 2017-old2 -// export, another for anything with "old" in its name. Both of those datasets -// have been removed, so only one branch was ever taken; the wording below is -// exactly what 2016 and 2017 have always printed. func Finish(db *sql.DB, dbPath string, st Stats) error { if _, err := db.Exec("VACUUM"); err != nil { return fmt.Errorf("vacuum: %w", err) diff --git a/parser/testdata/reader-fidelity-hashes.tsv b/parser/testdata/reader-fidelity-hashes.tsv index d6fa655..edc8822 100644 --- a/parser/testdata/reader-fidelity-hashes.tsv +++ b/parser/testdata/reader-fidelity-hashes.tsv @@ -1,13 +1,12 @@ -# Canonical cell-dump SHA-256 per input file, produced by the Rust/calamine -# ground truth (parser/examples/dump_cells.rs). The Go reader must reproduce -# each hash exactly. Hashes only - the dumps themselves are real student PII -# and are never committed, per parser/tests/fixtures/README.md. -# FROZEN. These hashes were produced by the Rust/calamine parser, which has -# since been removed, so they cannot be regenerated. They remain a real -# regression guard: any change to the Go reader that alters a single cell of any -# input file listed below fails the suite. If the inputs themselves ever change, -# the affected rows must be re-derived deliberately and reviewed, not refreshed -# in bulk. +# Canonical cell-dump SHA-256 per input file. The reader must reproduce each +# hash exactly. Hashes only - the dumps themselves are real student PII and are +# never committed. parser/cmd/dumpcells prints the stream a hash is taken over, +# which is how a failure is narrowed from a file to a cell. +# FROZEN. These cannot be regenerated, so a mismatch is a reader bug until +# proven otherwise, never a cue to refresh this file. Any change to the reader +# that alters a single cell of any input listed below fails the suite. If the +# inputs themselves ever change, re-derive only the affected rows, deliberately +# and reviewed, not in bulk. data/2016/023718c7d3cf7ace3a7116fabb12bd9cdhyduoccantho-1468920829104.xlsx fc3afd8da9ed28b0fc177192fab7b19a06535cb45d86841835d8e8828fc61c1c data/2016/08cdeb3636dcb2adaef829d62968274atravinh-1468902443859.xlsx 71f0e61918c63b95f08a16d502f5e9f2054e3c830bd83c725f1285bb1aabead1 data/2016/0b583c9875443e65ffa449d5ca76fae8ninhbinh-1468899778557.xlsx b0c2cfb9ab419442f151be5a014439f7f477fa7b31a9ae3f37b83073276bdb59 diff --git a/web/eslint.config.js b/web/eslint.config.js index 809a70e..2d571f0 100644 --- a/web/eslint.config.js +++ b/web/eslint.config.js @@ -4,9 +4,7 @@ import reactHooks from 'eslint-plugin-react-hooks' import reactRefresh from 'eslint-plugin-react-refresh' import { defineConfig, globalIgnores } from 'eslint/config' -// Scoped to this workspace. The other stages are Go, and the one remaining -// Node file outside it (parser/scripts/differential-parity.mjs, a diagnostic -// run by hand) is out of reach of a config that lives here. +// Scoped to this workspace; every other stage in the repository is Go. export default defineConfig([ globalIgnores(['dist']), { diff --git a/web/src/App.css b/web/src/App.css index 745a002..9f25414 100644 --- a/web/src/App.css +++ b/web/src/App.css @@ -781,11 +781,8 @@ footer { } } -/* --------------------------------------------------------------------------- - Table cells for the 2016-only identity columns. - Ported from the 2016 frontend; hidden automatically on the 2017 datasets - because every row there is NULL. - --------------------------------------------------------------------------- */ +/* Table cells for the 2016-only identity columns. The table hides them on other + datasets, where every row is NULL. */ .sbd-cell { font-family: ui-monospace, monospace; @@ -803,9 +800,7 @@ footer { white-space: nowrap; } -/* --------------------------------------------------------------------------- - Hub route (/thptqg/) — replaces the old standalone static index.html - --------------------------------------------------------------------------- */ +/* Hub route (/thptqg/) */ .hub { max-width: 42rem; diff --git a/web/src/App.jsx b/web/src/App.jsx index ba10a0c..e63b3b9 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -48,7 +48,7 @@ function DatasetApp({ dataset }) { const [results, setResults] = useState(null); const [searchError, setSearchError] = useState(null); const [activeTab, setActiveTab] = useState("search"); - // Query is owned here so we can bidirectionally bind it to the URL + // Owned here, not in SearchForm, so it can be bound to the URL both ways. const [query, setQuery] = useState(() => readUrlQuery()); const [totalCount, setTotalCount] = useState(null); @@ -98,16 +98,16 @@ function DatasetApp({ dataset }) { [db], ); - // Hydrate a ?q= deep link once, as soon as the database is ready. Fires at - // most once per mount, so it cannot cascade. + // Hydrate a ?q= deep link as soon as the database is ready. Keyed on `db` + // alone so it fires at most once per mount and cannot cascade. useEffect(() => { // eslint-disable-next-line react-hooks/set-state-in-effect if (db && query) handleSearch(query); // eslint-disable-next-line react-hooks/exhaustive-deps }, [db]); - // Read the candidate count for the footer once the database loads. One-shot - // per mount; the query result cannot change without a new database. + // Candidate count for the footer. One-shot per mount: the count cannot change + // without a new database. useEffect(() => { if (!db) return; const stmt = db.prepare("SELECT COUNT(*) AS c FROM student"); diff --git a/web/src/components/custom-query.jsx b/web/src/components/custom-query.jsx index 21b7af5..80321b8 100644 --- a/web/src/components/custom-query.jsx +++ b/web/src/components/custom-query.jsx @@ -3,8 +3,8 @@ import { useState, useCallback, useEffect } from "react"; const MAX_ROWS = 1000; export function CustomQuery({ db, disabled, presets = [] }) { - // Every preset set ends with a "Hệ thống" group whose first query dumps the - // table schema; that is what gets auto-run when the tab first opens. + // By convention every preset list ends with a "Hệ thống" group whose first + // query dumps the table schema. See lib/sql-presets.js. const schemaPreset = presets[presets.length - 1]?.queries[0]; const [sql, setSql] = useState(""); @@ -24,7 +24,8 @@ export function CustomQuery({ db, disabled, presets = [] }) { const trimmed = queryStr.trim(); if (!trimmed) return; - // Safety: only allow read-only statements + // Read-only statements only. The database is a per-browser copy, so this + // guards the user's own session against a typo, not the server. const upper = trimmed.toUpperCase(); const allowed = ["SELECT", "PRAGMA", "EXPLAIN", "WITH"]; if (!allowed.some((kw) => upper.startsWith(kw))) { @@ -34,7 +35,8 @@ export function CustomQuery({ db, disabled, presets = [] }) { return; } - // Auto-add LIMIT if user forgot + // Cap an unbounded SELECT: the tables run to hundreds of thousands of + // rows and rendering them all would lock the tab. let finalSql = trimmed; if ( upper.startsWith("SELECT") && diff --git a/web/src/components/hub.jsx b/web/src/components/hub.jsx index 69d9fb0..8761249 100644 --- a/web/src/components/hub.jsx +++ b/web/src/components/hub.jsx @@ -1,12 +1,6 @@ import { DATASETS, pathOf } from "../datasets"; -/** - * The /thptqg/ landing route. - * - * Replaces the hand-written static index.html the repo used to serve here, so - * the hub shares the app's styling and dataset list instead of repeating them. - * No database is fetched on this route. - */ +/** The /thptqg/ landing route. No database is fetched here. */ export function Hub() { const base = import.meta.env.BASE_URL; diff --git a/web/src/components/score-table.jsx b/web/src/components/score-table.jsx index 07f70ce..538844e 100644 --- a/web/src/components/score-table.jsx +++ b/web/src/components/score-table.jsx @@ -12,10 +12,9 @@ export function ScoreTable({ results }) { return

Không tìm thấy kết quả.

; } - // Drop columns where every row in the result set is NULL. This is what makes - // one table serve both exam years: 2016 rows surface Cụm thi / GT / Đức / - // Nhật and hide KHTN / KHXH / GDCD / Nga, and 2017 rows do the reverse — with - // no dataset conditional anywhere in this component. + // Drop columns that are NULL for every row in the result set. That is what + // lets one table serve both exam years with no dataset conditional here: each + // year's unused columns simply drop out. const visibleIdentity = IDENTITY_COLUMNS.filter((col) => hasAnyValue(results, col.key), ); diff --git a/web/src/components/student-detail.jsx b/web/src/components/student-detail.jsx index 1cdc399..7b42685 100644 --- a/web/src/components/student-detail.jsx +++ b/web/src/components/student-detail.jsx @@ -6,8 +6,8 @@ function fmt(n) { return n === null || n === undefined ? "—" : Number(n).toFixed(2); } -// Visible tier legend — keeps symbol + range + label co-located so users -// don't have to hover tiles to decode colors. +// Visible legend, so a user does not have to hover tiles to decode the colors. +// The ranges must stay in step with scoreTier() in lib/admission-blocks.js. const TIER_LEGEND = [ { key: "common", symbol: "·", range: "≤ 1", label: "Điểm liệt" }, { key: "uncommon", symbol: "○", range: "< 5", label: "Chưa đạt" }, diff --git a/web/src/datasets.js b/web/src/datasets.js index 6e5e401..99740b3 100644 --- a/web/src/datasets.js +++ b/web/src/datasets.js @@ -1,17 +1,13 @@ /** * The datasets this site serves. * - * `id` is the single identifier used end to end: - * - * data// → parser/configs/.yml → db/.db.gz → /thptqg// + * Which datasets exist is decided by the repository-root `datasets.json`, not + * here: the assembler is a Go program and cannot import this module. This file + * supplies only what the interface needs — titles, labels, search examples and + * SQL presets, keyed by id. * * Site path and database URL are derived from `id` rather than stored, so a * dataset cannot be misconfigured into pointing at the wrong database. - * - * Which datasets exist is not decided here. That lives in the repository-root - * `datasets.json`, which the assembler reads too — it is a Go program and - * cannot import this module. This file supplies only what the interface needs: - * titles, labels, search examples and SQL presets, keyed by id. */ import registry from "../../datasets.json"; @@ -24,17 +20,17 @@ const SUBTITLE = "Dữ liệu thí sinh toàn quốc · Hỗ trợ truy vấn SQ * Presentation, keyed by the ids declared in datasets.json. * * `source` is the full article URL the dataset's spreadsheets come from, shown - * in the footer as a link. The canonical copy of each URL is the crawler's - * `Article` field (crawler/internal/sources/source_.go); it is repeated here - * because a Go module and a Vite app cannot share a constant. + * in the footer as a link. The canonical copy is the crawler's `Article` field + * (crawler/internal/sources/source_.go); it is duplicated here because a Go + * module and a Vite app cannot share a constant. Keep the two in step. */ const CONTENT = { 2016: { label: "Kỳ thi 2016", title: "Tra cứu điểm thi THPT Quốc gia 2016", subtitle: SUBTITLE, - // The article the 119 cluster spreadsheets are fetched from — a school - // site that aggregated every cluster's file, not the ministry. + // A school site that aggregated every exam cluster's spreadsheet, not the + // ministry — hence the unexpected domain. source: "https://dtnt.bacninh.edu.vn/tin-tuc/tin-tuc-su-kien/cong-bo-diem-thi-thptqg-2016-toan-bo-120-cum-thi-da-co-diem.html", examples: ["17006021", "Nguyễn Thị Hoa"], @@ -51,10 +47,9 @@ const CONTENT = { }, }; -// A dataset in the registry with no content here would render a page with no -// title and no presets; content here for a dataset that no longer exists would -// offer a link to a database that was never built. Neither should reach a -// browser, so both fail at module load. +// Fail at module load rather than in the browser: a registry entry with no +// content here renders a page with no title and no presets, and content for an +// unregistered id links to a database that was never built. for (const { id } of registry.datasets) { if (!CONTENT[id]) { throw new Error(`datasets.json declares "${id}" but web/src/datasets.js has no content for it`); @@ -69,8 +64,8 @@ for (const id of Object.keys(CONTENT)) { export const DATASETS = registry.datasets.map((d) => ({ id: d.id, dbSizeMb: d.dbSizeMb, - // Derived, not written twice: the candidate count the hub shows is the same - // number the assembler enforces, so the two cannot drift apart. + // Derived from expectedRows so the count the hub shows is the same number the + // assembler enforces. blurb: `${d.expectedRows.toLocaleString("vi-VN")} thí sinh`, ...CONTENT[d.id], })); diff --git a/web/src/hooks/use-sqlite.js b/web/src/hooks/use-sqlite.js index 82ef903..06179c8 100644 --- a/web/src/hooks/use-sqlite.js +++ b/web/src/hooks/use-sqlite.js @@ -1,13 +1,15 @@ import { useState, useEffect, useRef } from "react"; import initSqlJs from "sql.js"; -// sql.js WASM file served from node_modules via Vite's public dir or CDN +// The sql.js engine itself, fetched from the upstream CDN rather than bundled. +// It is a hard runtime dependency: if this URL is unreachable, initSqlJs() +// rejects and no dataset can be opened at all, whatever the database fetch does. const SQL_WASM_URL = "https://sql.js.org/dist/sql-wasm.wasm"; /** - * Hook to load a SQLite database from a URL into sql.js. - * Returns { db, loading, error, progress }. + * Load a SQLite database from a URL into sql.js. A `.gz` URL is decompressed in + * the browser. Returns { db, loading, error, progress }. */ export function useSqlite(dbUrl) { const [db, setDb] = useState(null); @@ -21,10 +23,8 @@ export function useSqlite(dbUrl) { async function load() { try { - // Init sql.js WASM const SQL = await initSqlJs({ locateFile: () => SQL_WASM_URL }); - // Fetch the gzipped database with progress tracking const response = await fetch(dbUrl); if (!response.ok) throw new Error(`Failed to fetch database: ${response.status}`); @@ -45,11 +45,9 @@ export function useSqlite(dbUrl) { if (cancelled) return; - // Combine chunks into single ArrayBuffer const blob = new Blob(chunks); let arrayBuffer; - // If URL ends with .gz, decompress using DecompressionStream if (dbUrl.endsWith(".gz")) { const ds = new DecompressionStream("gzip"); const decompressed = blob.stream().pipeThrough(ds); diff --git a/web/src/lib/admission-blocks.js b/web/src/lib/admission-blocks.js index 15cb2f6..b99c600 100644 --- a/web/src/lib/admission-blocks.js +++ b/web/src/lib/admission-blocks.js @@ -1,13 +1,10 @@ -// Vietnamese university admission-block ("khối thi") definitions. -// Each block is the sum of 3 subjects; we only compute a block when ALL three -// subject scores exist for the student. +// Vietnamese university admission-block ("khối thi") definitions, per Circular +// 03/2017/TT-BGDĐT. Each block is the sum of exactly 3 subject scores. // -// One list serves both exam years. computeBlocks() skips any block with a -// missing subject score, so blocks needing GDCD self-exclude on 2016 rows and -// blocks needing Tiếng Đức / Nhật self-exclude wherever those weren't sat — -// no per-year branching required. -// -// Based on Circular 03/2017/TT-BGDĐT. +// One list serves both exam years with no per-year branching: computeBlocks() +// skips any block with a missing subject score, so blocks needing GDCD +// self-exclude on 2016 rows, and blocks needing a language the candidate did +// not sit self-exclude row by row. export const ADMISSION_BLOCKS = [ { code: "A00", subjects: ["toan", "vat_ly", "hoa_hoc"], label: "Toán + Lý + Hóa" }, { code: "A01", subjects: ["toan", "vat_ly", "tieng_anh"], label: "Toán + Lý + Anh" }, @@ -59,11 +56,8 @@ export const ADMISSION_BLOCKS = [ { code: "D14", subjects: ["ngu_van", "lich_su", "tieng_anh"], label: "Văn + Sử + Anh" }, { code: "D15", subjects: ["ngu_van", "dia_ly", "tieng_anh"], label: "Văn + Địa + Anh" }, - // German and Japanese blocks. Previously omitted because neither language - // appeared in the 2017 database — but that was a parser gap, not reality: - // unifying the subject patterns recovered German and Japanese scores in every - // dataset. Students who sat neither are unaffected, since computeBlocks() - // skips blocks with a missing subject. + // Out of code order. computeBlocks() sorts by total and ties break on list + // position, so resorting this list would reshuffle equal-scoring blocks. { code: "D05", subjects: ["toan", "ngu_van", "tieng_duc"], label: "Toán + Văn + Đức" }, { code: "D06", subjects: ["toan", "ngu_van", "tieng_nhat"], label: "Toán + Văn + Nhật" }, ]; @@ -81,9 +75,11 @@ export function computeBlocks(student) { return out.sort((a, b) => b.total - a.total); } -// Score tier: TFT rarity-style ladder. 6 stops match the Vietnamese exam -// reality (≤1 is "điểm liệt" — automatic fail regardless of other scores). -// Color is paired with a unicode symbol so meaning is never color-only. +// Score tier for a single subject, as a rarity-style ladder of 6 stops. The +// lowest stop is the Vietnamese "điểm liệt" rule: ≤ 1 is an automatic fail +// whatever the other scores are, so that boundary is inclusive while the rest +// are exclusive upper bounds. Every tier carries a unicode symbol as well as a +// color, so the meaning is never conveyed by color alone. export function scoreTier(score) { if (score === null || score === undefined) return null; if (score <= 1) return { key: "common", symbol: "·", label: "Điểm liệt" }; diff --git a/web/src/lib/query-mode.js b/web/src/lib/query-mode.js index 0b7b869..4a4f02d 100644 --- a/web/src/lib/query-mode.js +++ b/web/src/lib/query-mode.js @@ -1,9 +1,6 @@ /** - * Classify a search box query as an exam ID (số báo danh) or a name. - * - * Shared by App.jsx and search-form.jsx. They previously carried separate - * copies of this rule and had already drifted: the 2016 app matched - * `/^[A-Z]{2,4}\d+$/i || /^\d+$/` while the 2017 app matched only `/^\d+$/`. + * Classify a search box query as an exam ID (số báo danh) or a name. Shared by + * App.jsx and search-form.jsx so the two cannot disagree on what a query is. * * SBD formats across both exam years: * @@ -11,9 +8,8 @@ * 017006021 2016 — 9 digits including a leading zero * BAL000001 2016 — 2-4 letter exam-cluster code then digits * - * That last form is not an edge case: 616,593 of 877,461 candidates in the - * 2016 dataset (70.3%) have a letter-prefixed SBD. Matching digits only would - * break exam-ID lookup for most of that year. + * The letter prefix is not an edge case: 70% of the 2016 candidates have one, + * so a digits-only pattern would break exam-ID lookup for most of that year. */ const SBD_PATTERN = /^[A-Za-z]{0,4}\d+$/; diff --git a/web/src/lib/sql-presets.js b/web/src/lib/sql-presets.js index 5b5661f..82cdc73 100644 --- a/web/src/lib/sql-presets.js +++ b/web/src/lib/sql-presets.js @@ -1,13 +1,14 @@ /** * SQL presets shown in the "Truy vấn SQL" tab, per dataset. * - * These are genuinely dataset-specific and cannot be shared: + * These cannot be shared between years: * - 2016 has ten_cum_thi / gioi_tinh to group by, and no KHTN/KHXH/GDCD * - 2017 has the composite KHTN/KHXH scores, and its SBDs are province- * prefixed 8-digit numbers, which the Long An (49xxx) queries rely on * - * Both end with a "Hệ thống" group whose first query is run automatically when - * the SQL tab is first opened, so the schema is visible before writing a query. + * Each list must end with a "Hệ thống" group: custom-query.jsx runs that group's + * first query automatically when the SQL tab opens, so the schema is on screen + * before the user writes anything. */ export const PRESETS_2017 = [ @@ -60,11 +61,10 @@ WHERE so_bao_danh LIKE '49%' ORDER BY tong_khoi_a DESC LIMIT 10`, }, { - // Top 100 Long An students by their best of 49 official 2017 - // admission blocks. Each (student, block) pair is materialised via - // UNION ALL; ROW_NUMBER picks each student's #1 block; the outer - // SELECT ranks across students. Result includes the winning block - // code so the user sees which combination produced the score. + // Each (student, block) pair is materialised by UNION ALL, ROW_NUMBER + // picks each student's best block, and the outer SELECT ranks across + // students. The winning block code is returned so the user can see + // which combination produced the score. label: "Top 10 điểm khối cao nhất - Long An", sql: `WITH per_block AS ( SELECT so_bao_danh, ho_ten, ngay_sinh, 'A00' k, toan+vat_ly+hoa_hoc s FROM student WHERE so_bao_danh LIKE '49%' diff --git a/web/src/lib/subjects.js b/web/src/lib/subjects.js index 6c1d6e2..4402f86 100644 --- a/web/src/lib/subjects.js +++ b/web/src/lib/subjects.js @@ -1,13 +1,11 @@ /** - * The 16 subject columns of the canonical schema, in display order. + * The 16 subject columns of the canonical schema, in display order. Mirrors + * `ScoreFields` in parser/internal/schema/schema.go, and is the single list both + * score-table.jsx and student-detail.jsx render from. * - * Mirrors `ScoreFields` in parser/internal/schema/schema.go. Previously this list was - * maintained separately in score-table.jsx and student-detail.jsx, which is how - * they drifted out of sync with each other and with the database. - * - * No subject is dataset-specific here. Columns a given exam year has no data - * for are simply NULL for every row, and the UI hides all-NULL columns — so one - * list serves 2016 and 2017 without branching. + * Nothing here is dataset-specific. Columns an exam year has no data for are + * NULL on every row, and the UI hides all-NULL columns, so one list serves both + * years without branching. */ export const SUBJECTS = [ @@ -30,11 +28,9 @@ export const SUBJECTS = [ ]; /** - * Non-score columns worth showing in the results table. - * - * Only the 2016 dataset populates these; they are NULL throughout 2017 - * and get hidden by the same all-NULL filter that hides unused - * subjects, so no per-dataset conditional is needed. + * Non-score columns worth showing in the results table. Only the 2016 dataset + * populates these; the same all-NULL filter that hides unused subjects hides + * them elsewhere, so no per-dataset conditional is needed. */ export const IDENTITY_COLUMNS = [ { key: "ten_cum_thi", label: "Cụm thi" }, diff --git a/web/src/router.js b/web/src/router.js index f17117b..bd15b16 100644 --- a/web/src/router.js +++ b/web/src/router.js @@ -7,15 +7,9 @@ * /thptqg/2016/ → 2016 * /thptqg/2017/ → 2017 * - * That makes this an exact match on a single segment. The nested form this - * replaced (/thptqg/2017/old/) needed longest-prefix matching, because - * "/thptqg/2017/old/" also starts with "/thptqg/2017/" — an entire class of - * routing bug the flat scheme designs out. - * - * Those nested URLs used to be rewritten here so old bookmarks kept working. - * Both of them addressed the 2017-old and 2017-old2 datasets, which no longer - * exist, so there is nothing left to rewrite them to: they now fall through to - * the hub like any other unknown path. + * Keeping them flat makes this an exact match on a single segment; nested + * routes would need longest-prefix matching to disambiguate. Unknown paths fall + * through to the hub. */ import { DATASETS } from "./datasets"; diff --git a/web/vite.config.js b/web/vite.config.js index 8e62cfb..692a426 100644 --- a/web/vite.config.js +++ b/web/vite.config.js @@ -1,26 +1,17 @@ import { defineConfig } from "vite"; import react from "@vitejs/plugin-react"; -// One build serves every page. The app resolves which dataset to show from the -// URL (src/router.js), so there are no per-dataset build variants. +// One build serves every page; src/router.js resolves the dataset from the URL. // -// `base` is absolute, which is what lets the single emitted index.html work as -// an entry point at any depth: it references /thptqg/assets/... regardless of -// the directory it is served from. The deploy step copies it to each dataset -// path, so every URL is a real static file and no SPA 404-fallback is needed — -// and the existing ?q= deep links keep working, which that fallback would break. +// `base` must stay absolute. It makes the single emitted index.html work as an +// entry point at any depth, because asset URLs read /thptqg/assets/... no matter +// which directory serves the page. The deploy step copies that file to every +// dataset path, so each URL is a real static file and needs no SPA 404-fallback +// — a fallback would break the ?q= deep links. // -// publicDir holds only the gzipped databases, staged there by the assembler -// (assembler/internal/databases). Nothing uncompressed is ever placed in it. -// -// It sits at the repository root rather than inside this workspace because -// parser writes it, so the path has to climb out of web/. Vite resolves -// publicDir against the project root, which is this directory. -// -// outDir is left at its default, so the build lands in web/dist and this -// workspace stays self-contained. The Pages artifact (_site) is assembled at -// the repository root by the assembler (assembler/internal/site), since that is -// where the deploy action uploads from. +// publicDir points outside this workspace because the assembler stages the +// gzipped databases there (assembler/internal/databases). Vite resolves the path +// against the project root, which is this directory. export default defineConfig({ plugins: [react()], base: "/thptqg/",