From f6789e44534fe4dc44b0b3e68354db0cb21b1b4b Mon Sep 17 00:00:00 2001 From: Daniel Yudelevich Date: Wed, 23 Sep 2026 17:19:23 -0700 Subject: [PATCH] Address PR #32 review findings before the 0.4.0 release A leftover conflict marker from the find_emails rebase shipped in the 0.4.0 changelog. bulk companies skipped re-excluding the resumed CSV whenever any --exclusion-query-id was passed, assuming those IDs were the run's own round lists. A customer suppression list is the common case, so the next page could return only already-written companies and stop early. The CSV is now always re-excluded. File inputs caught only FileNotFoundError and JSON errors; a directory, permission error or non-UTF-8 file escaped as a traceback instead of the CLI error envelope. The SDK mirrored the platform's per-shape geo rules but not the cross-field ones (lat/lon pairing, radius without a centre, invalid standalone radius, 10-shape total), so those failed only at the API. --- CHANGELOG.md | 5 ++- .../src/discolike_cli/_inputs.py | 4 +++ .../discolike-cli/src/discolike_cli/bulk.py | 9 +++-- .../src/discolike_cli/queries.py | 2 ++ packages/discolike-cli/tests/test_bulk_cli.py | 34 +++++++++++++++++++ packages/discolike-cli/tests/test_inputs.py | 17 ++++++++++ packages/discolike/src/discolike/_models.py | 19 +++++++++++ packages/discolike/tests/test_models.py | 33 ++++++++++++++++++ 8 files changed, 115 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a09ab0..9283b6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ - SDK/CLI: `discogen.process` and `discogen.process_personas` take `typed_columns`, and the CLI gains `--typed-columns`. It opts the query into the detector answering yes/no, fixed-set and scale columns with a TypeSafe judgment model instead of generated prose; which columns are typed and how is decided server-side from the query text, not passed in by the caller. Off by default. See `include_confidence` below for the sibling flag that adds a confidence column to each typed column. - SDK/CLI: `discogen.process` and `discogen.process_personas` take `include_confidence`, and the CLI gains `--include-confidence`. It applies to typed columns, which a DiscoGen run answers with a TypeSafe judgment model rather than generated prose; with it on, each typed column gets a sibling confidence column. Off by default, and it changes display only - the answers and their probabilities come back either way. - SDK: `contacts.generate` can run without an LLM key. Pass `integration_id=NATIVE_ENGINE` (exported from `discolike`) for DiscoLike Groove, DiscoLike's native extractor, or omit `integration_id` to get your default LLM integration and native when you have none. The native engine returns every person the search surfaces with a title and does not validate titles against `icp_text`; a search provider is still required. `JobStatus` gains `title_validation` (`"llm"` or `"none"`, `None` on non-ContaGen jobs) so you can tell which engine ran. -- SDK: discover and count take `sub_industry` / `negate_sub_industry` (217 second-level labels scoped to their parent industry category; a bare label like `ROOFING` adds `CONSTRUCTION` to `category` server-side) and a multi-shape geo filter: `geo` (`lat,lon` or `lat,lon,radius`), `bbox` (`min_lat,min_lon,max_lat,max_lon`, longitudes may wrap the antimeridian) and `lat` / `lon` / `radius` (`50km`, `30mi`, or a bare number meaning kilometres; defaults to 50km). `geo` and `bbox` are lists, every circle and box is OR'd with the others and with the `lat`/`lon` centre, and a query carries at most 10 shapes. Passing a single `bbox` or `geo` string still works and is sent as a one-element list. The SDK applies the platform's own shape rules before the request leaves the client: a box is four finite numbers with `-90 <= min_lat < max_lat <= 90`, longitudes within ±180 and `min_lon` different from `max_lon`; a circle is a valid coordinate pair with an optional radius above 0 and at most 1000km. +- SDK: discover and count take `sub_industry` / `negate_sub_industry` (217 second-level labels scoped to their parent industry category; a bare label like `ROOFING` adds `CONSTRUCTION` to `category` server-side) and a multi-shape geo filter: `geo` (`lat,lon` or `lat,lon,radius`), `bbox` (`min_lat,min_lon,max_lat,max_lon`, longitudes may wrap the antimeridian) and `lat` / `lon` / `radius` (`50km`, `30mi`, or a bare number meaning kilometres; defaults to 50km). `geo` and `bbox` are lists, every circle and box is OR'd with the others and with the `lat`/`lon` centre, and a query carries at most 10 shapes. Passing a single `bbox` or `geo` string still works and is sent as a one-element list. The SDK applies the platform's own shape rules before the request leaves the client: a box is four finite numbers with `-90 <= min_lat < max_lat <= 90`, longitudes within ±180 and `min_lon` different from `max_lon`; a circle is a valid coordinate pair with an optional radius above 0 and at most 1000km. The cross-field rules are checked locally too: `lat` and `lon` come together, `radius` needs them and follows the same radius rule, and the centre, `geo` and `bbox` total at most 10 shapes. - SDK: `CompanyProfile` gains `sub_industry` (label:confidence, `None` when the domain was never scored against its category's sub-labels), `latitude`, `longitude` and `geo_precision`. The coordinates come back under their full names, and a response still using the pre-rename `lat` / `lon` keys parses into them, so the SDK works against an API on either side of that release. The `lat` / `lon` discover and count filters keep their names. - CLI: `discover` and `count` gain `--sub-industry`, `--negate-sub-industry`, `--lat`, `--lon`, `--radius`, `--geo` and `--bbox`. `--geo` and `--bbox` are repeatable, one flag per shape. - CLI: error envelopes on stderr gain a stable snake_case `code` (`validation_error`, `auth_required`, `auth_invalid`, `plan_access`, `rate_limited`, `network_error`, `not_found`, `server_error`, `job_failed`, `job_timeout`) and an `exit_code`, next to the existing `error` class name, `message`, and `status_code`. Agents and scripts should branch on `code` or the exit code. @@ -21,10 +21,9 @@ - CLI: domain lists from files — `--domains-file PATH` (CSV with a `domain` column, or one domain per line; merged with `--domain`) on `queries create-exclusion-list`, `contacts discover`, `contacts search`, `contacts count`, `contacts generate` and `discogen run`; `validate-icp --file` gains the `--domains-file` alias and reads the same shapes. `--domain` on `contacts generate` / `discogen run` is now optional when the file is given. - CLI: `--params-file PATH` on `discover`, `count`, `contacts discover|search|count` — a JSON object of API parameter names (an app form copied over), validated locally with the SDK request model before the call. Precedence: file < `--param` < flags. - CLI: `discover --exclude-domains-file PATH` merges a file into the inline `exclude_domain` list (100 max, checked locally). -- CLI: `discolike bulk companies|estimate|contacts` — the volume pipeline as plain CLI calls. `companies` loops `discover` at up to 10,000 per page, saves each page as an exclusion list (`-round-N`) for the next page, appends to `--out` as pages land and resumes from that CSV on rerun (warns at the 250,000-domain exclusion capacity). `estimate` sums the free `contacts count` over 1,000-domain slices and reports the upper bound at `--per-company`. `contacts` slices the domain list at `10000 / per-company` per `contacts discover` call (`results_by_company`, no `offset`), flattens to one row per contact, checkpoints finished slices in `.checkpoint` (stamped with the domains, `--per-company` and filters it was written for, so a rerun with different inputs is refused instead of silently skipped) and skips them on rerun. All three validate the request locally before the first billable call, keep one call in flight under `--rate-limit`, retry on 429/5xx, and print a JSON summary on stdout. +- CLI: `discolike bulk companies|estimate|contacts` — the volume pipeline as plain CLI calls. `companies` loops `discover` at up to 10,000 per page, saves each page as an exclusion list (`-round-N`) for the next page, appends to `--out` as pages land and resumes from that CSV on rerun, re-excluding every domain already in it even when `--exclusion-query-id` is given (warns at the 250,000-domain exclusion capacity). `estimate` sums the free `contacts count` over 1,000-domain slices and reports the upper bound at `--per-company`. `contacts` slices the domain list at `10000 / per-company` per `contacts discover` call (`results_by_company`, no `offset`), flattens to one row per contact, checkpoints finished slices in `.checkpoint` (stamped with the domains, `--per-company` and filters it was written for, so a rerun with different inputs is refused instead of silently skipped) and skips them on rerun. All three validate the request locally before the first billable call, keep one call in flight under `--rate-limit`, retry on 429/5xx, and print a JSON summary on stdout. - SDK: `ContactGenerateRequest.find_emails` (default `False`) asks the platform to run the email finder over every named, email-less row before the ContaGen job completes, filling `email` and `email_status` on those rows. Found addresses bill under the finder's rules; the rest of the job stays unbilled. - CLI: `discolike contacts generate --find-emails` sets the same flag, so a generate run can return name + title + email triples without chaining `email find-batch` afterwards. ->>>>>>> dffdd27 (feat: find_emails on ContactGenerateRequest and --find-emails on contacts generate) - SDK: OAuth refreshes now resend the RFC 8707 `resource` the token was issued for. `OAuthCredential` gains an optional `resource` field, filled in by `exchange_code` and persisted to the config file; credentials stored by earlier releases load with `resource=None` and refresh as before until the next `discolike auth login`. Without it, an authorization server configured with a default resource could re-bind a refreshed REST token to another audience. - SDK (behavior change, no code change): company `address.state` now comes back from the API as the subdivision name ("California", "Tokyo") instead of the ISO code ("CA", "13"). `CompanyAddress.state` is still `str | None` and needs no migration, but anything joining or grouping on that value as a code has to resolve it. The contact's own `state` is unchanged and stays a code. - SDK: state filters accept a code or a name, resolved server-side against the countries you selected. Discover/count still take one `country` value, but that value may be a region alias (`EU`, `APAC`, `DACH`) and the state resolves against every member. Contacts state filters accept multiple countries and drop a value they cannot resolve rather than erroring. diff --git a/packages/discolike-cli/src/discolike_cli/_inputs.py b/packages/discolike-cli/src/discolike_cli/_inputs.py index d0adf9f..9728c88 100644 --- a/packages/discolike-cli/src/discolike_cli/_inputs.py +++ b/packages/discolike-cli/src/discolike_cli/_inputs.py @@ -23,6 +23,8 @@ def read_domains_file(path: pathlib.Path) -> list[str]: rows = list(csv.reader(handle)) except FileNotFoundError as exc: raise typer.BadParameter(f"domains file not found: {path}") from exc + except (OSError, UnicodeDecodeError) as exc: + raise typer.BadParameter(f"domains file {path} could not be read: {exc}") from exc header = [cell.strip().lower() for cell in rows[0]] if rows else [] column = header.index(DOMAIN_COLUMN) if DOMAIN_COLUMN in header else 0 body = rows[1:] if DOMAIN_COLUMN in header else rows @@ -46,6 +48,8 @@ def read_params_file(path: pathlib.Path) -> dict[str, Any]: loaded = json.loads(path.read_text()) except FileNotFoundError as exc: raise typer.BadParameter(f"params file not found: {path}") from exc + except (OSError, UnicodeDecodeError) as exc: + raise typer.BadParameter(f"params file {path} could not be read: {exc}") from exc except json.JSONDecodeError as exc: raise typer.BadParameter(f"params file {path} must contain valid JSON: {exc}") from exc if not isinstance(loaded, dict): diff --git a/packages/discolike-cli/src/discolike_cli/bulk.py b/packages/discolike-cli/src/discolike_cli/bulk.py index cd5fa44..7e622ee 100644 --- a/packages/discolike-cli/src/discolike_cli/bulk.py +++ b/packages/discolike-cli/src/discolike_cli/bulk.py @@ -217,13 +217,12 @@ def arm_exclusion(domains: list[str], label: str) -> None: result = _call_with_retry(limiter, functools.partial(client.queries.create_exclusion_list, request)) exclusion_ids.append(str(result.query_id)) - if seen and not exclusion_ids: + if seen: + # --exclusion-query-id may be a suppression list unrelated to this CSV, so the file is always re-excluded. + supplied = len(exclusion_ids) for index, batch in _chunked(seen, MAX_RECORDS): arm_exclusion(batch, f"{run_name}-resume-{index}") - _log( - f"re-armed {len(exclusion_ids)} exclusion list(s) from the existing file " - "(pass the original --exclusion-query-id values to reuse them instead)" - ) + _log(f"re-armed {len(exclusion_ids) - supplied} exclusion list(s) from the existing file") added = 0 rounds = 0 diff --git a/packages/discolike-cli/src/discolike_cli/queries.py b/packages/discolike-cli/src/discolike_cli/queries.py index 1c79312..bd35625 100644 --- a/packages/discolike-cli/src/discolike_cli/queries.py +++ b/packages/discolike-cli/src/discolike_cli/queries.py @@ -99,6 +99,8 @@ def save_results_command( data = json.loads(input_path.read_text()) except FileNotFoundError as exc: raise typer.BadParameter(f"--input file not found: {input_path}") from exc + except (OSError, UnicodeDecodeError) as exc: + raise typer.BadParameter(f"--input file {input_path} could not be read: {exc}") from exc except json.JSONDecodeError as exc: raise typer.BadParameter(f"--input file {input_path} must contain valid JSON: {exc}") from exc diff --git a/packages/discolike-cli/tests/test_bulk_cli.py b/packages/discolike-cli/tests/test_bulk_cli.py index 57695fd..5863988 100644 --- a/packages/discolike-cli/tests/test_bulk_cli.py +++ b/packages/discolike-cli/tests/test_bulk_cli.py @@ -229,6 +229,40 @@ def test_bulk_companies_resumes_from_existing_csv( assert summary == {**summary, "companies": 28, "new": 3} +def test_bulk_companies_resume_still_excludes_the_csv_when_a_suppression_list_is_supplied( + install_build_client: Callable[[Handler], None], tmp_path: Path +) -> None: + out = tmp_path / "companies.csv" + out.write_text("domain,name,country,employees,similarity\n" + "".join(f"old{i}.com,,,,\n" for i in range(25))) + recorder = Recorder(discover=[_companies("n", 3)], queries_exclusion_list=[{"query_id": "q-resume"}]) + install_build_client(recorder) + result = runner.invoke( + app, + [ + "bulk", + "companies", + "--icp-prompt", + "x", + "--page-size", + "20", + "--max-companies", + "100", + "--run-name", + "r", + "--exclusion-query-id", + "q-customers", + "--out", + str(out), + *NO_LIMIT, + ], + ) + assert result.exit_code == 0, result.output + assert recorder.bodies("/v1/queries/exclusion-list") == [ + {"query_name": "r-resume-0", "domains": [f"old{i}.com" for i in range(25)]} + ] + assert recorder.params("/v1/discover")[0].get_list("exclusion_query_id") == ["q-customers", "q-resume"] + + def test_bulk_companies_overwrite_ignores_existing_csv( install_build_client: Callable[[Handler], None], tmp_path: Path ) -> None: diff --git a/packages/discolike-cli/tests/test_inputs.py b/packages/discolike-cli/tests/test_inputs.py index e88781c..0e14809 100644 --- a/packages/discolike-cli/tests/test_inputs.py +++ b/packages/discolike-cli/tests/test_inputs.py @@ -62,3 +62,20 @@ def test_read_params_file_rejects_invalid_json(tmp_path: Path) -> None: def test_read_params_file_missing_is_bad_parameter(tmp_path: Path) -> None: with pytest.raises(typer.BadParameter, match="not found"): read_params_file(tmp_path / "nope.json") + + +def test_read_domains_file_directory_is_bad_parameter(tmp_path: Path) -> None: + with pytest.raises(typer.BadParameter, match="could not be read"): + read_domains_file(tmp_path) + + +def test_read_domains_file_bad_encoding_is_bad_parameter(tmp_path: Path) -> None: + path = tmp_path / "latin1.csv" + path.write_bytes(b"caf\xe9.com\n") + with pytest.raises(typer.BadParameter, match="could not be read"): + read_domains_file(path) + + +def test_read_params_file_directory_is_bad_parameter(tmp_path: Path) -> None: + with pytest.raises(typer.BadParameter, match="could not be read"): + read_params_file(tmp_path) diff --git a/packages/discolike/src/discolike/_models.py b/packages/discolike/src/discolike/_models.py index ef24e29..914f48a 100644 --- a/packages/discolike/src/discolike/_models.py +++ b/packages/discolike/src/discolike/_models.py @@ -21,6 +21,7 @@ KM_PER_MILE = 1.609344 RADIUS_KM_SUFFIX = "km" RADIUS_MILE_SUFFIX = "mi" +MAX_GEO_SHAPES = 10 class DiscolikeModel(pydantic.BaseModel): @@ -47,6 +48,24 @@ def _validate_bbox(cls, value: object) -> object: def _validate_geo(cls, value: object) -> object: return _validate_shapes(value=value, name="geo", fmt=GEO_FORMAT, rejection=_geo_rejection) + @pydantic.model_validator(mode="after") + def _validate_geo_shapes(self) -> DiscolikeRequest: + if "lat" not in type(self).model_fields: + return self + lat, lon, radius = getattr(self, "lat", None), getattr(self, "lon", None), getattr(self, "radius", None) + if (lat is None) != (lon is None): + raise ValueError("lat and lon must be supplied together") + if radius is not None: + if lat is None: + raise ValueError("radius needs lat and lon") + reason = _radius_km_rejection(radius) + if reason is not None: + raise ValueError(reason) + total = (lat is not None) + len(getattr(self, "geo", None) or []) + len(getattr(self, "bbox", None) or []) + if total > MAX_GEO_SHAPES: + raise ValueError(f"{total} geo shapes (lat/lon, geo and bbox together); at most {MAX_GEO_SHAPES}") + return self + def to_wire(self) -> dict[str, Any]: return self.model_dump(mode="json", exclude_unset=True, by_alias=True) diff --git a/packages/discolike/tests/test_models.py b/packages/discolike/tests/test_models.py index c0d407f..044a554 100644 --- a/packages/discolike/tests/test_models.py +++ b/packages/discolike/tests/test_models.py @@ -2,6 +2,8 @@ import pytest from discolike._models import DiscolikeRequest +from discolike.requests import CountParams +from discolike.requests import DiscoverParams class _Probe(DiscolikeRequest): @@ -137,3 +139,34 @@ def test_bbox_rejects_boxes_the_platform_would_reject(value: str, message: str) def test_bbox_none_stays_unvalidated() -> None: assert _BboxProbe(bbox=None).to_wire() == {"bbox": None} + + +@pytest.mark.parametrize( + ("kwargs", "message"), + [ + ({"lat": 40.0}, "lat and lon must be supplied together"), + ({"lon": -74.0}, "lat and lon must be supplied together"), + ({"radius": "10km"}, "radius needs lat and lon"), + ({"lat": 40.0, "lon": -74.0, "radius": "wide"}, "must be a number optionally suffixed"), + ({"lat": 40.0, "lon": -74.0, "radius": "1001km"}, "greater than 0"), + ({"geo": ["30.27,-97.74"] * 11}, "11 geo shapes"), + ( + {"lat": 40.0, "lon": -74.0, "geo": ["30.27,-97.74"] * 5, "bbox": ["40.4,-74.3,41.0,-73.7"] * 5}, + "11 geo shapes", + ), + ], +) +def test_discover_params_rejects_geo_combinations_the_platform_would_reject( + kwargs: dict[str, object], message: str +) -> None: + with pytest.raises(pydantic.ValidationError, match=message): + DiscoverParams.model_validate(kwargs) + + +def test_discover_params_accepts_ten_shapes_and_a_centre_with_radius() -> None: + DiscoverParams(lat=40.0, lon=-74.0, radius="30mi", geo=["30.27,-97.74"] * 4, bbox=["40.4,-74.3,41.0,-73.7"] * 5) + + +def test_count_params_rejects_lat_without_lon() -> None: + with pytest.raises(pydantic.ValidationError, match="supplied together"): + CountParams(lat=40.0)