Skip to content

Conversation

EdenWuyifan
Copy link
Collaborator

No description provided.

@Copilot Copilot AI review requested due to automatic review settings September 7, 2025 17:50
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the value matching functionality with significant UI improvements and cleanup of unused scripts. The main changes focus on replacing the MaterialReactTable with a custom table implementation and enhancing the enum browsing experience.

  • Replaces MaterialReactTable with custom table implementation featuring sticky columns and improved editing
  • Adds GDC enum browsing with popover interface and search functionality
  • Removes unused Redis management scripts from package.json

Reviewed Changes

Copilot reviewed 10 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
package.json Removes unused Redis kill scripts and their pre-hooks
app/lib/heatmap/heatmap-helper.tsx Adds enumDef property to GDCAttribute interface
app/dashboard/components/value-comparisons/value-comparison-table.tsx Major refactor replacing MaterialReactTable with custom implementation
app/dashboard/components/fileuploading.tsx Adds close button to file upload form
app/dashboard/components/explanation/RelevantKnowledgeView.tsx Enhances enum display with tooltips using enumDef
api/utils.py Updates GDC ontology file path reference
api/tools/task_tools.py Code style improvements and cleanup
api/matching_task.py Code formatting improvements
api/langchain/memory.py Minor string formatting cleanup
api/index.py Updates GDC ontology file path reference

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 371 to 347
sourceCellRefs.current.set(row.id as number, el);
}}
onClick={() => {
if (!isEditing) {
setEditingRowId(row.id as number);
Copy link

Copilot AI Sep 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using type assertion 'as number' can be unsafe if row.id is not guaranteed to be a number. Consider adding proper type checking or ensuring the row interface has a typed id property.

Suggested change
sourceCellRefs.current.set(row.id as number, el);
}}
onClick={() => {
if (!isEditing) {
setEditingRowId(row.id as number);
if (typeof row.id === "number") {
sourceCellRefs.current.set(row.id, el);
}
}}
onClick={() => {
if (!isEditing && typeof row.id === "number") {
setEditingRowId(row.id);

Copilot uses AI. Check for mistakes.

Comment on lines 371 to 347
sourceCellRefs.current.set(row.id as number, el);
}}
onClick={() => {
if (!isEditing) {
setEditingRowId(row.id as number);
Copy link

Copilot AI Sep 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type assertion 'as number' is used without validation. Consider ensuring row.id is properly typed or add runtime type checking to prevent potential errors.

Suggested change
sourceCellRefs.current.set(row.id as number, el);
}}
onClick={() => {
if (!isEditing) {
setEditingRowId(row.id as number);
if (typeof row.id === "number") {
sourceCellRefs.current.set(row.id, el);
} else {
// Optionally handle the error, e.g. log a warning
console.warn("row.id is not a number:", row.id);
}
}}
onClick={() => {
if (!isEditing) {
if (typeof row.id === "number") {
setEditingRowId(row.id);
} else {
// Optionally handle the error, e.g. log a warning
console.warn("row.id is not a number:", row.id);
}

Copilot uses AI. Check for mistakes.

Copy link

github-actions bot commented Sep 7, 2025

Coverage report

St.
Category Percentage Covered / Total
🔴 Statements 56.87% 7444/13090
🟡 Branches 60.88% 484/795
🔴 Functions 39.1% 122/312
🔴 Lines 56.87% 7444/13090

Test suite run success

3 tests passing in 3 suites.

Report generated by 🧪jest coverage report action from 120da5c

Copy link

github-actions bot commented Sep 7, 2025

Coverage

Coverage Report
FileStmtsMissCoverMissing
api
   index.py58825557%39–40, 107, 115–119, 124–180, 185–241, 251–291, 309–314, 327–332, 382, 414, 433–459, 474, 495–508, 523, 539, 545, 557, 572–588, 603–609, 621–634, 646–657, 669–682, 695–708, 752, 761–771, 778–806, 835, 845, 851, 869, 906–909, 924, 941–944, 1023–1055, 1077, 1084–1086, 1091–1105, 1120–1142, 1152
   matching_task.py71826563%83–85, 113–159, 167–224, 280, 316, 351–353, 412, 416–431, 443, 454–517, 551, 684, 815, 858, 898, 962–973, 1014, 1034–1047, 1052–1105, 1109–1111, 1125–1163, 1173, 1176–1183, 1217, 1232–1241, 1249–1264, 1267, 1270, 1274, 1281, 1290, 1306–1309, 1333–1349, 1377, 1388, 1415–1421, 1429, 1435, 1488, 1523–1532, 1579, 1592, 1621, 1629–1631, 1644–1685, 1702, 1705–1709, 1712
   session_manager.py501178%25–26, 30–31, 39, 47–50, 53, 56
   utils.py32512262%33, 48, 59–61, 64–66, 95–96, 109–126, 130–131, 135–136, 141, 174–175, 200–202, 219, 222–231, 238–245, 255, 284, 294–309, 313–315, 327–367, 373–379, 420–422, 432–434, 438–440, 448–452, 460–463, 529–533, 549–553
api/candidate_quadrants
   candidate_quadrants.py902276%130, 136–152, 156–162, 165–168, 172
api/clusterer
   column_encoder.py431370%51, 55, 83, 92, 101–102, 111, 115, 125–126, 135–136, 144
   embedding_clusterer.py741974%59–76, 83, 102–110
   utils.py1299923%14–16, 20–22, 27–29, 34–35, 40–46, 50–60, 64–66, 125, 129, 140–172, 177–190, 207–208, 236–307
api/langchain
   agent.py1964577%85, 99–104, 106–109, 273, 281, 284–287, 296, 378–381, 400–403, 430–461, 466–475, 506–511
   memory.py2848072%147–173, 295, 304, 312–313, 378, 380, 382, 545, 565–566, 585, 664–675, 678–689, 692–706, 709–723, 726–740, 768–769, 817–818, 826–827
   rag.py38380%1–69
api/langgraph
   langgraph.py2012010%2–778
api/matcher
   bdikit.py41295%52, 113
   difflib.py23291%34, 48
   magneto.py25196%28
   rapidfuzz_value.py93990%49, 99, 106, 126, 148–151, 164–166
   utils.py10280%26, 49
   valentine.py40295%68, 79
api/tools
   candidate_tools.py651577%114–116, 136–138, 184–186, 227–229, 270–272
   online_research_tools.py1816763%31, 61, 76, 85–87, 116–132, 143–159, 170–186, 197–213, 241, 247, 257, 285–287, 302, 330–332, 351, 390–392, 411, 421, 455–457, 461–484
   query_tools.py781581%132, 160–167, 189–196, 218–219
   source_scraper.py10190%40
   task_tools.py521473%21, 50–54, 82, 96–101, 117–123
TOTAL3504130063% 

Tests Skipped Failures Errors Time
62 0 💤 1 ❌ 0 🔥 55.962s ⏱️

@EdenWuyifan EdenWuyifan merged commit ffa3fbf into main Sep 23, 2025
6 checks passed
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.

1 participant