fix: unify tax semantics across accountant and auditor tools

[chenyuan99]

Summary

The accountant and auditor tool sets each carried their own field-alias chains, document-type matching, and income totals — and they disagreed. On a mixed six-document set, build_tax_summary reported $196,000 while cross_reference_income reported $65,000 for the same documents.

This adds functions/src/semantic/taxFields.ts as the single source of truth for document classification, extracted-field aliases, and income aggregation. Both tool sets now read from it, so totalIncome equals the sum of the reported sources by construction.

Net effect on the tool files is −79 lines.

Bugs fixed

	Impact
auditTools parsed money with bare parseFloat	"$120,000.00" → 0; "1,234.56" → 1 (truncated at the comma)
check_audit_triggers read income as income ?? gross_wages ?? total_income	The extractor writes wages for a W-2, so the high-income audit triggers never fired on a W-2
cross_reference_income built its total from W-2 + 1099-NEC only	Interest, dividends, and Schedule C income were omitted from totalDocumentedIncome
The auditor's 1099-INT/1099-DIV filters did not strip separators	"1099 INT" was silently skipped

Tests

functions/ had no test framework. This adds Vitest — 57 tests, fully offline. The Genkit tools are invoked directly rather than mocked: their implementations are pure and never reach the model, so a dummy key in vitest.config.mts is sufficient.

• test/taxFields.test.ts — money parsing, alias resolution, document classification, income aggregation
• test/tools.test.ts — the five real tools, including the cross-tool agreement invariant (accountant and auditor must report the same total income), plus a named regression case for each bug above

Verified by mutation rather than by passing: reintroducing the two parsing bugs in taxFields.ts fails 22 of the 57 tests.

cd functions && npm test

Docs

docs/open-policy-agent.md — evaluation of OPA/Rego for the audit trigger rules. Conclusion: evaluated, not adopted. Eight hardcoded constants do not justify a second language, an opa binary in CI, and WASM cold-start cost; most of OPA's value (bundle distribution, decision logs) targets cross-service authorization, not single-function heuristic scoring. The doc records the two conditions that would change the answer — rules branching by tax year / state / filing status, or a CPA rather than an engineer owning them — and notes that OPA is not the right fix for the App.tsx role check (that needs Firebase custom claims).

CLAUDE.md / TASKS.md — both still described the superseded Python architecture. firebase.json deploys only functions/, and the frontend never references VITE_API_URL, the variable the Flask service is exposed under, so backend/queue/, backend/embedding/, backend/tax_forms/ and friends are unreachable from the running app. Two backlog items were also already done (the extraction pipeline ships in flows/extractor.ts; Dashboard.tsx already reads filing status from the user profile).

Note on the branch name

The branch was opened to explore Apache Ossie. Ossie's semantic-model spec is a genuine fit for this problem, but it is incubating at v0.2.0.dev0 with no stable SDK, so this lands the fix in plain TypeScript first. The module is shaped so an Ossie YAML model can drive it later; that option is recorded in TASKS.md alongside the OPA one.

🤖 Generated with Claude Code

[chenyuan99]

Addressed in f87ca57.

SonarCloud — all 10 issues fixed, none suppressed. All were in the new functions/src/semantic/taxFields.ts:

• Number.isNaN / Number.parseFloat in place of the coercing globals (3 issues)
• /[,$]/ character class in place of the /,|\$/ alternation (1 issue, MAJOR)
• Six String(unknown) sites that would produce "[object Object]" for a non-scalar. This one was worth more than a lint fix: extractedData is model-generated JSON, so a nested object where a scalar was expected is a real possibility, and "[object Object]" would have flowed into output as a document id or name. Added an asText helper returning "" for anything not a string/number/boolean, and routed every coercion through it — safeFloat, pickString, normalizeDocType, and the id/name/type reads in collectIncome and cross_reference_income.

Three tests added for the new explicit behavior — 60 passing.

upgrade check. Not caused by this PR: it has failed on every run since at least June — scheduled runs, pushes to main, and unrelated PRs (dependabot/*, codex/pwa-support). This PR only tripped its docs/** path filter. Root cause is in the log: create-pull-request rejects the detached HEAD that actions/checkout leaves on pull_request and push events without an explicit base.

Fixed by gating the PR-creating step to schedule / workflow_dispatch and supplying base: main. Opening a docs-upgrade PR as a side effect of an unrelated docs edit was never the intent. Happy to split this into its own PR if you'd rather keep this one focused.

One thing I did not address: Sonar reports 0.0% Coverage on New Code. The 60 tests do cover this code, but the Sonar scan isn't configured to ingest a coverage report from functions/. Fixing that means wiring @vitest/coverage-v8 output into the scanner config — out of scope here, and worth its own change.

🤖 Addressed by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[chenyuan99]

The three FOSSA checks are pre-existing failures on main, not introduced here.

They fail on every recent commit on the default branch, including this PR's own base (13000e4):

Check	main tip 13000e4	this PR 1815984	delta
Security Analysis	64 vulnerabilities	64 vulnerabilities	0
License Compliance	7 issues	7 issues	0
Dependency Quality	50 issues	51 issues	+1

Going further back, the same three have been red on every commit through bc7df86 (Security Analysis has ranged 64–84 as the Dependabot bumps landed).

So this PR adds zero vulnerabilities and zero license issues. The single delta is one dependency-quality issue from the vitest tree — added as a devDependency, so it does not reach the deployed Cloud Functions runtime.

I have not tried to fix these. Clearing 64 vulnerabilities is separate work with its own risk, and it overlaps the 22 Dependabot alerts open on the default branch (13 high, 8 moderate, 1 low) — worth its own PR rather than being folded into a semantic-layer change.

If the +1 is unwelcome, the alternative is Node 22's built-in node:test runner, which would add no dependencies at all. I'd recommend against it: the frontend already standardizes on Vitest, and +1 against a baseline of 50 is noise. Say the word if you'd rather have zero new dependencies.

🤖 Addressed by Claude Code