feat(docs): wire splunk-ao SDK reference doc auto-generation (HYBIM-725)#16
Conversation
Update all source paths and namespace references from galileo to splunk_ao as part of the Splunk AO rebrand. Blocked on HYBIM-717 (src/galileo → src/splunk_ao directory rename) before these changes can be validated. Changes: - pydoc-markdown.yml: search_path ./src/galileo → ./src/splunk_ao - pydoc-markdown-handlers.yml: search_path updated for handlers submodule - scripts/create_docs.py: SOURCE constant updated to ./src/splunk_ao - scripts/auto-generate-api-client.sh: OUTPUT_PATH → ../src/splunk_ao/resources - openapi-client-config.yaml: add package_name: splunk_ao Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
d010b44 to
827e7e7
Compare
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent. It may contain mistakes.
Verdict: request_changes — The added package_name key is not a recognized openapi-python-client config field and has no effect under --meta none; the path renames are otherwise correct.
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent. It may contain mistakes.
Verdict: needs_discussion — The OUTPUT_PATH rename targets src/splunk_ao/resources, which doesn't exist while the whole SDK still imports from galileo.resources, leaving the regen script in a broken/inconsistent state.
General Comments
- 🟡 minor (documentation): The PR description lists many changes (two new GitHub Actions workflows
publish-docs.yaml/regenerate-api-client.yaml, edits topydoc-markdown.yml,scripts/create_docs.py,openapi-client-config.yaml) that are not present in this branch's diff againstorigin/main— the actual change is 2 lines across 2 files. The workflow files do not exist in.github/workflows/. Please reconcile the description with the actual scope so reviewers and future readers aren't misled about what this PR ships.
Follow-ups
Suggested follow-up work that could be tracked as Shortcut stories:
src/splunk_ao/README_API_CLIENT.md:1-39: This README still refers to "Galileo API client" /GalileoApiClientand usesfrom galileo.resources...example imports. Once the galileo→splunk_ao rename is complete, update these references for consistency with the new namespace.
3fb1025 to
701ef17
Compare
…egen script (HYBIM-725) - Add .github/workflows/publish-docs.yaml: triggers on release or workflow_dispatch; runs create_docs.py, verifies output, and opens a PR against splunk/agent-observability-docs. On pull_request, only build+verify steps run (cross-repo steps skipped until PAT_AO_DOC_AUTOMATION is provisioned with write access to agent-observability-docs). - Add .github/workflows/regenerate-api-client.yaml: daily cron + manual dispatch; regenerates API client from OpenAPI spec and opens a PR. - Hardcode docs repo as splunk/agent-observability-docs (confirmed destination). - Keep OUTPUT_PATH at src/galileo/resources — resources not yet migrated to splunk_ao. - Add CI_ENV: dev (present in upstream, was missing from initial port). - Use PAT_CLATOOL for PR testing; rename final token secret to PAT_AO_DOC_AUTOMATION. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ca38c53 to
69d4a97
Compare
|
I'll run another automated review. |
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent. It may contain mistakes.
Verdict: request_changes — Two new workflows contain structural bugs that will fail or misbehave even after the pending secrets/identity TODOs are resolved: a read-only token that cannot push, a deprecated set-output command, and an ungated workflow_run trigger.
Follow-ups
Suggested follow-up work that could be tracked as Shortcut stories:
.github/workflows/regenerate-api-client.yaml:75-75:git push --force ... --no-verifyto a date-based branch will silently overwrite any prior same-day run's branch (and any manual edits to an existing open PR for that day). Consider using--force-with-leaseor including${{ github.run_id }}in the branch name (as publish-docs.yaml already does) to avoid clobbering..github/workflows/publish-docs.yaml:60-64: TheUpdate SDK docsstep doesrm -rf .../sdk-api/python/referencethencp -r ... .generated_docs/reference .../sdk-api/python/. Ifsdk-api/python/does not yet exist in the docs repo, thecpwill create a file namedpythonrather than the intended directory tree. Considermkdir -pon the target parent first to be robust..github/workflows/publish-docs.yaml:1-13: Neither new workflow declares aconcurrencygroup. Overlapping runs (e.g. a manual dispatch during a scheduled/workflow_run trigger) could race on branch creation/push. Adding aconcurrency:block keyed on the workflow + ref would prevent overlapping publish/regenerate runs.
…esting (HYBIM-725) PAT_AO_DOC_AUTOMATION is now provisioned with write access to both repos. Replace PAT_CLATOOL and remove the pull_request guard so the full pipeline (build → verify → checkout docs repo → push branch → open PR) runs on PR. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ity-docs (HYBIM-725) git push was failing with 403 — actions/checkout sets up auth for the initial clone but doesn't persist it for subsequent pushes. Explicitly set the remote URL with the PAT token before pushing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
d002a94 to
58f6cb5
Compare
…ent (HYBIM-725) - Remove pull_request trigger (was testing-only; pipeline now validated end-to-end) - Fix MDX acorn parse errors in create_docs.py: convert RST :: code blocks to fenced blocks, escape curly braces in all description/returns/raises fields, and pass through already-fenced example blocks without double-wrapping - Drop SSH signing from regenerate-api-client; use PAT_AO_DOC_AUTOMATION for push + PR creation (same pattern as publish-docs). SPLUNK_AO_API_URL is the only remaining unprovisioned secret needed to make the workflow operational. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
0ae5a74 to
2dc16bf
Compare
|
Running an automated review agent. |
|
Workflow created PR against agent-observability-docs |
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent. It may contain mistakes.
Verdict: request_changes — Both new workflows will fail their git commit step when there are no changes, causing recurring red runs (especially the daily cron); the doc-script example handling has a prose regression.
Follow-ups
Suggested follow-up work that could be tracked as Shortcut stories:
.github/workflows/regenerate-api-client.yaml:58-70:BRANCH_NAMEis derived only from the date (...update-api-client-<YYYY-MM-DD>). Re-running the workflow on the same day (manualworkflow_dispatchor a retry) force-pushes the same branch; if a PR for that branch already exists, the finalgh pr createstep will fail. Consider appending${{ github.run_id }}to the branch name, or making PR creation idempotent (check for an existing open PR before creating).scripts/create_docs.py:504-555: The new_escape_curly_braces,_convert_rst_code_blocks, and_sanitize_descriptionhelpers have no unit tests, and there are no tests forcreate_docs.pyat all. These functions have non-trivial edge cases (nested fences, RST::blocks with mixed indentation, curly braces adjacent to inline code). Adding focused unit tests would protect against regressions in the MDX-escaping logic.
… fencing (HYBIM-725) - publish-docs: guard git commit on non-empty staged diff; skip Create PR step when has_changes=false to avoid failed runs on no-docstring-change releases - regenerate-api-client: same no-op commit guard; make gh pr create idempotent (skip when branch was never pushed) - create_docs.py: note that description-only examples in this codebase are all code (not prose), so the else-fence branch is correct as-is; reviewer concern acknowledged in commit
…sions (HYBIM-725)
|
All feedback addressed:
All previously flagged bugs fixed in earlier commits. Pipeline validated end-to-end. |
|
Starting another automated review. |
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent. It may contain mistakes.
Verdict: request_changes — Doc-gen regression: prose-only examples are now wrapped in a ```python fence (unaddressed reviewer comment); pending operational TODOs are tracked.
Follow-ups
Suggested follow-up work that could be tracked as Shortcut stories:
.github/workflows/publish-docs.yaml:106-107: TheCreate PRstep injects the staticPR_BODYvia${{ env.PR_BODY }}into the shell command. It is a workflow-defined literal so there is no injection risk today, but referencing the shell variable directly (gh pr create --title "Updated Python SDK docs" --body "$PR_BODY") is more robust against future edits that add shell-special characters to the body..github/workflows/regenerate-api-client.yaml:167-169: Bot git identity (email/name) and theSPLUNK_AO_API_URLsecret are still TODO placeholders, and the same git-identity TODO exists in publish-docs.yaml. These are acknowledged in the PR description and tracked under HYBIM-725; surfacing here only so the workflows are not enabled in CI before the team provisions them.
|
@etserend Let's create a FAST ticket for the service account. Even though the team doesn't think service account is the correct approach but I feel like a ticket would help. Other than this LGTM! Not sure, if some of the issues pointed out in the code review are related to new changes in your PR or were already in the legacy pipelines. But would defer to Fernando's review. |
…ty (HYBIM-725)
- Replace placeholder splunk-ao-automation identity with github-actions[bot]
- Add mkdir -p before cp in publish-docs to handle missing sdk-api/python/ dir
- Fix PR_BODY shell reference (${{ env.PR_BODY }} → $PR_BODY) in publish-docs
- Remove header TODO block and SPLUNK_AO_API_URL TODO comment in regenerate-api-client
Prose-only docstring examples (description with no snippet) were incorrectly wrapped in a python fence. Track had_snippet on the example dict and only fence at emission when a real code snippet was present.
|
Resolved all outstanding review items:
Unit tests for the MDX helpers remain tracked as a follow-up (acknowledged in earlier comment). |
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-4-8). It may contain mistakes.
Verdict: request_changes — Two concrete workflow bugs undermine the auto-generation pipeline this PR wires up: an unquoted pip version constraint and a workflow_run trigger pointing at a workflow that doesn't exist.
General Comments
- 🟡 minor (testing): scripts/create_docs.py gains non-trivial new logic (
_convert_rst_code_blocks,_escape_curly_braces,_sanitize_description, and the rewritten example-emission branches keyed onhad_snippet), but there are no unit tests covering any of it. The author acknowledged this as a tracked follow-up, so it is non-blocking, but the example-fencing branches in particular (>>> stripping viacode[4:], prose-vs-code detection) are exactly the kind of string logic that regresses silently. Consider adding a few table-driven tests before this becomes load-bearing for published docs.
Follow-ups
Suggested follow-up work that could be tracked as Shortcut stories:
.github/workflows/regenerate-api-client.yaml:62-62:git push --forceis unnecessary here: BRANCH_NAME already embeds${{ github.run_id }}, so every run targets a fresh branch that cannot pre-exist. A plaingit push origin $BRANCH_NAMEavoids the surprise of a force-push in automation and fails loudly if the uniqueness assumption is ever violated..github/workflows/regenerate-api-client.yaml:54-54:${{ secrets.SPLUNK_AO_API_URL }}is interpolated unquoted into the shell. If the secret is empty the underlying import-openapi-yaml.sh prints usage and exits 1 (acceptable), but consider passing it via an env var and quoting ("$API_URL") to guard against whitespace/special characters and to keep the value out of the rendered command line.
| python-version: "3.11" | ||
|
|
||
| - name: Install Docstring Parser | ||
| run: pip install docstring-parser>=0.17.0 |
There was a problem hiding this comment.
🟠 major (bug): This argument is unquoted, so bash parses >=0.17.0 as an output redirection: the command runs pip install docstring-parser (unpinned, latest) and writes stdout to a stray file literally named =0.17.0 in the workspace. The >=0.17.0 version floor is silently dropped and never enforced. Quote the requirement so pip receives it as a single argument.
| run: pip install docstring-parser>=0.17.0 | |
| run: pip install 'docstring-parser>=0.17.0' |
🤖 Generated by Astra
| workflow_run: | ||
| workflows: ["Package Release"] | ||
| types: | ||
| - completed |
There was a problem hiding this comment.
🟠 major (bug): The workflow_run trigger keys off a workflow named "Package Release", but no workflow with that name exists in .github/workflows/ (the names present are: CI - Test, Update API Client, Publish Docs, CLA Assistant, Test splunk-ao-a2a, Test splunk-ao-adk). GitHub matches workflow_run.workflows by the name: field, so this trigger will never fire — the only working entry point is manual workflow_dispatch, contradicting the PR's stated "triggers on release" behavior. Either add/rename the release workflow to "Package Release", or point this at the actual workflow name. If the release workflow lands in a separate PR, please confirm the sequencing.
🤖 Generated by Astra
…ent (HYBIM-725) - Quote pip version constraint to prevent shell redirect bug - Remove dead workflow_run trigger; manual workflow_dispatch is the intended entry point - Pass SPLUNK_AO_API_URL via env var and quote it in shell - Drop --force from git push; run_id-scoped branch name already guarantees uniqueness
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-4-8). It may contain mistakes.
Verdict: approve — All prior review findings are resolved in the current HEAD; remaining items are minor/edge-case quality concerns and an acknowledged test follow-up.
General Comments
- 🟡 minor (design): The PR title/description states this "wires" doc auto-generation, but the release trigger in publish-docs.yaml is commented out (only workflow_dispatch is active) because the "Package Release" workflow doesn't exist yet. So publishing is manual-only for now. This appears intentional (matches the "Pending" section), but worth confirming that shipping the publish pipeline in a manual-only state is the desired end state for this PR rather than blocking on the release workflow landing first.
Follow-ups
Suggested follow-up work that could be tracked as Shortcut stories:
scripts/create_docs.py:507-557: Add focused unit tests for the MDX helpers (_escape_curly_braces,_convert_rst_code_blocks,_sanitize_description) and the example-emission branches (fenced vs. doctest vs. prose-only/had_snippet). These string transforms have many branches that can regress silently; the author already acknowledged this as a tracked follow-up..github/workflows/publish-docs.yaml:6-9: Once a "Package Release" workflow exists, uncomment/wire theworkflow_runtrigger (with thegithub.event.workflow_run.conclusion == 'success'guard previously discussed) so docs publish automatically on release rather than only via manual dispatch.
| if line.rstrip().endswith("::"): | ||
| # Emit the line with :: stripped (or removed if standalone) | ||
| stripped = line.rstrip()[:-2].rstrip() | ||
| out.append(stripped) |
There was a problem hiding this comment.
🟡 minor (bug): _convert_rst_code_blocks treats any description line ending in :: as the start of an RST literal block. This has two edge-case issues: (1) if a line ends in :: but no indented block follows (i.e. :: is incidental prose), the colons are silently stripped from the output text, altering the rendered prose; and (2) RST semantics render a trailing text:: as text: (one colon retained), whereas this strips both. Since there are no unit tests guarding this helper, consider only stripping/fencing when an indented block actually follows, and preserving a single colon for the text:: form.
🤖 Generated by Astra
| git config --global user.name "github-actions[bot]" | ||
| git remote set-url origin https://x-access-token:${PAT}@github.com/splunk/splunk-ao-python.git | ||
| git checkout -b $BRANCH_NAME | ||
| poetry run ./scripts/import-openapi-yaml.sh "$API_URL" |
There was a problem hiding this comment.
🟡 minor (bug): If the SPLUNK_AO_API_URL secret is unset/empty, $API_URL expands to an empty string and import-openapi-yaml.sh prints usage and exits 1, failing the daily cron. That's a loud failure (acceptable), but note import-openapi-yaml.sh has no set -e and uses curl -s without --fail, so a non-empty-but-unreachable URL would write an empty openapi.yaml and continue silently rather than failing. Worth confirming the URL is validated / curl uses --fail before this cron runs in production.
🤖 Generated by Astra
fercor-cisco
left a comment
There was a problem hiding this comment.
Approved. Please see the follow-up suggestions above.
Summary
Wires the splunk-ao SDK reference doc auto-generation pipeline for the Splunk AO rebrand. Updates source paths in the doc-gen configs and scripts, adds two new GitHub Actions workflows, and fixes a premature
OUTPUT_PATHrename.Changes
pydoc-markdown-handlers.ymlsearch_pathalready pointed to./src/splunk_ao/handlers(landed via HYBIM-717)scripts/auto-generate-api-client.shOUTPUT_PATHto../src/galileo/resources—src/galileo/resourcesstill exists andsplunk_aoimports from it via the compat shim; path moves when resources are migrated.github/workflows/publish-docs.yamlworkflow_dispatch; runscreate_docs.py, opens PR againstsplunk/agent-observability-docswith generated.mdxfiles.github/workflows/regenerate-api-client.yamlNote on
pydoc-markdown.ymlandscripts/create_docs.pyBoth files already point to
src/splunk_ao— those changes landed onmainas part of HYBIM-717 and are not part of this diff.Out of scope
scripts/import-openapi-yaml.shlogic —HOST_URLis a runtime arg;galileo_core__schemas__*yq keys are from the externalgalileo-corepackage and must stay unchangedsrc/splunk_ao/README_API_CLIENT.mdreferences — tracked as follow-upPending (requires team input before workflows can run in CI)
regenerate-api-client.yamlSPLUNK_AO_AUTOMATION_GITHUB_TOKEN,SPLUNK_AO_AUTOMATION_SSH_PRIVATE_KEY,DOCS_REPOTest plan
src/splunk_aoexists onmainpython3 scripts/create_docs.py— 73.mdxfiles generated cleanly fromsrc/splunk_ao/autogeneratedheader present in all output filespytest— 1813 passed, 134 skipped; 2 pre-existing optional-dep errors unrelated to this PR