From 714621a26d3ea9db3d608399f019ea7db206e001 Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 12:55:28 -0700 Subject: [PATCH 01/19] Improve docstrings for Citrine entry point and exceptions - Citrine class: add usage example, Raises section, explain each property's return type and available operations - exceptions.py: add module-level docstring explaining the exception hierarchy (retryable vs non-retryable) - All exception classes: expand one-line docstrings to explain what each exception means and what users should do about it - NonRetryableHttpException: document all public attributes - JobFailureError: document job_id and failure_reasons attrs Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/citrine.py | 81 +++++++++++++++++++---- src/citrine/exceptions.py | 131 +++++++++++++++++++++++++++++++++----- 2 files changed, 183 insertions(+), 29 deletions(-) diff --git a/src/citrine/citrine.py b/src/citrine/citrine.py index e89d61d57..08cf82173 100644 --- a/src/citrine/citrine.py +++ b/src/citrine/citrine.py @@ -11,18 +11,40 @@ class Citrine: """The entry point for interacting with the Citrine Platform. + Create an instance of this class to access projects, teams, + users, and other platform resources. All API calls are + authenticated using the provided API key. + Parameters ---------- - api_key: str - Unique key that allows a user to access the Citrine Platform. - Default: environ.get('CITRINE_API_KEY') - scheme: str - Networking protocol; usually https. Default: https - host: str - Host URL, generally '.citrine-platform.com'. - Default: environ.get('CITRINE_API_HOST') - port: Optional[str] - Optional networking port. Default: None + api_key : str, optional + API key for authentication. Obtain one from your + platform's account settings page. Falls back to the + ``CITRINE_API_KEY`` environment variable if not provided. + scheme : str, optional + URL scheme. Default: ``'https'``. + host : str, optional + Platform hostname, e.g. ``'mysite.citrine-platform.com'``. + Falls back to the ``CITRINE_API_HOST`` environment + variable if not provided. + port : str, optional + Network port. Default: ``None`` (use scheme default). + + Raises + ------ + ValueError + If ``host`` is not provided and ``CITRINE_API_HOST`` + is not set. + + Examples + -------- + >>> from citrine import Citrine + >>> client = Citrine( + ... api_key='your-api-key', + ... host='mysite.citrine-platform.com' + ... ) + >>> for project in client.projects.list(): + ... print(project.name) """ @@ -51,20 +73,51 @@ def __init__(self, @property def projects(self) -> ProjectCollection: - """Return a resource representing all visible projects.""" + """Access all projects visible to the authenticated user. + + Returns + ------- + ProjectCollection + A collection supporting ``.list()``, ``.get(uid)``, + and ``.register()`` operations on projects. + + """ return ProjectCollection(self.session) @property def users(self) -> UserCollection: - """Return the collection of all users.""" + """Access all users on the platform. + + Returns + ------- + UserCollection + A collection supporting ``.list()`` and ``.get(uid)`` + operations on users. + + """ return UserCollection(self.session) @property def teams(self) -> TeamCollection: - """Returns a resource representing all visible teams.""" + """Access all teams visible to the authenticated user. + + Returns + ------- + TeamCollection + A collection supporting ``.list()``, ``.get(uid)``, + and ``.register()`` operations on teams. + + """ return TeamCollection(self.session) @property def catalyst(self) -> CatalystResource: - """Return a resource representing Catalyst.""" + """Access the Catalyst multi-tenant data platform. + + Returns + ------- + CatalystResource + A resource for interacting with Catalyst services. + + """ return CatalystResource(self.session) diff --git a/src/citrine/exceptions.py b/src/citrine/exceptions.py index a528cce80..519102a1d 100644 --- a/src/citrine/exceptions.py +++ b/src/citrine/exceptions.py @@ -1,4 +1,18 @@ -"""Citrine-specific exceptions.""" +"""Exception hierarchy for the Citrine Python SDK. + +All Citrine-specific exceptions inherit from :class:`CitrineException`. +Exceptions are divided into two categories: + +* **Non-retryable** — the request will fail again if retried without + changes (e.g., bad input, missing resource, auth failure). +* **Retryable** — the request may succeed on a subsequent attempt + (e.g., server temporarily unavailable, workflow still validating). + +HTTP-specific exceptions inherit from +:class:`NonRetryableHttpException` and carry the status code, +response body, and parsed API error details (when available). + +""" from types import SimpleNamespace from typing import Optional, List from urllib.parse import urlencode @@ -8,31 +22,70 @@ class CitrineException(Exception): - """The base exception class for Citrine-Python exceptions.""" + """Base exception for all Citrine SDK errors. + + All exceptions raised by this library inherit from this + class, so ``except CitrineException`` will catch any + Citrine-specific error. + + """ pass class NonRetryableException(CitrineException): - """Indicates that a non-retryable error occurred.""" + """An error that will not succeed if the same request is retried. + + Common causes include invalid input, missing resources, or + insufficient permissions. Fix the underlying issue before + retrying. + + """ pass class RetryableException(CitrineException): - """Indicates an error occurred but it is retryable.""" + """An error that may succeed if the request is retried later. + + The server returned a transient error. Callers can safely + retry the request after a short delay. + + """ pass class UnauthorizedRefreshToken(NonRetryableException): - """The token used to refresh authentication is invalid.""" + """The API key used to refresh authentication is invalid. + + This typically means the API key has expired or been + revoked. Generate a new key from the platform's account + settings page. + + """ pass class NonRetryableHttpException(NonRetryableException): - """An exception originating from an HTTP error from a Citrine API.""" + """An HTTP error response from the Citrine Platform API. + + Attributes + ---------- + url : str + The API path that was requested. + code : int or None + The HTTP status code (e.g. 400, 404). + response_text : str or None + The raw response body. + api_error : ApiError or None + Parsed error details including validation errors, + if the response contained a JSON error body. + detailed_error_info : list[str] + Human-readable error lines joined in ``str(exc)``. + + """ def __init__(self, path: str, response: Optional[Response] = None): self.url = path @@ -82,7 +135,12 @@ def __init__(self, path: str, response: Optional[Response] = None): class NotFound(NonRetryableHttpException): - """A particular url was not found. (http status 404).""" + """The requested resource was not found (HTTP 404). + + Verify that the resource UID is correct and that it + exists in the expected project or dataset. + + """ @staticmethod def build(*, message: str, method: str, path: str, params: dict = {}): @@ -126,41 +184,78 @@ def build(*, message: str, method: str, path: str, params: dict = {}): class Unauthorized(NonRetryableHttpException): - """The user is unauthorized to make this api call. (http status 401).""" + """Authentication or authorization failed (HTTP 401/403). + + Check that your API key is valid and that you have + permission to access the requested resource. + + """ pass class BadRequest(NonRetryableHttpException): - """The user is trying to perform an invalid operation. (http status 400).""" + """The request was invalid (HTTP 400). + + Inspect ``api_error.validation_errors`` for details about + which fields failed validation. + + """ pass class WorkflowConflictException(NonRetryableHttpException): - """There is a conflict preventing the workflow from being executed. (http status 409).""" + """A conflict prevented the operation (HTTP 409). + + Another operation may be in progress on this resource. + Wait and retry, or check for concurrent modifications. + + """ pass -# A 409 is a Conflict, and can be raised anywhere a conflict occurs, not just in a workflow. +#: Alias for :class:`WorkflowConflictException`. A 409 can occur +#: in any context, not just workflows. Conflict = WorkflowConflictException class WorkflowNotReadyException(RetryableException): - """The workflow is not ready to be executed. I.e., still validating. (http status 425).""" + """The workflow is still validating (HTTP 425). + + This is a transient state. Use ``wait_while_validating()`` + to poll until the workflow is ready, or retry after a + short delay. + + """ pass class PollingTimeoutError(NonRetryableException): - """Polling for an asynchronous result has exceeded the timeout.""" + """The client-side polling timeout was exceeded. + + The server-side job may still be running. Increase the + ``timeout`` parameter to wait longer, or check the job + status manually. + + """ pass class JobFailureError(NonRetryableException): - """The asynchronous job completed with the given failure message.""" + """A server-side job completed with a failure status. + + Attributes + ---------- + job_id : UUID + The unique identifier of the failed job. + failure_reasons : list[str] + One reason string per failed task within the job. + + """ def __init__(self, *, message: str, job_id: UUID, failure_reasons: List[str]): super().__init__(message) @@ -169,7 +264,13 @@ def __init__(self, *, message: str, job_id: UUID, failure_reasons: List[str]): class ModuleRegistrationFailedException(NonRetryableException): - """A module failed to register.""" + """A module (predictor, design space, etc.) failed to register. + + The original exception from the API is included in the + message. Check that the module configuration is valid + and that all referenced resources exist. + + """ def __init__(self, moduleType: str, exc: Exception): err = 'The "{0}" failed to register. {1}: {2}'.format( From c87c1ca04d4bad030eb4997553b7a12e8ac8a17b Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 13:03:43 -0700 Subject: [PATCH 02/19] Improve docstrings for scores, objectives, and constraints - scores.py: add module docstring explaining the three score types and when to use each (LI for multi-objective sequential, EI for single-objective sequential, EV for no-baseline) - LIScore: explain "simultaneously exceeding" semantics - EIScore: clarify single-objective limit and magnitude focus - EVScore: explain equal-weight summing and how to influence relative weighting - Objectives: explain what descriptor_key must match - ScalarRangeConstraint: clarify inclusive/exclusive semantics and explain that violated constraints yield a zero score Co-Authored-By: Claude Opus 4.6 (1M context) --- .../constraints/scalar_range_constraint.py | 29 +++-- src/citrine/informatics/objectives.py | 40 +++++-- src/citrine/informatics/scores.py | 105 +++++++++++++----- 3 files changed, 121 insertions(+), 53 deletions(-) diff --git a/src/citrine/informatics/constraints/scalar_range_constraint.py b/src/citrine/informatics/constraints/scalar_range_constraint.py index 48793406b..d279e491f 100644 --- a/src/citrine/informatics/constraints/scalar_range_constraint.py +++ b/src/citrine/informatics/constraints/scalar_range_constraint.py @@ -8,20 +8,27 @@ class ScalarRangeConstraint(Serializable['ScalarRangeConstraint'], Constraint): - """Represents an inequality constraint on a real-valued material attribute. + """Constrain a real-valued property to a numeric range. + + Candidates whose predicted value for the specified + descriptor falls outside the range receive a score of + zero in the design execution. Parameters ---------- - descriptor_key: str - the key corresponding to a descriptor - lower_bound: float - the minimum value in the range - upper_bound: float - the maximum value in the range - lower_inclusive: bool - if True, will include the lower bound value in the range (default: true) - upper_inclusive: bool - if True, will include the max value in the range (default: true) + descriptor_key : str + The key of the descriptor to constrain. Must match + a descriptor key defined in the predictor. + lower_bound : float, optional + Minimum allowed value. If omitted, no lower limit. + upper_bound : float, optional + Maximum allowed value. If omitted, no upper limit. + lower_inclusive : bool, optional + Whether the lower bound is inclusive (``>=``) or + exclusive (``>``). Default: ``True`` (inclusive). + upper_inclusive : bool, optional + Whether the upper bound is inclusive (``<=``) or + exclusive (``<``). Default: ``True`` (inclusive). """ diff --git a/src/citrine/informatics/objectives.py b/src/citrine/informatics/objectives.py index 55231bfe4..a6cdaa6cb 100644 --- a/src/citrine/informatics/objectives.py +++ b/src/citrine/informatics/objectives.py @@ -1,4 +1,10 @@ -"""Tools for working with Objectives.""" +"""Objectives define optimization goals for design executions. + +An Objective specifies what property to optimize and in which +direction. Objectives are passed to :class:`~citrine.informatics.scores.Score` +instances to define the optimization strategy. + +""" from citrine._serialization import properties from citrine._serialization.serializable import Serializable from citrine._serialization.polymorphic_serializable import PolymorphicSerializable @@ -8,10 +14,12 @@ class Objective(PolymorphicSerializable['Objective']): - """ - An Objective describes a goal for a score associated with a single descriptor. + """Base class for optimization objectives. - Abstract type that returns the proper type given a serialized dict. + An Objective ties a scoring direction (maximize or minimize) + to a specific descriptor key. Use :class:`ScalarMaxObjective` + to maximize a property or :class:`ScalarMinObjective` to + minimize it. """ @@ -27,13 +35,17 @@ def get_type(cls, data): class ScalarMaxObjective(Serializable['ScalarMaxObjective'], Objective): - """ - Simple single-response maximization objective with optional bounds. + """Maximize a real-valued material property. + + The design execution will prefer candidates with higher + predicted values for the specified descriptor. Parameters ---------- - descriptor_key: str - the key from which to pull the values + descriptor_key : str + The key of the descriptor to maximize. Must match a + descriptor key defined in the predictor's outputs + (e.g. ``'Tensile Strength'``). """ @@ -48,13 +60,17 @@ def __str__(self): class ScalarMinObjective(Serializable['ScalarMinObjective'], Objective): - """ - Simple single-response minimization objective with optional bounds. + """Minimize a real-valued material property. + + The design execution will prefer candidates with lower + predicted values for the specified descriptor. Parameters ---------- - descriptor_key: str - the key from which to pull the values + descriptor_key : str + The key of the descriptor to minimize. Must match a + descriptor key defined in the predictor's outputs + (e.g. ``'Cost'``). """ diff --git a/src/citrine/informatics/scores.py b/src/citrine/informatics/scores.py index ebb452c53..ecb60913a 100644 --- a/src/citrine/informatics/scores.py +++ b/src/citrine/informatics/scores.py @@ -1,4 +1,20 @@ -"""Tools for working with Scores.""" +"""Scores rank candidate materials during design executions. + +A score combines one or more :class:`~citrine.informatics.objectives.Objective` +instances with optional :class:`~citrine.informatics.constraints.Constraint` +instances. The platform evaluates each candidate against the score and +returns candidates ranked from best to worst. + +Three score types are available: + +* :class:`LIScore` (Likelihood of Improvement) — multi-objective + sequential optimization. Requires baseline values. +* :class:`EIScore` (Expected Improvement) — single-objective + sequential optimization. Requires baseline values. +* :class:`EVScore` (Expected Value) — multi-objective sum of + predicted values. No baselines needed. + +""" from typing import List, Optional from citrine._serialization import properties @@ -11,9 +27,12 @@ class Score(PolymorphicSerializable['Score']): - """A Score is used to rank materials according to objectives and constraints. + """Base class for scoring strategies used in design executions. - Abstract type that returns the proper type given a serialized dict. + A Score ranks candidate materials by combining objectives + (what to optimize) with constraints (what limits to respect). + Use one of the concrete subclasses: :class:`LIScore`, + :class:`EIScore`, or :class:`EVScore`. """ @@ -31,18 +50,27 @@ def get_type(cls, data): class LIScore(Serializable['LIScore'], Score): - """Evaluates the likelihood of scoring better than some baselines for given objectives. + """Likelihood of Improvement — multi-objective sequential optimization. + + Ranks candidates by the probability that they simultaneously + improve on *every* baseline. Best for iterative experimental + campaigns where you want to beat your current best results + across all objectives at once. Parameters ---------- - objectives: list[Objective] - objectives (e.g., maximize, minimize, tune, etc.) - If multiple objectives are specified then this score evaluates the likelihood of - simultaneously exceeding all objectives. - baselines: list[float] - best-so-far values for the various objectives (there must be one for each objective) - constraints: list[Constraint] - constraints limiting the allowed values that material instances can have + objectives : list[Objective] + One or more objectives (e.g. ScalarMaxObjective, + ScalarMinObjective). Multiple objectives are treated + as a joint requirement: the score is the probability + of exceeding *all* baselines simultaneously. + baselines : list[float] + Current best-known value for each objective, in the + same order as ``objectives``. One baseline per + objective is required. + constraints : list[Constraint], optional + Constraints that candidates must satisfy. Candidates + violating any constraint receive a score of zero. """ @@ -66,18 +94,27 @@ def __str__(self): class EIScore(Serializable['EIScore'], Score): - """ - Evaluates the expected magnitude of improvement beyond baselines for a given objective. + """Expected Improvement — single-objective sequential optimization. + + Ranks candidates by the expected magnitude of improvement + beyond the baseline. Unlike LIScore, this considers *how + much* better a candidate is, not just the probability of + being better. Best for single-objective campaigns where + you want the largest possible gains. Parameters ---------- - objectives: list[Objective] - objectives (e.g., maximize, minimize, tune, etc.) - EIScore does not support more than 1 objective at this time. - baselines: list[float] - best-so-far values for the various objectives (there must be one for each objective) - constraints: list[Constraint] - constraints limiting the allowed values that material instances can have + objectives : list[Objective] + Exactly one objective. EIScore does not support + multiple objectives; use LIScore for multi-objective + optimization. + baselines : list[float] + Current best-known value for the objective. Must + contain exactly one value matching the single + objective. + constraints : list[Constraint], optional + Constraints that candidates must satisfy. Candidates + violating any constraint receive a score of zero. """ @@ -101,18 +138,26 @@ def __str__(self): class EVScore(Serializable['EVScore'], Score): - """ - Evaluates the expected value for given objectives. + """Expected Value — multi-objective optimization without baselines. + + Ranks candidates by the sum of predicted values across all + objectives. Unlike LIScore and EIScore, no baselines are + needed. Useful as a starting point when you have no + existing experimental results. + + When multiple objectives are specified, their individual + scores are summed with equal weight. The relative + weighting cannot be controlled directly; instead, adjust + the descriptor scales or units to influence the balance. Parameters ---------- - objectives: list[Objective] - objectives (e.g., maximize, minimize, tune, etc.) - If multiple objectives are specified, their scores are summed together. This allows - for simultaneous optimization of multiple objectives, although the weighting of the - various objectives cannot be directly specified. - constraints: list[Constraint] - constraints limiting the allowed values that material instances can have + objectives : list[Objective] + One or more objectives. Scores are summed across all + objectives. + constraints : list[Constraint], optional + Constraints that candidates must satisfy. Candidates + violating any constraint receive a score of zero. """ From 531b5ed7b5318b4a87bb7f6cbbcdc4e8ff5c1e10 Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 13:08:58 -0700 Subject: [PATCH 03/19] Improve docstrings for design execution and candidate classes - DesignExecution: explain the execution lifecycle and link to Score, candidates(), and hierarchical_candidates() - candidates(): document return ordering (by score), per_page, and return type with full Parameters/Returns sections - hierarchical_candidates(): explain the difference from candidates() (includes process history and sub-materials) - predict(): document PredictRequest input and DesignCandidate return with full Parameters/Returns sections - PredictRequest: add full Parameters section documenting all 5 constructor arguments - DesignCandidate: explain primary_score semantics - DesignVariable: list all subtypes and when each is used - DesignCandidateComment: add missing class docstring Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/informatics/design_candidate.py | 33 +++++++-- .../executions/design_execution.py | 67 +++++++++++++++++-- src/citrine/informatics/predict_request.py | 24 ++++++- 3 files changed, 109 insertions(+), 15 deletions(-) diff --git a/src/citrine/informatics/design_candidate.py b/src/citrine/informatics/design_candidate.py index 94fbcd275..10169d93d 100644 --- a/src/citrine/informatics/design_candidate.py +++ b/src/citrine/informatics/design_candidate.py @@ -21,6 +21,8 @@ class DesignCandidateComment(Serializable["DesignCandidateComment"]): + """A user comment attached to a design candidate.""" + message = properties.String('message') """:str: the text of the comment""" created_by = properties.UUID('created.user') @@ -30,10 +32,18 @@ class DesignCandidateComment(Serializable["DesignCandidateComment"]): class DesignVariable(PolymorphicSerializable["DesignVariable"]): - """Classes containing data corresponding to individual descriptors. + """A predicted or specified value for a single material descriptor. + + Materials are represented as (descriptor, value) pairs. Each + DesignVariable holds one value. The concrete subtype depends + on the descriptor type: + + * :class:`MeanAndStd` — continuous (real-valued) descriptors + * :class:`TopCategories` — categorical descriptors + * :class:`Mixture` — formulation descriptors + * :class:`ChemicalFormula` — chemical formula descriptors + * :class:`MolecularStructure` — SMILES-based descriptors - If you think of materials as being represented as a set of (descriptor, value) pairs, - these are simplified representations of the values. """ def __init__(self, arg): @@ -170,9 +180,14 @@ class HierarchicalDesignMaterial(Serializable["HierarchicalDesignMaterial"]): class DesignCandidate(Serializable["DesignCandidate"]): - """A candidate material generated by the Citrine Platform. + """A candidate material generated by a design execution. + + Each candidate contains predicted property values and a + ``primary_score`` indicating how well it satisfies the + objectives and constraints (higher is better). Retrieve + candidates via + :meth:`~citrine.informatics.executions.design_execution.DesignExecution.candidates`. - This class represents the candidate computed by a design execution. """ uid = properties.UUID('id') @@ -199,9 +214,13 @@ class DesignCandidate(Serializable["DesignCandidate"]): class HierarchicalDesignCandidate(Serializable["HierarchicalDesignCandidate"]): - """A hierarchical candidate material generated by the Citrine Platform. + """A design candidate with full material history. + + Like :class:`DesignCandidate`, but the material includes + the hierarchical structure (process history, ingredients, + sub-materials). Retrieve via + :meth:`~citrine.informatics.executions.design_execution.DesignExecution.hierarchical_candidates`. - This class represents the candidate computed by a design execution. """ uid = properties.UUID('id') diff --git a/src/citrine/informatics/executions/design_execution.py b/src/citrine/informatics/executions/design_execution.py index 0511cd06f..9875acf0e 100644 --- a/src/citrine/informatics/executions/design_execution.py +++ b/src/citrine/informatics/executions/design_execution.py @@ -13,10 +13,16 @@ class DesignExecution(Resource["DesignExecution"], Execution): - """The execution of a DesignWorkflow. + """A single run of a design workflow that generates candidates. - Possible statuses are INPROGRESS, SUCCEEDED, and FAILED. - Design executions also have a ``status_description`` field with more information. + A DesignExecution is created by triggering a + :class:`~citrine.resources.design_workflow.DesignWorkflow` + with a :class:`~citrine.informatics.scores.Score`. Once + the execution status reaches ``SUCCEEDED``, use + :meth:`candidates` to retrieve the generated candidates + ranked by score. + + Possible statuses: ``INPROGRESS``, ``SUCCEEDED``, ``FAILED``. """ @@ -47,7 +53,23 @@ def _build_candidates(cls, subset_collection: Iterable[dict]) -> Iterable[Design yield DesignCandidate.build(candidate) def candidates(self, *, per_page: int = 100) -> Iterable[DesignCandidate]: - """Fetch the Design Candidates for the particular execution, paginated.""" + """Fetch the design candidates generated by this execution. + + Returns an iterator of candidates ranked by score + (highest first). Each candidate contains predicted + material property values and a ``primary_score``. + + Parameters + ---------- + per_page : int, optional + Number of candidates per page. Default: 100. + + Returns + ------- + Iterable[DesignCandidate] + Paginated iterator over design candidates. + + """ path = self._path() + '/candidates' fetcher = partial(self._fetch_page, path=path, fetch_func=self._session.get_resource) @@ -63,7 +85,24 @@ def _build_hierarchical_candidates( yield HierarchicalDesignCandidate.build(candidate) def hierarchical_candidates(self, *, per_page: int = 100) -> Iterable[DesignCandidate]: - """Fetch the Design Candidates for the particular execution, paginated.""" + """Fetch candidates with full material history. + + Like :meth:`candidates`, but each candidate includes + the hierarchical material structure (process history, + ingredients, sub-materials) rather than a flat + representation. + + Parameters + ---------- + per_page : int, optional + Number of candidates per page. Default: 100. + + Returns + ------- + Iterable[HierarchicalDesignCandidate] + Paginated iterator over hierarchical candidates. + + """ path = self._path() + '/candidate-histories' fetcher = partial(self._fetch_page, path=path, fetch_func=self._session.get_resource) @@ -74,7 +113,23 @@ def hierarchical_candidates(self, *, per_page: int = 100) -> Iterable[DesignCand def predict(self, predict_request: PredictRequest) -> DesignCandidate: - """Invoke a prediction on a design candidate.""" + """Run a prediction for a specific material configuration. + + Uses the predictor from this execution's workflow to + generate predicted property values for the material + described in the request. + + Parameters + ---------- + predict_request : PredictRequest + The material configuration to predict on. + + Returns + ------- + DesignCandidate + A candidate with predicted property values. + + """ path = self._path() + '/predict' res = self._session.post_resource(path, predict_request.dump(), version=self._api_version) diff --git a/src/citrine/informatics/predict_request.py b/src/citrine/informatics/predict_request.py index b2c2adab0..0c5f93762 100644 --- a/src/citrine/informatics/predict_request.py +++ b/src/citrine/informatics/predict_request.py @@ -7,9 +7,29 @@ class PredictRequest(Serializable["PredictRequest"]): - """A Citrine Predict Request. + """A request to predict properties for a specific material. + + Typically constructed from an existing design candidate to + re-predict with modified inputs, or to run a what-if + analysis on a specific material configuration. + + Parameters + ---------- + material_id : UUID + Unique identifier of the material to predict on. + identifiers : list[str] + List of identifier strings for this material (e.g. + sample IDs, lot numbers). + material : DesignMaterial + The material definition containing descriptor values + to use as predictor inputs. + created_from_id : UUID + The UID of the design candidate this request was + derived from. + random_seed : int, optional + Seed for reproducible stochastic predictions. If + omitted, the platform chooses a random seed. - This class represents the candidate computed by a design execution. """ material_id = properties.UUID('material_id') From 529dd9fb65fb22ff43d7e12e9733aaa3616bc245 Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 13:13:02 -0700 Subject: [PATCH 04/19] Improve docstrings for dimensions and descriptors - dimensions.py: add module docstring explaining role in design spaces; list all Dimension subtypes in base class - ContinuousDimension/IntegerDimension: explain default bound behavior (falls back to descriptor bounds) - EnumeratedDimension: explain use cases and value parsing - descriptors.py: add module docstring explaining role in the platform; list all 6 Descriptor subtypes in base class Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/informatics/descriptors.py | 23 ++++++-- src/citrine/informatics/dimensions.py | 73 ++++++++++++++++++-------- 2 files changed, 71 insertions(+), 25 deletions(-) diff --git a/src/citrine/informatics/descriptors.py b/src/citrine/informatics/descriptors.py index 692dfe91e..b62730f00 100644 --- a/src/citrine/informatics/descriptors.py +++ b/src/citrine/informatics/descriptors.py @@ -1,4 +1,12 @@ -"""Tools for working with Descriptors.""" +"""Descriptors define the type and range of material properties. + +A Descriptor specifies what kind of values a material property +can hold (real numbers, integers, categories, chemical formulas, +molecular structures, or formulations). Descriptors are used +throughout the platform to define predictor inputs/outputs, +design space dimensions, and constraints. + +""" from typing import Type, Set, Union from gemd.enumeration.base_enumeration import BaseEnumeration @@ -30,9 +38,18 @@ class FormulationKey(BaseEnumeration): class Descriptor(PolymorphicSerializable['Descriptor']): - """A Descriptor describes the range of values that a quantity can take on. + """Base class for all descriptor types. + + Descriptors define the type and valid range of a material + property. Use one of the concrete subclasses: + + * :class:`RealDescriptor` — continuous real numbers + * :class:`IntegerDescriptor` — integer numbers + * :class:`CategoricalDescriptor` — discrete categories + * :class:`ChemicalFormulaDescriptor` — chemical formulas + * :class:`MolecularStructureDescriptor` — SMILES strings + * :class:`FormulationDescriptor` — mixture compositions - Abstract type that returns the proper type given a serialized dict. """ key = properties.String('descriptor_key') diff --git a/src/citrine/informatics/dimensions.py b/src/citrine/informatics/dimensions.py index db73e50aa..822ea3a46 100644 --- a/src/citrine/informatics/dimensions.py +++ b/src/citrine/informatics/dimensions.py @@ -1,4 +1,12 @@ -"""Tools for working with Dimensions.""" +"""Dimensions define the search space for design executions. + +Each dimension specifies the range of values that a single +descriptor can take during candidate generation. Dimensions +are assembled into a +:class:`~citrine.informatics.design_spaces.DataSourceDesignSpace` +to define the full search space. + +""" from typing import Optional, Type, List from citrine._serialization import properties @@ -10,9 +18,14 @@ class Dimension(PolymorphicSerializable['Dimension']): - """A Dimension describes the values that a quantity can take in the context of a design space. + """Base class for design space dimensions. + + A Dimension restricts one descriptor to a specific range + of values. Use the concrete subclasses: - Abstract type that returns the proper type given a serialized dict. + * :class:`ContinuousDimension` — real-valued range + * :class:`IntegerDimension` — integer range + * :class:`EnumeratedDimension` — finite set of values """ @@ -27,16 +40,21 @@ def get_type(cls, data) -> Type[Serializable]: class ContinuousDimension(Serializable['ContinuousDimension'], Dimension): - """A continuous, real-valued dimension. + """A continuous, real-valued dimension with inclusive bounds. + + If bounds are not specified, they default to the + descriptor's own lower and upper bounds. Parameters ---------- - descriptor: RealDescriptor - a descriptor of the single dimension - lower_bound: float - inclusive lower bound - upper_bound: float - inclusive upper bound + descriptor : RealDescriptor + The descriptor this dimension applies to. + lower_bound : float, optional + Inclusive lower bound for candidate values. Defaults + to the descriptor's ``lower_bound``. + upper_bound : float, optional + Inclusive upper bound for candidate values. Defaults + to the descriptor's ``upper_bound``. """ @@ -55,16 +73,21 @@ def __init__(self, class IntegerDimension(Serializable['IntegerDimension'], Dimension): - """An integer-valued dimension with inclusive lower and upper bounds. + """An integer-valued dimension with inclusive bounds. + + If bounds are not specified, they default to the + descriptor's own lower and upper bounds. Parameters ---------- - descriptor: IntegerDescriptor - a descriptor of the single dimension - lower_bound: int - inclusive lower bound - upper_bound: int - inclusive upper bound + descriptor : IntegerDescriptor + The descriptor this dimension applies to. + lower_bound : int, optional + Inclusive lower bound. Defaults to the descriptor's + ``lower_bound``. + upper_bound : int, optional + Inclusive upper bound. Defaults to the descriptor's + ``upper_bound``. """ @@ -83,14 +106,20 @@ def __init__(self, class EnumeratedDimension(Serializable['EnumeratedDimension'], Dimension): - """A finite, enumerated dimension. + """A dimension defined by a finite set of allowed values. + + Use this for categorical descriptors or when you want to + restrict a continuous descriptor to specific discrete + values (e.g. only certain temperatures). Parameters ---------- - descriptor: Descriptor - a descriptor of the single dimension - values: list[str] - list of values that can be parsed by the descriptor + descriptor : Descriptor + The descriptor this dimension applies to. + values : list[str] + Allowed values as strings. Each string must be + parseable by the descriptor (e.g. valid categories + for a CategoricalDescriptor). """ From 01dbe3edcc56462fce7e554db6f225579d43af13 Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 13:24:04 -0700 Subject: [PATCH 05/19] Improve docstrings for data source classes - Module docstring: explain what data sources are and that GemTableDataSource is the most common type - DataSource base: list all 4 subtypes with brief descriptions - GemTableDataSource: explain what GEM Tables are and how to find table IDs and versions Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/informatics/data_sources.py | 36 ++++++++++++++++++------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/citrine/informatics/data_sources.py b/src/citrine/informatics/data_sources.py index 77bed62d6..d4a126cd7 100644 --- a/src/citrine/informatics/data_sources.py +++ b/src/citrine/informatics/data_sources.py @@ -1,4 +1,10 @@ -"""Tools for working with Descriptors.""" +"""Data sources provide training data for predictors and design spaces. + +A DataSource specifies where the AI engine should read its data. +The most common type is :class:`GemTableDataSource`, which +references a GEM Table built from the platform's data model. + +""" from abc import abstractmethod from typing import Type, List, Mapping, Optional, Union from uuid import UUID @@ -21,10 +27,15 @@ class DataSource(PolymorphicSerializable['DataSource']): - """A source of data for the AI engine. + """Base class for data source specifications. - Data source provides a polymorphic interface for specifying different kinds of data as the - training data for predictors and the input data for some design spaces. + Use one of the concrete subclasses: + + * :class:`GemTableDataSource` — a GEM Table (most common) + * :class:`ExperimentDataSourceRef` — reference to experiment + data + * :class:`SnapshotDataSource` — a snapshot of data + * :class:`CSVDataSource` — a CSV file (deprecated) """ @@ -126,15 +137,20 @@ def to_data_source_id(self) -> str: class GemTableDataSource(Serializable['GemTableDataSource'], DataSource): - """A data source based on a GEM Table hosted on the data platform. + """A data source backed by a GEM Table on the platform. + + This is the most common data source type. GEM Tables are + built from the platform's GEMD data model via table + configurations. Use ``list_tables()`` on a project to find + available table IDs and versions. Parameters ---------- - table_id: UUID - Unique identifier for the GEM Table - table_version: Union[str,int] - Version number for the GEM Table. The first GEM table built from a configuration - has version = 1. Strings are cast to ints. + table_id : UUID + Unique identifier of the GEM Table. + table_version : int or str + Version number. The first table built from a + configuration has version 1. Strings are cast to int. """ From 09e5c9fd1dddd9d6c15f9ca53064f5fb17618f20 Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 13:26:48 -0700 Subject: [PATCH 06/19] Improve docstrings for Collection base class - Collection class: explain available CRUD operations and that concrete subclasses may add more - get(): add Parameters, Returns, and Raises sections - register(): add Parameters, Returns, and Raises sections - update(): add Parameters and Returns sections - delete(): add Parameters and Returns sections Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/_rest/collection.py | 87 +++++++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 5 deletions(-) diff --git a/src/citrine/_rest/collection.py b/src/citrine/_rest/collection.py index 6f83805ed..3da8075ed 100644 --- a/src/citrine/_rest/collection.py +++ b/src/citrine/_rest/collection.py @@ -18,7 +18,21 @@ class Collection(Generic[ResourceType], Pageable): - """Abstract class for representing collections of REST resources.""" + """Base class for server-backed resource collections. + + A Collection provides CRUD operations for a specific + resource type. All collections support at minimum: + + * ``get(uid)`` — fetch one resource by UID + * ``list()`` — paginate over all resources + * ``register(model)`` — create a new resource + * ``update(model)`` — update an existing resource + * ``delete(uid)`` — delete a resource + + Concrete collections (e.g. ``ProjectCollection``, + ``DatasetCollection``) may add additional operations. + + """ _path_template: str = NotImplemented _dataset_agnostic_path_template: str = NotImplemented @@ -50,7 +64,27 @@ def build(self, data: dict): """Build an individual element of the collection.""" def get(self, uid: Union[UUID, str]) -> ResourceType: - """Get a particular element of the collection.""" + """Fetch a single resource by its unique identifier. + + Parameters + ---------- + uid : UUID or str + The unique identifier of the resource. + + Returns + ------- + ResourceType + The resource object. + + Raises + ------ + ValueError + If ``uid`` is None (the object may not be + registered yet). + NotFound + If no resource with the given UID exists. + + """ if uid is None: raise ValueError("Cannot get when uid=None. Are you using a registered resource?") path = self._get_path(uid) @@ -59,7 +93,25 @@ def get(self, uid: Union[UUID, str]) -> ResourceType: return self.build(data) def register(self, model: CreationType) -> CreationType: - """Create a new element of the collection by registering an existing resource.""" + """Create a new resource on the platform. + + Parameters + ---------- + model : ResourceType + The resource to register. After registration, the + returned object will have a platform-assigned UID. + + Returns + ------- + ResourceType + The registered resource with server-assigned fields. + + Raises + ------ + ModuleRegistrationFailedException + If the platform rejects the resource. + + """ path = self._get_path() try: data = self.session.post_resource(path, model.dump(), version=self._api_version) @@ -94,14 +146,39 @@ def list(self, *, per_page: int = 100) -> Iterator[ResourceType]: per_page=per_page) def update(self, model: CreationType) -> CreationType: - """Update a particular element of the collection.""" + """Update an existing resource on the platform. + + Parameters + ---------- + model : ResourceType + The resource with updated fields. Must have a + valid ``uid`` (i.e., be previously registered). + + Returns + ------- + ResourceType + The updated resource as returned by the server. + + """ url = self._get_path(model.uid) updated = self.session.put_resource(url, model.dump(), version=self._api_version) data = updated[self._individual_key] if self._individual_key else updated return self.build(data) def delete(self, uid: Union[UUID, str]) -> Response: - """Delete a particular element of the collection.""" + """Delete a resource by its unique identifier. + + Parameters + ---------- + uid : UUID or str + The unique identifier of the resource to delete. + + Returns + ------- + Response + The server response. + + """ url = self._get_path(uid) data = self.session.delete_resource(url, version=self._api_version) return Response(body=data) From 49be1cc40ca723f02e4ed20f1f21194980bbffce Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 13:15:35 -0700 Subject: [PATCH 07/19] Improve docstrings for feature effects and Shapley classes All four Shapley classes had minimal (1-line) docstrings with no explanation of what Shapley values are or how to interpret them. Now: - Module docstring: explain Shapley values, positive/negative semantics, and the class hierarchy - ShapleyMaterial: document material_id and value attributes - ShapleyFeature: document feature name and materials list - ShapleyOutput: document output name and features list - FeatureEffects: document all attributes including status, failure_reason, and link to as_dict convenience property Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/informatics/feature_effects.py | 75 ++++++++++++++++++++-- 1 file changed, 71 insertions(+), 4 deletions(-) diff --git a/src/citrine/informatics/feature_effects.py b/src/citrine/informatics/feature_effects.py index d0cd04d6e..cb9f89961 100644 --- a/src/citrine/informatics/feature_effects.py +++ b/src/citrine/informatics/feature_effects.py @@ -1,3 +1,22 @@ +"""Feature importance via Shapley values for trained predictors. + +Shapley values quantify how much each input feature +contributes to a predictor's output for each material in the +training set. Positive values indicate the feature pushes the +prediction higher; negative values push it lower. The +magnitude reflects the strength of the effect. + +Access feature effects via +:attr:`~citrine.informatics.predictors.graph_predictor.GraphPredictor.feature_effects`. + +The class hierarchy is: + +* :class:`FeatureEffects` — top level, one per predictor +* :class:`ShapleyOutput` — one per predicted output +* :class:`ShapleyFeature` — one per input feature +* :class:`ShapleyMaterial` — one per training material + +""" from typing import Dict from uuid import UUID @@ -6,14 +25,33 @@ class ShapleyMaterial(Resource): - """The feature effect of a material.""" + """Shapley value for one material and one feature. + + Attributes + ---------- + material_id : UUID + Identifier of the training material. + value : float + Shapley value. Positive means the feature pushes + the prediction higher; negative means lower. + + """ material_id = properties.UUID('material_id', serializable=False) value = properties.Float('value', serializable=False) class ShapleyFeature(Resource): - """All feature effects for this feature by material.""" + """Shapley values for one input feature across all materials. + + Attributes + ---------- + feature : str + Name of the input feature. + materials : list[ShapleyMaterial] + Shapley values for each training material. + + """ feature = properties.String('feature', serializable=False) materials = properties.List(properties.Object(ShapleyMaterial), 'materials', @@ -26,7 +64,16 @@ def material_dict(self) -> Dict[UUID, float]: class ShapleyOutput(Resource): - """All feature effects for this output by feature.""" + """Shapley values for one predicted output, grouped by feature. + + Attributes + ---------- + output : str + Name of the predicted output. + features : list[ShapleyFeature] + Shapley values broken down by input feature. + + """ output = properties.String('output', serializable=False) features = properties.List(properties.Object(ShapleyFeature), 'features', serializable=False) @@ -38,7 +85,27 @@ def feature_dict(self) -> Dict[str, Dict[UUID, float]]: class FeatureEffects(Resource): - """Captures information about the feature effects associated with a predictor.""" + """Feature importance results for a trained predictor. + + Contains Shapley values showing how each input feature + affects each predicted output for every material in the + training set. Use :attr:`as_dict` for a convenient nested + dictionary representation. + + Attributes + ---------- + predictor_id : UUID + The predictor these results belong to. + predictor_version : int + The predictor version that was analyzed. + status : str + Computation status (e.g. ``'Succeeded'``). + failure_reason : str or None + Reason for failure, if status is not succeeded. + outputs : list[ShapleyOutput] or None + The computed Shapley values, grouped by output. + + """ predictor_id = properties.UUID('metadata.predictor_id', serializable=False) predictor_version = properties.Integer('metadata.predictor_version', serializable=False) From 64deb90437ba4061d2f6e403919960af83838f2c Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 13:30:58 -0700 Subject: [PATCH 08/19] Improve docstrings for jobs and waiting utilities - JobSubmissionResponse: explain what it is and how to use job_id for status polling - waiting.py: add module docstring explaining polling utilities - ConditionTimeoutError: explain that the server continues running independently and suggest increasing timeout Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/jobs/job.py | 12 ++++++++++-- src/citrine/jobs/waiting.py | 19 ++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/citrine/jobs/job.py b/src/citrine/jobs/job.py index 397a99971..bb6f71012 100644 --- a/src/citrine/jobs/job.py +++ b/src/citrine/jobs/job.py @@ -16,9 +16,17 @@ class JobSubmissionResponse(Resource['JobSubmissionResponse']): - """A response to a submit-job request for the job submission framework. + """Response from submitting an asynchronous job to the platform. + + Returned by operations that trigger server-side work (e.g., + ingestion, table building). Use the ``job_id`` to poll for + completion status. + + Attributes + ---------- + job_id : UUID + Unique identifier for tracking the submitted job. - This is returned as a successful response from the remote service. """ job_id = properties.UUID("job_id") diff --git a/src/citrine/jobs/waiting.py b/src/citrine/jobs/waiting.py index 62207947a..328d11f8d 100644 --- a/src/citrine/jobs/waiting.py +++ b/src/citrine/jobs/waiting.py @@ -1,3 +1,10 @@ +"""Utilities for waiting on asynchronous platform operations. + +These functions poll the platform at a configurable interval +until an asynchronous operation (predictor training, workflow +validation, design execution) completes or a timeout is reached. + +""" import time from pprint import pprint from typing import Union @@ -11,7 +18,17 @@ class ConditionTimeoutError(RuntimeError): - """Error that is raised when timeout is reached but the checked condition is still False.""" + """The client-side polling timeout was exceeded. + + Raised when an asynchronous operation (e.g. predictor + training, design execution) has not completed within the + specified ``timeout`` seconds. The server-side operation + continues running independently of this client timeout. + + Increase the ``timeout`` parameter to wait longer, or + poll the resource status manually. + + """ pass From 30750bbe8ba56feb3855d7d76ba9c68fcd78dd7e Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 11:30:55 -0700 Subject: [PATCH 09/19] Fix error message bugs: copy-paste, bare Exception, lambda, typo - Fix copy-paste bug in migrate_deprecated_argument where both sides of the error message showed new_arg_name instead of old_arg_name - Replace bare Exception with TypeError in validate_type for clearer error semantics - Fix NotFound.build() lambda signature (lambda self: -> lambda:) that silently prevented api_error from being populated - Fix "Cant" typo in WorkflowNotReadyException message Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/_session.py | 2 +- src/citrine/_utils/functions.py | 8 +++++--- src/citrine/exceptions.py | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/citrine/_session.py b/src/citrine/_session.py index c403a574b..2ef5d107c 100644 --- a/src/citrine/_session.py +++ b/src/citrine/_session.py @@ -205,7 +205,7 @@ def checked_request(self, method: str, path: str, raise Conflict(path, response) elif response.status_code == 425: logger.debug('%s %s %s', response.status_code, method, path) - msg = 'Cant execute at this time. Try again later. Error: {}'.format(response.text) + msg = 'Cannot execute at this time. Try again later. Error: {}'.format(response.text) raise WorkflowNotReadyException(msg) else: logger.error('%s %s %s', response.status_code, method, path) diff --git a/src/citrine/_utils/functions.py b/src/citrine/_utils/functions.py index a2c6c3a2a..7cde5aa1f 100644 --- a/src/citrine/_utils/functions.py +++ b/src/citrine/_utils/functions.py @@ -32,8 +32,10 @@ def validate_type(data_dict: dict, type_name: str) -> dict: data_dict_copy = data_dict.copy() if 'type' in data_dict_copy: if data_dict_copy['type'] != type_name: - raise Exception( - "Object type must be {}, but was instead {}.".format(type_name, data_dict['type'])) + raise TypeError( + "Object type must be '{}', but was '{}'. " + "Verify you are passing the correct object type." + .format(type_name, data_dict['type'])) else: data_dict_copy['type'] = type_name @@ -258,7 +260,7 @@ def migrate_deprecated_argument( if new_arg is None: return old_arg else: - raise ValueError(f"Cannot specify both \'{new_arg_name}\' and \'{new_arg_name}\'") + raise ValueError(f"Cannot specify both \'{new_arg_name}\' and \'{old_arg_name}\'") elif new_arg is None: raise ValueError(f"Please specify \'{new_arg_name}\'") return new_arg diff --git a/src/citrine/exceptions.py b/src/citrine/exceptions.py index 519102a1d..9502fd62d 100644 --- a/src/citrine/exceptions.py +++ b/src/citrine/exceptions.py @@ -178,7 +178,7 @@ def build(*, message: str, method: str, path: str, params: dict = {}): status_code=404, request=SimpleNamespace(method=method.upper()), reason="Not Found", - json=lambda self: {"code": 404, "message": message, "validation_errors": []} + json=lambda: {"code": 404, "message": message, "validation_errors": []} ) ) From e45013232e8d84e98fdf485d416500c2982d795c Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 12:00:48 -0700 Subject: [PATCH 10/19] Add request_id and method to NonRetryableHttpException Extract x-request-id or x-correlation-id from response headers and store as request_id attribute. Also store the HTTP method. Both are included in the error message for easier debugging and support escalation. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/exceptions.py | 16 +++++++++++++--- tests/test_session.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/citrine/exceptions.py b/src/citrine/exceptions.py index 9502fd62d..f925c86f4 100644 --- a/src/citrine/exceptions.py +++ b/src/citrine/exceptions.py @@ -90,19 +90,29 @@ class NonRetryableHttpException(NonRetryableException): def __init__(self, path: str, response: Optional[Response] = None): self.url = path self.detailed_error_info = [] + self.request_id = None + self.method = "unknown" if response is not None: self.response_text = response.text self.code = response.status_code - method = "unknown" if response.request is not None: - method = response.request.method + self.method = response.request.method + + headers = getattr(response, 'headers', {}) or {} + self.request_id = ( + headers.get('x-request-id') + or headers.get('x-correlation-id') + ) self.detailed_error_info.append( "{} (code: {}) returned from {} request to path: '{}'".format( - response.reason, self.code, method, path + response.reason, self.code, self.method, path ) ) + if self.request_id: + self.detailed_error_info.append( + "Request ID: {}".format(self.request_id)) try: resp_json = response.json() if isinstance(resp_json, dict): diff --git a/tests/test_session.py b/tests/test_session.py index f1e105bb3..147ecab09 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -5,6 +5,7 @@ BadRequest, Conflict, NonRetryableException, + NotFound, WorkflowNotReadyException, RetryableException) @@ -113,6 +114,41 @@ def test_get_not_found(session: Session): session.get_resource('/foo') +def test_request_id_captured_in_http_exception(session: Session): + with requests_mock.Mocker() as m: + m.get( + 'http://citrine-testing.fake/api/v1/foo', + status_code=404, + headers={'x-request-id': 'test-req-123'} + ) + with pytest.raises(NotFound) as exc_info: + session.get_resource('/foo') + assert exc_info.value.request_id == 'test-req-123' + assert 'test-req-123' in str(exc_info.value) + + +def test_method_stored_on_http_exception(session: Session): + with requests_mock.Mocker() as m: + m.get( + 'http://citrine-testing.fake/api/v1/foo', + status_code=404 + ) + with pytest.raises(NotFound) as exc_info: + session.get_resource('/foo') + assert exc_info.value.method == 'GET' + + +def test_request_id_none_when_header_absent(session: Session): + with requests_mock.Mocker() as m: + m.get( + 'http://citrine-testing.fake/api/v1/foo', + status_code=404 + ) + with pytest.raises(NotFound) as exc_info: + session.get_resource('/foo') + assert exc_info.value.request_id is None + + def test_status_code_409(session: Session): with requests_mock.Mocker() as m: url = '/foo' From 867678d06704f5310efa11180048118daa187fc2 Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 12:02:07 -0700 Subject: [PATCH 11/19] Fix silent error swallowing in session error handling - Replace silent ValueError pass with logger.debug for 401 JSON parsing (was completely invisible to users/developers) - Downgrade AttributeError from logger.error to logger.debug (this is an expected case, not a true error) - Upgrade JSONDecodeError in _extract_response_json from logger.info to logger.warning (returning empty dict silently can cause confusing downstream failures) Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/_session.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/citrine/_session.py b/src/citrine/_session.py index 2ef5d107c..f9817cd6d 100644 --- a/src/citrine/_session.py +++ b/src/citrine/_session.py @@ -171,13 +171,13 @@ def checked_request(self, method: str, path: str, self._refresh_access_token() response = self._request_with_retry(method, uri, **kwargs) except AttributeError: - # Catch AttributeErrors and log response - # The 401 status will be handled further down - logger.error("Failed to decode json from response: {}".format(response.text)) + # json() returned non-dict (e.g. list/string); 401 handled below + logger.debug("Response JSON is not a dict (status %s): %s", + response.status_code, response.text[:200]) except ValueError: - # Ignore ValueErrors thrown by attempting to decode json bodies. This - # might occur if we get a 401 response without a JSON body - pass + # Response has no JSON body (common for 401); 401 handled below + logger.debug("Response with status %s has no JSON body", + response.status_code) if 200 <= response.status_code <= 299: logger.info('%s %s %s', response.status_code, method, path) @@ -258,11 +258,11 @@ def _extract_response_json(path, response) -> dict: lacked the required 'application/json' Content-Type in the header.""") except JSONDecodeError as err: - logger.info('Response at path %s with status code %s failed json parsing with' - ' exception %s. Returning empty value.', - path, - response.status_code, - err.msg) + logger.warning( + 'Response at path %s with status code %s failed JSON ' + 'parsing (%s). Returning empty dict — downstream code ' + 'may behave unexpectedly.', + path, response.status_code, err.msg) return extracted_response From 348ea91160e1a5e5bb978657b0b4bcc6850cef22 Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 12:04:14 -0700 Subject: [PATCH 12/19] Add exception chaining and fix silent pagination error - Add 'from e' to exception re-raises in collection.py and ingestion.py so the original cause is preserved in tracebacks - Replace 'raise e' with bare 'raise' to preserve traceback context when re-raising the same exception - Add logger.warning in pageable.py when response data is not a dict (was silently swallowed, hiding type errors) Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/_rest/collection.py | 2 +- src/citrine/_rest/pageable.py | 7 +++++++ src/citrine/resources/ingestion.py | 10 +++++----- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/citrine/_rest/collection.py b/src/citrine/_rest/collection.py index 3da8075ed..03339e039 100644 --- a/src/citrine/_rest/collection.py +++ b/src/citrine/_rest/collection.py @@ -118,7 +118,7 @@ def register(self, model: CreationType) -> CreationType: data = data[self._individual_key] if self._individual_key else data return self.build(data) except NonRetryableException as e: - raise ModuleRegistrationFailedException(model.__class__.__name__, e) + raise ModuleRegistrationFailedException(model.__class__.__name__, e) from e def list(self, *, per_page: int = 100) -> Iterator[ResourceType]: """ diff --git a/src/citrine/_rest/pageable.py b/src/citrine/_rest/pageable.py index 129155cf7..306667011 100644 --- a/src/citrine/_rest/pageable.py +++ b/src/citrine/_rest/pageable.py @@ -1,6 +1,9 @@ +from logging import getLogger from typing import Optional, Iterable, Dict, Tuple, Callable, Union, Sequence from uuid import UUID +logger = getLogger(__name__) + class Pageable(): """Class that allows paging.""" @@ -87,6 +90,10 @@ def _fetch_page(self, try: next_uri = data.get('next', "") except AttributeError: + logger.warning( + "Response data is not a dict (type: %s); " + "pagination may be incomplete.", + type(data).__name__) next_uri = "" # A 'None' collection key implies response has a top-level array diff --git a/src/citrine/resources/ingestion.py b/src/citrine/resources/ingestion.py index 916426db1..2984ffc28 100644 --- a/src/citrine/resources/ingestion.py +++ b/src/citrine/resources/ingestion.py @@ -255,7 +255,7 @@ def build_objects(self, delete_templates=delete_templates) except IngestionException as e: if self.raise_errors: - raise e + raise else: return IngestionStatus.from_exception(e) @@ -331,9 +331,9 @@ def build_objects_async(self, ) except BadRequest as e: if e.api_error is not None: - raise IngestionException.from_api_error(e.api_error) + raise IngestionException.from_api_error(e.api_error) from e else: - raise e + raise def poll_for_job_completion(self, job: JobSubmissionResponse, @@ -560,11 +560,11 @@ def build_from_file_links(self, else: errors = [IngestionErrorTrace(msg=e.api_error.message)] if raise_errors: - raise IngestionException(errors=errors) + raise IngestionException(errors=errors) from e else: return FailedIngestion(errors=errors) else: - raise e + raise return self.build({ **response, "raise_errors": raise_errors From 22a60bcf2f3488f9b2e39e4af6f312eec12a2503 Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 12:07:59 -0700 Subject: [PATCH 13/19] Enhance timeout error messages with actionable guidance - PollingTimeoutError now includes job_id, timeout duration, and explains the server-side job continues running - ConditionTimeoutError now explains client timeout is independent of server-side work and suggests increasing timeout or polling manually Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/jobs/job.py | 7 ++++++- src/citrine/jobs/waiting.py | 9 ++++++--- tests/jobs/test_waiting.py | 6 ++++-- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/citrine/jobs/job.py b/src/citrine/jobs/job.py index bb6f71012..205b975bc 100644 --- a/src/citrine/jobs/job.py +++ b/src/citrine/jobs/job.py @@ -173,7 +173,12 @@ def _poll_for_job_completion(session: Session, logger.error(f'Job exceeded user timeout of {timeout} seconds. ' f'Note job on server is unaffected by this timeout.') logger.debug('Last status: {}'.format(status.dump())) - raise PollingTimeoutError('Job {} timed out.'.format(job_id)) + raise PollingTimeoutError( + 'Polling for job {} exceeded timeout of {} seconds. ' + 'The job may still be running on the server. ' + 'Increase the timeout parameter or check job ' + 'status manually.'.format(job_id, timeout) + ) if status.status == JobStatus.FAILURE: logger.debug(f'Job terminated with Failure status: {status.dump()}') if raise_errors: diff --git a/src/citrine/jobs/waiting.py b/src/citrine/jobs/waiting.py index 328d11f8d..a4c7baa01 100644 --- a/src/citrine/jobs/waiting.py +++ b/src/citrine/jobs/waiting.py @@ -92,9 +92,12 @@ def is_finished(): time.sleep(interval) if not is_finished(): raise ConditionTimeoutError( - "Timeout of {timeout_length} seconds " - "reached, but task {uid} is still in progress".format( - timeout_length=timeout, uid=resource.uid) + "Timeout of {} seconds reached, but task {} " + "is still in progress. The server-side task " + "continues running independently of this client " + "timeout. Increase the 'timeout' parameter to " + "wait longer, or poll status manually.".format( + timeout, resource.uid) ) current_resource = collection.get(resource.uid) diff --git a/tests/jobs/test_waiting.py b/tests/jobs/test_waiting.py index 62ce79f0d..27d8ab059 100644 --- a/tests/jobs/test_waiting.py +++ b/tests/jobs/test_waiting.py @@ -89,5 +89,7 @@ def test_wait_for_asynchronous_object(sleep_mock, time_mock): with pytest.raises(ConditionTimeoutError) as exception: wait_for_asynchronous_object(collection=collection, resource=resource, timeout=1.0) - assert str(exception.value) == ("Timeout of 1.0 seconds reached, " - "but task 123456 is still in progress") + msg = str(exception.value) + assert "1.0 seconds" in msg + assert "123456" in msg + assert "still in progress" in msg From fdb30439620581bb75a23ea3c4b100b6029aa51c Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 12:12:18 -0700 Subject: [PATCH 14/19] Enhance validation, batch, and deserialization error messages - Batcher collision error now explains what a collision means and suggests ensuring unique UIDs - Batcher dependency error now shows count and first 10 dependency names, suggests increasing batch_size - Collection uid=None error now explains the likely cause (unregistered object) and suggests calling .register() - Deserialization errors truncate data dicts to 200 chars and value reprs to 100 chars to avoid overwhelming output Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/_rest/collection.py | 7 +++++- src/citrine/_serialization/properties.py | 29 ++++++++++++++++++------ src/citrine/_utils/batcher.py | 17 ++++++++++++-- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/citrine/_rest/collection.py b/src/citrine/_rest/collection.py index 03339e039..7bd08bd81 100644 --- a/src/citrine/_rest/collection.py +++ b/src/citrine/_rest/collection.py @@ -86,7 +86,12 @@ def get(self, uid: Union[UUID, str]) -> ResourceType: """ if uid is None: - raise ValueError("Cannot get when uid=None. Are you using a registered resource?") + raise ValueError( + "Cannot retrieve a resource with uid=None. " + "This usually means the object has not been " + "registered with the platform yet. Call " + ".register() first to obtain a server-assigned " + "UID.") path = self._get_path(uid) data = self.session.get_resource(path, version=self._api_version) data = data[self._individual_key] if self._individual_key else data diff --git a/src/citrine/_serialization/properties.py b/src/citrine/_serialization/properties.py index 95cceef28..5dba37e35 100644 --- a/src/citrine/_serialization/properties.py +++ b/src/citrine/_serialization/properties.py @@ -95,9 +95,13 @@ def serialize(self, value: DeserializedType, base_class: typing.Optional[type] = None) -> SerializedType: if not isinstance(value, self.underlying_types): base_name = self._error_source(base_class) + value_repr = repr(value) + if len(value_repr) > 100: + value_repr = value_repr[:100] + "..." raise ValueError( - f'{type(value)} {value} is not one of valid types: ' - f'{self.underlying_types}{base_name}' + f'{type(value).__name__} is not one of valid ' + f'types: {self.underlying_types}{base_name}. ' + f'Value: {value_repr}' ) return self._serialize(value) @@ -107,9 +111,13 @@ def deserialize(self, value: SerializedType, if isinstance(value, self.underlying_types): return value # Don't worry if it was already deserialized base_name = self._error_source(base_class) + value_repr = repr(value) + if len(value_repr) > 100: + value_repr = value_repr[:100] + "..." raise ValueError( - f'{type(value)} {value} is not one of valid types: ' - f'{self.serialized_types}{base_name}' + f'{type(value).__name__} is not one of valid ' + f'types: {self.serialized_types}{base_name}. ' + f'Value: {value_repr}' ) return self._deserialize(value) @@ -129,9 +137,16 @@ def deserialize_from_dict(self, data: dict) -> DeserializedType: next_value = value.get(field) if next_value is None: if self.default is None and not self.optional: - msg = "Unable to deserialize {} into {}, missing a required field: {}".format( - data, self.underlying_types, field) - raise ValueError(msg) + data_preview = str(data) + if len(data_preview) > 200: + data_preview = data_preview[:200] + "..." + raise ValueError( + "Unable to deserialize into {}: " + "missing required field '{}'. " + "Data: {}".format( + self.underlying_types, + field, data_preview) + ) # This occurs if a `field` is unexpectedly not present in the data dictionary # or if its value is null. # Use the default value and stop traversing, even if we have not yet reached diff --git a/src/citrine/_utils/batcher.py b/src/citrine/_utils/batcher.py index dcc171807..1bf23b5b7 100644 --- a/src/citrine/_utils/batcher.py +++ b/src/citrine/_utils/batcher.py @@ -36,7 +36,11 @@ def batch(self, objects: Iterable[DataConcepts], batch_size) -> List[List[DataCo for obj in objects: if obj.to_link() in seen: # Repeat in the iterable; don't add it to the batch if seen[obj.to_link()] != obj: # verify that it's a replicate - raise ValueError(f"Colliding objects for {obj.to_link()}") + raise ValueError( + "Colliding objects for {}: two different " + "objects share the same identifier. Ensure " + "each object has a unique UID.".format( + obj.to_link())) else: by_type[obj.typ].append(obj) for scope in obj.uids: @@ -80,7 +84,16 @@ def batch(self, objects: Iterable[DataConcepts], batch_size) -> List[List[DataCo local_set = {index.get(x, x) for x in depends[obj] if index.get(x, x) in obj_set} full_set = set(local_set) if len(full_set) > batch_size: - raise ValueError(f"Object {obj.name} has more than {batch_size} dependencies.") + sample = [getattr(d, 'name', str(d)) + for d in list(full_set)[:10]] + raise ValueError( + "Object '{}' has {} dependencies, " + "exceeding batch_size={}. First {}: {}. " + "Increase batch_size or simplify the " + "dependency graph.".format( + obj.name, len(full_set), + batch_size, len(sample), sample) + ) for subobj in local_set: full_set.update(depends[subobj]) From bb21e26638feb7cd2a9427f946db24a3c885a3bd Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 12:16:20 -0700 Subject: [PATCH 15/19] Add programmatic error access to HTTP exceptions Add validation_errors property and has_failure() method to NonRetryableHttpException for structured access to API validation errors without parsing message strings. Enables patterns like: except BadRequest as e: for ve in e.validation_errors: print(f"Field '{ve.property}': {ve.failure_message}") if e.has_failure("missing_required_field"): # handle specifically Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/exceptions.py | 33 ++++++++++++++++++++++++++ tests/test_session.py | 49 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/src/citrine/exceptions.py b/src/citrine/exceptions.py index f925c86f4..b351818e3 100644 --- a/src/citrine/exceptions.py +++ b/src/citrine/exceptions.py @@ -143,6 +143,39 @@ def __init__(self, path: str, response: Optional[Response] = None): super().__init__("\n\t".join(self.detailed_error_info)) + @property + def validation_errors(self): + """Shortcut to api_error.validation_errors, or empty list. + + Returns + ------- + list + List of ValidationError objects, or [] if no + api_error is available. + """ + if self.api_error is not None: + return self.api_error.validation_errors + return [] + + def has_failure(self, failure_id): + """Check if a specific validation failure ID is present. + + Parameters + ---------- + failure_id : str + The failure_id to check for. + + Returns + ------- + bool + True if a validation error with the given + failure_id exists. + """ + return any( + ve.failure_id == failure_id + for ve in self.validation_errors + ) + class NotFound(NonRetryableHttpException): """The requested resource was not found (HTTP 404). diff --git a/tests/test_session.py b/tests/test_session.py index 147ecab09..dd00d2468 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -6,6 +6,7 @@ Conflict, NonRetryableException, NotFound, + Unauthorized, WorkflowNotReadyException, RetryableException) @@ -197,6 +198,54 @@ def test_status_code_400(session: Session): == resp_json['validation_errors'][0]['failure_message'] +def test_validation_errors_property(session: Session): + with requests_mock.Mocker() as m: + resp_json = { + 'code': 400, + 'message': 'validation failed', + 'validation_errors': [ + { + 'failure_message': 'field required', + 'failure_id': 'missing_field', + }, + ], + } + m.get('http://citrine-testing.fake/api/v1/foo', + status_code=400, json=resp_json) + with pytest.raises(BadRequest) as einfo: + session.get_resource('/foo') + assert len(einfo.value.validation_errors) == 1 + assert einfo.value.validation_errors[0].failure_id \ + == 'missing_field' + + +def test_has_failure_method(session: Session): + with requests_mock.Mocker() as m: + resp_json = { + 'code': 400, + 'message': 'bad', + 'validation_errors': [ + {'failure_id': 'field_x_required'}, + ], + } + m.get('http://citrine-testing.fake/api/v1/foo', + status_code=400, json=resp_json) + with pytest.raises(BadRequest) as einfo: + session.get_resource('/foo') + assert einfo.value.has_failure('field_x_required') + assert not einfo.value.has_failure('nonexistent') + + +def test_validation_errors_empty_without_api_error(session: Session): + with requests_mock.Mocker() as m: + m.get('http://citrine-testing.fake/api/v1/foo', + status_code=404) + with pytest.raises(NotFound) as einfo: + session.get_resource('/foo') + assert einfo.value.validation_errors == [] + assert not einfo.value.has_failure('anything') + + def test_status_code_401(session: Session): with requests_mock.Mocker() as m: m.get('http://citrine-testing.fake/api/v1/foo', status_code=401) From e9631c24aaf0933b1d5ba9e16a624b9be0e86619 Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 11:36:36 -0700 Subject: [PATCH 16/19] Add hint support to CitrineException base class Add optional 'hint' keyword argument to CitrineException that appends actionable guidance to error messages. When present, __str__ outputs the base message followed by "\n\nHint: ". This is the foundation for adding actionable hints to all exception subclasses in subsequent PRs. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/exceptions.py | 17 +++++++++++++- tests/test_exceptions.py | 49 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 tests/test_exceptions.py diff --git a/src/citrine/exceptions.py b/src/citrine/exceptions.py index b351818e3..3e4c2ae32 100644 --- a/src/citrine/exceptions.py +++ b/src/citrine/exceptions.py @@ -28,9 +28,24 @@ class CitrineException(Exception): class, so ``except CitrineException`` will catch any Citrine-specific error. + Parameters + ---------- + *args + Positional arguments passed to the base Exception class. + hint : str, optional + An actionable suggestion for how the user can resolve + the error. Displayed after the main message. """ - pass + def __init__(self, *args, hint=None): + super().__init__(*args) + self.hint = hint + + def __str__(self): + base = super().__str__() + if self.hint: + return "{}\n\nHint: {}".format(base, self.hint) + return base class NonRetryableException(CitrineException): diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py new file mode 100644 index 000000000..bc51071e0 --- /dev/null +++ b/tests/test_exceptions.py @@ -0,0 +1,49 @@ +"""Tests for the Citrine exception hierarchy.""" +import pytest + +from citrine.exceptions import CitrineException + + +def test_citrine_exception_without_hint(): + exc = CitrineException("something went wrong") + assert str(exc) == "something went wrong" + assert exc.hint is None + + +def test_citrine_exception_with_hint(): + exc = CitrineException("bad request", hint="Check your input.") + assert "bad request" in str(exc) + assert "Hint: Check your input." in str(exc) + assert exc.hint == "Check your input." + + +def test_citrine_exception_hint_format(): + exc = CitrineException("error", hint="Try again.") + expected = "error\n\nHint: Try again." + assert str(exc) == expected + + +def test_citrine_exception_no_args(): + exc = CitrineException() + assert str(exc) == "" + assert exc.hint is None + + +def test_citrine_exception_no_args_with_hint(): + exc = CitrineException(hint="Do something.") + assert "Hint: Do something." in str(exc) + + +def test_citrine_exception_is_catchable_as_exception(): + with pytest.raises(Exception): + raise CitrineException("test") + + +def test_hint_preserved_through_subclass(): + """Subclasses that call super().__init__ with hint should work.""" + class MyError(CitrineException): + pass + + exc = MyError("oops", hint="Fix it.") + assert exc.hint == "Fix it." + assert "Hint: Fix it." in str(exc) From 1f9b39904f9ea3af6e35acfffd1c24e13472c1dc Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 11:55:38 -0700 Subject: [PATCH 17/19] Add ServerError exception for 5xx HTTP responses Replace bare CitrineException(response.text) for 5xx errors with a structured ServerError that includes method, path, status code, response text (truncated to 500 chars), request ID, and an actionable hint about contacting support. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/_session.py | 9 +++++- src/citrine/exceptions.py | 41 ++++++++++++++++++++++++ tests/test_exceptions.py | 65 ++++++++++++++++++++++++++++++++++++++- tests/test_session.py | 8 +++-- 4 files changed, 119 insertions(+), 4 deletions(-) diff --git a/src/citrine/_session.py b/src/citrine/_session.py index f9817cd6d..ce069cae8 100644 --- a/src/citrine/_session.py +++ b/src/citrine/_session.py @@ -19,6 +19,7 @@ CitrineException, Conflict, NotFound, + ServerError, Unauthorized, UnauthorizedRefreshToken, WorkflowNotReadyException) @@ -209,7 +210,13 @@ def checked_request(self, method: str, path: str, raise WorkflowNotReadyException(msg) else: logger.error('%s %s %s', response.status_code, method, path) - raise CitrineException(response.text) + raise ServerError( + method=method, path=path, + status_code=response.status_code, + response_text=response.text, + request_id=response.headers.get( + 'x-request-id') + ) @staticmethod def _extract_response_stacktrace(response: Response) -> Optional[str]: diff --git a/src/citrine/exceptions.py b/src/citrine/exceptions.py index 3e4c2ae32..a3637e7d5 100644 --- a/src/citrine/exceptions.py +++ b/src/citrine/exceptions.py @@ -334,3 +334,44 @@ def __init__(self, moduleType: str, exc: Exception): err = 'The "{0}" failed to register. {1}: {2}'.format( moduleType, exc.__class__.__name__, str(exc)) super().__init__(err) + + +class ServerError(NonRetryableException): + """A server-side error (5xx) occurred on the Citrine API. + + Parameters + ---------- + method : str + The HTTP method of the request (e.g. GET, POST). + path : str + The API path that was requested. + status_code : int + The HTTP status code returned by the server. + response_text : str + The body of the error response (truncated to 500 chars). + request_id : str, optional + The server-assigned request ID for support reference. + """ + + def __init__(self, *, method, path, status_code, + response_text, request_id=None): + self.method = method + self.path = path + self.status_code = status_code + self.request_id = request_id + self.response_text = response_text[:500] if response_text else "" + + parts = ["Server error {} from {} {}".format( + status_code, method, path)] + if request_id: + parts.append("Request ID: {}".format(request_id)) + if self.response_text: + parts.append("Response: {}".format(self.response_text)) + + hint = ("This is a server-side issue. If it persists, " + "contact Citrine support") + if request_id: + hint += " with request ID '{}'.".format(request_id) + else: + hint += "." + super().__init__("\n".join(parts), hint=hint) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index bc51071e0..a0a0323a6 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -1,7 +1,7 @@ """Tests for the Citrine exception hierarchy.""" import pytest -from citrine.exceptions import CitrineException +from citrine.exceptions import CitrineException, NonRetryableException, ServerError def test_citrine_exception_without_hint(): @@ -47,3 +47,66 @@ class MyError(CitrineException): exc = MyError("oops", hint="Fix it.") assert exc.hint == "Fix it." assert "Hint: Fix it." in str(exc) + + +# --- ServerError tests --- + +def test_server_error_attributes(): + exc = ServerError( + method="POST", path="/api/v1/foo", + status_code=502, response_text="Bad Gateway", + request_id="abc-123" + ) + assert exc.method == "POST" + assert exc.path == "/api/v1/foo" + assert exc.status_code == 502 + assert exc.response_text == "Bad Gateway" + assert exc.request_id == "abc-123" + + +def test_server_error_message_includes_context(): + exc = ServerError( + method="GET", path="/api/v1/bar", + status_code=500, response_text="internal error" + ) + msg = str(exc) + assert "500" in msg + assert "GET" in msg + assert "/api/v1/bar" in msg + assert "internal error" in msg + + +def test_server_error_includes_request_id_in_message(): + exc = ServerError( + method="PUT", path="/x", + status_code=503, response_text="", + request_id="req-456" + ) + msg = str(exc) + assert "req-456" in msg + + +def test_server_error_truncates_long_response(): + long_text = "x" * 1000 + exc = ServerError( + method="GET", path="/", + status_code=500, response_text=long_text + ) + assert len(exc.response_text) == 500 + + +def test_server_error_has_hint(): + exc = ServerError( + method="GET", path="/", + status_code=500, response_text="err" + ) + assert exc.hint is not None + assert "server-side" in exc.hint.lower() + + +def test_server_error_is_non_retryable(): + exc = ServerError( + method="GET", path="/", + status_code=500, response_text="" + ) + assert isinstance(exc, NonRetryableException) diff --git a/tests/test_session.py b/tests/test_session.py index dd00d2468..64f85a520 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -6,6 +6,7 @@ Conflict, NonRetryableException, NotFound, + ServerError, Unauthorized, WorkflowNotReadyException, RetryableException) @@ -331,10 +332,13 @@ def test_failed_put_with_stacktrace(session: Session): json={'debug_stacktrace': 'blew up!'} ) - with pytest.raises(Exception) as e: + with pytest.raises(ServerError) as e: session.put_resource('/bad-endpoint', json={}) - assert '{"debug_stacktrace": "blew up!"}' == str(e.value) + assert e.value.status_code == 500 + assert e.value.method == "PUT" + assert e.value.path == "/bad-endpoint" + assert "blew up!" in e.value.response_text def test_cursor_paged_resource(): From 2b9354a55382d1a65d8fef5f4e282e62ef83c492 Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 12:18:13 -0700 Subject: [PATCH 18/19] Add default hints to HTTP exception subclasses Each HTTP exception subclass now has a _default_hint providing actionable guidance: - NotFound: verify resource UID - Unauthorized: check API key validity - BadRequest: check validation errors - WorkflowConflictException: wait and retry Hints are wired through NonRetryableHttpException.__init__ to the base CitrineException hint mechanism. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/exceptions.py | 27 +++++++++++++++++++++++---- tests/test_exceptions.py | 31 ++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/citrine/exceptions.py b/src/citrine/exceptions.py index a3637e7d5..01067abf2 100644 --- a/src/citrine/exceptions.py +++ b/src/citrine/exceptions.py @@ -102,6 +102,8 @@ class NonRetryableHttpException(NonRetryableException): """ + _default_hint = None # Subclasses may set this + def __init__(self, path: str, response: Optional[Response] = None): self.url = path self.detailed_error_info = [] @@ -156,7 +158,9 @@ def __init__(self, path: str, response: Optional[Response] = None): self.code = None self.api_error = None - super().__init__("\n\t".join(self.detailed_error_info)) + super().__init__( + "\n\t".join(self.detailed_error_info), + hint=self._default_hint) @property def validation_errors(self): @@ -200,6 +204,11 @@ class NotFound(NonRetryableHttpException): """ + _default_hint = ( + "Verify the resource UID exists in the target " + "project/dataset. UIDs are case-sensitive." + ) + @staticmethod def build(*, message: str, method: str, path: str, params: dict = {}): """ @@ -249,7 +258,10 @@ class Unauthorized(NonRetryableHttpException): """ - pass + _default_hint = ( + "Check that your API key is valid and has access " + "to this resource. Regenerate your key if expired." + ) class BadRequest(NonRetryableHttpException): @@ -260,7 +272,10 @@ class BadRequest(NonRetryableHttpException): """ - pass + _default_hint = ( + "Check the validation errors above for specific " + "field issues." + ) class WorkflowConflictException(NonRetryableHttpException): @@ -271,7 +286,11 @@ class WorkflowConflictException(NonRetryableHttpException): """ - pass + _default_hint = ( + "Another operation may be in progress on this " + "resource. Wait and retry, or check for " + "concurrent modifications." + ) #: Alias for :class:`WorkflowConflictException`. A 409 can occur diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index a0a0323a6..73c042d6b 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -1,7 +1,10 @@ """Tests for the Citrine exception hierarchy.""" import pytest -from citrine.exceptions import CitrineException, NonRetryableException, ServerError +from citrine.exceptions import ( + BadRequest, CitrineException, NonRetryableException, NotFound, + ServerError, Unauthorized, WorkflowConflictException, +) def test_citrine_exception_without_hint(): @@ -110,3 +113,29 @@ def test_server_error_is_non_retryable(): status_code=500, response_text="" ) assert isinstance(exc, NonRetryableException) + + +# --- Default hint tests for HTTP exception subclasses --- + +def test_not_found_has_default_hint(): + exc = NotFound("/test/path") + assert exc.hint is not None + assert "UID" in exc.hint + + +def test_unauthorized_has_default_hint(): + exc = Unauthorized("/test/path") + assert exc.hint is not None + assert "API key" in exc.hint + + +def test_bad_request_has_default_hint(): + exc = BadRequest("/test/path") + assert exc.hint is not None + assert "validation" in exc.hint.lower() + + +def test_conflict_has_default_hint(): + exc = WorkflowConflictException("/test/path") + assert exc.hint is not None + assert "retry" in exc.hint.lower() From 201516c7fa3e0d29f069523de948e740343ee410 Mon Sep 17 00:00:00 2001 From: Gregory Mulholland Date: Wed, 25 Mar 2026 12:19:38 -0700 Subject: [PATCH 19/19] Enhance auth error message with actionable guidance UnauthorizedRefreshToken now includes a descriptive message and hint directing users to regenerate their API key at the platform URL or set the CITRINE_API_KEY environment variable. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/citrine/_session.py | 11 ++++++++++- tests/test_session.py | 5 ++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/citrine/_session.py b/src/citrine/_session.py index ce069cae8..a43cabc32 100644 --- a/src/citrine/_session.py +++ b/src/citrine/_session.py @@ -118,7 +118,16 @@ def _refresh_access_token(self) -> None: json=data) if response.status_code != 200: - raise UnauthorizedRefreshToken() + raise UnauthorizedRefreshToken( + "Failed to refresh authentication token.", + hint=( + "Your API key may have expired or been " + "revoked. Generate a new one at " + "{}://{}/account/api-keys or set the " + "CITRINE_API_KEY environment variable." + .format(self.scheme, self.authority) + ) + ) self.access_token = response.json()['access_token'] self.access_token_expiration = datetime.fromtimestamp( jwt.decode( diff --git a/tests/test_session.py b/tests/test_session.py index 64f85a520..521b02074 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -97,8 +97,11 @@ def test_get_refresh_token_failure(session: Session): with requests_mock.Mocker() as m: m.post('http://citrine-testing.fake/api/v1/tokens/refresh', status_code=401) - with pytest.raises(UnauthorizedRefreshToken): + with pytest.raises(UnauthorizedRefreshToken) as exc_info: session.get_resource('/foo') + msg = str(exc_info.value) + assert "refresh authentication token" in msg + assert "api-keys" in msg def test_get_no_refresh(session: Session):