Skip to content

Conversation

@baptiste-olivier
Copy link
Collaborator

@baptiste-olivier baptiste-olivier commented Oct 8, 2025

Claude Code 4.5 Sonnet MR description (08/10)

📋 Analyse Complète de la Feature LAB-3976

🎯 Description Générale de la MR

Cette MR introduit une refonte architecturale majeure du Kili Python SDK en ajoutant une nouvelle API orientée domaine (Domain API) qui coexiste avec l'API
legacy existante. L'objectif est de fournir une interface plus moderne, typée et organisée pour les utilisateurs du SDK, tout en maintenant une compatibilité
ascendante complète.


🏗️ Explication Détaillée de la MR

Problématique Résolue

L'ancienne architecture du SDK présentait plusieurs limitations :

  1. Manque de typage : Retour de dictionnaires génériques au lieu d'objets typés
  2. API plate : Toutes les méthodes au niveau du client (ex: kili.count_assets(), kili.append_to_labels())
  3. Découvrabilité limitée : Difficulté à explorer les méthodes disponibles par domaine
  4. Pas de séparation des préoccupations : Logique métier mélangée avec la couche API

Solution Proposée

La nouvelle architecture introduit :

  1. Client Domain Séparé (client_domain.py)
  • Nouveau point d'entrée avec namespaces organisés par domaine
  • Lazy loading via @cached_property pour optimiser les performances
  • Accès ergonomique : kili.assets.list() au lieu de kili.assets()
  1. Module domain_api - Couche de Namespaces
  • Organisation logique par domaine (Assets, Labels, Projects, Users, etc.)
  • Support de sous-namespaces imbriqués (ex: assets.workflow.step.invalidate())
  • Base commune DomainNamespace avec weak references pour éviter les fuites mémoire
  1. Module domain_v2 - Contrats TypedDict
  • Définition stricte des contrats de données via TypedDict
  • Validation runtime avec typeguard
  • Views ergonomiques (dataclasses) pour accès aux données
  • Helpers de transformation (adapters, validators)
  1. Framework de Tests d'Équivalence
  • Harness automatisé pour comparer legacy vs v2
  • Normalizers pour aligner les réponses avant comparaison
  • Comparators personnalisables par type de méthode
  • Rapports détaillés de parité

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@baptiste-olivier baptiste-olivier force-pushed the feature/lab-3976-aa-sdk-user-i-see-typed-objects-and-no-dict-when-using branch from efd4680 to 2507cdc Compare October 8, 2025 08:23
@RuellePaul
Copy link
Contributor

RuellePaul commented Oct 8, 2025

Rename exposed name to client View (example: IssueView  -> Issue) for clarity

Capture d’écran 2025-10-08 à 16 53 14

@RuellePaul
Copy link
Contributor

RuellePaul commented Oct 8, 2025

Ideally, we should have typing corresponding to requested fields (is it feasible) ?
-> I don't think static type checkers can infer which fields exist based on runtime fields arg

>>> issues = kili.issues.list(project_id="cmggpzer4098xs30wb8815kt6", asset_id="cmggpzerm0991s30w7qnvdd25", fields=['id'])
>>> print(issues[0].status)
None

@RuellePaul
Copy link
Contributor

We should distinguish the methods :

  • which can truly create multiples objects by batch : kili.assets.create -> kili.assets.create_many
  • from one that cannot : kili.issues.solve

@RuellePaul
Copy link
Contributor

Exception are still painful in v2 :

issue_ids = kili.issues.create(project_id="cmggpzer4098xs30wb8815kt6",
                               label_id_array=[""],
                               text_array=["test"])
Creating issues:   0%|          | 0/1 [00:00<?, ?it/s]
Not friendly exception raised

Traceback (most recent call last):
File "/Users/paulruelle/kili-python-sdk/src/kili/core/graphql/graphql_client.py", line 265, in execute
return self._execute_with_retries(document, variables, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/.venv/lib/python3.12/site-packages/tenacity/init.py", line 336, in wrapped_f
return copy(f, *args, **kw)
^^^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/.venv/lib/python3.12/site-packages/tenacity/init.py", line 475, in call
do = self.iter(retry_state=retry_state)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/.venv/lib/python3.12/site-packages/tenacity/init.py", line 376, in iter
result = action(retry_state)
^^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/.venv/lib/python3.12/site-packages/tenacity/init.py", line 398, in
self._add_action_func(lambda rs: rs.outcome.result())
^^^^^^^^^^^^^^^^^^^
File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/concurrent/futures/_base.py", line 449, in result
return self.__get_result()
^^^^^^^^^^^^^^^^^^^
File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/concurrent/futures/_base.py", line 401, in __get_result
raise self._exception
File "/Users/paulruelle/kili-python-sdk/.venv/lib/python3.12/site-packages/tenacity/init.py", line 478, in call
result = fn(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/src/kili/core/graphql/graphql_client.py", line 313, in _execute_with_retries
return self._raw_execute(document, variables, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/src/kili/core/graphql/graphql_client.py", line 322, in _raw_execute
res = self._gql_client.execute(
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/.venv/lib/python3.12/site-packages/gql/client.py", line 484, in execute
return self.execute_sync(
^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/.venv/lib/python3.12/site-packages/gql/client.py", line 248, in execute_sync
return session.execute(
^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/.venv/lib/python3.12/site-packages/gql/client.py", line 1028, in execute
raise TransportQueryError(
gql.transport.exceptions.TransportQueryError: {'message': '[badRequest] Bad request parameters. -- This can be due to: Error: Issue #1 is of type ISSUE, and so must contain the related labelID\n at (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/issue/mutations.ts:39:21)\n at Array.forEach ()\n at resolver (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/issue/mutations.ts:37:10)\n at resolver (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/converters/issue.ts:46:26)\n at (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/combineResolvers.ts:21:36)\n at fieldConfig.resolve (/Users/paulruelle/kili/services/label-backend-v2/src/directives/dynamicComplexity.ts:77:26)', 'locations': [{'line': 2, 'column': 3}], 'path': ['data'], 'extensions': {'code': 'OPERATION_RESOLUTION_FAILURE', 'stacktrace': ['Error: Issue #1 is of type ISSUE, and so must contain the related labelID', ' at (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/issue/mutations.ts:39:21)', ' at Array.forEach ()', ' at resolver (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/issue/mutations.ts:37:10)', ' at resolver (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/converters/issue.ts:46:26)', ' at (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/combineResolvers.ts:21:36)', ' at fieldConfig.resolve (/Users/paulruelle/kili/services/label-backend-v2/src/directives/dynamicComplexity.ts:77:26)'], 'context': {'projectID': 'cmggpzer4098xs30wb8815kt6', 'trace': 'Error: Issue #1 is of type ISSUE, and so must contain the related labelID\n at (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/issue/mutations.ts:39:21)\n at Array.forEach ()\n at resolver (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/issue/mutations.ts:37:10)\n at resolver (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/converters/issue.ts:46:26)\n at (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/combineResolvers.ts:21:36)\n at fieldConfig.resolve (/Users/paulruelle/kili/services/label-backend-v2/src/directives/dynamicComplexity.ts:77:26)', 'errorKey': 'badRequest', 'complexity': 2, 'dynamicComplexity': 1, 'kiliClient': {'callTime': '2025-10-08T13:12:03.383845Z', 'callUUID': '63ba39dc-1133-4adb-88a7-50223c08cd94', 'languageName': 'Python', 'languageVersion': '3.12.6', 'methodName': 'is_api_key_valid', 'name': 'python-sdk-domain', 'platformName': 'Darwin', 'platformVersion': 'Darwin Kernel Version 23.6.0: Fri Jul 5 17:55:37 PDT 2024; root:xnu-10063.141.1~2/RELEASE_ARM64_T6030', 'source': 'external', 'version': '25.2.3'}, 'userEmail': '[email protected]', 'userID': 'user-1', 'userOrganizationID': 'first-organization', 'resolverName': 'createIssues', 'tokenType': 'X-API-Key', 'id': 'createIssues-bdbc1d57-d5bd-4c27-975a-c090cca44724', 'startTime': '2025-10-08T13:12:03.967Z', 'endTime': '2025-10-08T13:12:03.977Z', 'duration': 0.01}, 'http': {'status': 400}}}

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "/Users/paulruelle/kili-python-sdk/scripts/add_or_set_metadata.py", line 8, in
issue_ids = kili.issues.create(project_id="cmggpzer4098xs30wb8815kt6",
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/src/kili/domain_api/issues.py", line 316, in create
issue_ids = issue_use_cases.create_issues(project_id=ProjectId(project_id), issues=issues)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/src/kili/use_cases/issue/init.py", line 30, in create_issues
return self._kili_api_gateway.create_issues(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/src/kili/adapters/kili_api_gateway/issue/init.py", line 54, in create_issues
result = self.graphql_client.execute(GQL_CREATE_ISSUES, payload)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/paulruelle/kili-python-sdk/src/kili/core/graphql/graphql_client.py", line 286, in execute
raise kili.exceptions.GraphQLError(error=err.errors, context=context) from err
kili.exceptions.GraphQLError: GraphQL error: "[badRequest] Bad request parameters. -- This can be due to: Error: Issue #1 is of type ISSUE, and so must contain the related labelID
at (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/issue/mutations.ts:39:21)
at Array.forEach ()
at resolver (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/issue/mutations.ts:37:10)
at resolver (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/converters/issue.ts:46:26)
at (/Users/paulruelle/kili/services/label-backend-v2/src/resolvers/combineResolvers.ts:21:36)
at fieldConfig.resolve (/Users/paulruelle/kili/services/label-backend-v2/src/directives/dynamicComplexity.ts:77:26)"

The relevant information is hidden in a lot of logs.

With new client, it should be better and would have huge impact on SDK DX to add a clean exception handler :

KiliException : Issue #1 is of type ISSUE, and so must contain the related labelID

@RuellePaul
Copy link
Contributor

API seems complicated to use with multiple objects (we must deal with list of the same size) :

issue_ids = kili.issues.create(project_id="cmggpzer4098xs30wb8815kt6",
                               label_id_array=["cmghz8ia801dlos0wcspg4ssa", "cmghz8ia801dlos0wcspg4ssa"],
                               text_array=["test", "test 2"])

why not something like

issues = kili.issues.create(project_id="cmggpzer4098xs30wb8815kt6",
                            label_id="cmghz8ia801dlos0wcspg4ssa",
                            comments=["test", "test2"])

or

issues = kili.issues.create(project_id="cmggpzer4098xs30wb8815kt6",
                            issues=[{"label_id": "cmghz8ia801dlos0wcspg4ssa", "text": "test"},
                                    {"label_id": "cmghz8ia801dlos0wcspg4ssa", "text": "test 2"}])

or even

issues = kili.issues.create(project_id="cmggpzer4098xs30wb8815kt6",
                            issues=[IssueInput(label_id="cmghz8ia801dlos0wcspg4ssa", text="test"),
                                    IssueInput(label_id="cmghz8ia801dlos0wcspg4ssa", text="test 2")])

@RuellePaul
Copy link
Contributor

kili.issues.solve returns a list of StatusResponse, with a try catch for each :

            try:
                result = issue_use_cases.update_issue_status(
                    issue_id=IssueId(issue_id), status="SOLVED"
                )
                results.append({"id": issue_id, "status": "SOLVED", "success": True, **result})
            except (ValueError, TypeError, RuntimeError) as e:
                results.append(
                    {"id": issue_id, "status": "SOLVED", "success": False, "error": str(e)}
                )

but when we run into an error, the code stops with an exception

response = kili.issues.solve(issue_ids=['cmghz8ger01ddos0wcu7u5ol3'])
print(response)

We should always return an exception on a fail, it'll be better for DX. For example, user could do :

kili.issues.solve(issue_ids=['cmghz8ger01ddos0wcu7u5ol3'])

and think it worked, but didn't find out that it fails.

The best DX would be something like this :

Creating issues:   70%|▣▣▣▣▣▣▣         | 7/10 [00:00<?, ?it/s]
KiliException : Exception when trying to create issue #7 

@RuellePaul
Copy link
Contributor

The model representation could be prettier :

>>> response = kili.issues.open(issue_ids=['cmghz8ger01ddos0wcu7u5ol4'])
>>> print(response)
[StatusResponse(_data={'id': 'cmghz8ger01ddos0wcu7u5ol4', 'status': 'OPEN', 'success': True})]

Ideally, should be :

[StatusResponse(id='cmghz8ger01ddos0wcu7u5ol4', status='OPEN', success=True)]

@RuellePaul
Copy link
Contributor

kili.assets.external_ids.update feels weird (why not use kili.assets.update) ?

@RuellePaul
Copy link
Contributor

RuellePaul commented Oct 8, 2025

why keeping a large kili.assets.update , whereas issues does not have a kili.issues.update but specialized methods (cancel, open, ...)

-> we should remove generic update methods like kili.assets.update and use specialized ones (kili.assets.update_priority, kili.assets.update_external_id) when it makes sense

e.g kili.labels.honeypots.create vs kili.labels.create_honeypot

@RuellePaul
Copy link
Contributor

🔍 Investigation for Claude Code : Pydantic V2 usage over TypedDict
https://docs.pydantic.dev/latest/concepts/models/

@RuellePaul
Copy link
Contributor

kili.labels.predictions.list is weird alongside kili.labels.list: predictions are labels. We should not keep bad methods for V2

@RuellePaul
Copy link
Contributor

Consider rename kili.projects.users.add -> kili.projects.members.add

@RuellePaul
Copy link
Contributor

I made a first review with a focus on issues domain usage

A lot of comments are about global re-definition of SDK client V2 and should be discussed in tech weekly 👀

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants