Refactor asset detail retrieval for unified model support

[rhythmatician]

Enhance asset detail retrieval to support a unified model for both standard and client-specific queries. Update comments to clarify model relationships in asset retrieval.

[Copilot]

Pull request overview

This PR refactors asset detail retrieval to support a unified model (AssetToClientAssetManagerResponseModel) for both standard and client-specific asset queries. The changes enhance documentation to clarify the model relationship and add error logging for cleanup failures.

Key changes:

• Updated return type documentation for get_details() methods to explain the safe cast from AssetToAssetManageResponseModel to AssetToClientAssetManagerResponseModel
• Added comprehensive inline comments explaining why the type cast is safe (superset relationship with 8 additional optional fields)
• Added error logging in async cleanup path to match existing sync implementation

[Copilot]

@rhythmatician I've opened a new pull request, #95, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

src/qualer_sdk/quick_collection.py:222

• The new client_company_id parameter introduced in both sync and async get_details methods lacks test coverage. The existing unit tests in test_asset_collection.py only stub get_asset_manager_list but don't cover the new branching logic that calls get_asset_manager_list_get_2 when client_company_id is provided. Tests should be added to verify both the standard path (client_company_id=None) and the client-specific path (client_company_id provided).

        if not client_company_id:
            # Cast standard assets to client model (superset type). Safe because:
            # 1. AssetToClientAssetManagerResponseModel has all fields from AssetToAssetManageResponseModel
            # 2. Additional client-specific fields are optional and will be None
            # 3. Both models parse the same base asset JSON structure
            result = get_asset_manager_list.sync(
                client=self.client,
                model_filter_type=FilterType.COLLECTED_ASSETS,
                model_search_string=search_string,
                model_page=page,
                model_page_size=page_size,
            )
            return cast(list[AssetToClientAssetManagerResponseModel], result or [])
        else:
            return (
                get_asset_manager_list_get_2.sync(
                    client_company_id=client_company_id,
                    client=self.client,
                    query_filter_type=AssetFilterType.CLIENT_ASSETS_COLLECTED,
                    query_search_string=search_string,
                    query_page=page,
                    query_page_size=page_size,
                )
                or []
            )

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.