feat: add inputs.json file for dataset traceability #310
Conversation
WalkthroughAdds output-related defaults and a graceful shutdown constant; updates logging and exporters to use the new defaults; introduces Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant DatasetManager
participant RequestConverterFactory
participant RequestConverter
participant Conversations
participant FS
User->>DatasetManager: _profile_configure_command()
activate DatasetManager
DatasetManager->>DatasetManager: _configure_dataset(...)
DatasetManager->>DatasetManager: _generate_inputs_json_file()
DatasetManager->>RequestConverterFactory: get(model_endpoint)
RequestConverterFactory-->>DatasetManager: RequestConverter
DatasetManager->>Conversations: iterate sessions & turns
loop per session
DatasetManager->>RequestConverter: format_payload(turn)
RequestConverter-->>DatasetManager: payload dict
DatasetManager->>DatasetManager: accumulate SessionPayloads
end
DatasetManager->>FS: write OutputDefaults.INPUTS_JSON_FILE (InputsFile JSON)
FS-->>DatasetManager: success / error
DatasetManager-->>User: done
deactivate DatasetManager
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)aiperf/dataset/dataset_manager.py (5)
🪛 Ruff (0.13.1)aiperf/dataset/dataset_manager.py162-162: Do not catch blind exception: (BLE001) 🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
aiperf/common/models/dataset_models.py (1)
92-105: Avoid mutable defaults on the new Pydantic models.Using
Field(default=[])relies on Pydantic copying the list every time. Switch todefault_factory=listso we never risk shared state or unnecessary deep copies.- payloads: list[dict[str, Any]] = Field( - default=[], + payloads: list[dict[str, Any]] = Field( + default_factory=list, description="List of formatted payloads in the session (one per turn). These have been formatted for the model and endpoint.", ) … - data: list[SessionPayloads] = Field( - default=[], description="List of all dataset sessions." + data: list[SessionPayloads] = Field( + default_factory=list, description="List of all dataset sessions." )tests/dataset/test_dataset_manager_inputs_json.py (1)
124-133: Make payload conversion failures easier to diagnose.By default
asyncio.gathermasks individual coroutine contexts when an exception is raised, making the source turn harder to track. Passingreturn_exceptions=Truewould hide errors entirely. A better middle ground is to callasyncio.gatherwithreturn_exceptions=False(the default) but wrap eachformat_payloadcall with additional context, or iterate sequentially. For clarity during failures, consider refactoring to iterate over turns or add context to raised errors so the failing turn/session is obvious.tests/dataset/conftest.py (1)
123-124: Silence Ruff ARG001 by accepting*args, **kwargs.Ruff flags the unused
file_path/modeparameters inmock_open. Broaden the signature to*args, **kwargsand drop the unused names to keep lint green.Apply this diff:
- def mock_open(file_path, mode="r"): + def mock_open(*args, **kwargs): return AsyncContextManager(capture)aiperf/dataset/dataset_manager.py (2)
124-134: Guard against unbounded concurrency on large datasets.
asyncio.gatherwill spawn one coroutine per turn in the conversation. For big datasets (thousands of turns) this can blow past file descriptors/memory and back-pressure the loop. Consider batching, streaming per turn, or usingasyncio.Semaphoreto cap outstanding conversions.
160-164: Log full stack traces for easier debugging.Catching bare
Exceptionand logging only the message hides stack context. Useself.exception/logger.exception(if available) or includeexc_info=Trueso operators can trace the failure source.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
aiperf/common/config/config_defaults.py(1 hunks)aiperf/common/constants.py(1 hunks)aiperf/common/logging.py(4 hunks)aiperf/common/models/__init__.py(3 hunks)aiperf/common/models/dataset_models.py(2 hunks)aiperf/controller/system_controller.py(2 hunks)aiperf/dataset/dataset_manager.py(5 hunks)aiperf/exporters/csv_exporter.py(2 hunks)aiperf/exporters/json_exporter.py(2 hunks)tests/data_exporters/test_csv_exporter.py(5 hunks)tests/data_exporters/test_json_exporter.py(2 hunks)tests/dataset/__init__.py(1 hunks)tests/dataset/conftest.py(1 hunks)tests/dataset/test_dataset_manager_inputs_json.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
aiperf/common/models/__init__.py (1)
aiperf/common/models/dataset_models.py (2)
InputsFile(98-105)SessionPayloads(86-95)
tests/data_exporters/test_json_exporter.py (1)
aiperf/common/config/config_defaults.py (1)
OutputDefaults(114-121)
aiperf/exporters/csv_exporter.py (1)
aiperf/common/config/config_defaults.py (1)
OutputDefaults(114-121)
aiperf/common/models/dataset_models.py (1)
aiperf/common/models/base_models.py (1)
AIPerfBaseModel(25-53)
aiperf/controller/system_controller.py (1)
aiperf/common/config/config_defaults.py (1)
OutputDefaults(114-121)
aiperf/dataset/dataset_manager.py (5)
aiperf/clients/model_endpoint_info.py (4)
ModelEndpointInfo(117-155)from_user_config(47-54)from_user_config(103-114)from_user_config(130-135)aiperf/common/config/config_defaults.py (1)
OutputDefaults(114-121)aiperf/common/factories.py (16)
RequestConverterFactory(475-480)create_instance(183-209)create_instance(284-315)create_instance(354-364)create_instance(375-381)create_instance(390-395)create_instance(406-414)create_instance(425-430)create_instance(439-447)create_instance(456-464)create_instance(491-499)create_instance(536-550)create_instance(561-573)create_instance(584-596)create_instance(607-611)create_instance(620-628)aiperf/common/models/dataset_models.py (3)
Conversation(73-83)InputsFile(98-105)SessionPayloads(86-95)aiperf/common/protocols.py (3)
RequestConverterProtocol(416-423)format_payload(419-423)info(69-69)
tests/dataset/test_dataset_manager_inputs_json.py (5)
aiperf/common/config/config_defaults.py (1)
OutputDefaults(114-121)aiperf/common/factories.py (1)
RequestConverterFactory(475-480)aiperf/common/models/dataset_models.py (2)
InputsFile(98-105)SessionPayloads(86-95)tests/dataset/conftest.py (4)
populated_dataset_manager(80-91)capture_file_writes(95-127)empty_dataset_manager(66-76)user_config(19-28)aiperf/dataset/dataset_manager.py (1)
_generate_inputs_json_file(136-164)
aiperf/exporters/json_exporter.py (1)
aiperf/common/config/config_defaults.py (1)
OutputDefaults(114-121)
tests/data_exporters/test_csv_exporter.py (1)
aiperf/common/config/config_defaults.py (1)
OutputDefaults(114-121)
aiperf/common/logging.py (2)
aiperf/common/config/config_defaults.py (1)
OutputDefaults(114-121)tests/dataset/conftest.py (1)
user_config(19-28)
tests/dataset/conftest.py (3)
aiperf/common/config/output_config.py (1)
OutputConfig(15-34)aiperf/common/models/dataset_models.py (3)
Conversation(73-83)Text(24-27)Turn(43-70)aiperf/dataset/dataset_manager.py (1)
DatasetManager(52-355)
🪛 Ruff (0.13.1)
aiperf/dataset/dataset_manager.py
160-160: Do not catch blind exception: Exception
(BLE001)
tests/dataset/test_dataset_manager_inputs_json.py
185-185: Unused function argument: args
(ARG001)
185-185: Unused function argument: kwargs
(ARG001)
186-186: Avoid specifying long messages outside the exception class
(TRY003)
tests/dataset/conftest.py
123-123: Unused function argument: file_path
(ARG001)
123-123: Unused function argument: mode
(ARG001)
🔇 Additional comments (12)
tests/dataset/__init__.py (1)
1-2: License header alignment looks goodSPDX metadata matches project conventions and keeps test package compliant.
aiperf/common/constants.py (1)
12-13: Addition ofGRACEFUL_SHUTDOWN_TIMEOUTmatches new shutdown flow.New constant is well documented and consistent with existing timeout defaults.
aiperf/common/config/config_defaults.py (1)
115-121: No hard-coded references to inputs.json or aiperf.log
I ran a repo-wide search and found no occurrences outside config_defaults.py; confirm any documentation or CLI defaults reference these new constants.aiperf/common/models/__init__.py (2)
24-27: Consistent public surface for new dataset modelsRe-exporting
InputsFileandSessionPayloadshere keeps the public API aligned with their introduction indataset_models; no gaps spotted.
91-109: all kept in syncThe additions to
__all__match the new imports, so downstream consumers get the expected symbols.tests/data_exporters/test_json_exporter.py (2)
11-11: Test now tracks shared constantSwitching to
OutputDefaultskeeps the expectation coupled to the runtime constant—solid guard against future drift.
95-97: Expectation mirrors exporter pathThe assertion now mirrors the exporter’s default path logic, so the test will flag any future divergence.
aiperf/exporters/json_exporter.py (1)
10-51: Exporter path follows config defaultsUsing
OutputDefaults.PROFILE_EXPORT_AIPERF_JSON_FILEkeeps this exporter aligned with the centralized defaults; everything else remains untouched.aiperf/exporters/csv_exporter.py (1)
12-46: CSV exporter path centralizedGreat to see the CSV exporter adopting the same
OutputDefaultsconstant—consistent artifact naming across exporters.aiperf/controller/system_controller.py (1)
13-520: Log file path honors artifact rootThe controller now reports the log location under the configured artifact directory, matching the logging setup—looks good.
aiperf/common/logging.py (1)
14-163: Logging defaults sourced from OutputDefaultsAll file-handling paths now flow through
OutputDefaults, ensuring every logging entry point honors the same folder/file defaults. Nice consolidation.tests/data_exporters/test_csv_exporter.py (1)
11-293: Tests aligned with exporter defaultsUpdating the expectations to use
OutputDefaults.PROFILE_EXPORT_AIPERF_CSV_FILEguarantees the tests mirror the runtime behavior. Everything checks out.
ajcasagrande
requested review from
debermudez,
ganeshku1,
lkomali and
the-david-oy
3 participants
Format is slightly different from GenAI-Perf. Looking for feedback on this:
payloadwas renamed topayloadspayloadsrepresents a turn in a conversation/sessionsession_idfield was added, so it can be used for correlation purposes later. this will allow us to link requests to payloads for use in the future2 Turns Data:
CLI Command:
Multi-Modal Data:
CLI Command:
Summary by CodeRabbit