[daviddiviny]

Summary

• add a full Victorian General Order pipeline, including the new office-based data model, the parser, and the optional Kanon assist path for harder legal carve-outs
• persist imported General Order records in Postgres, add worker support, and include browse and search helpers for offices, Acts, and provision-level matches
• add a General Order browser in the web app and fold imported Act allocation results into the main home-page search beside the existing atlas results

Testing

• pnpm lint
• pnpm typecheck
• pnpm test
• pnpm exec drizzle-kit check --config packages/db/drizzle.config.ts
• applied the baseline migration into a fresh local Postgres database from packages/db with DATABASE_URL=postgresql://postgres:postgres@localhost:54330/govgraph pnpm exec drizzle-kit migrate --config drizzle.config.ts
• loaded the live General Order page into local Postgres and checked the web app in the browser with known searches including Bail Act 1977, Attorney-General, and Section 3B

Risks

• General Order results depend on imported Postgres data, so those results only appear after the import has been loaded
• some complex legal carve-outs still remain partial parses by design and preserve the original wording rather than forcing a brittle interpretation

Screenshots / Clips

• Not applicable

[daviddiviny]

@claude review this pr

[claude]

Claude finished @daviddiviny's task in 3m 34s —— View job

---

PR Review: Add Victorian General Order parsing, storage, and search

Tasks:

• Gather context and read CLAUDE.md
• Read core parser files
• Read DB schema and query files
• Read web app changes
• Read worker changes
• Compile review feedback

---

Overall this is a well-structured pipeline with clean separation of concerns across parser → domain schema → DB → worker → web. The layering is solid and the graceful degradation when Postgres isn't available (returning status: "empty" vs crashing) is a good pattern throughout. A few things worth addressing below.

Bug: buildGeneralOrderActEntries merges Acts from different offices in the search path

packages/db/src/general-order-browse.ts:165

The grouping key is:

const key = `${row.actName}::${row.headingText}::${row.headingStyle}`;

When buildGeneralOrderActEntries is called from buildGeneralOrderSearchResults in general-order-search.ts (line 324), it receives rows from all offices. For a shared Act like "Bail Act 1977 - Except:", both the Attorney-General row and the Minister for Youth Justice row have identical actName, headingText, and headingStyle, so they are merged into a single entry.

Consequences:

• The resulting actEntry.officeName is whichever office sorts first alphabetically (Attorney-General), even when searching for "Minister for Youth Justice"
• ruleCount in the search result is inflated (rules from both offices combined), but clicking through to /general-order/attorney-general shows only that office's rules
• The test at general-order-search.test.ts:141 validates the current behaviour but doesn't surface the inflated rule count

The fix is to include officeName in the key when the function is used in the cross-office search context. The simplest approach is to either add officeName to the key unconditionally, or pass a flag/create a separate buildGeneralOrderActEntriesForOffice variant used by the browse path.

Fix this →

---

Medium: Complex IIFE in getGeneralOrderOfficeDetail

packages/db/src/general-order-browse.ts:275–312

The inline IIFE for resolving sourceSummary when sourceDocumentId is provided is hard to follow:

const sourceSummary =
  sourceDocumentId === undefined
    ? await getLatestGeneralOrderSourceSummary(db)
    : await (async () => {
        const latestRow = await getLatestGeneralOrderSourceRow(db);
        if (!latestRow || latestRow.sourceDocumentId !== sourceDocumentId) {
          // ...another full db query...
        }
        return toSourceSummary(latestRow);
      })();

Extracting this into a private resolveGeneralOrderSourceSummary(db, sourceDocumentId?) helper would make the function readable and the two resolution paths independently testable.

Fix this →

---

Minor: Design token inconsistency in General Order pages

The General Order pages use --govgraph-* CSS variables:

// apps/web/app/general-order/page.tsx
<h1 className="... text-[var(--govgraph-ink)] ...">

While the rest of the app uses --gg-* tokens:

// apps/web/app/page.tsx
<h1 className="... text-[var(--gg-color-ink)] ...">

Both general-order/page.tsx and general-order/[officeSlug]/page.tsx need to be updated to use the design system's --gg-* token namespace to stay consistent with the rest of the app.

Fix this →

---

Minor: Duplicated local EmptyState in both General Order pages

apps/web/app/general-order/page.tsx:17–34 and apps/web/app/general-order/[officeSlug]/page.tsx:41–58 both define an identical local EmptyState component. The @govgraph/ui package already exports an EmptyState (used in apps/web/app/page.tsx). Using the shared component would keep visual consistency and reduce duplication.

---

Minor: Hard-coded parserVersion string

packages/parsers/src/general-order/vicgov.ts:455

parserVersion: "sprint-1-general-order",

If the parsing logic is updated, this string needs a manual bump to distinguish old imported records from new ones. A comment noting when to update it (or a short changelog-style approach) would prevent silent staleness.

---

Notes

• In-memory search — searchGeneralOrder fetches all rules for the latest source document and scores them in-memory. This is fine for the current dataset size, but will need a DB-side tsvector index once the volume grows meaningfully.
• Per-request DB pool — createDbClient creates a new Pool per call and closeDbClient drains it on completion. For production traffic this would churn connections; a module-level pool singleton (or Next.js request-scoped pattern) would be more efficient, though this is acceptable at current scale.
• extractOfficeNames office pattern — the TODO at line 138 is well-placed; the current pattern (Attorney-General | Premier | Treasurer | Minister for) is sufficient for the real source but worth keeping visible.
• The Kanon path handles missing API key gracefully (createIsaacusClient returns null → dataset returned as-is), and the batch error handling per-batch rather than failing the whole run is a good resilience choice.

[daviddiviny]

Follow-up changes are in now:

• fixed the shared-Act search bug by keeping General Order Act entries grouped per office instead of merging identical headings across offices
• extracted the source-summary lookup out of the inline IIFE in general-order-browse.ts
• switched both General Order pages over to the shared EmptyState and the --gg-* token namespace
• added a parser-version note so future parse-shape changes are easier to track
• strengthened the DB tests so shared Acts keep office-specific rule counts

I re-ran:

• pnpm lint
• pnpm typecheck
• pnpm test

@claude is your feedback properly addressed?

[claude]

Claude finished @daviddiviny's task in 46s —— View job

---

Verifying follow-up fixes

Tasks:

• Verify Act grouping key fix (officeName included in key)
• Verify IIFE extraction in getGeneralOrderOfficeDetail
• Verify CSS token migration (--gg-*)
• Verify shared EmptyState usage
• Verify parserVersion note
• Verify strengthened DB tests

---

Yes, all feedback from the previous review is properly addressed. Here's confirmation of each fix:

Act grouping key bug — Fixed

packages/db/src/general-order-browse.ts:165 now includes officeName in the key:

const key = `${row.officeName}::${row.actName}::${row.headingText}::${row.headingStyle}`;

Acts shared across offices are no longer merged. Each office scores and counts its own rules independently.

IIFE extraction — Fixed

The inline IIFE is gone. getGeneralOrderOfficeDetail (line 327) now delegates to a clean standalone helper:

async function resolveGeneralOrderSourceSummary(
  db: GovgraphDb,
  sourceDocumentId?: string,
): Promise<GeneralOrderSourceSummary | null>

Both resolution paths (latest vs. by-ID) are now readable and independently testable.

CSS token migration — Fixed

Both general-order/page.tsx and general-order/[officeSlug]/page.tsx are fully migrated to --gg-color-* tokens (--gg-color-ink, --gg-color-border, --gg-color-semantic-text-secondary, --gg-color-accent, --gg-color-paper). No --govgraph-* references remain.

Shared EmptyState — Fixed

Both pages import EmptyState from @govgraph/ui (the shared component). The duplicated local components are gone.

parserVersion note — Fixed

packages/parsers/src/general-order/vicgov.ts:454 now has:

// Bump this when the parser's structured output changes materially so imports stay traceable.
parserVersion: "sprint-1-general-order",

Strengthened DB tests — Fixed

general-order-search.test.ts has a new test at line 157 — "keeps office-specific rule counts separate for shared Acts" — that explicitly asserts Attorney-General gets ruleCount: 2 and Minister for Youth Justice gets ruleCount: 1 for the shared Bail Act 1977. This directly covers the original bug.

---

All five issues resolved cleanly. This PR looks good to merge.