-
Notifications
You must be signed in to change notification settings - Fork 1
feat: show leftover pairs on the grouping comparison strip (v2.12.17) #521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -607,7 +607,8 @@ many scored posts entered the factorization. Results persist to | |
| `report_period_score` / `report_member_score`. | ||
| `GET /api/reports/{grouping}` lists the trend; | ||
| `GET /api/reports/{grouping}/{period}` is ABAC-filtered; | ||
| `GET /api/reports/compare/{period}` is the home-page grouping strip; | ||
| `GET /api/reports/compare/{period}` is the home-page grouping strip | ||
| and carries the same ABAC-filtered leftover pairs (ADR 0149); | ||
| `POST .../rebuild` scores every grouping kind (post_admin). `make seed` | ||
| folds A-100/B-200 Event Lineage fixtures (and the Riverbend calendar | ||
| post) that already have constructed IRT cells into the same shared | ||
|
|
@@ -620,6 +621,9 @@ closest/farthest pairs (signed residual `R`, observed `Y`, expected | |
| effects) above the member list, leftover-map axis share for residual | ||
| SVD axes 1 and 2, and complete-case coverage captions (map used N of M | ||
| scored posts), plus the | ||
|
|
||
| closest/farthest pairs above the member list, leftover pairs on the | ||
| grouping comparison strip, and the | ||
|
Comment on lines
+624
to
+626
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: Garbled duplicated clause in ARCHITECTURE.md The edit inserts a blank line and a duplicated clause into a sentence that already lists the closest/farthest pairs above the member list, producing a broken, doubly-listed sentence. Prose only, but likely an editing slip. Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| PU / corp / thread comparison -- never a placeholder. TEPP is unchanged. | ||
|
|
||
| ## Phase 6b: Knowledge Graph as a real Ontology + Semantic Layer | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # 2.12.17 — Leftover pairs on the grouping comparison strip | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 Changelog fragment reuses a released version number The new changelog fragment and PR title use 2.12.17, but CHANGELOG.md already ships released [2.12.17] and [2.12.18] sections. The feature itself lands under [Unreleased], so behavior is unaffected, but the reused version label can confuse changelog assembly. Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| ## Added | ||
|
|
||
| - `GET /api/reports/compare/{period}` carries ABAC-filtered leftover | ||
| pairs (ADR 0149). After `make seed`, the A-100 comparison row names | ||
| leftover pairs; click opens that post. Hidden leftover posts stay | ||
| hidden. Do not invent leftover numbers. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,3 +54,60 @@ migration replay (ADR 0166), docstring coverage, and the measurement | |
| boundary are all stated in [AGENTS.md](AGENTS.md) -- read it before | ||
| changing code, tests, or runtime policy rather than restating anything | ||
| here. | ||
| ======= | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Merge conflict marker committed into CLAUDE.md A literal Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
| period-report run on the same snapshot (ADR 0013 / ADR 0024). The TEPP path goes through `tepp_client`. A missing | ||
| transport or an unused accepted envelope is Failed | ||
| (`tepp_not_available` / `tepp_result_not_persisted`). Do not invent a | ||
| theta or a local psychometric substitute. The home list caption stays | ||
| `kind · status · entity`; the machine failure code is detail-only | ||
| (ADR 0014). Open a Failed TEPP row, then connect a live TEPP | ||
| transport. A failed lineage row retries reconstruction -- it does not | ||
| mention TEPP. A failed period-report row rebuilds the report. A | ||
| pending TEPP row does not claim a calibrated measurement. A pending | ||
| lineage row says reconstruction has not started yet. | ||
| Digest prefixes stay audible; hover a prefix to read the full digest. | ||
| Opening a cutoff title shows the live post. Titles marked updated | ||
| after cutoff were rewritten after the run; the opened body names | ||
| both clocks and shows **Body this run knew** beside the live | ||
| rewrite. Compare those two texts before treating the live body as | ||
| reconstructed evidence (ADR 0016 / 0025). | ||
| `POST /api/analysis-runs` records Pending lineage only on an | ||
| authorized cutoff capture (ADR 0017). TEPP and period-report kinds | ||
| are 422. The Request button waits until affiliated corps load; choose | ||
| a corp if the token walks more than one. `POST /api/analysis-runs/{id}/start` | ||
| commits Running plus a durable outbox row, then reconstructs that | ||
| frozen cutoff bag (ADR 0021 / ADR 0023) or submits TEPP through | ||
| `tepp_client` (ADR 0022). A missing transport or unused accepted | ||
| envelope is Failed. Failed TEPP is terminal — connect a TEPP | ||
| transport from that Failed row. Create does not invent a Pending | ||
| TEPP row. Do not invent a theta. Hover the Result prefix to read | ||
| the parent-choice digest. | ||
| After `make seed`, open **Period report · Succeeded · Demo Corp**, | ||
| then **Open period report 2026-W02**. The home week is already | ||
| 2026-W02, so the grouping comparison strip lands on Demo Corp. Report | ||
| grouping is Corporate entity and Demo Corp is current. The focused | ||
| chip name contains `Corporate entity: Demo Corp` and the persisted | ||
| mean θ. The period-report panel says Demo Corp is the opened grouping | ||
| and to read its mean θ and member posts, then open a post. Those | ||
| members land immediately under that next action, ahead of Other Corp | ||
| and the week strip. After `make seed`, leftover closest/farthest pairs | ||
| sit above the member list with leftover-map rank (rank 0 names no | ||
| leftover structure) and unexplained leftover `U` next to leftover-map | ||
| distance `d`. Leftover-map axis share badges name Gabriel inertia | ||
| of axes 1 and 2; open a leftover pair to read the post–criterion cell. | ||
| The shares do not invent a leftover score. Opening Public post names the next action: read | ||
| Event Lineage, Keyman, and evaluation on that post. The popup Event | ||
| Lineage DAG marks that post current. After that current node, the | ||
| popup names Keyman and evaluation as the next read. After landed | ||
| evaluation, the popup names the first Keyman as the next read. After | ||
| landed Ada West related, the popup names the first related node as | ||
| the next read. After that next action, the popup lands Priya Nair | ||
| related nodes. After those related nodes land, the popup names Ask | ||
| about this lineage as the next read. After that next action, the | ||
| popup lands Ask about this lineage. After landed chat, the popup names | ||
| the first Ask. After that next action, the popup lands the first Ask | ||
| answer. After landed first Ask answer, the popup names the first | ||
| cited source. After that next action, the popup lands the first cited | ||
| evidence. Changing the week first still | ||
| focuses the report period field. Mean θ stays on the period-report | ||
| panel. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2430,7 +2430,24 @@ async def compare_period_groupings( | |
| ] | ||
| if not members: | ||
| continue | ||
| visible.append({**row, "members": [], "post_count": len(members)}) | ||
| leftover_pairs = [ | ||
| pair | ||
| for pair in row.get("leftover_pairs", []) | ||
| if _can_see_post(account, pair) | ||
| and not _is_synthetic_demo_member(pair, demo_entity_ids) | ||
| ] | ||
| leftover_pairs = [ | ||
| {key: value for key, value in pair.items() if key != "has_real_source_context"} | ||
| for pair in leftover_pairs | ||
| ] | ||
|
seonghobae marked this conversation as resolved.
|
||
| visible.append( | ||
| { | ||
| **row, | ||
| "members": [], | ||
| "leftover_pairs": leftover_pairs, | ||
| "post_count": len(members), | ||
| } | ||
| ) | ||
|
seonghobae marked this conversation as resolved.
Comment on lines
+2433
to
+2450
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: Comparison strip changes from counts-only to per-post identifiers The compare endpoint sets Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| return {"period_code": period_code, "groupings": visible} | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -978,6 +978,28 @@ async def fetch_period_comparison( | |
| members_by_key: dict[tuple[str, str], list[asyncpg.Record]] = defaultdict(list) | ||
| for row in members: | ||
| members_by_key[(row["grouping_kind"], row["grouping_key"])].append(row) | ||
| # Safe SQL: the source-context expression is an immutable schema fragment; grouping filters are bound. | ||
| leftover = await conn.fetch( # nosemgrep: python.lang.security.audit.sqli.asyncpg-sqli.asyncpg-sqli | ||
| f""" | ||
| select lp.grouping_kind, lp.grouping_key, lp.pair_kind, lp.post_id, | ||
| lp.criterion_code, lp.leftover_distance, lp.leftover_residual, | ||
| p.post_title, p.visibility_code, p.corporate_entity_id, | ||
| ({_SOURCE_CONTEXT_PRESENT_SQL}) as has_real_source_context | ||
| from report_leftover_pair lp | ||
| join source_post p on p.post_id = lp.post_id | ||
| where lp.period_code = $1 and lp.rubric_version = $2 | ||
| and lp.grouping_kind = any($3::text[]) | ||
| order by lp.grouping_kind, lp.grouping_key, | ||
| case lp.pair_kind when 'closest' then 0 else 1 end, | ||
| p.post_title | ||
| """, | ||
| period_code, | ||
| RUBRIC_VERSION, | ||
| list(GROUPING_KINDS), | ||
| ) | ||
| leftover_by_key: dict[tuple[str, str], list[asyncpg.Record]] = defaultdict(list) | ||
| for row in leftover: | ||
| leftover_by_key[(row["grouping_kind"], row["grouping_key"])].append(row) | ||
| payload: list[dict[str, Any]] = [] | ||
| for row in rows: | ||
| label = await resolve_grouping_label(conn, row["grouping_kind"], row["grouping_key"]) | ||
|
|
@@ -997,6 +1019,20 @@ async def fetch_period_comparison( | |
| } | ||
| for member in members_by_key.get((row["grouping_kind"], row["grouping_key"]), []) | ||
| ], | ||
| "leftover_pairs": [ | ||
| { | ||
| "pair_kind": str(pair["pair_kind"]), | ||
| "post_id": str(pair["post_id"]), | ||
| "post_title": pair["post_title"], | ||
| "criterion_code": str(pair["criterion_code"]), | ||
| "leftover_distance": float(pair["leftover_distance"]), | ||
| "leftover_residual": float(pair["leftover_residual"]), | ||
| "visibility_code": pair["visibility_code"], | ||
| "corporate_entity_id": str(pair["corporate_entity_id"]), | ||
| "has_real_source_context": bool(pair["has_real_source_context"]), | ||
| } | ||
| for pair in leftover_by_key.get((row["grouping_kind"], row["grouping_key"]), []) | ||
| ], | ||
|
Comment on lines
1019
to
+1035
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: Comparison leftover query omits residual-detail columns
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| } | ||
| ) | ||
| return payload | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -55,3 +55,6 @@ next action, not only the distance. | |||||||||
| Depends on [ADR 0048](0048-persist-lsirm-leftover-pairs.md) and | ||||||||||
| [ADR 0003](0003-fast-mlsirm-report-integration.md). Complete-case | ||||||||||
| coverage of the leftover map is [ADR 0168](0168-leftover-map-complete-case-coverage.md). | ||||||||||
|
|
||||||||||
| [ADR 0003](0003-fast-mlsirm-report-integration.md). The grouping | ||||||||||
| comparison strip reuses this leftover store ([ADR 0149](0149-leftover-pairs-on-comparison-strip.md)). | ||||||||||
|
Comment on lines
+59
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Orphaned ADR link in Related section The appended Related paragraph opens with a stray
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # ADR 0149 — Leftover pairs on the grouping comparison strip | ||
|
|
||
| **Decision status:** Accepted | ||
| **Date:** 2026-08-24 | ||
|
|
||
| ## Context | ||
|
|
||
| ADR 0048 persists closest and farthest leftover post–criterion pairs. | ||
| ADR 0049 shows those pairs above the period-report member list. The | ||
| home-page grouping comparison strip (`GET /api/reports/compare/{period}`) | ||
| already names mean θ per PU / corp / thread so a buyer can switch | ||
| grouping without opening the period-report list first. That strip | ||
| does not yet name leftover pairs, so the buyer still has to switch | ||
| grouping before they can open a leftover post. | ||
|
|
||
| Leftover pairs are already authorized rows. Denormalizing them onto | ||
| the comparison strip would invent a second leftover store. | ||
|
|
||
| ## Decision | ||
|
|
||
| The comparison payload carries the same ABAC-filtered `leftover_pairs` | ||
| as the period-report payload. Each visible comparison row may name | ||
| closest and farthest leftover pairs. Clicking a leftover pair on the | ||
| strip opens that post with the same handler as a leftover pair on the | ||
| period-report list. A leftover pair for a hidden post is omitted the | ||
| same way a hidden member is. | ||
|
|
||
| Do not invent leftover numbers or a second theta. Missing leftover | ||
| rows render nothing. | ||
|
|
||
| ## Consequences | ||
|
|
||
| After `make seed`, the A-100 comparison row names leftover pairs. | ||
| Open a leftover pair from the strip to read the post–criterion cell. | ||
| The Period reports member list still shows leftover pairs above | ||
| members (ADR 0049). This slice only adds the same authorized leftover | ||
| store to the comparison strip. | ||
|
|
||
| ## Related | ||
|
|
||
| Depends on [ADR 0048](0048-persist-lsirm-leftover-pairs.md) and | ||
| [ADR 0049](0049-leftover-pair-report-ui.md). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3588,6 +3588,37 @@ function ReportsPanel({ | |
| <span className="post-badge">mean θ {row.mean_theta.toFixed(2)}</span> | ||
| <span className="post-badge">{row.post_count} posts</span> | ||
| </button> | ||
| {row.leftover_pairs && row.leftover_pairs.length > 0 && ( | ||
| <ul className="ticket-list" aria-label={`Leftover pairs for ${row.grouping_label}`}> | ||
| {row.leftover_pairs.map((pair) => { | ||
| const kindLabel = | ||
| pair.pair_kind === "farthest" ? "Farthest leftover" : "Closest leftover"; | ||
| const nextAction = | ||
| pair.pair_kind === "farthest" | ||
| ? "Open this post to read the criterion it sat farthest from after main effects." | ||
| : "Open this post to read the criterion it sat closest to after main effects."; | ||
| const criterion = criterionShortLabel(pair.criterion_code); | ||
| return ( | ||
| <li | ||
| key={`${row.grouping_kind}:${row.grouping_key}:${pair.pair_kind}:${pair.post_id}:${pair.criterion_code}`} | ||
| className="ticket-list-item" | ||
| > | ||
| <button | ||
| className="post-list-item" | ||
| aria-label={`Open leftover ${pair.pair_kind} pair from comparison: ${pair.post_title} · ${criterion}`} | ||
| onClick={() => onSelectPost(pair.post_id)} | ||
| > | ||
| <span className="ticket-title"> | ||
| {kindLabel}: {pair.post_title} · {criterion} | ||
| </span> | ||
| <span className="post-badge">{nextAction}</span> | ||
| <span className="post-badge">d {pair.leftover_distance.toFixed(2)}</span> | ||
| </button> | ||
| </li> | ||
| ); | ||
| })} | ||
| </ul> | ||
| )} | ||
|
seonghobae marked this conversation as resolved.
Comment on lines
+3591
to
+3621
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: Comparison strip duplicates leftover-pair markup instead of reusing the shared component The report panel renders leftover pairs through the shared Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| </li> | ||
| ))} | ||
| </ul> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Info: Duplicate leftover-pairs paragraph in AGENTS.md
The change adds a second 'Period leftover pairs' paragraph beside the existing one, re-listing a shorter ADR set and dropping the R/Y/E/rank/U/d detail the original documents. Two overlapping statements of the same policy invite drift.
Was this helpful? React with 👍 or 👎 to provide feedback.