Skip to content

refactor: isolate compact holdings parsing - #682

Merged
Pigbibi merged 3 commits into
mainfrom
refactor/architecture-compact-holdings-20261031
Nov 3, 2026
Merged

Pigbibi merged 3 commits into
mainfrom
refactor/architecture-compact-holdings-20261031

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

AI assistance

Implemented with OpenAI GPT-6 Astra Pro. Independent AI review by /root/phase1k_evidence_review covered base 50176bfaf66d34eeb6c465f7bff542b9a6dea565, candidate tree 840a589dd1dee84a32803d6f76021cb695b8ba47, and the complete two-file change. Published commit 76cb272eb2a8594c6a025d25bb504746ea741e43 points to that reviewed tree. The reviewer found no defects.

What changed

adapt_compact_sections currently parses the rendered dashboard and composes the final notification sections in one function. This change extracts its existing holdings loop into _compact_holding_lines, keeping composition and supplemental-line handling in the public adapter.

The extraction preserves the public signature, supported headings and section boundaries, separator precedence, quantity parsing, localization, output order, duplicate handling, lazy input consumption and propagated exceptions. It does not change valuation, routing, delivery or trading behavior. Inlining the private helper reconstructs the complete original module AST.

Only notifications/compact_adapter.py and a new characterization test module change. All 14 currently open LongBridge PR file inventories were checked; neither path overlaps those changes. In particular, the renderer changes in #680 and startup work remain separate.

How verified

  • Characterization tests were written and executed against the unchanged source first. The focused set passed 19 tests before and 19 tests after the extraction, including the three existing adapter tests.
  • The 16 new characterization methods cover supported and unsupported headings, section termination, blank/repeated headings, full-width separator precedence, nonzero thresholds, localization, falsey inputs, duplicate supplements, evaluation order, lazy iteration and exception identity.
  • Whole-module AST equality after inlining, public signature preservation, Python compilation and whitespace checks passed.
  • The unchanged repository risk classifier categorizes both changed paths as low risk.
  • Independent review independently reproduced the same 19 tests on both versions, compared 280 differential scenarios, and verified 16 same-interpreter cleanup cases. The original module AST, public signature and docstring match after inlining.
  • Native CI completed on checkout 694b33574430ed22f4c60e7127655f0a051517a2; its tree matches the independently reviewed tree. The full unit suite passed 2,504 tests, with 1 skipped and 343 passing subtests. Frozen installation, Ruff, QPK pin/lock checks and offline workflow tests passed.
  • Documentation checks passed with 36 checker tests, zero blocking findings and one advisory. Native Gitleaks scanned the actual one-commit PR range without findings; commitlint passed. The PR risk gate classified the change as low risk and the template check passed.
  • Inherited CI limitation: the public-configuration step returned success although its log reports rg: command not found. That remote scan did not execute and is not counted as a completed check. Existing repair drafts ci: fail closed when the public configuration scan cannot run #679/ci: verify public configuration scan completion #681 are separate; this refactor does not modify their workflow paths.
  • Supplemental offline verification checked all 30 workflow files against the exact published tree, confirmed the actual ripgrep input inventory, and ran the unchanged scan body with ripgrep 15.2.0. The scanner actually executed, returned the expected no-match exit 1, and the step returned 0. This is offline content verification, not a CI rerun or a repair of the inherited step.

Local verification used an existing Python test runner. It did not install or change dependencies. The snapshot contains 311 verified baseline blobs; four unrelated baseline entries were not materialized and are preserved unchanged in the authoritative base tree. No full local frozen install or local Ruff result is claimed. The native run covered frozen installation, lint, the complete test suite, documentation and governance. The public-configuration execution limitation is reported above.

Scope and rollback

This is a draft proposal. Reverting the two-file change restores the original layout. Production entry points, fingerprints, runtime configuration, dependency pins, accounts, risk controls and deployment settings remain outside this change. No owner labels are changed.

@coderabbitai

coderabbitai Bot commented Oct 31, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@Pigbibi

Pigbibi commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T17:03:36.916736Z 16abf05 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: dd9906bdab

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Pigbibi
Pigbibi marked this pull request as ready for review November 3, 2026 17:00
@Pigbibi Pigbibi added the owner-approved Owner approval for high-risk PRs. AI agents must never apply this label. label Nov 3, 2026
@Pigbibi

Pigbibi commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 16abf053c6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Pigbibi
Pigbibi merged commit c9cb033 into main Nov 3, 2026
8 of 9 checks passed
@Pigbibi
Pigbibi deleted the refactor/architecture-compact-holdings-20261031 branch November 3, 2026 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

owner-approved Owner approval for high-risk PRs. AI agents must never apply this label.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant