Core and connector follow-ups from the codebase review (#197)

[richardmhope]

The core and connectors sections of #197, after the API section in #205. The web/deploy section follows; #197 stays open until it lands.

1. The shared pytest plugin broke minimal environments

iceberg-core publishes iceberg_core.testing under pytest11, so pytest imports it in every environment where the package is installed — including one carrying the engine's dependency shape: plain iceberg-core, no db extra, no SQLAlchemy (ADR 0002). The import failed at collection, before a single test ran, in a package that deliberately does not depend on what it needed.

The ORM imports are guarded and the database fixtures pytest.skip with a sentence naming what to install. They stay registered rather than vanishing — a fixture that disappeared would be an obscure fixture 'session' not found, and a test that wanted a session is now reported skipped rather than passing on nothing.

Tested in a subprocess with sys.modules[name] = None, because the workspace this suite runs in has the extra installed and can never reproduce the condition in-process.

2. Jira spliced stored state into quoted JQL literals

Three values reach f'{field} >= "{start}"' with only an isinstance(str) check: a discovery window's bounds (from the lease), a resume point (from the checkpoint), and a watermark (from the cursor). All three round-trip through the API's database after this connector writes them, and a " in one closes the literal and changes the query rather than failing it.

Each is now checked against the single shape _minute writes (YYYY-MM-DD HH:MM). An unusable value is dropped, not fatal: the query widens, so the cost is re-reading issues that dedupe on their fingerprint — the same trade the resume logic already states a few lines down ("re-reading costs a duplicate, skipping costs the secret"). A checkpoint carrying one restarts the spec, exactly as an unknown checkpoint version does.

3. Support code copied into each connector is now shared

_LazySandbox was byte-identical in Confluence and Jira. The extraction-outcome mapping was in all three — and had already drifted:

	Confluence / Jira	file share
REJECTED_BOMB	parse_error	size_limit

Same file, same refusal, two manifest reasons depending on which connector met it. Both move to extraction.py, beside the outcomes they translate and the is_incomplete property they must stay in lockstep with.

Resolved to size_limit, which is the reading the enum's own names point at: REJECTED_* is a decision this code made about how big something would become, FAILED_* is something that broke. An operator sent to look for a malformed file by a parse_error would find a perfectly well-formed one. No test pinned either behaviour, which is how it drifted — there is one now, plus a check that every non-EXTRACTED outcome has an entry so a new member cannot fall into some connector's default.

4. The enum CHECK-constraint question, decided

No constraint, and the reasoning is now in the docstring rather than the question being left open:

• A CHECK reintroduces exactly the cost native_enum=False was chosen to avoid — adding a value becomes DROP/ADD CONSTRAINT, a table scan under an ACCESS EXCLUSIVE lock on Postgres.
• It defends against a writer this architecture says does not exist. Only the API writes, so "out of band" means someone holding the database credentials, who can drop the constraint as easily as violate it. It stops a typo in a manual UPDATE, not an adversary.
• The value could never be believed anyway. Reading an unknown one raises LookupError naming the column's enum and its legal values — verified, not assumed. That property is what the trade rests on, so test_enum_columns.py pins it; if SQLAlchemy ever starts coercing instead of raising, the decision should be revisited and the test is what will say so.

Validation

Every behavioural fix has a test confirmed to fail against the code before it (stashed and re-run). New: test_pytest_plugin.py, test_enum_columns.py, test_extraction_reasons.py, plus JQL tests in test_jira_connector.py.

make check green: ruff, mypy, docs check, 2007 passed / 2 skipped.

---

Generated by Claude Code

[icebergai-review-bot]

Verdict

APPROVE

Completed bounded review across 1 immutable scope(s). No actionable findings in the supplied immutable change.

Scope health

Convergence: healthy. Review mode: initial.
Recommended action: CONTINUE_INCREMENTAL.

• No escalation signals.

Prior findings

Finding	Status
—	No prior finding state

New findings

No new findings.

Fix-induced regressions

• None evidenced.

Uncertainty

• No material uncertainty recorded.

Validation

• Reviewed the supplied diff; exact-head CI is reported as passed.

Residual risks

• None identified.