Conversation
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: antidodo <albin2993@gmail.com>
# Conflicts: # flamesdk/flame_core.py # flamesdk/resources/client_apis/clients/data_api_client.py # flamesdk/resources/client_apis/clients/po_client.py # flamesdk/resources/client_apis/clients/storage_client.py # flamesdk/resources/client_apis/storage_api.py
…V conversion Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: antidodo <albin2993@gmail.com>
# Conflicts: # flamesdk/resources/utils/fhir.py
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
📝 WalkthroughWalkthroughTwo files modified: whitespace adjustments in rest_api.py and a refactoring of FHIR data extraction logic in utils/fhir.py. The FHIR refactor reorganizes data processing with batch-friendly loops, converts dot-delimited key sequences to lists, simplifies traversal logic, updates resource-specific extraction paths, and improves logging and error handling throughout. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
flamesdk/resources/utils/fhir.py (1)
134-163:⚠️ Potential issue | 🟠 MajorAvoid IndexError on fallback key lookup; use isinstance for Ruff E721.
The
[0]fallback at line 155 raisesIndexErrorwhen no substring match exists, since theif key:check at line 156 occurs after indexing (never executes). Additionally,type(...) == dict/listtrips Ruff E721 and misses subclasses. Usenext(..., None)withisinstance():🛠️ Proposed fix
- if (current < (len(keys) - 1)) or (type(fhir_entry) == list): - if type(fhir_entry) == dict: + if (current < (len(keys) - 1)) or isinstance(fhir_entry, list): + if isinstance(fhir_entry, dict): @@ - elif type(fhir_entry) == list: + elif isinstance(fhir_entry, list): @@ - if type(fhir_entry) == dict: + if isinstance(fhir_entry, dict): try: value = fhir_entry[key] except KeyError: - key = [k for k in fhir_entry.keys() if key in k][0] - if key: - value = fhir_entry[key] - else: - flame_logger.new_log(f"Unable to find field '{key}' in fhir data at level={current + 1} " - f"(keys found: fhir_entry.keys())", - log_type='warning') - return None + fallback_key = next((k for k in fhir_entry.keys() if key in k), None) + if fallback_key is None: + flame_logger.new_log(f"Unable to find field '{key}' in fhir data at level={current + 1} " + f"(keys found: fhir_entry.keys())", + log_type='warning') + return None + value = fhir_entry[fallback_key]
🤖 Fix all issues with AI agents
In `@flamesdk/resources/utils/fhir.py`:
- Around line 61-76: The loop handling input_resource == 'QuestionnaireResponse'
uses the per-page index i as the row key which can collide across paged batches;
update the logic in that branch (where col_id and value are populated via
_search_fhir_resource and df_dict is updated) to use a stable row identifier
instead of str(i) — for example derive row_key from the QuestionnaireResponse
resource id (entry['resource']['id']) or a running current_count variable that
is incremented across pages; ensure you check for None id and fall back to a
globally incremented counter, then use df_dict[col_id][row_key] = value instead
of df_dict[col_id][str(i)] to avoid overwrites across pages.
| elif input_resource == 'QuestionnaireResponse': | ||
| for item in entry['resource']['item']: | ||
| col_id = _search_fhir_resource(fhir_entry=item, | ||
| flame_logger=flame_logger, | ||
| key_sequence=col_key_seq, | ||
| keys=col_keys, | ||
| current=2) | ||
| value = _search_fhir_resource(fhir_entry=item, | ||
| flame_logger=flame_logger, | ||
| key_sequence=value_key_seq, | ||
| keys=value_keys, | ||
| current=2) | ||
| if col_id_filters is not None: | ||
| if (col_id is None) or (not any([col_id_filter in col_id for col_id_filter in col_id_filters])): | ||
| continue | ||
| if col_id not in df_dict.keys(): | ||
| df_dict[col_id] = {} | ||
| df_dict[col_id][str(i)] = value |
There was a problem hiding this comment.
Row IDs for QuestionnaireResponse can collide across pages.
Using the per-page i index resets on each paged batch, so later pages can overwrite earlier rows for the same column. Use a stable row key (e.g., current_count or resource id).
🛠️ Proposed fix
for i, entry in enumerate(fhir_data['entry']):
current_count += 1
@@
- elif input_resource == 'QuestionnaireResponse':
- for item in entry['resource']['item']:
+ elif input_resource == 'QuestionnaireResponse':
+ row_key = str(entry.get("resource", {}).get("id", current_count))
+ for item in entry['resource']['item']:
@@
- df_dict[col_id][str(i)] = value
+ df_dict[col_id][row_key] = value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif input_resource == 'QuestionnaireResponse': | |
| for item in entry['resource']['item']: | |
| col_id = _search_fhir_resource(fhir_entry=item, | |
| flame_logger=flame_logger, | |
| key_sequence=col_key_seq, | |
| keys=col_keys, | |
| current=2) | |
| value = _search_fhir_resource(fhir_entry=item, | |
| flame_logger=flame_logger, | |
| key_sequence=value_key_seq, | |
| keys=value_keys, | |
| current=2) | |
| if col_id_filters is not None: | |
| if (col_id is None) or (not any([col_id_filter in col_id for col_id_filter in col_id_filters])): | |
| continue | |
| if col_id not in df_dict.keys(): | |
| df_dict[col_id] = {} | |
| df_dict[col_id][str(i)] = value | |
| elif input_resource == 'QuestionnaireResponse': | |
| row_key = str(entry.get("resource", {}).get("id", current_count)) | |
| for item in entry['resource']['item']: | |
| col_id = _search_fhir_resource(fhir_entry=item, | |
| flame_logger=flame_logger, | |
| keys=col_keys, | |
| current=2) | |
| value = _search_fhir_resource(fhir_entry=item, | |
| flame_logger=flame_logger, | |
| keys=value_keys, | |
| current=2) | |
| if col_id_filters is not None: | |
| if (col_id is None) or (not any([col_id_filter in col_id for col_id_filter in col_id_filters])): | |
| continue | |
| if col_id not in df_dict.keys(): | |
| df_dict[col_id] = {} | |
| df_dict[col_id][row_key] = value |
🤖 Prompt for AI Agents
In `@flamesdk/resources/utils/fhir.py` around lines 61 - 76, The loop handling
input_resource == 'QuestionnaireResponse' uses the per-page index i as the row
key which can collide across paged batches; update the logic in that branch
(where col_id and value are populated via _search_fhir_resource and df_dict is
updated) to use a stable row identifier instead of str(i) — for example derive
row_key from the QuestionnaireResponse resource id (entry['resource']['id']) or
a running current_count variable that is incremented across pages; ensure you
check for None id and fall back to a globally incremented counter, then use
df_dict[col_id][row_key] = value instead of df_dict[col_id][str(i)] to avoid
overwrites across pages.
Summary by CodeRabbit
Bug Fixes
Refactor
Style