Skip to content

fix(reports): normalize nonfinite primary cross share - #1007

Closed
seonghobae wants to merge 1 commit into
feat/leftover-map-compare-cross-share-v2530from
repair/1006-primary-report-finite-cross-share
Closed

fix(reports): normalize nonfinite primary cross share#1007
seonghobae wants to merge 1 commit into
feat/leftover-map-compare-cross-share-v2530from
repair/1006-primary-report-finite-cross-share

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Closes #1006 only if this lane itself becomes the accepted successor; it is currently a Draft recovery/evidence lane.

Gap and valid delta

The primary fetch_period_reports() read model can serialize persisted report_leftover_pair.leftover_map_cross_share with raw float(...). PostgreSQL numeric accepts NaN/Infinity/-Infinity, while strict JSON rejects them. The valid repair is narrow: reuse _finite_float_or_none() for this one primary-report field, preserve finite positive/zero/negative values exactly, map null/non-finite to None, and leave ABAC/source-context/grouping/pair ordering and psychometric ownership unchanged.

This lane also contains useful regression evidence: a fake-boundary contract and a live-PostgreSQL test covering finite positive, zero, negative, SQL null, NaN, +Inf, -Inf, plus strict json.dumps(..., allow_nan=False) delivery.

Wrong-base finding

This branch was authored from protected main. Retargeting PR metadata to #831 did not reconstruct branch ancestry. Head d678bbbbf8b5b49654352d7cae82c7df1c0eba71 still diverges from #831 8d7d8fe17de30ef08121d9f38fb1e7279e64354a at protected main@83eba56149eb802cd63642c507c324c9976ec78e; the earlier merge ref therefore was not corrected-#831 acceptance. This PR remains Draft and must not be merged as the product lane.

Current successor / closure rule

#1008 is the live repair lane on the actual #831 parent. Current #1008 head is d581a73c0be5a4906d9e6b19578f5dff4498d4c4, an ordinary descendant of exact #831. Its diff now contains the same production normalization, a fake-boundary finite/zero/negative/null/NaN/+Inf/-Inf strict-JSON contract, and an extracted live-PostgreSQL regression carrying this lane's useful integration evidence. The predecessor #1008 candidate reached product tests but exposed a test-fixture dependency error; current d581a73... supplies the missing synthetic demo_analyst_token fixture and its exact-head repository acceptance run is still in progress.

Keep #1007 open until #1008 reaches terminal exact-current-parent GREEN and is normally accepted into #831. Only then does verified complete successor inheritance permit closing this wrong-base lane. No force push, destructive rebase, self-approval, bypass, gate weakening, or stale receipt transfer.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 94c360c2-ce40-4e4c-8cd3-82998809077f

📥 Commits

Reviewing files that changed from the base of the PR and between 83eba56 and d678bbb.

📒 Files selected for processing (3)
  • backend/app/report_ingestion.py
  • backend/tests/test_api.py
  • backend/tests/test_period_report_cross_share.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

fetch_period_reportsleftover_map_cross_share의 비유한 값을 None으로 정규화합니다. 유한한 양수, 0, 음수 값과 SQL null은 유지합니다. 실통합 테스트와 회귀 테스트가 이 동작과 strict JSON 직렬화를 검증합니다.

Changes

보고서 cross-share 정규화

Layer / File(s) Summary
cross-share 정규화 구현
backend/app/report_ingestion.py
_finite_float_or_none 헬퍼가 유한한 값만 반환하도록 추가되었습니다. fetch_period_reportsleftover_map_cross_share에 이 헬퍼를 사용합니다.
정규화 회귀 검증
backend/tests/test_api.py, backend/tests/test_period_report_cross_share.py
실제 데이터베이스와 모의 연결 테스트가 유한 값, null, NaN, Infinity, -Infinity 처리를 검증합니다. strict JSON 직렬화도 검증합니다.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to d678b

Reports now serialize non-finite cross-share values as null while preserving valid signed values, with regression coverage for the affected read path.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed 구현은 [#1006]의 핵심 요구사항을 충족합니다. 기존 _finite_float_or_none 헬퍼를 사용하고 leftover_map_cross_share만 정규화합니다. 유한한 양수, 0, 음수 값은 유지하고 SQL NULL 및 NaN, Infinity, -Infinity는 None으로 변환합니다. 추가된 fake-connection 및 PostgreS…
Out of Scope Changes check ✅ Passed 변경 사항은 [#1006]의 기본 fetch_period_reports() 직렬화 수정과 해당 회귀 테스트에 한정됩니다. 테스트 픽스처와 PostgreSQL 통합 테스트도 요구된 동작 검증에 직접 사용됩니다. 범위를 벗어난 코드 변경은 확인되지 않습니다.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목은 fetch_period_reports의 primary cross-share 값에 대한 비유한 값 정규화를 정확히 설명합니다. 변경 목적과 직접 관련되며 간결하고 구체적입니다.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch repair/1006-primary-report-finite-cross-share

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.

@seonghobae
seonghobae changed the base branch from main to feat/leftover-map-compare-cross-share-v2530 September 11, 2026 02:54
@seonghobae
seonghobae marked this pull request as draft September 11, 2026 02:55
@seonghobae
seonghobae marked this pull request as ready for review September 11, 2026 02:55
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

Copy link
Copy Markdown
Contributor Author

Fresh ancestry verification found a stricter wrong-base defect than the PR body currently describes. The PR metadata was retargeted to #831 (8d7d8fe17de30ef08121d9f38fb1e7279e64354a), but the actual head d678bbbbf8b5b49654352d7cae82c7df1c0eba71 still diverges from #831 with merge-base protected main 83eba56149eb802cd63642c507c324c9976ec78e. More importantly, refs/pull/1007/merge still resolved to 2d010c7fd671bd55f80e18a486c50bc08d23108e, whose parents are main@83eba561... and d678bbbb..., not #831. Therefore Tests run 34556497425 cannot be promoted as corrected-#831 integration evidence even if it terminates GREEN. I converted this PR back to Draft so that invalid acceptance does not continue consuming a runner or get mistaken for a promotion gate.

Concurrent #1008 is not treated as a race: its head 9cc136c7eeb2d8639f6770b102e758491b149f9b is an actual descendant of #831 (ahead_by=1, merge-base exactly 8d7d8fe...), and its merge ref c11ae62606c54c68521f358a689920657a18ad08 has parents #831 + #1008 head. #1008 will therefore be the live repair/successor lane. #1007 stays open as recovery/evidence until #1008 has inherited the useful real-PostgreSQL strict-JSON regression as well as the finite/zero/negative/null/NaN/+Inf/-Inf contract and reaches GREEN; only then is supersession complete enough to close this lane.

Copy link
Copy Markdown
Contributor Author

Verified successor #1008 reached exact-parent GREEN on d581a73c0be5a4906d9e6b19578f5dff4498d4c4 (Tests 34563424670: success) and was normally merged into the live #831 branch as 9189a3e718665b29845a75b9e8360971eb9f7556. #1008 preserves the production normalization plus both fake-boundary and live-PostgreSQL finite/zero/negative/null/NaN/+Inf/-Inf strict-JSON regressions from this wrong-base lane. Under the no-drop rule, complete successor inheritance is now verified; this historical wrong-base recovery lane can close without losing valid delta/evidence.

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.

bug(reports): normalize non-finite persisted cross-share on the primary report read model

1 participant