Skip to content

test: add a real GPCM parameter-recovery test for fast-mlsirm - #452

Closed
seonghobae wants to merge 5 commits into
mainfrom
worktree-fix-gpcm-recovery-test
Closed

test: add a real GPCM parameter-recovery test for fast-mlsirm#452
seonghobae wants to merge 5 commits into
mainfrom
worktree-fix-gpcm-recovery-test

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Follow-up to test: add a real GRM parameter-recovery test for fast-mlsirm #451 (real GRM parameter-recovery test), extending the same pattern to GPCM per this cycle's resume instructions — period_report.py's production code fits GRM and GPCM and picks between them.
  • Checked fast_mlsirm's installed source directly (polytomous.py) rather than guessing at the GPCM formula: PolytomousFit's own docstring documents GPCM as "additive category intercepts" with a "Muraki step reparametrization" — the standard Muraki (1993) generalized partial credit model, distinct from GRM's cumulative-logistic thresholds.
  • tests/test_fast_mlsirm_gpcm_recovery.py implements that exact formula (cumulative step logits, softmax over categories) to generate synthetic responses from known true item discriminations/steps and person thetas, fits via the real fit_polytomous(..., model="gpcm"), scores via score_polytomous.
  • Measured recovery: theta RMSE ≈ 0.30, correlation ≈ 0.95 — stronger than the GRM test's ≈0.38/≈0.92 at the same sample size, consistent with GPCM's additive structure. Asserted with loose-but-meaningful margins (RMSE < 0.55, correlation > 0.8).
  • Bonus finding while reading the source: fast_mlsirm.cat_simulate_polytomous is a real computerized-adaptive-test simulator over a fitted GRM/GPCM item bank (Dodd, De Ayala & Koch, 1995) — directly relevant to the "CAT" part of the same gap-baseline entry. Noted as a concrete next step, not attempted this cycle (staying to one well-scoped addition per cycle).
  • Updated the gap-baseline "Testing" entry: both GRM and GPCM recovery now real and tested; FIPC and CAT still explicitly noted as open, with CAT's next step now concrete (cat_simulate_polytomous exists and is ready to exercise).

Test plan

  • uv run --frozen python -m pytest -q tests/test_fast_mlsirm_gpcm_recovery.py — 1 passed
  • uv run --frozen python -m pytest -q (full backend suite) — 754 passed/17 skipped, no regressions (this branch doesn't yet include test: add a real GRM parameter-recovery test for fast-mlsirm #451's GRM test since that's unmerged, so the count reflects origin/main + this one new test)

🤖 Generated with Claude Code


Open in Devin Review

Two TypeScript build errors on main (blocking every open PR's
"Frontend lint, test, build" check, including this repo's own review
bot's ability to approve them):

- App.tsx imported rememberOidcReturnUrl/returnUrlFromLocation from
  oidcReturnUrl.ts but never called them -- the login button built its
  own unsanitized returnUrl inline instead of using the safe helper
  (oidcReturnUrl.ts's isSafeReturnUrl guard against an open-redirect-
  shaped value) or persisting it as the sessionStorage/localStorage
  fallback restoreOidcReturnUrl (already wired up on the callback side
  in main.tsx) reads when the OIDC state round-trip drops it.
- The unauthenticated login screen unconditionally rendered
  <AdminPanel accessToken={accessToken} /> when destination === "admin"
  -- accessToken is string | undefined here (always undefined while
  unauthenticated), a real type error, and the render was unreachable
  through normal navigation (destination only changes via the
  authenticated nav) -- dead code, removed.

uv run --frozen python -m pytest -q: 753 passed, 17 skipped.
pnpm run test: 140 passed. pnpm run lint / build: clean.
Simulates polytomous GPCM responses from known true item parameters
and person thetas using Muraki (1993)'s additive category-intercept
formula (fast_mlsirm ships no polytomous simulator, so it's
implemented directly, matching PolytomousFit's own documented
parameterization), fits them with the same fit_polytomous(...,
model="gpcm") period_report.py's production code can select, and
asserts recovered EAP thetas match true thetas within a
literature-typical bound. Also notes cat_simulate_polytomous (a real
CAT simulator over a fitted GRM/GPCM bank) exists as a concrete next
step for the CAT part of the same gap entry.
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 16 minutes

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9b959609-35b7-4f2c-9d84-b60082474e23

📥 Commits

Reviewing files that changed from the base of the PR and between ef6f5a5 and 09e0ec0.

📒 Files selected for processing (4)
  • docs/product-technical-gap-baseline.md
  • frontend/src/App.test.tsx
  • frontend/src/App.tsx
  • tests/test_fast_mlsirm_gpcm_recovery.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

@github-actions
github-actions Bot disabled auto-merge August 23, 2026 06:55
@seonghobae
seonghobae enabled auto-merge (squash) August 23, 2026 06:55
devin-ai-integration[bot]

This comment was marked as resolved.

@opencode-agent
opencode-agent Bot disabled auto-merge August 23, 2026 07:38
The GRM recovery test and its measured RMSE/correlation numbers live
in a separate, still-open PR and are not part of this branch. Only
claim what this PR itself adds (test_fast_mlsirm_gpcm_recovery.py)
and note the GRM test as still in progress elsewhere, so the doc
can't be merged into a state where it cites a test file that does
not exist in the repo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B5M79L945DMyMs3sg5yJ14
@seonghobae
seonghobae enabled auto-merge (squash) August 23, 2026 08:11
seonghobae added a commit that referenced this pull request Aug 23, 2026
PR454's tree only adds tests/test_fast_mlsirm_fipc_recovery.py; the
GRM/GPCM/CAT recovery tests live on separate open PRs (#451/#452/#453).
Claiming "(Resolved)" with all four files here is false if this PR
merges independently, so scope the claim to FIPC and note the other
three are pending on their own PRs.
devin-ai-integration[bot]

This comment was marked as resolved.

@github-actions
github-actions Bot disabled auto-merge August 23, 2026 08:17
seonghobae added a commit that referenced this pull request Aug 23, 2026
docs/product-technical-gap-baseline.md claimed all three fast-mlsirm
recovery tests (GRM, GPCM, CAT) exist and were "Mostly resolved," but
this PR only adds the CAT test -- GRM (#451) and GPCM (#452) are still
open, unmerged PRs. Scope the entry to what this PR actually ships and
note the other two as in flight.

test_fast_mlsirm_cat_recovery.py's MAX_MEAN_ITEMS_USED=20 bound was
loose enough that a non-adaptive (random item order) run of the same
fixture/seed also passes (measured mean_items_used ~14.97), so the
test would not catch a silent regression that dropped the `adaptive`
flag. Tighten the bound to 12, close to the real adaptive-run measurement
(~8.7) and verified to fail the non-adaptive fallback case.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment thread frontend/src/App.tsx
@seonghobae

Copy link
Copy Markdown
Contributor Author

Reviewed the remaining informational thread. The login/AdminPanel changes are the shared OIDC root fix and are covered by their dedicated tests. @opencode-agent please independently review exact head 09e0ec034ca175ec14ecebdc04851e1a8989db45.

@seonghobae
seonghobae enabled auto-merge (squash) August 23, 2026 08:37
@seonghobae

Copy link
Copy Markdown
Contributor Author

Reviewed the remaining informational thread. The login/AdminPanel changes are the shared OIDC root fix and are covered by their dedicated tests. @opencode-agent please independently review exact head 09e0ec034ca175ec14ecebdc04851e1a8989db45.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Closing as out of lane for LineageWeave. IRT/linking recovery tests belong in fast-mlsirm, not this repo. Unique product slice remains #258. Do not restack onto the GNB-four head.

@seonghobae seonghobae closed this Aug 24, 2026
auto-merge was automatically disabled August 24, 2026 01:12

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant