Preserve structured topic references during finalization#20
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request refactors the handling of source references during topic finalization. It introduces a fallback mechanism in the parsing logic to restore missing references from original topics using their slugs if the LLM output is incomplete. Additionally, it standardizes the reference payload format within prompts to include specific fields like source ID and path. Review feedback suggests improving the robustness of the JSON parsing by ensuring that null values returned by the LLM are converted to empty strings rather than the literal string "None", which could otherwise break fallback lookups and data integrity.
| source_id = str(raw_ref.get("source_id", "")).strip() | ||
| source_path = str(raw_ref.get("source_path", "")).strip() | ||
| locator = str(raw_ref.get("locator", "")).strip() |
There was a problem hiding this comment.
Using str(raw_ref.get(key, "")) can lead to the string "None" if the LLM returns a null value for a field in the JSON payload. It is safer to use the or operator to ensure a None value is converted to an empty string before calling str().
| source_id = str(raw_ref.get("source_id", "")).strip() | |
| source_path = str(raw_ref.get("source_path", "")).strip() | |
| locator = str(raw_ref.get("locator", "")).strip() | |
| source_id = str(raw_ref.get("source_id") or "").strip() | |
| source_path = str(raw_ref.get("source_path") or "").strip() | |
| locator = str(raw_ref.get("locator") or "").strip() |
| locator = str(raw_ref.get("locator", "")).strip() | ||
| if source_id and source_path and locator: | ||
| refs.append(SourceRef(source_id=source_id, source_path=source_path, locator=locator)) | ||
| slug = str(raw_topic.get("slug", "")).strip() |
There was a problem hiding this comment.
Similar to the reference parsing, using str(raw_topic.get("slug", "")) will result in the string "None" if the LLM returns a null slug. This can break the fallback reference lookup on line 49. Using or "" ensures None is handled correctly.
| slug = str(raw_topic.get("slug", "")).strip() | |
| slug = str(raw_topic.get("slug") or "").strip() |
| TopicDigest( | ||
| slug=str(raw_topic.get("slug", "")).strip(), | ||
| slug=slug, | ||
| title=str(raw_topic.get("title", "")).strip(), |
Summary
Testing