Repository navigation
Commit 83e4e51
authored
Authority packs and cross-corpus deep research (#2215)
* LLM framework: support models that require the OpenAI Responses API
OpenAI's GPT-5.6 family rejects function tools on /v1/chat/completions when
reasoning_effort is set. Every agent here carries function tools, and turning
reasoning off to keep the old endpoint discards the capability the model is
chosen for, so requires_responses_api() routes the family to /v1/responses on
both the DB-credential and env-credential paths. Routed automatically because
the names are selectable in the System Settings LLM picker; otherwise choosing
one 400s every agent in the install with the reason visible only in a worker
log.
The Responses API also rejects a function_call whose paired reasoning item is
missing, which in-run compaction was producing: it dropped ThinkingParts from
older messages, including responses that carried a tool call. Chat Completions
tolerated the split for years. Reasoning now stays on any response that called
a tool — an invariant, not a provider switch, since reasoning that produced a
tool call is part of that call's record.
Context windows are registered explicitly. There is no bare gpt-5 entry to
inherit, so an unlisted name falls to the 128K default and sizes a 1M-window
model at an eighth of its budget — which moves when a deep-research run
compacts, not just a reported number.
Also adds error_type to CoreAgent.chat()'s response metadata, matching what the
streaming wrapper already returned: that path swallows the exception entirely,
so the class name is the only thing a caller can branch on.
* Authority packs: a framework for curating public regulatory sources
The machinery for turning a published body of regulation into corpora that
can be cited: pack configuration and install services, source-plan and
discovery provider base classes, a frontier service that tracks what remains
unfetched, a gate service for what may be installed, and the governance graph
that relates authorities to one another.
Domain-neutral by construction — nothing here knows about any particular
regulator. The provider base classes are the extension point; a pack supplies
its own providers and source plans as data.
Fetching is constrained by an SSRF host allowlist derived from each pack's
declared source hosts, matched exact-or-dot-suffix so notercot.com cannot
satisfy ercot.com. Authority packs need an endpoint identity distinct from
document identity, because one regulator's order can adopt another's revision
and be cited by a third's tariff before any of them is ingested.
* Authority packs: the DFW public-authority pack, and its console
Four packs — Texas electric statutes, PUCT rules and proceedings, ERCOT
large-load rules, Oncor delivery tariffs — as DATA under enrichment/data, plus
the per-regulator source and discovery providers they need. Ten corpora, 354
documents of public regulatory material.
This is the pack that exercises the framework in the previous commit. It is
deliberately data rather than code: what is specific to a body of regulation
belongs in a manifest, not in the product's logic.
Also the Authority Console — the admin surface for reviewing and installing a
pack, with a preflight modal that shows what an install would fetch before it
fetches anything — and the three generic management commands that provision
and maintain packs.
The deployment topology that assembles these corpora into customer-facing
groups is NOT here. That is a property of a deployment, not of the software,
and lives in an external repo.
* Structured finding cards, with thresholds the corpus declares
A deep-research finding is ordinarily a claim plus the annotation ids that
support it. A card is a finding that also commits to a shape, so the report,
the interface and any downstream consumer read the same fields instead of
re-deriving them from prose. Two shapes: a regime card, built around a
half-open effective interval whose END IS EXCLUSIVE — an inclusive end lets
both regimes claim the boundary day — and an obligation card, built around
what must be done, by whom, and by when.
The obligation card keeps roles and dates apart because a single owed_by and a
single deadline conflate things a reader cannot afford to conflate: whoever
prepares a filing is often not who submits it, and an approval date is not an
effective date is not the date an application is due.
The thresholds an obligation attaches at are CONFIGURATION. A corpus declares
its own scale in its Readme.CAML —
[component:obligation-schema unit=MW steps=25,50,75,100 label=ramp]
— read by a new parse_component_props helper. Declared where a corpus already
describes itself rather than in a settings column nobody discovers, and
tolerant of a typo: a malformed marker degrades to the unconfigured default
instead of failing a run. Unconfigured corpora keep the guard that matters — a
phase-triggered obligation must still say where it bites — and lose only the
membership check, which would have nothing to check against.
Note the props regex is deliberately separate from _COMPONENT_MARKER_RE, which
strips markers out of user-controlled corpus metadata. That one must stay a
blunt remove-any-marker pattern; giving it groups would let an author-supplied
prop survive stripping.
* Deep research over corpus groups, and a loop that knows its own budget
GROUP SCOPE. A ResearchReport gains a corpus_group, and the agent a
search_across_group tool that fans retrieval over every member corpus the
CALLER may read — membership resolved per query, never a config-time snapshot.
Reachable from the interface as well as from chat: startResearchReport takes
corpusGroupId and the Research tab offers a group picker. A group the caller
cannot see is refused rather than ignored, because a silently narrowed run
produces a report that reads as group-wide and is not.
BUDGET. A run is capped on model REQUESTS as well as tokens, and could not see
that counter: it was cut off mid-work, and the salvage composition that
replaced its report body silently lost whatever it had not yet written.
Observed at exactly 60 of 60 permitted requests, with two of four required
sections missing from the salvaged body. Past 65% of the step budget every tool
return now carries a notice; past 80% it says to finalize and what is lost
otherwise.
DIAGNOSIS. Every ending that was not an explicit finalize was recorded as one
opaque budget_exhausted — token budget, step budget and an agent that simply
stopped, all indistinguishable — and chat() swallows the framework exception,
so the worker log said nothing either. Reports now carry a terminal_reason.
finalize_report is terminal in fact rather than only in its docstring: a second
call used to re-run composition, leaving the later body stored and both passes'
warnings accumulated.
AUDIT. tool_call_log covered only the closures this module builds. Everything
from the agent's default toolset — similarity_search among them — was
invisible, so the log held about half of what a run did while the interface
presented that half as the whole. Reading a tool's ABSENCE off that half-log is
how this work briefly convinced itself the agent never searched by meaning,
which was false. Now covered, with the closures skipped by name because the
factory re-wraps caller tools and a marker attribute never survives.
CITATIONS. A material obligation with no citation is refused when recorded. An
obligor no cited passage names is kept but marked inferred, with a count
surfaced as a report warning — refusing it lost the obligation along with the
bad attribution, and gave the agent a prose escape hatch it filled with
placeholder party names.
* Document import, versioning and workspace support for pack ingestion
What installing an authority pack needs from the ingestion side: chunked and
REST import paths, V2 import/export round-tripping with re-ingest remapping,
file-converter handling for the formats regulators actually publish, and the
document-versioning transitions a pack update depends on when a revision
supersedes an earlier one.
Also the workspace service that files generated artifacts — research reports
among them — into the creator's personal corpus, and the corpus-service and
slug handling those paths rely on.
* GraphQL surface and frontend for corpus groups, research and imports
The API and interface changes the preceding commits need: schema additions
with the golden SDL regenerated, corpus-group management, the import modal,
document and markdown viewer updates, and the chat/workspace surfaces.
The golden SDL in config/graphql/schema.graphql is the wire contract that
test_schema_parity pins; it is regenerated here deliberately rather than
drifting.
* Docs, changelog fragments and stack configuration
Deployment and architecture docs for authority packs, the acceptance
procedure for the readiness question — written against the GUI and the GraphQL
API, because a harness that reaches inside the application is a second
implementation of the product's own checks — plus changelog fragments, the
isolated e2e compose profile, and two new CLAUDE.md pitfalls covering the
Responses API family and the audit log's blind spot.
* Authority packs are sideloaded deployment data, not product code
The four Texas / large-load packs (texas_electric_law, puct_electric,
ercot_large_load, oncor_delivery -- ten corpora, their in-pack providers,
charters, specs, personas, relationship declarations and rights fixtures) no
longer ship in opencontractserver/enrichment/data/authority_packs/. They were
moved to their own repository along with the three test modules that asserted
their data contract, source plans and provider parsers, the deployment runbook,
and the full-stack import spec that drives their ten corpora.
A body of regulation has its own legal-review state, refresh cadence and
publisher relationships. Shipping one in the product meant every install
carried, catalogued, and imported the providers of one deployment's regulatory
topology. bolivia stays as the worked example the format needs.
Sideloading:
- AUTHORITY_PACK_ROOTS (new): directories *of* packs, the same shape as the
in-tree root, so a pack repository mounts with one variable instead of one
entry per pack. AUTHORITY_PACK_PATHS is unchanged and both are honoured.
- authority_pack_dirs() de-duplicates by resolved path -- a pack reachable
through both settings would otherwise import its providers twice under the
same generated module name -- and a root scan now skips subdirectories with
no pack.yaml, so a root pointed one level too deep contributes nothing
instead of surfacing a pack's own charters/ and specs/ as broken packs.
Explicit AUTHORITY_PACK_PATHS entries are deliberately not filtered that way:
the operator named that directory, so a missing manifest must surface as an
invalid pack rather than vanish.
- load_authority_pack --check runs the same validation as the Authority Console
preflight and writes nothing. The Console needs an authority-admin browser
session; a headless deployment had no equivalent.
Tests keep the machinery coverage without the data:
- New fixture pack tests/fixtures/authority_packs/example_utility -- a complete,
deliberately fictional schema-v2 pack. It lives under tests/fixtures/ because
authority_pack_dirs() enumerates every immediate subdirectory of the in-tree
root, so a fixture placed there would ship as an installable pack.
- test_authority_pack_loader.py (was test_grid_dossier_authority_pack_loader):
convergence, idempotency, curator-state preservation, atomic abort and
prefix-binding validation, against the fixture pack.
- test_authority_discovery_runtime.py (was test_grid_dossier_discovery_runtime):
unchanged in substance; it always defined its own providers.
- test_authority_pack_sideload.py (new): the sideload contract itself.
- test_authority_source_hosts.py built its in-pack provider assertion by
importing a pack module; it now builds a real sideloaded pack, which is the
only shape available once packs live outside the tree.
- test_generic_grammars.py loses three assertions that depended on pack-declared
abbreviations; requires_section_marker is already covered neutrally by
test_authority_pack_taxonomy.py.
The PUCT/ERCOT citation shapes stay in the shared Tier-2 grammar: a pack's
declarative vocabulary (shape_rules / abbreviations) cannot express a bespoke
regex, and inventing a citation_patterns mechanism inside GenericCitationExtractor
is not warranted by this change.
* Let a sideloaded pack import its own modules
In-pack component modules are imported by file path under a synthetic module
name, so their parent packages never existed. A pack module therefore could not
import a sibling: `from ..helper import x` had nothing to resolve against.
A pack living in the tree hid this, because it was ALSO importable as a real
Python package -- so two of the four packs just removed reached their own
pack-root helpers by absolute dotted path
(`opencontractserver.enrichment.data.authority_packs.<pack>.<helper>`). That
resolves only while the pack is in the tree. Sideloaded, the import raised and
15 providers silently failed to register: exactly the thing the pack format
promises cannot happen ("copy-to-add, copy-to-port").
The registry now creates each pack's parent packages before importing its
components, so ordinary relative imports work identically in-tree and out. The
pack-root package is shared across component families, so a pack's helper is
one module object however many families import it -- otherwise a pack's source
and discovery providers would hold diverging copies of the same helper.
`__path__` is re-pointed on every discovery pass rather than skipped when the
module already exists, so `reset_registry()` and a swapped pack directory stay
correct.
Drops the now-redundant `module_ns` parameter of
`_discover_pack_component_classes`: the component subdirectory name is part of
the synthetic module path, so the two families cannot collide.
Covered by PackSiblingImportTests in test_authority_pack_providers.py, which
builds a pack whose provider imports a pack-root helper and asserts both that it
registers and that the helper resolves to exactly one module.
* Purge cached pack submodules when a pack name changes directory
The previous commit's docstring claimed re-pointing a synthetic package's
__path__ was enough for a re-mounted pack name to resolve against its new
directory. It is not: an import resolves against sys.modules["<pkg>.<sub>"]
before it ever consults the parent's __path__, so a helper cached from the
previous directory was handed straight back and the newly mounted pack silently
ran the old pack's code.
Component modules were unaffected -- they are re-registered in sys.modules and
re-executed on every discovery pass -- so only helper modules reached through
the package machinery were stale, which is why the two existing tests (distinct
pack names) did not catch it.
Cached submodules are now dropped when the path actually changes.
test_same_pack_name_from_a_new_directory_reloads_its_helper mounts the same pack
name from two tempdirs with different helper values; it fails against the
un-purged version and passes now.
Also:
- Removed changelog.d/remove-corpus-specific-commands.removed.md. Every command
it announced removing was added on this same branch and never appeared in a
release, so like the acceptance-gate pair it nets to nothing -- and it was the
last fragment carrying deployment-specific narrative. The move is recorded in
the sideload fragment instead.
- Documented why E2E_RUN_AUTHORITY_IMPORTS has no in-tree consumer: it is set by
a deployment running its own pack-import acceptance spec against this
frontend.
- Documented the in-tree consequence of requiring pack.yaml in a root scan: a
committed pack with a broken manifest now vanishes from the catalog rather
than surfacing as repairable, which is acceptable for a reviewed in-tree
commit and is not applied to explicit AUTHORITY_PACK_PATHS entries.
(--no-verify: lint-staged cannot spawn prettier from a git worktree, whose
frontend/node_modules is not populated. pre-commit run --all-files passes, and
playwright.config.ts was checked with the repo's prettier 2.8.8 directly.)
* Drop the last deployment name from the mypy baseline header
The section header still described its entries as GridDossier branch modules;
the modules it lists are the authority-pack and deep-research ones that remain.
* Drop 42 regenerated screenshots that show no change
The screenshot workflow rewrote 54 PNGs on this branch. Compared pixel-by-pixel
against main, most differ only in backdrop grey and subpixel text positioning --
a different capture environment, not a different UI. corpus--import-modal--
confirm-step.png was the extreme case: 99.999% of its pixels "changed" because
the modal overlay's grey shifted by a value or two, and the two images are
indistinguishable side by side.
Reverted those 42 to main. The 14 that remain each show something this branch
actually changed:
- corpus--intelligence-overview / --intelligence-panel (new): the redesigned
IntelligencePanel -- stat tiles replaced by a document listing
- corpus-groups--management-panel--with-groups / --anonymous (new): the Owner
column, and the page listing groups the viewer does not own
- corpus--toc--hybrid-index / --expanded-children: per-entry descriptions
- authorities--console-* (5): the Authority Packs nav entry and the reworded
console subtitle
- knowledge-base--document-viewer--unsupported-file / --unsupported-loading:
the DocumentViewer rework behind MarkdownDocumentViewer
- corpus--import-modal--upload-step: Select File as the active stepper state
Verified by rendering each pair side by side rather than trusting the byte
delta; every image in the diff was compared, not sampled.
* Address review: preserve Responses-API redirect on DB-creds fallback, fix ERCOT guide case-insensitivity, surface corpus-group load error
Address review: fixed model_factory.py's DB-credential fallback paths
silently dropping the openai-responses: prefix, added the missing
re.IGNORECASE on _ERCOT_GUIDE_RE, surfaced a silently-swallowed
GraphQL error in StartResearchModal's corpus-group picker, pinned the
step-budget notice's known reasoning-only-request gap with a
regression test, added an SSRF allowlist regression test for the
notercot.com/ercot.com dot-boundary case, and documented the
bootstrap()/load_declarations() relationship-sync ordering dependency.
* Fix pre-existing CI failures: async ORM in test body, missing WorkspaceService export, stale mock signature/variables
Pre-existing bugs in this branch's own commits (not caused by the
review-fix commit), surfaced only once CI ran a fresh full suite:
- test_nested_approval_gates.py: the new cross-corpus-group test called
Corpus/Document/CorpusGroup ORM writes directly inside an async test
method body, raising SynchronousOnlyOperation. Moved the writes into
a sync helper invoked via sync_to_async.
- test_corpus_services_package.py: opencontractserver/corpuses/services
added WorkspaceService to __all__ but the structural re-export test's
expected list was never updated.
- test_llm_model_factory.py: TestCredentialCache's _construct_model mock
predates the responses_api kwarg the Responses-API commit added to
the real signature; the mock never accepted it.
- CamlIntelligenceEmbeds.ct.tsx: IntelligencePanel's collection-docs
query gained includeCaml: true (workspace-only corpora fix) but this
test's mock variables were never updated to match, while the other
two IntelligencePanel-covering test files already had it — an
oversight in the same PR, not this one.
* Fix cross-corpus-group test: context must not be document-scoped for a dynamic-document call
_validate_resource_id_params (a defense-in-depth IDOR guard) rejects a
tool call whose document_id argument differs from ctx.deps.document_id
when the latter is set. _make_ctx() fixes deps.document_id to
self.document for every other test in this class (all document-scoped
agent calls), but the cross-corpus-group test deliberately calls
ask_document for a DIFFERENT document via corpus_group - the shape a
corpus-level agent (document_id=None) actually has. Added an
explicit-override sentinel to _make_ctx so this test can opt out of
the document-scoped default instead of tripping its own IDOR guard.
* Add coverage for authority_html HTML/URL normalization helpers
opencontractserver/pipeline/base/authority_html.py had 0% patch coverage
(151 untested lines). Adds opencontractserver/tests/test_authority_html.py
exercising normalize_html_text, AuthorityLink.attribute, canonicalize_authority_url
(tracking-param stripping, scheme/host validation, fragment handling, relative
resolution), extract_authority_links (dedup, nested/unclosed anchors, hidden
tag suppression, img alt/aria-label/title fallbacks), visible_html_text (block
boundaries, blank-line collapsing, hidden tags), extract_labeled_value
(same-line and next-line label forms), and stable_source_slug (URL stem
extraction, truncation with hash suffix, fallback to "document").
* Add component test coverage for ResearchFindingsEmbed
Covers the CAML research-findings embed's four render states: no
reportId, a report with no qualifying findings, REGIME cards (open vs
closed intervals, with/without primary-authority-effective-from and
unresolved qualifications), and OBLIGATION cards (with/without form
reference and deadline). Brings the component to 100% statement,
branch, function and line coverage under COVERAGE=true.
* Extend authority_sources test coverage for parsing edge cases and error paths
Adds tests for previously-uncovered branches in authority_sources.py: date
parsing type checks, host_matches_declared_sources (untested), canonical-key
and content validation on AuthoritySourceRecord/SourceRelationship/
AuthorityPublisherEvidence, retrieved_at normalization, MIME inference edge
cases (PNG/OLE/PPTX/XLSX/malformed ZIP/unsupported extensions/binary
fallback), PPTX/XLSX/generic-XML text extraction (success and malformed
input), PDF parse-error wrapping, PNG OCR (success, megapixel cap, generic
OCR failure, missing Pillow/pytesseract), the fetch factory's max_bytes
propagation and Gotenberg-converter ImportError fallback, and the previously
untested extract_authority_archive_members entry point plus the private ZIP
budget helpers. Raises opencontractserver/enrichment/authority_sources.py
coverage to 100% under this test file; no production code changed.
* Extend authority pack service test coverage for validation and permission paths
authority_pack_service.py sat at 85.62% patch coverage on PR #2215 (90 lines
missing). Add direct-call unit tests against the private
classmethods/staticmethods (matching the existing convention in these files)
for: catalog()/preflight() fault-isolation into invalid AuthorityPackPlan
entries, duplicate pack-id detection, manifest read/parse failures,
schema_version and authority_prefixes-without-mappings validation,
install_path() fingerprint staleness, _pack_approval_status mixed/pending
branches, declarative_fingerprint's non-dict corpora tolerance, _hash_files
and _pack_file directory-escape guards, corpus-entry structural validation
(title/slug/description/charter/default_authority_weight), authority-prefix
shape/uniqueness validation, metadata-schema validation (version/fields/data
types/choices), corpus-identity resolution against existing DB rows
(ambiguous title, conflicting slug, legacy-unslugged adoption), declared-
prefix cross-checking, relationships.yaml structural validation, the
manifest+section-spec duplicate relationship-edge guard, authority-namespace
prefix binding conflicts, and metadata-column type-conflict rejection.
Verified against a live postgres: 626 statements, 4 remain uncovered (381,
501-502, 916) — each a defensive branch that is unreachable without mocking
core Django internals (see file docstrings for the reasoning), deliberately
left untested rather than asserting a raise fires under a fabricated mock.
* Extend authority_import_artifacts test coverage for validation and error paths
Adds ~96 new test methods to test_authority_import_artifacts.py covering the
validation gauntlet in read_source_plan, pack-relative CA certificate
resolution, collection-path error branches (discovery failures, unresolvable
providers, duplicate/expanded canonical keys, mixed rights_status, gate
interactions), the link-only AuthoritySection build path in
build_authority_import_artifacts/_normalize_records (previously entirely
untested), corpus persona/post_processors/alias resolution, manifest and
source-plan-path validation, publisher-source filename/sidecar-member
derivation, _build_export_payload conflict guards, and the small validation
primitives (_json_safe, _validate_authority_metadata, etc).
Raises coverage on opencontractserver/enrichment/authority_import_artifacts.py
from 76% to 99% (205 -> 4 missing lines). The 4 remaining lines are either
unreachable given upstream validation gates (dead defensive code) or require
an engineered sha256-prefix collision not worth the resulting test fragility.
* Add component test coverage for authority pack preflight modal and packs tab
Adds AuthorityPackPreflightCoverage.ct.tsx, covering branches the existing
AuthorityPacks.ct.tsx suite (run through the full AuthorityConsole shell)
never reaches: PacksTab's own loading/error/empty/refresh states, card-level
fallback text, every packStatus() badge, approvalTone()'s neutral and
review-only branches, every pack-summary badge inside PackPreflightModal,
the preflight query's loading/error/null-pack states, install failure
(ok:false, with and without a server message), a thrown network error, a
successful publish that falls back to the default toast text, and the
close-blocked-while-installing guard.
PacksTabTestWrapper.tsx mounts PacksTab directly (no AuthorityConsole shell)
so a corpus-ZIP-importer bridge can be omitted, exercising PacksTab's
`onImportCorpus ? ... : undefined` branch that the console (which always
supplies the callback) can never reach.
Verified via nyc/lcov against the local Playwright CT run: PacksTab.tsx is
100% statement/branch/line covered; PackPreflightModal.tsx is 98.5%
statement/98.2% branch/98.4% line covered, with the single remaining
uncovered line being handleInstall's early-return guard, which duplicates
the Install button's own `disabled` expression and is therefore unreachable
through any real user interaction.
* Address PR review: audit-log race, pack namespace collision, import isolation
Fixes the findings from the two bot reviews that are self-contained (no
contract or dependency change). The two High-severity findings are NOT
addressed here -- both need a design decision from the author and are
tracked in issues #2219 (resumed research runs silently delete previously
valid findings) and #2220 (relationships spanning an unchanged document are
dropped on incremental reimport).
Fixed:
- research_reports.py: append_tool_call / append_finding did a non-atomic
refresh -> append -> save. Every tool call now routes through
append_tool_call via _audited, and reap_stalled_research can re-enqueue a
RUNNING report while the original worker is still writing, so the later
save() clobbered the earlier append -- dropping an audit entry and
undercounting the calls_made behind the step-budget notice. Both now do the
read-modify-write under select_for_update() inside transaction.atomic().
research_tasks.py has no enclosing atomic and Celery runs autocommit
(ATOMIC_REQUESTS is HTTP-only), so this is a brief per-call row lock.
- registry.py: the synthetic pack namespace was keyed on the pack directory
basename while authority_pack_dirs() de-duplicates by resolved path, so two
different packs sharing a basename collided in sys.modules and
_ensure_synthetic_package re-pointed the first pack's package at the second
pack's files. New _pack_namespaces() keeps the plain basename when unique
and appends a short path digest on collision.
- import_tasks_v2.py:540: _attach_publisher_original_file sat ahead of the try
that isolates every other per-document step, so one document's storage error
aborted the whole pack import. It now returns (None, {}) like the outer
handler.
- Moved MAX_DERIVED_MESSAGE_TITLE_CHARS and MAX_WORKSPACE_FILENAME_CHARS into
constants/truncation.py; replaced the inline 1 MiB CA-certificate cap with
MAX_EXTRA_CA_CERTIFICATE_BYTES in constants/safe_http.py.
- research_tools.py: comment recording why astart_deep_research returns a
PermissionError as text instead of propagating (the check stays enforced in
ResearchReportService.start; propagating would kill the chat turn).
Dismissed, with the reason recorded in code or tests:
- XML entity expansion: the shipped runtime (python:3.12.13-slim-bookworm,
expat 2.7.4) enables expat's input-amplification limit by default, and both
ET.fromstring sites already translate ParseError into a "malformed" error.
Pinned by a new regression test rather than adding defusedxml.
- load_authority_pack Command MRO: the AuthorityPackService base is
load-bearing, not vestigial -- ~30 call sites in test_authority_pack_loader
resolve validation helpers through Command. Documented in a comment.
Tests: 4 new backend tests (audit-log and findings lost-update, pack basename
collision, XML bomb on both paths) and one component test covering the
SaveMessageToWorkspace thrown-error branch.
* Fix 600s test deadlock: re-seed PipelineSettings singleton after flush
The pytest job failed with 5 errors in
test_core_agents.py::TestCoreAgentFactoriesDefaults -- two tests timing out at
exactly 600.01s and three asserting 'Mocked default prompt'. Neither this PR
nor the preceding review-fix commit touches that class, PipelineSettings, or
the embedder path; the failures are a latent suite-wide landmine that surfaced
when four added backend tests shifted the --dist loadscope distribution.
Root cause. Migration 0031_add_pipeline_settings seeds the singleton with a
one-shot RunPython, so its row lives outside any test transaction. Django's
flush -- run on every TransactionTestCase teardown -- truncates
documents_pipelinesettings along with every other table, and with
serialized_rollback disabled (the default) nothing restored it. Under
pytest -n 4 --dist loadscope, any TransactionTestCase running before an
embedder-touching test on the same worker left the install with NO singleton
for the rest of that worker's session.
Because get_instance() re-creates the row lazily, the absence did not fail
loudly -- it deadlocked. An async test under Django's TestCase reaches
get_embedder -> get_default_embedder_path -> get_instance on the asgiref
executor thread, which holds its own DB connection. That connection cannot see
the main thread's uncommitted row, so it issues a competing INSERT and blocks
on the test transaction's lock, while the main thread is parked in
AsyncToSync.__call__ waiting for that very executor. Neither side advances
until pytest-timeout fires, and the hang leaves unittest.mock.patch decorators
un-exited -- which is why one hung test failed three later tests in the class
sharing the patched target.
Fix: an idempotent post_migrate receiver
(opencontractserver/documents/_pipeline_settings_seed.py, connected in
documents/apps.py) that re-seeds the singleton after each flush and repairs
reused/poisoned CI volumes at DB setup. This mirrors the existing
enrichment/_namespace_seed.py convergence, which solves the identical problem
for the shipped AuthorityNamespace rows -- including its fallback to the global
app registry when post_migrate arrives without an apps kwarg, which is exactly
how flush emits it (flush.py vs migrate.py).
Verification. The failure reproduces deterministically before the fix with
`pytest test_pipeline_settings.py test_core_agents.py::
TestCoreAgentFactoriesDefaults` (2 timeouts + 3 'Mocked default prompt'), and
that command now exits 0. A flush-then-async-agent run also passes 11/11. New
tests cover the receiver on the apps-less emission, idempotency against
operator edits, and the real flush command end to end.
* Address review round two: slug oracle, finalize race, missing terminal row
Three correctness findings from the review posted after the last push.
1. Corpus-group slug-enumeration oracle (research_tools.py::_load_group).
The slug was resolved with an unfiltered CorpusGroup.objects.filter(
slug=...). No unauthorized access followed -- ResearchReportService.start
still refused the run -- but the two failure modes replied DIFFERENTLY: a
nonexistent slug returned this tool's not-found string, a real-but-invisible
one fell through to start()'s PermissionError text. That difference let a
caller enumerate every group slug in the install. Now routed through
CorpusGroupService.get_group_by_ref, matching the sibling
core_tools/multi_corpus.py::_resolve_group_corpora and CLAUDE.md's
services-layer rule; both cases return one refusal.
2. finalize_report's double-call guard was check-then-act. refresh_from_db +
status check cannot stop two concurrent callers: pydantic-ai dispatches
parallel tool calls, and reap_stalled_research can put a second worker on
the same report, so both could read RUNNING before either committed
COMPLETED and both compose -- reproducing the double-composition the guard
exists to prevent. New ResearchReportService.finalize_once runs the check
and the composition as ONE critical section under select_for_update, the
same treatment append_finding/append_tool_call already get. The salvage
path in _run_deep_research_async uses it too, so a late salvage can no
longer overwrite a genuine report committed by another worker.
3. Incremental reimport skipped the terminal row for an unchanged, source-less
document (import_tasks_v2.py). A document matched by canonical_key whose
content is unchanged and whose source is not reingestable (the NUL
placeholder the V2 exporter writes for text/markdown members) returned
early without the DONE PendingDocumentAnnotations row that
_reingest_document_with_deferred_remap creates for the documents it
converges. The document was invisible to
finalize_corpus_import_relationships: the corpus-level coordination row
could not reach DONE on its account and expected_doc_count undercounted.
Re-shipping a pack with an unchanged markdown member is the ordinary case
that hit it. The row's id_map is empty for the same reason the converged
path's is -- relationship endpoints on unchanged documents stay tracked in
issue #2220.
Tests. Three added: invisible-vs-missing group returning one refusal;
finalize_once refusing the second call without composing again or accumulating
warnings; and the converged source-less rerun recording exactly one DONE row.
The third was validated against the fix -- it fails (exit 1) with the change
reverted and passes with it, so it guards the branch rather than merely
covering it. Full affected suite (research/, authority_targeted_import,
import_v2_reingest_remap, corpus_export_import_v2) passes.
* Pin the direction of the group-refusal invariant, not just its symmetry
Comparing the two refusal strings for equality would also pass if BOTH paths
started leaking in some new way. Assert that nothing which exists only when the
row was actually found (the group's title, or permission-flavoured wording from
start()) reaches the caller.
* Address review round three: post-commit install faults, gate fault isolation, registry race
Four Major correctness findings from the latest PR review.
1. Authority pack install reported a committed install as failed. Both the
reactive relink (inside _install_plan) and the response refresh (in
install()) run AFTER the atomic block commits; a failure in either surfaced
as ok=False for a pack already in the database, and the idempotent retry
then no-ops. Both now degrade to post_commit_warnings carried on the result,
surfaced in the GraphQL message + result.warnings and printed by
load_authority_pack. The refresh failure falls back to the approved
pre-install plan.
2. Concurrent first install of the same pack raced on
uniq_corpus_slug_per_creator_cs and escaped as a raw IntegrityError. Caught
separately and returned as a curated retry message; the loser's atomic block
rolls back so nothing of its attempt survives.
3. An exception in discover_and_bootstrap's gate stretch (approval fingerprint
-> evaluate -> audit record) left the frontier row wedged in in_progress —
invisible to dequeue_queued forever — and aborted the crawl batch. That
stretch gets its own fault handler, deliberately separate from the bootstrap
handler below (which builds its audit record from an unbound `decision`
there). AuthorityGateService._verify_rich_publisher_evidence also catches
Exception now rather than three enumerated types: verify_publisher_evidence
is a provider override with an open-ended exception vocabulary. Fail-closed —
the return is still False, so a broken verifier can only refuse a record.
4. PipelineComponentRegistry construction was an unsynchronised check-then-act
and get_registry()'s lru_cache does not serialise the wrapped call on a
miss, so a second thread could read component tuples that discovery had not
filled yet (_initialized is set before discovery runs). Construction and
reset_registry() now hold a class-level RLock; same-thread re-entrancy from
a pack module's import is unchanged.
Each new test verified to fail with its fix reverted and pass with it.
* Surface post-commit install warnings as a warning toast, not a success toast
The Authority Console renders the server's install message, so the round-three
post-commit warnings already reached the operator — but through toast.success,
because the server keeps ok:true (the pack really is installed). A green toast
reading "...re-linking existing corpora failed" defeats the point of the fix.
PackPreflightModal now branches on the structured result.warnings the payload
already carries and toasts a warning instead. InstallAuthorityPackOutputs.result
is typed for the keys the client reads rather than unknown.
Component test verified to fail with the branch reverted.
* Fix blank obligation-card heading, terminal-state finalize guard, and review nits
Remediates the latest review on #2215.
Blank heading (high). ResearchFindingsEmbed read `card.owed_by`, a field
ObligationCard has never had — the stored card is that model's `model_dump()`,
which serialises `responsible_party`. Every real obligation card rendered an
empty <h4> in production. The component suite stayed green because its mocks
were hand-authored to the wrong shape AND nothing asserted the heading at all;
renaming the mock alone would have left the same gap. Heading assertions now
cover both card shapes, and CardFieldContractTests pins both models' full
field-name sets against the TypeScript interface that consumes them. The
agent-facing either/or error also named `owed_by` as a parameter; corrected.
The embed additionally dropped `obligor_grounded`, so an obligor the cited
passages never name rendered identically to one they do. Marked now.
finalize_once refused only a COMPLETED report. CANCELLED and FAILED are equally
"the run already ended", and reap_stalled_research can put a second worker on
one report — so a salvage finalize could overwrite a row the other worker had
just marked CANCELLED by soft time limit or FAILED with a traceback, yielding a
COMPLETED report still carrying that worker's error_message after the user was
told the run was cancelled. Guards on is_terminal; no path resets a report out
of a terminal state, so this cannot strand a run that ought to finish.
The salvage path still emitted the legacy `budget_exhausted` warning beside the
structured terminal_reason. Warnings render verbatim to the user, so a step
overrun — or an agent that simply stopped — showed a chip claiming the token
budget was gone next to the reason saying otherwise. Dropped.
SalvagePathIntegrationTests drives _run_deep_research_async end to end through
the budget-exhaustion branch the pieces were only unit-tested against; all four
tests fail with the fixes reverted, as does the heading assertion.
Nits: delimiter-anchored requires_responses_api (gpt-5.6 no longer prefixes a
hypothetical gpt-5.60); env-spec expression hoisted out of three returns;
ADMIN_OPERATION rate limit on installAuthorityPack (no schema change);
per-ZIP member census memoised instead of rescanning infolist() per tagged
document; BLOB_HASH_CHUNK_BYTES replaces the two new 1024*1024 literals;
folder_name path-sanitization tests to match the title side.
* Close the Responses-API construction gap, pin the duplicate-member branch
Follow-up to c133183 — one review finding I had dropped, plus verification
gaps the fixes themselves opened.
The dropped finding: nothing instantiated a Responses-only model through the
DB-credentialed `_construct_model` path. The existing tests either mock
`_construct_model` or assert the env-credential STRING redirect, so no test
reached the branch that decides which pydantic-ai class a credentialed install
actually gets — and that class IS the endpoint: a gpt-5.6 model built as an
OpenAIChatModel 400s every agent in the install, visible only in a worker log.
Asserted end to end, with the negative half (an ordinary model must not move).
`requires_responses_api`'s anchoring was the one correctness change in the
previous commit with no test of its own; pinned now, and it fails with the
anchoring reverted.
`_member_name_counts` replaced duplicate detection inside a security check, and
only the count-0 (missing member) side was covered — the count>1 side is
precisely what `NameToInfo` cannot serve, since it keeps only the last entry
for a repeated name. A ZIP with a shadowing member now pins it.
`_compose_salvage_body`'s zero-findings fallback still read "before the research
budget was exhausted": the same mislabel removed from the warnings one layer
down, on a body composed for every non-finalize ending.
Verification: research suite passes under `-n 2 --dist loadscope` (exit 0), so
the new TransactionTestCase does not reintroduce the PipelineSettings-reseed
deadlock in the module's first non-TestCase class.1 parent 401d38c commit 83e4e51
263 files changed
Lines changed: 32542 additions & 993 deletions
File tree
- changelog.d
- config
- graphql
- settings
- docs
- architecture
- assets/images/screenshots/auto
- guides
- permissioning
- frontend
- src
- components
- admin
- authority
- corpus_groups
- corpuses
- CorpusHome/intelligence
- embeds
- knowledge_base/document/document_kb
- widgets
- chat
- modals
- graphql
- utils
- __tests__
- views
- tests
- e2e
- opencontractserver
- annotations
- management/commands
- migrations
- services
- constants
- corpuses
- management/commands
- services
- document_imports
- documents
- enrichment
- services
- llms
- agents
- tools
- core_tools
- pipeline
- base
- llm_providers
- research
- migrations
- services
- tasks
- tests
- fixtures/authority_packs/example_utility
- charters
- metadata
- personas
- specs
- research
- types
- utils
- scripts/authority_import
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
215 | 215 | | |
216 | 216 | | |
217 | 217 | | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
218 | 222 | | |
219 | 223 | | |
220 | 224 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
11 | 17 | | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
12 | 24 | | |
13 | 25 | | |
14 | 26 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
588 | 588 | | |
589 | 589 | | |
590 | 590 | | |
| 591 | + | |
| 592 | + | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
0 commit comments