feat(export): expose include_code_metric_metadata on export_records (HYBIM-788)

[etserend]

Summary

Port of upstream commit e42a569 from rungalileo/galileo-python (#587) — first commit in the HYBIM-788 migration, applied in chronological order.

• Adds include_code_metric_metadata: bool = False param to ExportClient.records() and module-level export_records()
• When True, includes per-row scorer metadata (the dict returned by code-based scorers via the (score, metadata) tuple-return contract) on each MetricSuccess in the export — off by default to keep payloads small
• Patches LogRecordsExportRequest model (log_records_export_request.py) to carry the new field through to_dict() / from_dict()
• Adds 2 tests: default-False and opt-in-True

Namespace translation applied

All galileo.* references rewritten to splunk_ao.* per HYBIM-788 translation rules. galileo-core and galileo_core.* intentionally unchanged (external dep).

Test plan

• poetry run pytest tests/test_export.py -v — 17/17 passed
• poetry run ruff check src/splunk_ao/export.py tests/test_export.py — clean

Part of

HYBIM-788 — commits applied in order: 1 of 3 (e42a569 → fa93ab2 → 6ccd06a)

[fercor-cisco]

🤖 This review was generated by the Astra agent (claude-opus-4-8). It may contain mistakes.

Verdict: request_changes — New field was hand-added to generated code without updating openapi.yaml, so it will be silently dropped on the next client regen and break export_records; the author's two blocking Jira questions are also unresolved.

General Comments

• 🟠 major (bug): src/splunk_ao/resources/models/log_records_export_request.py is generated code. scripts/auto-generate-api-client.sh backs up, deletes, and fully regenerates the entire resources/ directory from openapi.yaml with --overwrite, and the LogRecordsExportRequest schema in openapi.yaml (lines 28475-28555) was NOT updated to add include_code_metric_metadata. Consequences on the next regen (HYBIM-853, the 0.29.0 regen the author flagged in the ticket is already pending):

1. The manual include_code_metric_metadata field is dropped from the regenerated model.
2. export.py still passes include_code_metric_metadata=... into the LogRecordsExportRequest(...) constructor (lines 60 and 142), which will then raise TypeError: __init__() got an unexpected keyword argument, breaking export_records() entirely.

The repo already has an established pattern for regen-surviving patches (patch_http_validation_error.py, wired in as a post-generation step). This change uses no such mechanism. Please add the property to the LogRecordsExportRequest schema in openapi.yaml (mirroring the redact entry, with default: false) so it survives regeneration — or, if the intent is to wait for HYBIM-853, coordinate as the author's own ticket question #1 asks before merging. This is the same concern the author raised in HYBIM-788 and it appears to be unanswered.

• 🟡 minor (question): The new field defaults to False rather than UNSET, so to_dict() always emits include_code_metric_metadata: false on every export request (unlike optional fields such as filters/sort which are omitted when unset). This matches upstream and mirrors redact (also default-non-UNSET), but the author explicitly asked in HYBIM-788 whether always sending this field is intended on the backend side, and whether the backend even accepts the field yet. That question is unresolved, and the field is not part of the API contract in openapi.yaml. Please confirm the backend accepts (and does not reject) an unknown/extra field before this ships.

Follow-ups

Suggested follow-up work that could be tracked as Shortcut stories:

• src/splunk_ao/resources/models/log_records_export_request.py:324-337: Round-trip asymmetry: from_dict defaults include_code_metric_metadata to UNSET when the key is absent, while the constructor/__init__ default is False. So deserializing a payload lacking the key yields UNSET, not False. This is consistent with how openapi-python-client generates other fields and is harmless here, but worth noting if the field is ever added to the spec with a non-UNSET default (regen would produce d.pop(..., False) instead).

[fercor-cisco]

🟠 major (bug): This field was manually added to a file generated by openapi-python-client, but the corresponding property is missing from the LogRecordsExportRequest schema in openapi.yaml. Running scripts/auto-generate-api-client.sh will regenerate this file and silently drop the field, after which the constructor calls in export.py (include_code_metric_metadata=...) will raise TypeError. Add the property to openapi.yaml so the codegen output includes it.

🤖 Generated by the Astra agent

[fercor-cisco]

@etserend were the changes to src/splunk_ao/resources/models/log_records_export_request.py done manually or via the source code generation script?

If they were done manually, they would be lost in the next regeneration.

[etserend]

@etserend were the changes to src/splunk_ao/resources/models/log_records_export_request.py done manually or via the source code generation script?

If they were done manually, they would be lost in the next regeneration.

Yes, manually — same as upstream e42a569 (they also hand-patched without updating openapi.yaml).
Fixed in 64b29da: include_code_metric_metadata is now in openapi.yaml so it will be regenerated from next run.

[fercor-cisco]

🤖 This review was generated by the Astra agent (claude-opus-4-8). It may contain mistakes.

Verdict: approve — Correct, well-tested param plumbing; the prior openapi.yaml concern is resolved. One non-blocking coverage gap in sibling wrappers.

General Comments

• 🟡 minor (design): This PR exposes include_code_metric_metadata on the module-level export_records() and ExportClient.records(), but the two sibling convenience wrappers — LogStream.export_records() (src/splunk_ao/log_stream.py:747) and Experiment.export_records() (src/splunk_ao/experiment.py:1374) — also delegate to ExportClient().records() and forward redact yet do not expose or forward the new parameter. As a result, users of those higher-level APIs silently get include_code_metric_metadata=False with no way to opt in. If this is intentional for this port (commit 1 of 3), consider a short note; otherwise forwarding the param through both wrappers would keep the feature symmetric with redact.

Follow-ups

Suggested follow-up work that could be tracked as Shortcut stories:

• src/splunk_ao/log_stream.py:747-817: Consider forwarding include_code_metric_metadata through LogStream.export_records() and Experiment.export_records() (experiment.py:1374) so all three export entry points expose the new feature consistently, matching how redact is handled.

[fercor-cisco]

minor (design): ... the two sibling convenience wrappers — LogStream.export_records() and Experiment.export_records() — also delegate to ExportClient().records() and forward redact yet do not expose or forward the new parameter.

Verified: this asymmetry is inherited from upstream e42a569, not introduced by this port.

I checked the source commit against the two wrappers:

• Source commit e42a569 touches only three files (export.py, resources/models/log_records_export_request.py, tests/test_export.py). It does not touch log_stream.py or experiment.py.
• At that commit, both wrappers already existed (galileo/log_stream.py:747, galileo/experiment.py:1374) and both accept/forward redact: bool = True but do not expose or forward include_code_metric_metadata — their signatures end at redact, identical to what this PR carries over.

So the redact-forwarded-but-include_code_metric_metadata-not asymmetry is exactly how upstream shipped it. This PR faithfully reproduces the upstream scope; it neither introduced nor closed the gap.

Keeping it as-is preserves fidelity with the port (commit 1 of 3). If upstream later closes this gap, it would come through in a subsequent ported commit. Leaving this note here to document that the divergence is intentional rather than an oversight.

[fercor-cisco]

Port verification vs. upstream e42a569

Verdict: ✅ The PR correctly ports the commit — faithfully reproduces e42a569 with the expected splunk_ao refactoring applied.

Line-for-line match with the source commit

All three files touched by the upstream commit are reproduced exactly, with only the namespace translation applied:

Change	Source (galileo)	PR (splunk_ao)	Match
records() gets include_code_metric_metadata: bool = False param	✔	✔	✅
Threaded into request body construction	✔	✔	✅
export_records() param + docstring section	✔	✔	✅
LogRecordsExportRequest model: field decl, to_dict(), from_dict()	✔ (11 lines)	✔ (identical 11 lines)	✅
2 tests (default-False, opt-in-True)	✔	✔	✅

Refactoring/renaming correctly applied

• galileo.export.export_records_stream → splunk_ao.export.export_records_stream in the @patch decorators ✅
• Docstring "Exports records from a Galileo project." → "...from a Splunk AO project." ✅ (this is the extra 9 additions/1 deletion vs. the source's 8 additions — an intentional, correct namespace fix)
• ExportClient.records structure preserved (line 19/32) ✅

One legitimate addition beyond the source commit

The PR adds an openapi.yaml change (5 lines) that the upstream commit did not have. This is expected: in galileo-python that model is codegen'd from a separate spec, whereas splunk-ao-python vendors openapi.yaml in-repo. I confirmed it's placed under the correct schema (LogRecordsExportRequest, line 28475), adjacent to redact, with matching default: false and a description consistent with the code. This is a correct adaptation, not a discrepancy.