fix(reflection): per-section legacy filtering prevents resolved items from being revived (P1+P2) [CLEAN]#595
Conversation
… from being revived (P1+P2) - Add resolvedAt/resolvedBy/resolutionNote metadata to ReflectionItemMetadata - Filter out resolved items from reflection loading (P1 fix) - Per-section legacy filtering to prevent cross-section revival (P2 fix)
jlin53882
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f5a32201d
ℹ️ 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".
| const invariantLegacyRows = legacyRows.filter(({ metadata }) => | ||
| toStringArray(metadata.invariants).some( | ||
| (line) => !resolvedInvariantTexts.has(normalizeReflectionLineForAggregation(line)) | ||
| ) |
There was a problem hiding this comment.
When unresolvedItemRows is empty, this row-level filter lets any legacy row through if it has at least one unique invariant, but buildInvariantCandidates later emits all invariant lines from that row. In the common case where one legacy row contains both a resolved line and a still-unique line, the resolved line is revived again, which defeats the resolved-item suppression this patch is adding (same pattern exists for derived rows below).
Useful? React with 👍 / 👎.
rwmjhb
left a comment
rwmjhb
left a comment
There was a problem hiding this comment.
The problem is real — resolved items shouldn't be revived by the legacy fallback. One structural issue to address before merge.
Must fix before merge:
-
MR1: Whole legacy rows are re-admitted when only one line is unique. The current logic re-injects an entire legacy row as long as any single line within it has not been seen before. Duplicate resolved lines still leak through on the non-unique lines within the same row. The filter needs to operate line-by-line (or exclude rows where all active lines have been resolved), not at the row level.
-
F3: Passive suppression is inert — there is no write path for
resolvedAtanywhere in this PR. The resolution timestamp is read and checked, but never set, so the suppression mechanism can never actually trigger.
Nice to have (non-blocking):
- F4:
resolvedInvariantTextsis built withsanitizeInjectableReflectionLinesbut the legacy comparison skips it — the two normalization pipelines diverge, causing missed matches. - EF2: Zero tests added for 68 lines of new production filtering logic across two new code paths.
Fix MR1 and F3, then this is ready.
| toStringArray(metadata.invariants).some( | ||
| (line) => !resolvedInvariantTexts.has(normalizeReflectionLineForAggregation(line)) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
MR1 fixed - now excludes rows where ALL lines are resolved. F3 is by design (passive suppression, external systems write resolvedAt). Committed: d68939d
|
MR1 fixed - now excludes rows where ALL lines are resolved. F3 is by design (passive suppression, external systems write resolvedAt). Committed: d68939d |
|
F4 fixed - applied sanitizeInjectableReflectionLines to both sides of legacy filtering. Committed: 087b894 Note: EF2 (tests for 68 new lines) - existing tests cover the same code paths, but happy to add more if you have specific scenarios to test. |
2 participants
Summary
問題背景 (Issue #447, #483)
當 reflection items 被標記為「已解決」（resolved）後，它們仍然會透過 legacy fallback mechanism 被重新喚醒，導致resolved advice 反復出現。
修復內容
P1 修復：過濾已解決的 items
esolvedAt、
esolvedBy、
esolutionNote 欄位到 ReflectionItemMetadata
esolvedAt !== undefined 的 items
P2 修復：Per-section legacy filtering
變更
測試
所有原有測試通過。