Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds filtering and sorting functionality for items in the aggregation display based on a priority list. Items not present in item_list_priority.json are filtered out from the summary tables (unless they are event items, points, or QP), and known items are displayed in priority order.
Changes:
- Added
item_list_priority.jsoncontaining 138 items with priority metadata - Implemented filtering logic to exclude unknown items from summary tables
- Implemented sorting by dropPriority (descending) then by id (descending)
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| viewer/tsconfig.json | Enabled resolveJsonModule to support JSON imports in TypeScript |
| viewer/src/summaryUtils.ts | Added filtering logic to exclude unknown items from normal category using isKnownItem() |
| viewer/src/summaryUtils.test.ts | Added test case verifying unknown items are filtered out |
| viewer/src/itemPriority.ts | New module implementing item filtering and priority-based sorting logic |
| viewer/src/itemPriority.test.ts | Comprehensive test suite for filtering and sorting functions |
| viewer/src/data/item_list_priority.json | JSON data file containing 138 items with id, rarity, shortname, and dropPriority fields |
| viewer/src/components/SummaryTable.tsx | Applies priority sorting to normal items in the summary table |
| viewer/src/components/ReporterSummary.tsx | Applies priority sorting to item columns in reporter detail tables |
| gen_item_list_priority.py | Python script to generate item_list_priority.json from source data |
| SPEC.md | Documentation of the filtering and sorting behavior |
| Makefile | Added gen-priority-file target to generate the priority JSON file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import logging | ||
| import sys | ||
|
|
||
| logger = logging.getLogger(__name__) |
There was a problem hiding this comment.
The logger is created but never configured with basicConfig or a handler. When warnings are logged at line 57, they won't be displayed to the user unless the logging level is configured. Consider adding logging.basicConfig() in the main function or in the if name == "main" block to ensure warning messages are visible.
No description provided.