Feat/itsm connector client changes - #3
Merged
Merged
Conversation
Adds format_ev_datetime, the inverse the interval filter builders need. Behaviour-preserving for reporting: its eight existing tests are unchanged.
ev_since_filter/ev_between_filter emit FIELD:(a;b), the only range grammar this API honours. ev_contains_filter/ev_starts_with_filter use '~' with an explicit wildcard, correcting the belief that '~' is exact-match only.
_TIMESTAMP_RE accepted well-shaped but impossible timestamps (9999-99-99, 25:61:61) and Unicode digits, and a dropped condition returns the whole table rather than an error, so a typo'd watermark silently degraded a sync sweep to a full-table read. _interval_bound now also requires parse_ev_datetime(text) to succeed. Switch \d -> [0-9] and .match()+$ -> fullmatch() so the anchoring no longer depends on the prior .strip(). Add acceptance-side coverage for the five renderings measured live, and soften the ev_since_filter docstring's "cannot be malformed" overclaim about datetime input. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…unsorted EasyVista honours 'FIELD DESC' but silently drops 'FIELD:DESC', returning the default order. Measured live 2026-08-17; closes O-DIR-1.
The colon-vs-space rule was measured on LAST_UPDATE (a date column), never on RFC_NUMBER. Applying it to RFC_NUMBER is sound inference from a syntactic rule, not a live-verified fact about that field -- Task 9's live guard is what actually pins RFC_NUMBER DESC. Prose-only: no constant, assertion, or behaviour changed.
BREAKING: Request/Employee timestamp fields are datetime, not str. The format is offset-bearing ISO 8601 with ms precision (verified live), so no caller-side server_timezone is needed. Write models untouched: write format unverified. Also fixes tests/test_reporting.py::test_window_excludes_missing_or_unparseable_dates (renamed to test_window_excludes_missing_dates): its "garbage"-dated ticket can no longer be constructed, since Request.model_validate now rejects a malformed CREATION_DATE_UT before aggregate_tickets ever sees it. That coverage moved to models/tests/test_common.py's new unparseable-timestamp test.
CRITICAL: _fields._text() and references._scalar() returned "" / None for
anything that wasn't a str/int, so a retyped timestamp column silently
vanished from every consumer of a model_dump(by_alias=True) dict:
TicketContext.to_markdown() dropped its Created/Updated rows, and
Request.reference("LAST_UPDATE") / aggregate_tickets(dimensions=(...)) on a
timestamp column resolved to nothing. Both extractors now render a datetime
via format_ev_datetime (falling back to plain .isoformat() for the naive case,
since neither extractor may raise), fixed once at the shared root rather than
patched per call site.
Also fixes the Important finding that OptionalDateTime didn't honour its own
"aware" promise for a datetime passed in directly (only strings routed through
parse_ev_datetime's naive->UTC normalization; a bare second isinstance(value,
str) guard skipped it for everything else) -- dropped, so every value now
routes through parse_ev_datetime uniformly.
Three minors: test_employee.py now directly asserts last_update parses (it
had no failing pre-existing assertion, so was previously only covered
transitively); the unparseable-timestamp test now pins that the
ValidationError names the field (errors()[0]["loc"] == ("when",)), which is
the entire reason the coercer hands the original value back instead of
raising itself; reporting.py's docstring now explains why the
now-partially-unreachable missing/unparseable guard is kept rather than
deleted as dead code.
CHANGELOG.md intentionally untouched -- Task 10 owns changelog consolidation.
All present on the item-level GET and reachable via a fields= projection; previously available only as untyped extras. Closes EV-R1.
…laim The docstring and field comment said the default LIST projection omits all ten new fields while the same sentence listed ACTION_NUMBER and DONE_BY_ID among what it carries. The real split is three-way: six fields genuinely absent from the default list row, two already present top-level, and two (ACTION_TYPE_ID, REQUEST_ID) present on a list row only nested inside ACTION_TYPE/REQUEST -- so the declared top-level-aliased field reads None off a list row despite the data being present. Also notes that only one of a fresh ticket's ~12 auto-spawned actions is human-authored and that generated actions carry an undeclared STATUS_ID_ON_CREATE. Parametrized the empty-string-sentinel test over all eight OptionalInt fields so its name matches what it actually checks. No field, alias or type changed.
The actions list honours fields= and grants every scalar requested, so a caller can read all timestamps and authors for a ticket in one request instead of one item fetch per action. Closes EV-R3.
Review found the "*" isn't-a-wildcard and dotted-path-is-dropped caveats only lived in the private builder comment, unreachable from help()/IDE tooltips. Adds one sentence to the client-facing docstring, mirrored verbatim into the sync tree. No behavior change.
Both live-verified by re-reading the record, not by status code. Uses the
top-level actions/{id} and nested requests/{rfc}/documents/{id} paths; the
alternatives return 403. Closes EV-R4.
Each verified writable by re-reading the ticket. severity_id and urgency_id deliberately excluded: one is refused, the other returns 590 while still applying (tracked as O-590-PARTIAL). Closes EV-R9, EV-R10.
Adds a differential change-window characterization (a single count cannot prove a range filter works on this API) and corrects three tilde tests whose assertions were right but whose stated conclusion over-generalized. Also settles live whether '%' is a wildcard for '~' (it is, matching '*' exactly), verifies ActionUpdate's lowercase-cased body actually lands through update_action, rewords ActionUpdate's docstring to name the field rather than quote a body it no longer sends, and corrects this module's own inherited assumption that every LAST_UPDATE comparison-operator rendering is silently dropped -- two of the three instead raise a hard type-mismatch error (590), only the colon-free rendering is structurally unparseable enough to be dropped.
Round-1 review found three Important defects in the change-window characterization: split_instants leaked a raw datetime into the comparison-operator f-strings (space separator, 6-digit fraction) despite its str-literal contract; that same test asserted two 590s with no control isolating them to the embedded operator rather than the column rejecting FIELD:"value" outright; and the closed-interval test was a single count that could not distinguish a real upper bound from one silently dropped down to the open-ended form. Also fixes RECENT_TICKETS_SORT's guard, which computed an unsorted baseline but never compared against it (monotonicity-only, same fate EV-R6's sibling test was written to avoid), a strict assertion in the percent-wildcard probe that could redden on a data-availability gap instead of skipping, two compound is-not-None asserts that risked printing a live instant on failure, and a stale ticket/action/update count in conftest.py's mutation-footprint docstring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
'~' is a pattern operator requiring an explicit wildcard (* or %), not exact-match-only -- degenerates to equality without one. Corrects the claim in the user guide, the search-syntax and asset-workflow skills, and the changelog; adds ev_contains_filter/ev_starts_with_filter examples in their place. Documents the change-window builders (ev_since_filter/ev_between_filter), the timestamp helpers (parse_ev_datetime/format_ev_datetime), and the two-fate comparison-operator behaviour (silent-drop vs. HTTP 590 type-mismatch, depending on whether FIELD: syntax survives). Closes the O-DIR-1 sort-token hedges in both the reporting-and-context and search-syntax skills now that FIELD DESC is live-confirmed. Consolidates the CHANGELOG's Unreleased section: merges the duplicate Changed headings, strikes the now-false "no datetime parsing is claimed" line, adds the scope note that only Employee.last_update is a true break relative to 0.1.0, and fills in the entries this branch was still missing (the four filter builders, list_actions(fields=...), update_action/ delete_document/ActionUpdate, Action's new fields, RequestUpdate's widening, and the RECENT_TICKETS_SORT fix).
Same failure mode as the earlier easyvista-asset-workflow catch: skill and changelog content outside the brief's file list still described a pre-task-9 API. delete_document, update_action and list_actions(fields=) now exist in easyvista-document-workflow and easyvista-ticket-actions; RequestUpdate's impact_id/owner_id/external_reference widening is documented in easyvista-ticket-workflow; skills/README.md's index no longer contradicts the skills it indexes. Rewrites the CHANGELOG's BREAKING scope note: the 0.1.0 git tag resolves to a commit ~150 commits after the 0.1.0 release commit the changelog documents, and at the tag commit all seven timestamp fields -- not just Employee.last_update -- were already str. Verified against both commits before rewriting. Also fixes a genuine dangling reference (ChangedRef, which never existed) in a test docstring to the real field it was describing (Action.updated_at).
…l map Add ActionUpdate to the _WRITE_MODELS dict so its snippets in easyvista-ticket-actions are now validated for keyword correctness. Add a new test_write_models_map_is_complete() test that asserts every EasyvistaWriteModel subclass exported from the package appears in the map. This prevents silent validation skips when a new write model is added without updating the map. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round-2 live probing found that EasyVista *accepts* an offset-less timestamp literal and reads it in a different zone. Measured against one instance, the same wall-clock text enumerated 13 rows with its offset and 11 without: the offset-less form moves the bound later and skips records, with no error of any kind. A watermark that silently skips is the worst outcome this grammar has. `format_ev_datetime` already refused a naive `datetime` on exactly this reasoning, and said so in its docstring. The string path was unguarded, so the identical hazard reached the wire by the other route. Both paths now refuse. A bare date stays legal -- day granularity has no time to misplace, and it is a form measured live as honoured. Also withdraws a claim this package shipped: `RequestUpdate`'s docstring said `DESCRIPTION` is empty on every ticket of the verified instance. A pooled 77-row sample across four orderings found `COMMENT` on 57 rows, `DESCRIPTION` on 27 and both on 24, the proportions flipping by slice. The earlier 0/15 reading was drawn from probe-authored tickets. The load-bearing claim is untouched and still verified -- `RequestUpdate.description` writes the `COMMENT` memo -- but the generalisation is gone, and with it any hope of auto-detecting an instance's body memo by sampling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`download_document` returns `bytes`, so a consumer mirroring attachments between systems has to hold the whole file. With the base64 inflation an upload applies, a 32 MB attachment peaked near 76 MB of worker memory for what is conceptually a pass-through. `stream_document` hands the body over in 64 KiB chunks (`chunk_size=` to change it) so the file never has to exist in memory whole. It accepts exactly what `download_document` accepts and resolves the URL through the same `resolve_url`, so the same-origin refusal, `follow_redirects=True` and the error mapping are shared rather than re-derived. Only the download direction can stream. EasyVista takes an attachment as base64 inside a JSON body, so `add_document` must materialise the whole payload before it can send anything; the asymmetry is the API's. The retry decision, which is the part worth questioning: a streamed response cannot be restarted once bytes have reached the caller, because restarting would deliver them twice, and this method will not silently duplicate data. But refusing to retry at all would make streaming strictly less reliable than `download_document`, which retries. So the retried unit is "open the download AND take its first chunk" -- everything inside `_open_stream`, which produces nothing the caller has seen yet and is therefore safe to repeat under the same policy `get_bytes` uses (same attempt count, same backoff, 590 still not retried). From the first chunk onwards nothing is retried: a transport failure surfaces as `EasyvistaConnectionError` and a partly consumed stream is never resumed, which is stated in the docstring because it is the caller's problem to handle. A test asserts the request count on a mid-body failure, so making this resumable fails loudly. Two consequences of streaming forced small decisions. `_raise_for_response` reads `.content`, which a streaming response refuses until the body has been read, so the error path reads it first -- that is what keeps a 403 on the streaming path identical to a 403 on the buffered one, asserted as an equality between the two rather than against a hardcoded type. And httpx spells its streaming methods with a leading `a` rather than the `Async` prefix unasync's convention knows about, so `aread` and `aiter_bytes` join `aclose` in TOKEN_REPLACEMENTS, with the rationale recorded there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of 8ac38fa found seven statements about `stream_document` that nothing backed, plus one connection released later than it reads. What was false and is now true: - The document-workflow skill's sync/async banner carved `async for` out for the `iter_*` methods only, so a reader following it literally wrote `await client.stream_document(doc)` -- a TypeError, because the method is deliberately not named `iter_*`. The carve-out now names it, and says why awaiting it fails. Only this skill's copy of the shared banner changed; it is the only one with a byte-streaming method. - `unasync_build.py`'s rationale for the `aread`/`aiter_bytes` entries claimed an unmapped httpx async name would raise `AttributeError`. It would not: httpx 0.28.1 defines `aread`, `aiter_bytes` AND `aclose` on `httpx.Response` itself, so in sync code the attribute exists and merely misbehaves -- an un-awaited coroutine leaving the body unread (then `ResponseNotRead` from the error mapping instead of the mapped EasyVista exception), or `TypeError: 'async_generator' object is not iterable`. Only `aclose` on the *client* is genuinely absent. The comment now says so, which is a stronger argument for mapping every one of these names than the version it replaces. - CHANGELOG called "a 32 MB attachment peaked near 76 MB of worker memory" a measurement of ours. It is not ours, appeared nowhere else in the repo, and pointed at the wrong half: of that peak only the download buffer is what this change removes, while the base64 payload `add_document` builds is untouched. The bullet now states the motivation without borrowing a number. - `docs/user_guide.rst`'s new streaming subsection showed only the sync loop while the guide tells async readers every method is a coroutine. It now spells out `async for chunk in client.stream_document(...)`, matching the treatment the pagination section already gives `iter_tickets`. - A test comment said 10240 bytes was "not a multiple of the chunk size" at chunk_size=1024. It was exactly ten chunks, and no streaming test anywhere used a ragged body, so a short final chunk was never exercised. Both the transport and the client case are now genuinely off the boundary and assert the tail. Claims that were true but untested, now pinned: - `test_stream_bytes_retries_a_failure_fetching_the_first_chunk`: the design decision three documents assert -- the first chunk is fetched inside the retried unit, so a failure before any byte reaches the caller is still a safe restart. The rejected alternative (retry the open alone) passed all twelve existing `stream_bytes` tests; it fails this one. - `test_stream_bytes_chunks_at_the_documented_default_size`: every other chunk-counting test passed `chunk_size` explicitly, so `DEFAULT_STREAM_CHUNK_SIZE` could change to anything and leave "64 KiB by default" false in three places with a green suite. - `test_stream_document_closes_the_inner_stream_when_stopped_early`, with the fix it needs: `stream_document` iterated the inner `stream_bytes` generator and never closed it, so a caller that stopped early left the response -- and its pooled connection -- checked out until the generator became garbage, which on the async surface means the collector plus the event loop's asyncgen finalizer. An explicit try/finally releases it at once on both surfaces; `contextlib.aclosing` could not be used because the codegen cannot map it to its sync twin. `stream_bytes` is now annotated `AsyncGenerator[bytes, None]`, which is what it always was. Gates: 729 passed (was 723), 99 skills-contract, docs examples 24 passed, mypy clean over 38 files, ruff check + format clean, `unasync_build.py --check` up to date, sphinx -W succeeded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The verify pass on the streaming download found six pieces of prose that were wrong rather than merely untidy. Four are the same defect class this branch has been closing all along: a comment or docstring in `_async/` is copied verbatim into `_sync/`, so one that is false there is false twice. - A test helper's comment said its unreachable `yield` stops the function being a coroutine. In the generated sync tree there is no coroutine to avoid; the yield is simply what makes it a generator function. Now says that. - `_ClosableStream`'s docstring named only `aclose()`. unasync rewrites the method to `close()` but not the name inside a docstring, so the sync twin documented a method it does not have. Names both, as the rest of the tree does. - A comment pointed at "the join above" when the assertion is eight lines below. - The new `try/finally` comment claimed it fixes the `break` case. It does not, and the report that introduced it argued so correctly: unwinding this generator is itself deferred to the event loop's finaliser on the async surface, so a bare `break` still defers. What the `finally` removes is the second wait, for the inner generator to become garbage on its own. Says that now, including why the sync surface never needed it. - `skills/README.md` still told readers to `await` every method while its own inventory advertises `stream_document`, which is an async generator. That is the third file to carry this exact defect, after the skill's banner and the user guide. - One `--` inside a CHANGELOG paragraph whose other prose uses an em dash. Markdown applies no smart typography, so both rendered in the published notes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Minor, not patch. The release carries two breaking changes, and CHANGELOG.md's own policy line says breaking changes land between minor versions while the package is pre-1.0 — 0.1.1 would have contradicted the file it appears in: - read-model timestamps are timezone-aware `datetime` instead of `str`; - `ev_since_filter` / `ev_between_filter` now refuse a bound whose time carries no UTC offset, which they previously passed to the wire. The version lives in more places than the two the release workflow names, and two of them are enforced: `testing/test_public_api.py` asserts `__version__` outright, and `scripts/tests/test_skills_contract.py` asserts every skill's frontmatter version equals it — with a comment saying a release that bumps `__version__` and forgets the skills fails there. It does. All eight are bumped. The link block is also repaired. `[0.1.0]` pointed at `releases/tag/v0.1.0`, which has never existed: the tag that was pushed is bare `0.1.0`, so that link 404s in the published changelog today. It now points at the real tag. `[0.2.0]` is written v-prefixed to match the convention `.github/workflows/release.yml` documents and validates against; GitHub's compare view accepts the mixed pair. No tag is created here. Tagging is outward-facing and deliberately left to a human — use `v0.2.0`, both to satisfy the workflow's convention and to avoid repeating the bare-tag mistake that broke the 0.1.0 link. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…k stamps
Three measured behaviour changes, each closing a gap between what the code
claimed and what the wire does.
1. An interval bound naming a time is now NORMALISED, not passed through.
`_TIMESTAMP_RE` admitted renderings the API rejects: measured live
2026-08-18, `LAST_UPDATE:(2025-11-28T16:14:41+01:00;)` returns HTTP 590, as
do minute precision, `seconds+00:00` and a space separator instead of `T`
(what `str(aware_datetime)` produces). Only a bare date and
millisecond-precision-with-offset are honoured. That mattered because the
offset gate added earlier on this branch makes an offset MANDATORY on a
time, and the obvious way to comply with a stored `"2026-08-17T20:26:40"`
watermark is to append `+02:00` -- which 590s every sweep. An admitted
string bound is now re-rendered through
`format_ev_datetime(parse_ev_datetime(text))`, a bare date passes through
unchanged, and the string and datetime paths finally emit byte-identical
bounds. The comment at filters.py claiming the regex "accepts only the
renderings measured live" was false; it is now the admission gate and says
so. `test_since_emits_the_open_ended_interval` pinned the 590-ing literal as
canonical and no longer does.
Two sub-cases handled with it: the RENDERED bound is validated, so a zone
whose UTC offset is not a whole number of minutes (any pre-1900 zoneinfo
entry) raises locally instead of emitting `+05:53:20` that the string path
would refuse; and lowercase `z` is now accepted, since `parse_ev_datetime`
accepts it on the read path and the gate rejected it with a misleading "not
a timestamp" message.
2. `ev_contains_filter`/`ev_starts_with_filter` now refuse `_` and `[` as well
as `*` and `%`. All four are metacharacters to `~`: measured live,
replacing one character of an RFC that matched 1 row with `_` matched 9, and
`[0-9]` likewise, while `[<realchar>x]` matched 1. No escape exists -- `\_`
matched 0 rows, so the backslash is compared literally. `_` is pervasive in
EasyVista codes, so `ev_contains_filter("ASSET_TAG", "LAPTOP_01")` silently
also matching `LAPTOP-01` with HTTP 200 was a routine input producing wrong
rows. Refusing is the rationale already written there for `*`/`%`.
3. A malformed timestamp column now RAISES instead of falling through to
pydantic. The fallthrough defeated its own purpose: `"20260817"` became
`1970-08-23T12:00:17Z`, 56 years off and silent, and `1755434441610` -- what
an epoch-millis format change looks like -- became a wholly credible
`2025-08-17T12:40:41.610Z`, absorbing the one signal the guard exists to
raise. The docstring already promised a raise; the obsolete epoch-seconds
paragraph is gone. The `""` unset sentinel still becomes `None`.
Also documents the deliberate read/write asymmetry in `timestamps.py`:
`parse_ev_datetime` assumes UTC for an offset-less literal because a read must
never fail a record, while `_interval_bound` refuses the same shape because a
mis-zoned bound skips records silently.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nk_size `list_actions` sent no `max_rows` and never paginates, so a ticket's action log was truncated at the server's own default (25 on the verified instance) -- the one search-backed call on this client that did not inject `config.default_max_rows`. It now passes it explicitly, so the truncation point is the caller's to see and to raise. Pagination is deliberately NOT added here: it changes behaviour and needs live verification that this endpoint's `@next` behaves like the others'. What this branch newly did was attach completeness claims to that truncation -- `list_actions`'s docstring said "read every action's timestamps and author in one request", `models/action.py` said "every one of these top-level in one request", and the CHANGELOG said "a whole ticket's action metadata". All three are false for any ticket with more actions than one page, and a freshly created ticket already carries about twelve. They now say "a page", and both `list_actions` and `get_ticket_context` state plainly that at most one page is returned, that nothing paginates, and that the excess is dropped with no error -- `get_ticket_context` because it consumes the list, so `TicketContext.to_markdown()` renders a silently truncated log. `Transport.stream_bytes` now rejects a non-positive `chunk_size` locally. Measured: `chunk_size=0` escaped as "ValueError: range() arg 3 must not be zero" and `-8` as "IndexError: list index out of range", both thrown from inside httpx's ByteChunker several frames below this client, so a caller computing a size read a library bug rather than bad input. Prose corrections on the same surfaces, all previously false or absent: - `resolve_url` said "The API is trusted to describe its own instance, not to redirect us off it". Both download paths run `follow_redirects=True` and a `302` to another host IS followed; the credential is dropped, but the foreign bytes are returned as the attachment. The docstring now states what is actually guaranteed. Behaviour unchanged -- signed-location hops need it. - `stream_document` documents that stopping early on the async surface needs an explicit `aclose()`, or the response stays checked out of the pool for a GC cycle. This was written only in a comment inside the method body. - `iter_tickets` documents the accepted sort token: space-separated `FIELD DESC` works, `FIELD:DESC`/`-FIELD`/`DESC(FIELD)` are silently ignored, and the sort is load-bearing on a change-window sweep. - `update_action`'s return value is the API's unverified echo and may be sparse; re-read with `get_action`. - `RECENT_TICKETS_SORT` sorts a varchar, so `get_department_context`'s "newest-first" is now "descending RFC_NUMBER" -- the live test proves a string ordering, and on an instance issuing more than one RFC prefix letter every `R...` ticket outranks every `I...` one regardless of date. - `aggregate_tickets` records that an offset-less `created_since` bound is read as UTC, which silently shortens the window by the instance's offset. - `_fields.py` named `reporting` as a co-consumer; it has never imported the module (`references.resolve_reference` is its path). And "byte-identical to what the API sent" is false for any input whose fraction is not 3 digits -- the repo's own fixture shows it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…pdate's writes Adds the guards the prose in the previous two commits now depends on, and repairs four assertions that could report green while asserting nothing. New: - `test_only_some_timestamp_renderings_are_accepted_as_an_interval_bound` walks the matrix normalisation rests on: a bare date, `ms+offset` and `ms+Z` are honoured; `seconds+offset`, `minutes+offset` and a space separator each raise 590. The seconds case is the one that matters -- it is how a caller naturally satisfies the offset rule, and the unit suite used to pin it as canonical. - `test_the_ascending_sort_token_the_docs_recommend_is_honoured`. Round 1 measured bare `LAST_UPDATE` and `LAST_UPDATE ASC` as ascending, but only the DESC form was pinned; the sweep guidance now tells callers to use the ascending one, so it is pinned rather than remembered. Skips when the default page order is already ascending, so it cannot pass for a coincidental reason. - `test_request_update_writes_impact_owner_and_external_reference` (test_live_ticket_identity.py). `RequestUpdate`'s three new fields had no live read-back, while their unit test's docstring read as though one existed. Under this branch's own measured rule -- a 200 on a PUT is not a receipt, a field the API cannot honour is silently dropped -- that is exactly the gap that ships a field which does nothing. One field per PUT so a failure names the field; the impact and owner ids are sampled from the instance rather than hardcoded, because writing back the value `ticket_factory` already set would pass even if the field were dropped. The unit test's docstring no longer credits itself with a verification it does not perform. - The `%`-wildcard characterization is extended to `_` and `[0-9]`, the two metacharacters the builders newly refuse, plus a `\_` probe showing no escape exists. Renamed to say what it now covers. Distinguishes "matched nothing" (compared literally -- the regression) from "matched no more than exact" (a sparse sample -- a skip). - The `update_action` live test now characterizes the PUT's echo, which had never been captured. It asserts the echo never names a DIFFERENT action; asserting it names THIS one would pin a shape nobody has measured, which is why the docstring and skill instead say to re-read with `get_action`. Repaired: - The comparison-operator control asserted `0 <= control <= baseline`, which is unfalsifiable: `_count` never returns a negative and a filtered count cannot exceed the unfiltered one. Its failure message described a state that could not occur. Now `0 < control < baseline`, which additionally proves the literal was honoured rather than merely not rejected -- a strictly stronger licence for attributing the two 590s to the embedded comparison syntax. - The `LAST_UPDATE DESC` monotonicity check guarded the length of the RFC list while checking the timestamp list, so it passed vacuously whenever fewer than two timestamps came back. Guards the list it actually checks. - The tilde test asserted `exact <= by_prefix`, satisfied by `by_prefix == exact == 1` -- the state its own sibling test skips as inconclusive, and in which it proved nothing about `~` being a pattern operator. Now skips there and asserts the strict bound. - `RECENT_TICKETS_SORT`'s failure message said "newest-first" for what is a string ordering on a varchar. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…se claims Prose only; no code in this commit. CHANGELOG: - The `Changed` section asserted, as a verified live fact, that 0/15 sampled tickets have a non-empty `DESCRIPTION` -- a figure the `Fixed` section fifty lines below explicitly retracts as a sampling artifact (a pooled 77-row sample found `DESCRIPTION` populated on 27 rows). One release entry must not contradict itself, and the CHANGELOG was the last place in the repo still asserting the withdrawn number. The load-bearing claim -- `RequestUpdate.description` writes `COMMENT` -- is untouched. - The BREAKING retype bullet documented the seven changed types but not the consequence: `json.dumps(ticket.classify_fields().official)` now raises `TypeError`, and `mode="json"` appeared nowhere in the repo. Added to the migration note, the user guide's Timestamps section and the ticket-workflow skill. - "during this same unreleased cycle" -> "during this 0.2.0 cycle": the section is headed `[0.2.0]`, so a released entry described itself as unreleased. - "~150 commits" -> "117 commits" (`git rev-list --count 6df6a75..3216a33`). The note's other three facts are correct; a wrong count invites distrust of them. - New entries for the interval normalisation, the `_`/`[` refusal, the malformed-timestamp raise, the inclusive lower bound and the sweep-sort hazard. The watermark-sweep hazard, documented in three places plus the docstring: an unsorted offset sweep over a change window can skip a record permanently, because the rows the filter selects are by construction the rows that are changing -- a ticket touched between pages can land before the read cursor, and the next sweep starts from a later watermark. Sorting ascending on the filtered column moves such a row toward the tail so it is seen twice; every sweep example now carries `sort="LAST_UPDATE"` and de-duplicates by `rfc_number`. Nothing in the branch had acknowledged pagination stability at all. Release documentation: - `docs/publishing.rst` and `release.yml` both said the repository's existing tags are v-prefixed. `git tag -l` prints one line: `0.1.0`, unprefixed. The workflow's tag-stripping logic is right; the reason given for it was untrue. - `publishing.rst` said bump the version in "both places". Four tracked sites hardcode it, and two of them are gated -- so the documented procedure guaranteed a red CI run on every release. All four are now named. - `twine>=5.1` -> `twine>=7.0`: hatchling stamps `Metadata-Version: 2.5` and twine <=6.2 caps its valid-metadata list at 2.4, so `twine check` fails a perfectly good wheel and sdist (measured: 6.2.0 fails both, 7.0.0 passes). - The coverage comment's "1272 statements / 99.21% exactly" is now 1437 / 99.37%; re-stated as a snapshot rather than a canary, since it moves with every added line. - README: the pre-1.0 paragraph -- the second thing a PyPI visitor reads, on an artifact that cannot be re-uploaded -- had three grammar errors, and three relative links 404 on the project page because PyPI does not rewrite them. Skills and user guide, each a claim that was incomplete or false: - search-syntax: `*` and `%` are not the only `~` metacharacters; a dotted relation path (`REQUEST.RFC_NUMBER`) IS honoured in `search`, which `list_actions` depends on, while the "only top-level scalars" rule is about bare nested sub-keys; the ascending sort tokens are named. - ticket-actions: the one-page cap, the unverified PUT echo, and the `created_at`/`updated_at` divergence from `Request`/`Employee`, which a consumer would otherwise meet as an `AttributeError`. - reporting-and-context: "genuinely sorted newest-first" attached "verified live" to an inference about a varchar sort; and an offset-less `created_since` is read as UTC. - client-setup, the designated async reference: "returns coroutines" is false for `stream_document` and every `iter_*`. - document-workflow: the async early-exit close, the `chunk_size` guard, and that streamed bytes are not proof of instance origin. - user guide: the JSON note, the declared-vs-undeclared date column split, and that a `datetime` in `custom_fields` will not serialise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tightening the timestamp validator so junk raises instead of becoming a bogus epoch instant also made it raise on `None`. That is wrong: a JSON `null` is an ordinary wire absence on a column whose own type is `datetime | None`, and a caller passing the field's default explicitly is not an error either. Only `""` and junk were meant to change behaviour. Regression guard added, because this was introduced by the very change that was meant to make the validator stricter — the two absences it must accept are now named in the docstring and pinned by a test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two commits ago this branch published the opposite ruling on every surface that documents a watermark sweep: sort ASCENDING, on the reasoning that ascending turns a permanent miss into a duplicate. That reasoning was wrong, and the recommendation with it. Offset pagination over a set sorted by the column being mutated can drop a row in either direction; what differs is where the dropped row's own stamp lands relative to the next watermark. Ascending, the re-touched row moves tail-ward and the row that crosses the cursor is a neighbour whose stamp did NOT change: it falls below the new watermark and no later sweep selects it -- lost. Descending, the row that slips behind the cursor is the re-touched one, whose stamp is now above the watermark, so the next sweep re-selects it -- deferred and self-healing. So `sort="LAST_UPDATE DESC"` plus de-duplication is now the guidance in ev_since_filter, iter_tickets, the user guide, the search-syntax skill and the CHANGELOG, each carrying the real reason instead of the old one. Keyset pagination is named as the fully robust alternative for a caller who cannot tolerate even a deferred miss, with the honest note that iter_tickets cannot express it because it owns its own offset. The live sort characterization now runs with the change window applied: the guidance is exclusively about a FILTERED sweep, and `sort` has the same silent-ignore fate a search condition has, so "honoured alongside a search" was unmeasured. The ascending pin is kept -- both tokens really are honoured, which the docs still state -- but renamed and re-framed so it no longer reads as the recommended sweep form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…re recommended The last round taught the search-syntax skill that `_` and `[` are metacharacters under `~` and that ev_contains_filter / ev_starts_with_filter now raise for them, but it stopped there. The primary published doc still enumerated only `*` and `%` and steered callers to those two builders using an ASSET_TAG example -- the very column whose codes are underscore-pervasive -- as did the README and the asset-workflow skill. A reader following any of the three passed `LAPTOP_01` and got a ValueError none of them warned about. The user guide's grammar bullet, the README snippet, the user guide's asset example and the asset skill (snippet plus its own Gotcha) now all carry the `_` / `[` / no-escape facts. They also carry the exit none of them offered: `:` does not expand a wildcard, so an exact match on a value containing a metacharacter is expressible with ev_equals_filter, and only pattern-matching AROUND a literal metacharacter is impossible. That exit is now stated by the builders' own ValueError message and their docstrings too, so the person who hits it at runtime is not left without a path -- and it is measured rather than remembered: the live characterization now also probes `RFC_NUMBER:"<stem>_"` and requires it to return strictly fewer rows than the `~` probe on the identical pattern, which a wildcard-expanding `:` (or a silently dropped condition) could not do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… direction Five smaller accuracy gaps the verify lenses found on the previous round. filters.py: a string bound whose only fault is a sub-minute UTC offset (`...41+05:53:20`, what `isoformat()` gives for any pre-1900 zoneinfo instant) was refused with the generic "is not an EasyVista timestamp ... pass a datetime to be certain". That was wrong twice: the value IS valid ISO 8601, and passing the datetime raises too. It now gets its own message naming the offset as the cause and saying the datetime path refuses it for the same reason; the unit test pins both paths to the same diagnosis instead of accepting "timestamp". _interval_bound's docstring said the sub-millisecond truncation happens but not which way. It truncates DOWN, and that is not symmetric: harmless on the inclusive lower bound (it can only re-read), but on an upper bound it moves the bound up to 999 microseconds earlier and silently NARROWS the window. Documented, with the advice to pass millisecond precision when an exact upper bound matters -- and ev_between_filter's docstring, which said nothing about the normalisation at all, now states both. CHANGELOG, user guide and ticket-workflow skill prescribed `mode="json"` for two breakages but it only fixes one: classify_fields() takes no arguments, so there is nowhere to put the keyword. All three now split the two paths and give the classified-bucket case a remedy that exists. The live rendering matrix asserted `0 < got <= baseline` for each honoured interval rendering, which the silently-dropped whole-table fate also satisfies -- not an assertion by this project's own rule. Each rendering is now a differential across two bounds in that same rendering, which a dropped condition cannot pass. The live RequestUpdate read-back wrote an OWNER_ID / IMPACT_ID sampled off an arbitrary other ticket. Those are foreign keys constrained by the ticket's catalog and domain, so a refusal was a plausible false red reported as "RequestUpdate cannot write OWNER_ID". It now tries several sampled candidates, treats a validation refusal as a skip naming only the column, and asserts EXTERNAL_REFERENCE -- the one column needing no instance-side legality -- first and unconditionally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The change-window sort ruling was corrected to descending two commits ago (49ef8d8) on solid reasoning that still holds, but the correction left a trap undocumented: because DESC yields the newest row first, the watermark reaches its final value on page 1 of any sweep. A sweep that is interrupted, or capped with max_records -- exactly the pattern the guide's, the skill's and this package's own pagination examples use elsewhere -- still ends up holding the newest stamp. Advancing the watermark from that permanently excludes every row the incomplete sweep never read, with no error of any kind. Under the ascending advice that correction retracted, the same interruption was resumable; DESC is not, unless the caller advances the watermark only after a sweep runs to completion. State that caveat at all four sites that recommend the DESC sweep: ev_since_filter's docstring, the user guide's change-window warning, the search-syntax skill, and the changelog entry that made the correction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ity live test _write_first_accepted PUT each IMPACT_ID/OWNER_ID candidate and treated "no exception raised" as "the instance accepted it". But this API's documented refusal mode -- restated in the very test's own docstring -- is HTTP 200 with the field silently dropped, not a raised error. When the instance declined a sampled candidate this way, the helper returned it anyway, never tried the rest, and the final assertion failed claiming "RequestUpdate cannot write IMPACT_ID" when the truth was "that impact is not valid for this ticket" -- the exact false red this helper exists to prevent. Move the read-back inside the loop: after each PUT, re-fetch the ticket and compare the column, and only return a candidate the instance demonstrably applied. A mismatch now falls through to the next candidate the same way a raised EasyvistaValidationError already did, so both refusal shapes end in a clean skip instead of a failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…hange-window tests Three fixes to test_live_change_window.py ahead of a live run against a shared preprod instance: - The comment above the interval-rendering differential claimed the counts "are not printed (P2)". False: pytest's assertion rewriter reprints both operands of a comparison even when the assert carries a message, so binding a count to a local does not close that channel -- only binding the comparison itself to a bool does, which is already this module's own idiom (is_non_increasing, colon_did_not_expand). Bind the two comparisons and correct the comment. - test_descending_sort_needs_the_space_separated_token now runs with a window applied, which turned its colon-token check into a membership race: a ticket below the bound can be touched by anyone in the seconds between the stale unsorted snapshot and the colon-token snapshot, enter the filtered set, and make the test fail claiming "LAST_UPDATE:DESC now reorders results" for an ordinary concurrent write. Take the unsorted snapshot immediately before the colon-token snapshot instead of reusing one from three round trips earlier, and retry once on mismatch before failing. - test_a_comparison_operator_never_narrows_the_result asserted the filtered count equals the session-cached tickets_baseline, so one concurrent create anywhere on the shared instance fails it claiming "a bare comparison operator was honoured" -- the opposite of what happened. Compare against a same-instant unfiltered re-read instead, keeping the property under test without assuming the instance is quiescent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The module docstring claimed a full run issues "4 updates". That was already off by 2 before this round, and the new ticket-identity test's IMPACT_ID/OWNER_ID read-back (up to 5 candidate PUTs per column, some of which the instance may reject) makes the true count variable. Restate it as the real range -- 6 to 14 ticket updates -- so anyone deciding whether to point this suite at a given instance is reading an honest upper bound. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three call shapes were wrong. Each was measured against a live instance,
and in every case the instance was correctly configured -- the fault was
ours. docs/API_Info.md settles all three; it had been under-read.
PostRequest's docstring claimed a ticket needs "at minimum catalog_code
plus title". Wrong, and expensively so: the documented body is
catalog_code + origin + title + description + department_id + urgency_id
+ impact_id, and the same body minus those four ids is accepted on some
catalogs and rejected on others with the IDENTICAL remaining bytes. The
rejection is 590/2013 whose message is a bare SQL parser error naming no
field, at a position that never moves with our text -- so it reads like a
server defect and is not one. Every id in the documented body was
verified to persist by reading it back under an explicit projection;
these columns are absent from the default projection, like TITLE, so an
unprojected read shows None whatever the server stored.
Also documented: a rejected create may still have created the ticket. 12
attempts returned 3 RFC_NUMBERs and afterwards all 12 tickets existed.
BREAKING CHANGE: RequestUpdate.status_id is removed. There is no flat
status update on this API and the field never worked -- sent alone the
PUT is rejected 590/2013, and sent beside any other field it returns 200,
applies the other field, and drops the status in silence. A write that
reports success and stores nothing is worse than one that fails, so
extra="forbid" now makes RequestUpdate(status_id=...) raise at
construction. Use set_status.
set_status(rfc, status_guid=..., comment=None) is added on both clients,
with build_set_status beside it. It sends the documented
{"closed": {"status_GUID": ...}} body -- the same request close_ticket
sends, under a name that says what it does, because "close" is what the
wire calls it and not what it is limited to: given six different status
GUIDs in turn, a fresh ticket landed on exactly the status requested
every time, non-terminal ones included.
test_live_smoke leaked one ticket per live run. Its
test_missing_mandatory_field_raises_validation_error asserted that a
create with a catalog but no title is rejected "(no ticket created), so
this stays read-only-safe by construction" -- both halves false. title is
not the mandatory field (the full documented body with no title creates
fine), and the rejection does write a row. Replaced with a test that
omits the ids which really are required and reconciles the leftover
ticket by its external_reference marker, which survives the failed insert
and is searchable. Two tests added beside it: the documented body lands
every id, and set_status reaches a NON-terminal status.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.