2
HISv3 Code Review
padmanto edited this page 2026-07-01 06:23:25 +07:00

HISv3 Code Review — Installation & Usage

Comprehensive code review for HISv3 repositories (go-modules, backend, frontend). Checks compliance against 43 HISv3 coding rules, detects breaking changes, audits databases, and reviews frontend forms.

Prerequisites

  • pi coding agent installed
  • The hisv3-mcp extension loaded (installed by default at ~/.pi/agent/extensions/hisv3-mcp/)
  • MCP server (for rule corpus lookups) or use local auditors without the server

One-Time Setup

1. Install the skill files

mkdir -p ~/.pi/agent/skills/hisv3-code-review

Copy SKILL.md, references/rules-index.md, and templates/ into that directory. The skill ships with the pi agent installation — verify it exists:

ls ~/.pi/agent/skills/hisv3-code-review/
# SKILL.md  references/  templates/

2. Confirm the extension is loaded

The hisv3-mcp extension lives at ~/.pi/agent/extensions/hisv3-mcp/. It should auto-load on pi start. If missing, copy it from the pi extensions directory.

3. Set the MCP server URL

The extension connects to an MCP server for rule lookups. Set the URL in your shell profile:

export HISV3_MCP_URL=http://192.168.30.153:8783

Verify it's reachable:

curl http://192.168.30.153:8783/health
# {"status":"ok","service":"hisv3-mcp-server","corpora":{"rules":{"count":43},...}}

If you don't have access to the MCP server, the local auditors (DB audit, breakage detection, FE form audit) still work.

4. Restart pi

pi

Activate the tools inside pi:

hisv3_activate

Usage — Realistic Examples

Scenario 1: Pre-merge compliance check on a feature branch

User: I'm on branch feature/add-blood-type-field. Run a full compliance check before I merge.

Agent detects repo → his-backend, runs hisv3_compliance_check("his-backend"):

PASSED   storage-cancel-and-error-enrich      All defer cancel() calls have error enrichment
PASSED   auth-guard-middleware                  Authorization middleware present on routes
VIOLATED migration-required                     ALTER TABLE patients ADD COLUMN blood_type VARCHAR(5)
                                                 → Missing migration file
VIOLATED index-on-fk                            FK patient_id in orders has no index
                                                 → Missing: idx_orders_patient_id
N/A      fe-double-click-prevention             No frontend changes in this diff
PASSED   context-propagation                     All handlers pass ctx to storage

Agent flags 2 violations:
  - Add migration file: migrations/20260701_add_blood_type.sql
  - Add index: CREATE INDEX idx_orders_patient_id ON orders(patient_id)

You fix both, re-run, all PASSED. Merge with confidence.

Scenario 2: Breaking change caught before it ships

User: I changed the GetPatient signature to accept a context. Check for breakage.

Agent runs hisv3_audit_breakage on the diff:

BREAKING  GetPatient(ctx context.Context, id int) (*Patient, error)
          Previously: GetPatient(id int) (*Patient, error)
          Affected callers (42 locations):
            internal/service/registration/service.go:156
            internal/service/billing/service.go:89
            internal/service/pharmacy/service.go:234
            ... and 39 more

WARNING   PatientFilter struct
          Added field: BloodType string  (OK — zero value compatible)

Agent suggests the non-breaking pattern:
  Keep GetPatient(id int) as a wrapper that calls GetPatientCtx(ctx, id)
  Add // Deprecated comment to the old function
  Migrate callers gradually

Scenario 3: DB migration audit before deployment

User: I added two new tables and changed a column. Audit the DB changes.

Agent runs hisv3_audit_db on the diff:

[+] Migration file found: migrations/20260701120000_add_lab_results.sql (up + down)
[+] Index on FK: idx_lab_results_patient_id on lab_results(patient_id)
[+] Index on FK: idx_lab_results_order_id on lab_results(order_id)
[-] Missing index on FK: lab_tests.lab_result_id has no index
    → Add: CREATE INDEX idx_lab_tests_lab_result_id ON lab_tests(lab_result_id)
[!] LEFT JOIN pitfall in query:
    SELECT p.name, lr.result_value
    FROM patients p LEFT JOIN lab_results lr ON p.id = lr.patient_id
    WHERE lr.result_value > 100  ← NULL guard missing!

    Fix:
    SELECT p.name, COALESCE(lr.result_value, 0) AS result_value
    FROM patients p LEFT JOIN lab_results lr ON p.id = lr.patient_id
    WHERE COALESCE(lr.result_value, 0) > 100

Scenario 4: Frontend form double-submit vulnerability

User: I built a new patient registration form. Check for double-click issues.

Agent runs hisv3_audit_fe_forms on the diff:

[-] Missing double-click guard in: src/pages/registration/NewPatientForm.tsx
    Line 84:
      <Button type="submit" onClick={handleSubmit(onSubmit)}>
        Daftar Pasien
      </Button>

    ↓ Missing: disabled={isSubmitting}, loading={isSubmitting}

    Fix:
      const { formState: { isSubmitting } } = useForm();

      <Button type="submit" disabled={isSubmitting} loading={isSubmitting}
        onClick={handleSubmit(onSubmit)}>
        {isSubmitting ? 'Mendaftarkan...' : 'Daftar Pasien'}
      </Button>

[+] Guard present in: src/pages/registration/EditPatientForm.tsx
      disabled={isSubmitting} loading={isSubmitting} ✓

Scenario 5: End-to-end: fix, commit, PR

User: I fixed the LEFT JOIN pitfall in lab_results. Generate a commit message and PR description.

Agent generates commit:
  fix(storage): add COALESCE guard on lab_results LEFT JOIN

  The query SELECT p.name, lr.result_value FROM patients p
  LEFT JOIN lab_results lr had no NULL guard on result_value,
  causing WHERE lr.result_value > 100 to silently drop rows.

  Added COALESCE(lr.result_value, 0) on both SELECT and WHERE.

Agent generates PR description:

  ## Summary
  Fix LEFT JOIN pitfall in lab_results query — missing COALESCE on result_value
  caused rows to be silently excluded from the WHERE clause.

  ## Changes
  - storage/lab_results.go: Added COALESCE(lr.result_value, 0) to SELECT and WHERE
  - Added migration index idx_lab_tests_lab_result_id

  ## Affected Rules
  - left-join-pitfall (VIOLATED → PASSED)
  - index-on-fk (VIOLATED → PASSED)

  ## Risk
  Low — only changes NULL handling, no schema changes.

  ## Testing
  - Unit test added for NULL result_value case
  - Manual test: query patients with no lab results → shows 0, not excluded

Quick Commands

Full compliance check

Run full compliance check on this branch

Runs all 43 rules against your diff. Returns PASSED / VIOLATED / N/A per rule.

Breaking change detection

Audit this diff for breaking changes

Scans Go code for exported function/struct/interface signature changes that break callers.

Database audit

Check this SQL for DB issues

Flags: missing migration files, missing indexes on FK columns, LEFT JOIN without NULL guards.

Frontend form audit

Review these forms for double-click prevention

Checks submit buttons for disabled/loading props and isSubmitting guards.

Generate commit message

Generate a commit message for these changes

Produces a conventional commit with type/scope detected from the diff.

Generate PR description

Generate a PR description for this branch

Structured template with Summary, Changes, Affected Rules, Risk, Testing sections.

What It Checks

Go Breakage Detection

Pattern Flag
Added parameter to exported function BREAKING
Changed return type BREAKING
Removed exported function BREAKING
Changed struct field type WARNING
Added field to struct OK

Database Rules

Pattern Flag
DDL without migration file Missing migration
FK column without index Missing index
LEFT JOIN without COALESCE/NULL guard Pitfall

Frontend Rules

Pattern Flag
Submit button without disabled prop Missing guard
Form without isSubmitting check Missing guard

MCP Rules (43 total)

Search across repos:

Repo Coverage
his-go-modules Go module patterns, error handling, storage
his-backend Service layer, resolvers, authorization
his-frontend Components, forms, state management

How It Works

User prompt: "Run compliance check"
       │
       ▼
┌──────────────────────────────────────────┐
│ hisv3_compliance_check(repo)              │
│                                          │
│ 1. Fetches all 43 rules via MCP          │
│ 2. Runs local auditors (DB, breakage, FE)│
│ 3. Merges results per rule               │
│ 4. Returns PASSED / VIOLATED / N/A       │
└──────────────────────────────────────────┘
       │
       ▼
Agent reads violations, cross-references
  with repo code, produces fix recommendations

Local auditors work without the MCP server:

hisv3_audit_db         → scans SQL diffs
hisv3_audit_breakage   → scans Go diffs
hisv3_audit_fe_forms   → scans TSX diffs

Troubleshooting

Symptom Likely Cause Fix
MCP server unreachable URL not set or wrong Check echo $HISV3_MCP_URL, verify curl .../health
MCP server unreachable On different network Run scripts/tunnel.sh or set correct IP
No rules returned Wrong repo name Use his-go-modules, his-backend, or his-frontend
Local audits show nothing No additions in diff Ensure changes include new code, not just deletions
Tools grayed out Not activated Run hisv3_activate inside pi
Commit/PR generation fails Changes not staged Stage changes with git add first