Skip to content

feat: name leftover-map unexplained share on leftover pairs (v2.12.27) - #537

Open
seonghobae wants to merge 5 commits into
mainfrom
feat/leftover-map-unexplained-share-v21227
Open

feat: name leftover-map unexplained share on leftover pairs (v2.12.27)#537
seonghobae wants to merge 5 commits into
mainfrom
feat/leftover-map-unexplained-share-v21227

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Buyer next action

After make seed, leftover post–criterion pairs sit above the member list with unexplained leftover share U²/R̃² next to leftover-map distance d. Click the pair to open that post. The share badge is omitted when the pair has no complete-case leftover map (fallback rows do not invent a share). Never invent leftover score or theta.

A leftover residual R is not the leftover the two leftover-map axes do not reconstruct. Unexplained leftover share names s = U_c² / R̃² of centered leftover R̃ = R − center after two-axis reconstruction R̂_c = ξ_{1:2} · ζ_{1:2} (U_c = R̃ − R̂_c). Using raw R in the denominator (or U = R − recon) leaves the grand mean inside the named leftover, so a fully reconstructed rank-1 cell would look unexplained whenever center ≠ 0. This PR centers first. R̂_c and U_c stay internal and are not persisted.

Scope (this PR only)

  • ADR 0183 leftover-map unexplained leftover share
  • migration 0183 nullable leftover_map_unexplained_share on leftover pair rows
  • persist/fetch/seed leftover_map_unexplained_share when complete-case ξ, ζ exist (s = U_c² / R̃²; unused axes pad with 0; hidden SVD axes after the second are dropped)
  • frontend U²/R̃² badge + named next action; i18n en/ko/zh/ja/vi
  • rank-1 share is 0 even when mean(R) ≠ 0; rank-3 synthetic residual proves s ≠ R, ≠ leftover_distance, and uncentered U; leftover_distance stays full-rank Euclidean
  • rank-0 maps store s = 0; fallback omits share
  • do not persist leftover_map_unexplained or leftover_map_reconstruction
  • tsc: optional AdminPanel accessToken (login screen has no token) and drop unused oidcReturnUrl import so leftover UI typechecks
  • release 2.12.27

Independent of leftover stacks already in review

Do not mix into:

Merge contract

Persist unexplained leftover share s = U_c² / R̃² of centered leftover
on leftover post–criterion pairs (ADR 0183). Two-axis reconstruction
R̂_c and centered leftover U_c stay internal. A rank-1 cell stores
share 0 even when mean(R) ≠ 0. Fallback omits the share. Do not persist
leftover-map unexplained leftover U or reconstruction R̂.

Copy link
Copy Markdown
Contributor Author

Merge contract for exact head 776de53558c81c0c076582277028cbb295ca6bfe:

Requested reviewers stay empty: the only org collaborator is the author, so review cannot be requested from this account.

Independent of leftover stacks already in review, including #535 leftover-map unexplained leftover. Devin’s grand-mean-in-U note on #535 is addressed here by centering leftover before naming share, not by patching that head.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 40 minutes.

View limit details

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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f8b1d267-0d66-4747-a0dc-f964690cb76a

📥 Commits

Reviewing files that changed from the base of the PR and between eeecd6e and b1d01d1.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (27)
  • AGENTS.md
  • ARCHITECTURE.md
  • CHANGELOG.d/2.12.27-leftover-map-unexplained-share.md
  • CHANGELOG.md
  • CLAUDE.md
  • backend/app/report_ingestion.py
  • backend/tests/test_api.py
  • docs/adr/0003-fast-mlsirm-report-integration.md
  • docs/adr/0048-persist-lsirm-leftover-pairs.md
  • docs/adr/0049-leftover-pair-report-ui.md
  • docs/adr/0183-leftover-map-unexplained-share.md
  • frontend/package.json
  • frontend/src/App.test.tsx
  • frontend/src/api.ts
  • frontend/src/components/LeftoverPairList.tsx
  • frontend/src/i18n.test.ts
  • frontend/src/i18n.ts
  • frontend/src/leftoverMapUnexplainedShare.test.ts
  • frontend/src/leftoverMapUnexplainedShare.ts
  • lineageweave/leftover_pairs.py
  • migrations/0183_report_leftover_map_unexplained_share.sql
  • migrations/rollback/0183_report_leftover_map_unexplained_share.sql
  • pyproject.toml
  • scripts/seed_demo_data.py
  • tests/test_leftover_pairs.py
  • tests/test_period_report.py
  • tests/test_schema.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 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 2 potential issues.

Open in Devin Review

Comment thread lineageweave/leftover_pairs.py Outdated
Comment on lines +156 to +162
filled_sq = float(filled * filled)
unexplained_sq = float(unexplained * unexplained)
if filled_sq > _LEFTOVER_SINGULAR_FLOOR:
return float(unexplained_sq / filled_sq)
if unexplained_sq <= _LEFTOVER_SINGULAR_FLOOR:
return 0.0
return None

@devin-ai-integration devin-ai-integration Bot Aug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Per-cell share can exceed 1

_unexplained_leftover_share computes a per-cell ratio U_c²/R̃², not an aggregate. Two-axis reconstruction can overshoot one cell with opposite sign, so unexplained can exceed |filled| and s can exceed 1. The check constraint only rejects negatives and the frontend accepts any finite non-negative value, so a badge above 1.00 can render. Consistent with ADR 0183, which only forbids negative or non-finite shares.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged — per-cell share can exceed 1 when two-axis reconstruction overshoots a cell. ADR 0183 only forbids a negative or non-finite share; the check constraint and badge follow that. Not clamping to 1.00 in this increment (that would hide honest overshoot). Author will not self-approve.

Copy link
Copy Markdown
Contributor Author

write-probe cycle 2026-08-24T08:04KST

Copy link
Copy Markdown
Contributor Author

Cycle 2026-08-24T08:15KST write retry.

Copy link
Copy Markdown
Contributor Author

Cycle 2026-08-24T08:03KST (Grok Build).

Copy link
Copy Markdown
Contributor Author

Independent next leftover gap is now open as #539 (explained share e = R̂_c² / R̃², v2.12.28, exact head b15d0ee31f6dc71f5dae2839a45d43b6716e5911). Do not mix #539 into this unexplained-share stack. This PR still needs independent APPROVE on exact head 776de535; author will not self-approve. Issues #79 and #87 stay open.

Drop the unused oidcReturnUrl import (pre-existing on main) and accept
an optional AdminPanel accessToken so the login screen typechecks. Do
not change leftover unexplained-share arithmetic.

Copy link
Copy Markdown
Contributor Author

Pushed tsc repair 9a6395546b03f6edc040492fef8f597bdca94212 (drop unused oidcReturnUrl import on main; optional AdminPanel accessToken for the login screen). Leftover unexplained-share arithmetic is unchanged. New exact head is required. Author will not self-approve. Independent of #539.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae
seonghobae enabled auto-merge August 24, 2026 06:46
@opencode-agent
opencode-agent Bot disabled auto-merge August 24, 2026 07:00
@seonghobae
seonghobae enabled auto-merge (squash) August 24, 2026 08:03
@opencode-agent
opencode-agent Bot disabled auto-merge August 24, 2026 08:42

@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 2 new potential issues.

Open in Devin Review

Comment on lines 51 to 75
let nextAction: string;
if (rankBadge !== null && observedExpected !== null) {
if (rankBadge !== null && observedExpected !== null && shareBadge !== null) {
nextAction =
pair.leftover_map_rank === 0
? tf(
"Leftover map rank 0 means no leftover structure after IRT main effects. Read observed Y {observed}, expected E {expected}, and unexplained share {value}, then open this post.",
{
observed: Number(pair.observed_response).toFixed(2),
expected: Number(pair.expected_response).toFixed(2),
value: Number(pair.leftover_map_unexplained_share).toFixed(2),
},
)
: tf(
"Read leftover map rank {rank}, observed Y {observed}, expected E {expected}, and unexplained share {value} after IRT main effects, then open this post.",
{
rank: String(pair.leftover_map_rank),
observed: Number(pair.observed_response).toFixed(2),
expected: Number(pair.expected_response).toFixed(2),
value: Number(pair.leftover_map_unexplained_share).toFixed(2),
},
);
} else if (rankBadge !== null && observedExpected !== null) {
nextAction =
pair.leftover_map_rank === 0
? tf(

@devin-ai-integration devin-ai-integration Bot Aug 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Share badge shows but next-action text can omit it

In LeftoverPairList.tsx the share badge renders whenever shareBadge is non-null, but the next-action sentence names the share only when both rankBadge and observedExpected are also present. A pair carrying a share but lacking rank or observed/expected shows the badge while the sentence stays silent. Freshly computed pairs always carry an integer rank, so this only reaches legacy rows without rank.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +161 to +182
def _unexplained_leftover_share(filled: float, reconstruction: float) -> float | None:
"""Return ``s = U_c² / R̃²`` when both terms are finite; otherwise omit.

``filled`` is centered leftover ``R̃ = R − center``. Gabriel
reconstruction recovers that centered matrix, not raw residual
``R``. Using raw ``R`` in the denominator (or subtracting
reconstruction from ``R``) leaves the grand mean inside the named
leftover and makes a fully reconstructed rank-1 cell look
unexplained whenever ``center ≠ 0``.
"""
if not np.isfinite(filled) or not np.isfinite(reconstruction):
return None
unexplained = filled - reconstruction
if not np.isfinite(unexplained):
return None
filled_sq = float(filled * filled)
unexplained_sq = float(unexplained * unexplained)
if filled_sq > _LEFTOVER_SINGULAR_FLOOR:
return float(unexplained_sq / filled_sq)
if unexplained_sq <= _LEFTOVER_SINGULAR_FLOOR:
return 0.0
return None

@devin-ai-integration devin-ai-integration Bot Aug 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Centering-first share matches two-axis reconstruction

In leftover_pairs.py, reconstruction is the inner product of the padded two-axis coordinates, the rank-2 approximation of the same centered matrix the SVD factorized, and _unexplained_leftover_share uses centered leftover in both numerator and denominator. Rank-1/rank-2 maps reconstruct exactly, so share is 0 even when mean(R) != 0; only rank>=3 yields nonzero share. The floor guard returns 0.0 only when the unexplained part is also negligible, otherwise omits, avoiding unstable ratios. The result is always non-negative, satisfying the DB check.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@seonghobae
seonghobae enabled auto-merge August 24, 2026 11:50
# Conflicts:
#	AGENTS.md
#	ARCHITECTURE.md
#	CHANGELOG.md
#	CLAUDE.md
#	backend/tests/test_api.py
#	docs/adr/0003-fast-mlsirm-report-integration.md
#	frontend/package.json
#	lineageweave/leftover_pairs.py
#	pyproject.toml
#	tests/test_leftover_pairs.py
#	tests/test_period_report.py
#	tests/test_schema.py
#	uv.lock
@github-actions
github-actions Bot disabled auto-merge August 24, 2026 15:56

@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 2 new potential issues.

Open in Devin Review

Comment thread scripts/seed_demo_data.py
Comment on lines +122 to +123
cur.execute((migrations / "0169_report_leftover_map_axis.sql").read_text())
cur.execute((migrations / "0183_report_leftover_map_unexplained_share.sql").read_text())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Seed adds previously-missing axis migration

seed_demo_data.py adds 0169_report_leftover_map_axis.sql to the seed migration list. _persist_seed_period_report already inserts into report_leftover_map_axis, so before this change make seed would fail at that insert because the table was never created. This PR fixes that gap while adding 0183.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines 175 to 180
person,
item,
max(distance, 0.0),
None,
)
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Rank-0 origin map yields share 0.0, fallback yields None

For a constant (rank-0 after centering) complete-case rectangle, _complete_case_positions returns zeros arrays rather than None, so candidates are built with filled=0, reconstruction=0, giving share 0.0 with rank 0. The distinct fallback path (no complete-case rectangle at all) omits the share as None. Both rank-0 cases are handled distinctly per ADR 0183.

(Refers to this code)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@seonghobae
seonghobae enabled auto-merge August 24, 2026 16:13
@opencode-agent
opencode-agent Bot disabled auto-merge August 24, 2026 16:50
@seonghobae
seonghobae enabled auto-merge August 24, 2026 17:04
@opencode-agent
opencode-agent Bot disabled auto-merge August 24, 2026 17:58
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