From b9695624ff2e374a78b814a6bb60e1cd15f21b63 Mon Sep 17 00:00:00 2001 From: Byron Pullutasig <115118857+bpulluta@users.noreply.github.com> Date: Thu, 6 Aug 2026 08:07:52 -0400 Subject: [PATCH 1/2] Improve geothermal qualitative schema and harden LiteLLM kwarg handling Restore geothermal schema/plugin to the latest tuned recall state (v2.2.2 + collection prompt tuning), and add adaptive OpenAI service retry behavior that removes provider-rejected call kwargs (e.g., temperature) and retries safely. Also adds runtime-focused tests for unsupported-kwarg recovery and non-recoverable error passthrough. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../geothermal_schema.json | 30 +++---- .../geothermal_electricity/plugin_config.yaml | 31 ++++++- compass/services/openai.py | 84 +++++++++++++++--- .../unit/services/test_services_openai.py | 88 +++++++++++++++++++ 4 files changed, 202 insertions(+), 31 deletions(-) diff --git a/compass/extraction/geothermal_electricity/geothermal_schema.json b/compass/extraction/geothermal_electricity/geothermal_schema.json index 272bec3f5..62db443e0 100644 --- a/compass/extraction/geothermal_electricity/geothermal_schema.json +++ b/compass/extraction/geothermal_electricity/geothermal_schema.json @@ -1,14 +1,14 @@ { "title": "Geothermal Electricity Ordinance Extraction Schema", "description": "Single-shot structured extraction schema for utility-scale geothermal electricity ordinances. This schema guides an LLM to extract all relevant features in one call and returns an outputs array where each object represents one row in the extracted long-form table.", - "version": "2.1.2", + "version": "2.2.2", "type": "object", "required": ["outputs"], "additionalProperties": false, "properties": { "outputs": { "type": "array", - "description": "Sparse long-form extraction table. Include only features with an enacted, explicit requirement and emit exactly one row per feature — never two rows with the same feature name. For array-valued features (primary use districts, special use districts, accessory use districts, prohibited use districts, required permits), merge all qualifying items from the entire document into a single consolidated array in that one row. Never infer, imply, or guess a requirement from related context.", + "description": "Sparse long-form extraction table. Include only features with an enacted, explicit requirement and emit exactly one row per feature — never two rows with the same feature name. For array-valued features (primary use districts, special use districts, required permits), merge all qualifying items from the entire document into a single consolidated array in that one row. Never infer, imply, or guess a requirement from related context.", "items": { "type": "object", "required": [ @@ -48,8 +48,6 @@ "seismic monitoring plan", "primary use districts", "special use districts", - "accessory use districts", - "prohibited use districts", "required permits", "bond requirement", "decommissioning", @@ -101,7 +99,7 @@ "description": "Only extract requirements that apply to utility-scale geothermal electricity generation and directly associated geothermal electricity infrastructure, including exploration or drilling operations, production or injection wells, geothermal power plants, associated substations, and gen-tie lines only when the ordinance text explicitly governs them. Exclude geothermal heat pumps, HVAC, direct-use geothermal, district heating, greenhouse heating, residential geothermal, and other non-generation systems unless the ordinance explicitly governs utility-scale electricity generation as well. State statutes and regulations are in scope when they impose enforceable siting, zoning, drilling, permitting, monitoring, bonding, setback, or decommissioning requirements on geothermal electricity projects in the jurisdiction. This schema must work across nationwide ordinance styles, including county, municipal, township, parish, borough, tribal, and state-level regulatory text." }, "technology_applicability_gate": { - "description": "Extract a row only when the cited evidence clearly applies to geothermal electricity. If the excerpt does not explicitly mention geothermal, the summary must include clear evidence that the governing section or table applies to geothermal electricity projects (for example a geothermal-specific chapter heading, geothermal-defined use class, or explicit cross-reference in the same provided text). If applicability is ambiguous or technology-neutral without clear geothermal linkage, omit the feature." + "description": "Extract a row only when the cited evidence clearly applies to geothermal electricity. If the excerpt does not explicitly mention geothermal, the summary may establish applicability through ordinance context that is explicitly tied to geothermal in the same governing structure (for example a geothermal-specific chapter heading, a geothermal-defined use class, a table of uses listing geothermal, or table notes/standards that apply to that listed geothermal use). If applicability remains ambiguous or technology-neutral without a clear geothermal linkage, omit the feature." }, "nationwide_jurisdiction_handling": { "description": "Preserve the ordinance's own governance vocabulary instead of normalizing it to one jurisdiction type. Districts may appear as zoning districts, use districts, overlays, resource areas, exclusive farm use zones, planned development areas, or similar land-use categories. Permits may appear as conditional use permits, special use permits, administrative permits, use permits, site certificates, plans of operation, drilling authorizations, county approvals, state board approvals, or similar authorizations. If a jurisdiction is effectively unzoned or the controlling requirements come from state regulation without district tables, omit district features rather than forcing a district classification." @@ -116,7 +114,7 @@ "description": "When multiple numeric values apply to the same feature, keep one row and select the controlling most restrictive value for that feature. Restrictiveness rules: distance features choose the largest minimum separation distance; noise choose the lowest allowed numeric limit; maximum height choose the lowest maximum height; minimum lot size choose the highest minimum lot size. Keep condition-specific alternatives in summary only when the ordinance text explicitly shows they all apply to the same geothermal electricity feature." }, "district_and_permit_routing": { - "description": "Use the district features only for zoning districts, overlay districts, or land-use districts that explicitly classify geothermal electricity facilities. Use 'primary use districts' when the use is allowed by right or as a principal permitted use. Use 'special use districts' when the use requires a conditional use permit, special use permit, or comparable discretionary approval. Use 'accessory use districts' only when the ordinance explicitly allows geothermal electricity as an accessory or secondary use. Use 'prohibited use districts' only for districts where geothermal electricity facilities are explicitly and unconditionally prohibited. Use 'required permits' only for explicit permits or approvals that are mandatory to initiate geothermal exploration, geothermal drilling, geothermal power plant construction, or geothermal electricity operation. Do not use 'required permits' for generic county-wide permit administration language, fee schedules, broad permit references that apply to all development, or permit provisions that are not clearly tied to geothermal electricity in the same evidence. Do not route permit requirements into district features, and do not route district tables into permit features. ONE ROW PER FEATURE: When multiple districts or permits qualify for the same feature category, combine ALL of them into a single array in one row. Never emit two or more rows with the same feature name — the one-row-per-feature rule is absolute for district and permit features. For example, if both 'Exclusive Farm Use (EFU)' and 'Forestry (F)' are conditional use districts, the single 'special use districts' row must have value: ['Exclusive Farm Use (EFU)', 'Forestry (F)'], not two separate rows." + "description": "Use the district features only for zoning districts, overlay districts, or land-use districts that explicitly classify geothermal electricity facilities. Use 'primary use districts' when the use is allowed by right or as a principal permitted use. Use 'special use districts' when the use requires a conditional use permit, special use permit, or comparable discretionary approval. Use 'required permits' only for explicit permits or approvals that are mandatory to initiate geothermal exploration, geothermal drilling, geothermal power plant construction, or geothermal electricity operation. Do not use 'required permits' for generic county-wide permit administration language, fee schedules, broad permit references that apply to all development, or permit provisions that are not clearly tied to geothermal electricity in the same evidence. Do not route permit requirements into district features, and do not route district tables into permit features. ONE ROW PER FEATURE: When multiple districts or permits qualify for the same feature category, combine ALL of them into a single array in one row. Never emit two or more rows with the same feature name — the one-row-per-feature rule is absolute for district and permit features. For example, if both 'Exclusive Farm Use (EFU)' and 'Forestry (F)' are conditional use districts, the single 'special use districts' row must have value: ['Exclusive Farm Use (EFU)', 'Forestry (F)'], not two separate rows." }, "prohibition_boundary": { "description": "Use 'prohibitions' only for currently effective bans or moratoria on geothermal electricity exploration, drilling, well development, power plant siting, facility construction, or related project deployment. A ban on hydraulic fracturing or fracking counts only when the ordinance explicitly uses that ban to prohibit, limit, or condition geothermal electricity development. Do not treat ordinary permit conditions, environmental standards, or operational restrictions as prohibitions when the project remains allowable subject to compliance." @@ -190,7 +188,7 @@ "description": "Operational, design, permit, monitoring, and end-of-life requirements that are primarily textual obligations.", "properties": { "fencing": { - "description": "Extract explicit fencing, enclosure, controlled-access, or perimeter barrier requirements for geothermal electricity facilities, wells, plants, or related equipment. Do not map generic security plans here unless the ordinance explicitly requires fencing or enclosure." + "description": "Extract any explicit fencing, enclosure, perimeter barrier, or access control requirement that applies to geothermal electricity facilities or any component thereof, including individual well sites, wellheads, substations, transformers, production facilities, injection facilities, sumps, retention ponds, or other associated equipment. Fencing requirements that apply to specific sub-components only (e.g., wellheads only, substations only, or sumps only) are in scope — extract them and preserve the specific component scope in summary. Ordinance provisions that conditionally require fencing ('shall be required when', 'may be required', 'must be installed if') are in scope when they represent an enacted regulatory standard rather than discretionary agency guidance. Do not map generic security plans, access management policies, or operational procedures unless the ordinance explicitly requires a physical fence, enclosure, or barrier." }, "color requirements": { "description": "Extract explicit color, paint, finish, or color-treatment requirements for geothermal electricity structures or equipment. Do not map generic visual mitigation here unless the ordinance expressly requires a color treatment." @@ -199,10 +197,10 @@ "description": "Extract explicit lighting requirements, shielding, directionality, glare controls, night-lighting limits, or aviation-lighting conditions for geothermal electricity facilities or related equipment." }, "visual impact assessment": { - "description": "Extract explicit requirements for a visual impact assessment, visual resource study, scenic impact analysis, photo simulation, line-of-sight analysis, or similar visual review document. Do not use this feature for ordinary screening requirements unless the ordinance explicitly requires an assessment or study." + "description": "Extract any requirement to formally assess, document, describe, or demonstrate mitigation of visual effects or visual impacts of a geothermal electricity facility. This includes: visual impact assessments, visual resource studies, scenic impact analyses, viewshed analyses, photo simulations, line-of-sight analyses, requirements to submit maps or descriptions of scenic vistas, viewsheds, or visual resources, requirements to describe or demonstrate the visual effect of the proposed facility, and requirements to document visual impact mitigation measures. The requirement must be a formal submission or study obligation — not a general design standard. Do not use this feature for color, screening, or landscaping requirements unless they are explicitly imposed as part of a required visual assessment or mitigation plan (those belong under color requirements or lighting requirements)." }, "seismic monitoring plan": { - "description": "Extract explicit requirements for a seismic monitoring plan, induced seismicity monitoring plan, geophysical monitoring plan, or similar seismicity-management document or program. Do not use this feature for generic environmental monitoring unless the ordinance explicitly ties it to seismic or fault-related monitoring." + "description": "Extract any ordinance requirement related to seismicity, induced seismicity, land subsidence, seismic/geologic hazard characterization, or seismic risk management for geothermal electricity projects. This includes: seismic monitoring plans, induced seismicity monitoring programs, subsidence prevention or detection programs, requirements to participate in county or government seismic monitoring programs, requirements to monitor for or mitigate induced seismicity or land subsidence, requirements to design or operate facilities to minimize seismic activity or subsidence from geothermal fluid withdrawal or injection, requirements to submit precautionary measures for seismic/subsidence risk, and project application requirements to provide special seismic considerations or geologic hazard/site-condition analyses where seismicity, faults, or ground subsidence are explicitly part of the required submission criteria. Do not use this feature for generic environmental impact statements or broad geologic descriptions that do not create a specific seismic/subsidence-related requirement." }, "required permits": { "description": "Extract explicit permits, approvals, authorizations, reviews, certifications, or comparable entitlements that must be filed or obtained to initiate geothermal exploration, drilling, facility construction, or facility operation. This includes conditional use permits, special use permits, zoning permits, drilling permits, county permits, municipal permits, use permits, site plan approvals, plans of operation, state board approvals, siting certificates, and similar authorizations. Preserve exact permit names in value. Always output value as an array of strings, even when only one permit is required. Extract only when the same cited evidence explicitly ties the permit requirement to geothermal electricity facilities or geothermal exploration/drilling for electricity generation. Exclude generic permitting frameworks, administrative filing workflows, fee schedules, broad references to permit requirements for all uses, and non-filing actions such as appeals, interpretations, lot line adjustments, variances not required for geothermal initiation, temporary-event permits, or unrelated permits unless the ordinance explicitly makes them mandatory for geothermal project initiation." @@ -223,12 +221,6 @@ }, "special use districts": { "description": "Extract all districts, zones, overlays, or similar land-use categories where geothermal electricity facilities are allowed only through special use, conditional use, special exception, discretionary review, or comparable approval. Use an array of district names in value and preserve the approval posture in summary. Geothermal applicability must be explicitly established for each included district: either the district clause itself mentions geothermal, or cite an explicit in-document definition or use class that unambiguously links to geothermal power generation (e.g., 'Commercial Power Generating Facility' defined to include geothermal power, or 'Extensive Impact Services' explicitly listed to include geothermal operations). Do not route the permit type itself here unless it is part of the district allowance language. If the ordinance states geothermal is conditional in a district, never classify that district as primary use." - }, - "accessory use districts": { - "description": "Extract all districts, zones, overlays, or similar land-use categories where geothermal electricity facilities are explicitly allowed only as an accessory, incidental, or subordinate use to another principal use. Use an array of district names in value and preserve the accessory-use condition in summary. Geothermal applicability must be explicitly established for each included district: either the district clause itself mentions geothermal, or cite an explicit in-document definition or use class that unambiguously links to geothermal power generation." - }, - "prohibited use districts": { - "description": "Extract all districts, zones, overlays, or similar land-use categories where geothermal electricity facilities are explicitly and unconditionally prohibited. Use an array of district names in value and quote the prohibitory language in summary. Geothermal applicability must be explicitly established for each included district: either the district clause itself mentions geothermal, or cite an explicit in-document definition or use class that unambiguously links to geothermal power generation. Do not include districts where geothermal electricity may still be allowed with a permit, variance, overlay, or other conditional path." } } }, @@ -304,7 +296,7 @@ "$instructions": { "general": [ "Use direct text excerpts and quotes in summary whenever possible.", - "Each feature may appear at most once in outputs — the one-row-per-feature rule is absolute. If multiple ordinance clauses or sections match the same feature, consolidate them into exactly one row as follows: for array-valued features (primary use districts, special use districts, accessory use districts, prohibited use districts, required permits), combine ALL matching items into a single array in that one row and capture all source clauses in summary; for numeric features, keep the controlling most restrictive value in value and list alternatives in summary.", + "Each feature may appear at most once in outputs — the one-row-per-feature rule is absolute. If multiple ordinance clauses or sections match the same feature, consolidate them into exactly one row as follows: for array-valued features (primary use districts, special use districts, required permits), combine ALL matching items into a single array in that one row and capture all source clauses in summary; for numeric features, keep the controlling most restrictive value in value and list alternatives in summary.", "Feature IDs are strict canonical keys. Do not output aliases, prefixed variants, or paraphrased feature names not present in the enum.", "For any numeric feature, the summary must support the same requirement that produced value and units for that row. Never pair a numeric value from one clause with qualitative-only language from another clause that has no numeric threshold.", "Standardize units in the units field using this schema's canonical vocabulary, while preserving ordinance-specific wording in summary.", @@ -316,7 +308,7 @@ "Preserve exact local or state regulatory terminology in summary and, where applicable, in value. Do not rename district categories, permit names, or agency approvals into a preferred local template.", "If ordinance text shows an amended or superseding requirement, extract the current operative requirement as written rather than a superseded historical value unless the ordinance text clearly keeps both rules active.", "For moratoria and temporary prohibitions, preserve explicit timing windows in summary and omit clearly expired moratoria.", - "If text is suggestive but not explicit for the target feature, omit the feature.", + "If text is genuinely ambiguous as to whether it imposes an enforceable requirement, or if technology applicability is unclear, omit the feature. Do not omit a feature merely because the ordinance uses different terminology than the feature name — terminology equivalences are defined in each feature's description in $definitions.", "If the text references a different chapter or external document for the controlling value but does not restate that value here, omit the feature instead of outputting blanks or placeholders.", "If a provision is written for renewable or energy facilities generally, extract it only when the same provided evidence clearly ties that provision to geothermal electricity." ], @@ -335,7 +327,7 @@ "For drilling schedule requirements, extract explicit start and end times into 'drilling start time' and 'drilling end time' using 24-hour HH:MM format and units 'HH:MM (24-hour)'." ], "qualitative": [ - "For qualitative features, output only when an explicit enforceable requirement is present.", + "For qualitative features, output only when the ordinance text unambiguously imposes an enforceable requirement. 'Explicit' means the text clearly establishes a legal obligation — it does not require the ordinance to use the exact feature name or schema terminology. The full range of equivalent terminology for each feature is defined in $definitions.", "For fencing, color requirements, lighting requirements, visual impact assessment, seismic monitoring plan, bond requirement, and decommissioning, prefer value=null and units=null unless the ordinance states a specific numeric threshold or an explicit list that should be preserved in value.", "For required permits, always use an array of strings in value, even when only one permit is required.", "For required permits, require a direct geothermal-electricity applicability anchor in the same cited evidence, such as geothermal power plant, geothermal generating facility, geothermal exploration or production wells, renewable energy provision explicitly including geothermal, or a geothermal-specific use table row.", @@ -349,7 +341,7 @@ "For all district features, use an array of district or zone names and set units to null.", "Use the exact district names or codes as they appear in the ordinance text whenever possible.", "For each district listed in value, require an explicit geothermal-electricity applicability anchor: either the district clause directly mentions geothermal, or cite a clearly defined use class in the same ordinance that explicitly includes geothermal power (e.g., 'Commercial Power Generating Facility' defined to include geothermal, or 'Geothermal Overlay Zone', or ordinance language stating a use category 'includes geothermal operations'). Do not include districts based on generic utility or infrastructure terminology alone unless the same document explicitly states those categories encompass geothermal power generation.", - "Use 'primary use districts' for by-right or principal-use authorization, 'special use districts' for conditional or discretionary authorization, 'accessory use districts' for accessory-only authorization, and 'prohibited use districts' for unconditional district-level bans.", + "Use 'primary use districts' for by-right or principal-use authorization and 'special use districts' for conditional or discretionary authorization.", "Preserve the legal approval posture in summary, but keep only the district names in value.", "If the jurisdiction is unzoned, statewide, or otherwise does not use district-style land-use categories for the operative geothermal rule, omit district features rather than inventing a district mapping.", "If the ordinance does not explicitly list district names, omit the feature rather than paraphrasing a generic zoning statement." diff --git a/compass/extraction/geothermal_electricity/plugin_config.yaml b/compass/extraction/geothermal_electricity/plugin_config.yaml index 6fe547bbb..7a0824d68 100644 --- a/compass/extraction/geothermal_electricity/plugin_config.yaml +++ b/compass/extraction/geothermal_electricity/plugin_config.yaml @@ -141,4 +141,33 @@ heuristic_keywords: - "cannabis cultivation" - "commercial cannabis" -collection_prompts: True +collection_prompts: + - key: contains_relevant_text + label: geothermal applicability or governing standards + prompt: |- + You are validating whether an ordinance text chunk should be retained + for utility-scale geothermal electricity extraction. + + Return JSON (not markdown) with exactly two keys: + 1) "{key}" (boolean) + 2) "explanation" (string) + + Set "{key}" = true when the chunk contains substantive regulatory text + that is likely usable for geothermal electricity extraction, including + any of the following: + - explicit geothermal electricity language; OR + - geothermal exploration/drilling/wells/injection/production terms; OR + - operative zoning or development standards, use-table notes, permit + requirements, siting constraints, hazard/site-condition requirements, + decommissioning, bonding, lighting, fencing, color, or visual-impact + requirements that can govern listed geothermal or utility facility uses + within the same ordinance structure. + + Treat enforceable table notes and standards as relevant even when the + same chunk does not repeat the word "geothermal", as long as the text is + substantive ordinance language (not only headings/indexes). + + Set "{key}" = false only when the chunk is clearly irrelevant + (e.g., table of contents, navigation text, or content focused on + unrelated technologies such as solar, wind, battery storage, cannabis, + or geothermal heat pumps/HVAC/direct-use only). diff --git a/compass/services/openai.py b/compass/services/openai.py index a089d6cc9..c699287cc 100644 --- a/compass/services/openai.py +++ b/compass/services/openai.py @@ -1,6 +1,7 @@ """COMPASS Ordinances OpenAI service amd utils""" import logging +import re import openai from elm.base import ApiBase @@ -14,6 +15,14 @@ logger = logging.getLogger(__name__) +_UNSUPPORTED_PARAM_MSG_MARKERS = ( + "does not support", + "unsupported value", + "unsupported parameter", + "unknown parameter", + "only the default", + "not supported", +) def usage_from_response(current_usage, response): @@ -136,6 +145,7 @@ def __init__( service_tag=service_tag, ) self.client = client + self._unsupported_call_kwargs = set() async def process( self, @@ -222,18 +232,39 @@ def _update_pb_cost(self, response): ) async def _call_gpt(self, **kwargs): """Query Chat GPT with user inputs""" - try: - return await self.client.chat.completions.create(**kwargs) - except openai.BadRequestError: - messages = kwargs.get("messages") - if messages: - logger.exception( - "Got 'BadRequestError' for the following messages:\n\t%s", - "\n\t".join([f"{m!r}" for m in messages]), + active_kwargs = dict(kwargs) + self._drop_known_unsupported_kwargs(active_kwargs) + + while True: + try: + return await self.client.chat.completions.create(**active_kwargs) + except openai.BadRequestError as error: + unsupported_kwarg = _unsupported_call_kwarg(error, active_kwargs) + if unsupported_kwarg is None: + messages = kwargs.get("messages") + if messages: + logger.exception( + "Got 'BadRequestError' for the following " + "messages:\n\t%s", + "\n\t".join([f"{m!r}" for m in messages]), + ) + else: + logger.exception("Got 'BadRequestError'") + raise + + self._unsupported_call_kwargs.add(unsupported_kwarg) + active_kwargs.pop(unsupported_kwarg, None) + logger.warning( + "Retrying %r call without unsupported kwarg %r after " + "provider rejected it", + self.model_name, + unsupported_kwarg, ) - else: - logger.exception("Got 'BadRequestError'") - raise + + def _drop_known_unsupported_kwargs(self, kwargs): + """Drop kwargs previously rejected for this service instance""" + for kwarg in self._unsupported_call_kwargs: + kwargs.pop(kwarg, None) def _get_response_message(response): @@ -241,3 +272,34 @@ def _get_response_message(response): if response is None: return None return response.choices[0].message.content + + +def _unsupported_call_kwarg(error, request_kwargs): + """str | None: Recoverable unsupported top-level call kwarg name""" + error_body = getattr(error, "body", {}) or {} + kwarg = None + message = str(error) + + if isinstance(error_body, dict): + error_info = error_body.get("error", {}) + kwarg = error_info.get("param") + message = error_info.get("message") or message + elif isinstance(error_body, str): + message = error_body or message + + if not isinstance(kwarg, str): + match = re.search(r"['\"]param['\"]:\s*['\"]([^'\"]+)['\"]", message) + if match: + kwarg = match.group(1) + + message = message.casefold() + + if not isinstance(kwarg, str): + return None + if kwarg not in request_kwargs: + return None + if kwarg in {"messages", "model"}: + return None + if any(marker in message for marker in _UNSUPPORTED_PARAM_MSG_MARKERS): + return kwarg + return None diff --git a/tests/python/unit/services/test_services_openai.py b/tests/python/unit/services/test_services_openai.py index 4e0e21321..61f258804 100644 --- a/tests/python/unit/services/test_services_openai.py +++ b/tests/python/unit/services/test_services_openai.py @@ -10,6 +10,7 @@ count_tokens, usage_from_response, OpenAIService, + _unsupported_call_kwarg, ) from compass.services.usage import UsageTracker from compass.utilities.enums import LLMUsageCategory @@ -122,5 +123,92 @@ async def _test_response(*args, **kwargs): # ruff:ignore[unused-async] } +@pytest.mark.asyncio +async def test_openai_service_retries_without_unsupported_kwarg( + sample_openai_response, monkeypatch +): + """Unsupported kwargs are removed and retried once dynamically""" + call_kwargs = [] + + async def _test_response(*args, **kwargs): # ruff:ignore[unused-async] + call_kwargs.append(kwargs.copy()) + if len(call_kwargs) == 1: + response = httpx.Response(400) + response.request = httpx.Request(method="POST", url="https://test") + raise openai.BadRequestError( + "unsupported parameter", + response=response, + body={ + "error": { + "message": ( + "Unsupported value: 'temperature' does not support 0" + ), + "type": "invalid_request_error", + "param": "temperature", + "code": "400", + } + }, + ) + + return sample_openai_response(kwargs=kwargs) + + client = openai.AsyncOpenAI(api_key="dummy") + monkeypatch.setattr( + client.chat.completions, + "create", + _test_response, + raising=True, + ) + openai_service = OpenAIService(client, model_name="gpt-5.6-terra") + + message = await openai_service.process( + messages=TEST_MESSAGES_1, temperature=0, timeout=300 + ) + assert message == "test_response" + assert len(call_kwargs) == 2 + assert "temperature" in call_kwargs[0] + assert "temperature" not in call_kwargs[1] + assert "temperature" in openai_service._unsupported_call_kwargs + + +def test_unsupported_call_kwarg_ignores_non_recoverable_bad_request(): + """Context-length bad requests are not treated as unsupported kwargs""" + response = httpx.Response(400) + response.request = httpx.Request(method="POST", url="https://test") + error = openai.BadRequestError( + "context too long", + response=response, + body={ + "error": { + "message": "Input tokens exceed the configured limit", + "type": "invalid_request_error", + "param": "messages", + "code": "context_length_exceeded", + } + }, + ) + unsupported = _unsupported_call_kwarg( + error, {"messages": TEST_MESSAGES_2, "timeout": 300} + ) + assert unsupported is None + + +def test_unsupported_call_kwarg_parses_param_from_error_message(): + """Fallback parser recovers kwarg when error body is unstructured""" + response = httpx.Response(400) + response.request = httpx.Request(method="POST", url="https://test") + error = openai.BadRequestError( + "Error code: 400 - {'error': {'message': \"Unsupported value: " + "'temperature' does not support 0\", 'type': " + "'invalid_request_error', 'param': 'temperature', 'code': '400'}}", + response=response, + body=None, + ) + unsupported = _unsupported_call_kwarg( + error, {"messages": TEST_MESSAGES_2, "temperature": 0} + ) + assert unsupported == "temperature" + + if __name__ == "__main__": pytest.main(["-q", "--show-capture=all", Path(__file__), "-rapP"]) From b61ab60bcc61e264e0565c6bd1291a04fee8b00e Mon Sep 17 00:00:00 2001 From: Byron Pullutasig <115118857+bpulluta@users.noreply.github.com> Date: Thu, 6 Aug 2026 08:28:57 -0400 Subject: [PATCH 2/2] Cap unsupported-kwarg retries and fix docstring typo Address PR review feedback: correct the module docstring typo and bound the number of unsupported top-level kwarg drops per request so provider rejections cannot trigger unbounded paid retries. --- compass/services/openai.py | 27 ++++++++- .../unit/services/test_services_openai.py | 59 ++++++++++++++++++- 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/compass/services/openai.py b/compass/services/openai.py index c699287cc..92597ea51 100644 --- a/compass/services/openai.py +++ b/compass/services/openai.py @@ -1,4 +1,4 @@ -"""COMPASS Ordinances OpenAI service amd utils""" +"""COMPASS Ordinances OpenAI service and utils""" import logging import re @@ -23,6 +23,7 @@ "only the default", "not supported", ) +_MAX_UNSUPPORTED_KWARG_DROPS = 3 def usage_from_response(current_usage, response): @@ -234,12 +235,31 @@ async def _call_gpt(self, **kwargs): """Query Chat GPT with user inputs""" active_kwargs = dict(kwargs) self._drop_known_unsupported_kwargs(active_kwargs) + num_dropped_kwargs = 0 while True: try: - return await self.client.chat.completions.create(**active_kwargs) + return await self.client.chat.completions.create( + **active_kwargs + ) except openai.BadRequestError as error: - unsupported_kwarg = _unsupported_call_kwarg(error, active_kwargs) + unsupported_kwarg = _unsupported_call_kwarg( + error, active_kwargs + ) + if ( + unsupported_kwarg is not None + and num_dropped_kwargs >= _MAX_UNSUPPORTED_KWARG_DROPS + ): + logger.warning( + "Exceeded maximum number of unsupported kwarg " + "retries (%d) for %r call; last rejected kwarg " + "was %r", + _MAX_UNSUPPORTED_KWARG_DROPS, + self.model_name, + unsupported_kwarg, + ) + unsupported_kwarg = None + if unsupported_kwarg is None: messages = kwargs.get("messages") if messages: @@ -254,6 +274,7 @@ async def _call_gpt(self, **kwargs): self._unsupported_call_kwargs.add(unsupported_kwarg) active_kwargs.pop(unsupported_kwarg, None) + num_dropped_kwargs += 1 logger.warning( "Retrying %r call without unsupported kwarg %r after " "provider rejected it", diff --git a/tests/python/unit/services/test_services_openai.py b/tests/python/unit/services/test_services_openai.py index 61f258804..48d0a76bc 100644 --- a/tests/python/unit/services/test_services_openai.py +++ b/tests/python/unit/services/test_services_openai.py @@ -10,6 +10,7 @@ count_tokens, usage_from_response, OpenAIService, + _MAX_UNSUPPORTED_KWARG_DROPS, _unsupported_call_kwarg, ) from compass.services.usage import UsageTracker @@ -141,7 +142,8 @@ async def _test_response(*args, **kwargs): # ruff:ignore[unused-async] body={ "error": { "message": ( - "Unsupported value: 'temperature' does not support 0" + "Unsupported value: 'temperature' does not " + "support 0" ), "type": "invalid_request_error", "param": "temperature", @@ -171,6 +173,61 @@ async def _test_response(*args, **kwargs): # ruff:ignore[unused-async] assert "temperature" in openai_service._unsupported_call_kwargs +@pytest.mark.asyncio +async def test_openai_service_caps_unsupported_kwarg_retries(monkeypatch): + """Unsupported kwarg drops are capped to avoid unbounded retries""" + call_kwargs = [] + rejected_kwargs = [ + "temperature", + "top_p", + "presence_penalty", + "frequency_penalty", + "seed", + ] + + async def _test_response(*args, **kwargs): # ruff:ignore[unused-async] + call_kwargs.append(kwargs.copy()) + param = rejected_kwargs[len(call_kwargs) - 1] + response = httpx.Response(400) + response.request = httpx.Request(method="POST", url="https://test") + raise openai.BadRequestError( + "unsupported parameter", + response=response, + body={ + "error": { + "message": ( + f"Unsupported value: {param!r} does not support 0" + ), + "type": "invalid_request_error", + "param": param, + "code": "400", + } + }, + ) + + client = openai.AsyncOpenAI(api_key="dummy") + monkeypatch.setattr( + client.chat.completions, + "create", + _test_response, + raising=True, + ) + openai_service = OpenAIService(client, model_name="gpt-5.6-terra") + + with pytest.raises(openai.BadRequestError): + await OpenAIService._call_gpt.__wrapped__( + openai_service, + messages=TEST_MESSAGES_1, + temperature=0, + top_p=1, + presence_penalty=0, + frequency_penalty=0, + seed=42, + ) + + assert len(call_kwargs) == _MAX_UNSUPPORTED_KWARG_DROPS + 1 + + def test_unsupported_call_kwarg_ignores_non_recoverable_bad_request(): """Context-length bad requests are not treated as unsupported kwargs""" response = httpx.Response(400)