Skip to content

feat: return data_source_info and data_source_detail_dict - #32

Open
tomerqodo wants to merge 3 commits into
augment_only-issues-20260113-augment-codex-sentry_base_feat_return_data_source_info_and_data_source_detail_dict_pr179from
augment_only-issues-20260113-augment-codex-sentry_head_feat_return_data_source_info_and_data_source_detail_dict_pr179
Open

feat: return data_source_info and data_source_detail_dict#32
tomerqodo wants to merge 3 commits into
augment_only-issues-20260113-augment-codex-sentry_base_feat_return_data_source_info_and_data_source_detail_dict_pr179from
augment_only-issues-20260113-augment-codex-sentry_head_feat_return_data_source_info_and_data_source_detail_dict_pr179

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from qodo-benchmark#179

@augmentcode

augmentcode Bot commented Jan 15, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR updates the document-detail API to return both the raw data-source payload and a richer “detail” representation, including better support for local_file sources.

Changes:

  • Document detail endpoint now includes data_source_detail_dict alongside data_source_info in responses (for both metadata=without and the default response).
  • Default document detail response switches data_source_info to the raw JSON form (data_source_info_dict), while still returning the enriched data_source_detail_dict.
  • Document.data_source_detail_dict now parses and returns raw JSON for local_file sources (previously not covered).
  • Added unit tests exercising data_source_info_dict and data_source_detail_dict behavior for key data source types, including local_file.

Technical Notes: The detailed representation for upload_file still performs a DB lookup, while notion_import/website_crawl/local_file detail currently derives directly from JSON in data_source_info.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. 4 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread api/models/dataset.py
}
}
elif self.data_source_type in {"notion_import", "website_crawl"}:
elif self.data_source_type in {"notion_import", "website_crawl", "local_file"}:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

data_source_detail_dict now parses JSON for local_file; unlike data_source_info_dict, this path doesn’t catch JSONDecodeError, so malformed data_source_info could raise and turn the document-detail API into a 500. Consider handling decode failures consistently (e.g., returning {}) to match data_source_info_dict’s behavior.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

"position": document.position,
"data_source_type": document.data_source_type,
"data_source_info": data_source_info,
"data_source_info": document.data_source_detail_dict,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

After this change, data_source_info is raw JSON for the default response but remains data_source_detail_dict for metadata=without, so the response shape for the same key depends on the query param. If clients expect a stable meaning for data_source_info, it may be worth aligning semantics between branches (also applies to the else branch below).

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎



# ERROR: Expected LocalFileInfo, but got WebsiteCrawlInfo
case_2: LocalFileCase = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

case_2 is unused (not included in cases) and the # ERROR marker appears misleading, which can confuse readers and may hide real test intent. If it’s not meant to be exercised, consider dropping it to keep the test focused.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

assert detail_result == website_data

def test_local_file_data_source_detail_dict_without_db(self):
"""Test that local_file returns empty data_source_detail_dict (this doesn't need DB context)."""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This docstring/comments say local_file returns an empty dict / returns {} early, but the assertion expects parsed JSON ({"file_path": ...}), which matches the updated model behavior. Updating the description would prevent future confusion about intended semantics.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

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.

2 participants