test(optimiser): add unit tests and fix linting for optimiser

[thejus03]

Resolves #4406

Summary

• Add comprehensive unit tests for _models, _modules, and _solver packages (hasConflict, copyState, scoreTimetableState, beamSearch helpers, mergeAndFilterModuleSlots, nusmods link serialization, etc.)
• Fix golangci-lint configuration to scan all packages including those with _ prefix (previously ./... silently skipped them)
• Fix CI and package.json lint commands to explicitly list all packages
• Fix linting issues caught by the corrected scan: comment formatting, VenuesJson → VenuesJSON, error string capitalisation, wasted assignment, shadowed variables, type assertion safety
• Add formatting check step to CI with actionable error message

Test plan

• Verify the optimiser still works end-to-end via pnpm start:optimiser
• pnpm lint:optimiser:code passes with 0 issues
• pnpm lint:optimiser:styles reports no formatting issues

🤖 Generated with Claude Code

[vercel]

@thejus03 is attempting to deploy a commit to the modsbot's projects Team on Vercel.

A member of the Team first needs to authorize it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.76%. Comparing base (988c6fd) to head (b4bf645).
⚠️ Report is 243 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4410      +/-   ##
==========================================
+ Coverage   54.52%   56.76%   +2.23%     
==========================================
  Files         274      317      +43     
  Lines        6076     7059     +983     
  Branches     1455     1699     +244     
==========================================
+ Hits         3313     4007     +694     
- Misses       2763     3052     +289     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[hacktron-app]

⏭️ Hacktron Security Check — Skipped

Reason: Unable to determine the PR author. The scan cannot proceed without a valid author.

[jloh02]

@greptile

[greptile-apps]

Greptile Summary

This PR adds comprehensive unit tests for the _models, _modules, and _solver Go packages in the timetable optimiser, and fixes the golangci-lint configuration so that packages with _ prefixes are scanned. Several linting issues surfaced by the corrected scan are also resolved (comment formatting, VenuesJson → VenuesJSON, error string capitalisation, type assertion safety, wasted assignment, shadowed variables).

• Test coverage: New _test.go files cover ParseTimeToMinutes, ParseModuleSlotFields, mergeAndFilterModuleSlots (free-day/time-range/recording bypass/dedup/E-venue logic), hasConflict, copyState, scoreTimetableState, beamSearch helpers, and NUSMods shareable link serialisation.
• Lint fix: .golangci.yaml updated to disable overly strict rules (gochecknoglobals, gocognit, mnd) and CI/package.json updated to explicitly pass . ./_*/ so _-prefixed packages are included in scans.
• Source fixes: _client, _constants, _modules, _solver cleaned up for comment style, naming conventions, and minor Go idioms (for i := range limit, maps.Copy).

Confidence Score: 5/5

Safe to merge — changes are additive test files plus minor linting clean-ups with no behavioural impact on the optimiser's runtime logic.

All production-path changes are cosmetic (comment wording, identifier naming, whitespace). No logic is altered in the hot-path. New tests exercise the existing code and all pass. The only finding is a documentation wording issue in the README.

No files require special attention.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CI: optimiser job] --> B[go vet / build]
    B --> C[golangci-lint run . ./_*/]
    C --> D{formatting diff?}
    D -- yes --> E[fail with fix hint]
    D -- no --> F[run unit tests loop]
    F --> G["for dir in ./_*/\nexclude _test/\ngo test -count=1 -v dir"]
    G --> H[start integration server\nport 8020]
    H --> I[go test ./_test/. -v]
    I --> J[stop server]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[CI: optimiser job] --> B[go vet / build]
    B --> C[golangci-lint run . ./_*/]
    C --> D{formatting diff?}
    D -- yes --> E[fail with fix hint]
    D -- no --> F[run unit tests loop]
    F --> G["for dir in ./_*/\nexclude _test/\ngo test -count=1 -v dir"]
    G --> H[start integration server\nport 8020]
    H --> I[go test ./_test/. -v]
    I --> J[stop server]

Loading

_{Reviews (2): Last reviewed commit: "Merge branch 'master' into tests/new-tes..." | Re-trigger Greptile}

[greptile-apps]

pnpm lint now requires golangci-lint on all developer machines

The existing "lint": "run-p \"lint:**\"" script uses a glob that matches any script whose name begins with lint:, including the newly added lint:optimiser:code and lint:optimiser:styles. This means running pnpm lint, pnpm check, or pnpm ci now silently requires golangci-lint to be installed. Frontend developers who run pnpm lint to check JS/TS changes will get a command-not-found failure if they don't have Go tooling set up. The CI correctly uses the individual commands, so this only affects local development.

Prompt To Fix With AI

This is a comment left during a code review.
Path: website/package.json
Line: 20-21

Comment:
**`pnpm lint` now requires `golangci-lint` on all developer machines**

The existing `"lint": "run-p \"lint:**\""` script uses a glob that matches any script whose name begins with `lint:`, including the newly added `lint:optimiser:code` and `lint:optimiser:styles`. This means running `pnpm lint`, `pnpm check`, or `pnpm ci` now silently requires `golangci-lint` to be installed. Frontend developers who run `pnpm lint` to check JS/TS changes will get a command-not-found failure if they don't have Go tooling set up. The CI correctly uses the individual commands, so this only affects local development.

How can I resolve this? If you propose a fix, please make it concise.

[thejus03]

I think this is fine? Since there are explicit commands for linting just website frontend code (eg; pnpm lint:code). @jloh02

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
nusmods-export	Ignored	Preview	Jun 21, 2026 2:42pm
nusmods-website	Ignored	Preview	Jun 21, 2026 2:42pm