Skip to content

fix - send community when creating report#302

Open
mjbradford89 wants to merge 5 commits into
developfrom
fix/llm_create_community
Open

fix - send community when creating report#302
mjbradford89 wants to merge 5 commits into
developfrom
fix/llm_create_community

Conversation

@mjbradford89
Copy link
Copy Markdown
Contributor

@claude

This comment was marked as resolved.

@claude

This comment was marked as resolved.

@claude

This comment was marked as resolved.

@claude

This comment was marked as resolved.

@claude
Copy link
Copy Markdown

claude Bot commented May 26, 2026

Review — clean.

  • Correctness: report.download_urlreport.url in aio.llm_report_download is the actual fix — ReportLLMPostProcessing.__init__ only sets self.url (resources.py:1239), so the old async path was an AttributeError on first call. Sync was unaffected because it goes through task.download_report(), which already uses self.url with Authorization: None (resources.py:1242–1258).
  • Community on POST body: added in both sync (api.py:1072) and async (aio/__init__.py:1732) — matches the pattern used by every other community-scoped endpoint in this client.
  • S3-URL guard: the inline comment + test_llm_report_download_no_community_on_s3_url are preventative — the diff itself doesn't add params to the S3 call, but the test pins the invariant so a future "helpful" refactor can't reintroduce SigV4 breakage. Worth keeping.
  • Spec / Gitflow: base is develop ✓. No public-surface change → no version bump needed (correct per AGENTS.md — bumps belong to develop → master). No new endpoint, so no artifact-index ordering concern.
  • Tests: sync + async POST-body assertions, async S3-URL guard. Sync llm_report_download is unchanged and its path was already correct, so no missing case worth flagging.

Optional, non-blocking: the async llm_report_download has a defensive hasattr(result, "handle") block that the sync path lacks (api.py:1082 calls result.handle.close() unconditionally). Pre-existing, not introduced here — mentioning only because you're in the area.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants