[upstream-queue] PR-5 (5a): core suggestion generation — engine, provenance, enforcement, dedup#8
Open
damienriehl wants to merge 8 commits into
Conversation
Author
|
Pairs with web PR-5 (5b): alea-institute/ontokit-web#8 (suggestion review UI — accept/reject/edit, provenance surfacing, editor mounts). Merge/retarget together. |
Author
|
Eval-gate note (L4.12 rubric): the rubric-gated quality eval of generated suggestions (rubric + folio-python mechanical checks + FOLIO-MCP semantic judgment) requires real LLM generations — deferred with today's no-paid-API constraint and the pure-mirror dev deploy (this route isn't deployed until the queue PR lands). It should run with the first live generation exercise; the pipeline's mechanical contracts (per-type payload, provenance completeness, confidence normalization, gate ladder, dedup verdicts) are pinned by the 1688-test suite. |
…ss-branch search) Minimal phase-12 chain needed by the suggestion generation pipeline: - DuplicateRejection model + duplicate_check schemas (from 8226d9e) - StructuralSimilarityService — folio-python Jaccard, fail-soft to 0.0 (from 9a97712) - EmbeddingService.semantic_search_all_branches + SemanticSearchResultWithBranch schema (from e4f8a71), ported to the CAST(:param AS vector) bindparam style this branch already uses (::vector after a bindparam is silently dropped by SQLAlchemy text()) - DuplicateCheckService with 40/40/20 exact/semantic/structural scoring (from a884e27); reviewer-flow hunks (suggestion.py reject-linking fields, suggestion_service.py) dropped — they belong to the PR-6 review flow - HNSW index + duplicate_rejections migration (from 11d1d3a); also repairs the orphaned u9v0w1x2y3a4 down_revision (t8u9v0w1x2y3 was never extracted) to chain onto 47cc27515626 so alembic history is a single linear head again Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…nts/annotations/edges) Extracted from the phase-13 lineage (029d309, 220c88c, 4b54f96, c6d3cb7, 3399045, 481284a + final-state tests from 3b1874a/1fa317e): - schemas/generation.py: request/response contract, Provenance literal, GeneratedSuggestion with confidence + provenance + embedded validation and duplicate results - services/validation_service.py: VALID-01..06 rules, mint_iri, detect_project_namespace - services/context_assembler.py: ~2-4K-token ontology context assembly (quality_filter.py from the same lineage commit dropped — PR-6 reviewer concern, nothing in the generate path imports it) - services/llm/prompts/: 5 prompt templates + typed PROMPT_BUILDERS dispatch - services/suggestion_generation_service.py: context -> LLM -> parse -> validate -> dedup pipeline; confidence normalization (clamp 0-1, /100 for 0-100 scales); dedup failures fail-soft to verdict=pass - api/routes/generation.py: POST generate-suggestions + validate-entity; generate enforces role gate (403), rate limit (429, Redis fail-open), budget (402) BEFORE any LLM call — closes the PR-4 follow-up where the rate limiter shipped without enforcement - tests: generation schemas, entity validation, suggestion generation (GEN-01..09), context assembler; quality_filter-only tests trimmed Typing tightened for this branch's strict mypy gate (dict type args, typed PROMPT_BUILDERS, cast on JSON parse, isinstance guard on confidence). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
Per-suggestion provenance now carries model + prompt identity, not just the audit log (D-08: metadata only — raw prompt text is never persisted): - GeneratedSuggestion gains model: str | None and prompt_template: str | None - SuggestionGenerationService.generate() accepts model_id and stamps model=model_id, prompt_template=<PROMPT_BUILDERS key> on every suggestion - generate-suggestions route threads config.model into the pipeline - tests pin: provenance='llm-proposed', confidence in [0,1], model id, and prompt_template key on every generated suggestion; model stays None when no model_id is threaded (user-written paths) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…22/502 + success shape) Route-level coverage for the enforcement boundary, which the extracted lineage left thin: - 401/403 unauthenticated + non-member + viewer role-gate - 429 rate-limit exceeded — pins that check_rate_limit fires BEFORE any provider construction or budget check (the PR-4 'shipped but not wired' follow-up), plus the documented Redis fail-open path - 402 budget exhausted / daily cap, provider never constructed - 400 missing LLM config, 422 batch_size bounds, 404 unknown project and unknown class_iri, 502 provider auth error - success shape pins provenance/model/prompt_template on the wire and that config.model is threaded into the pipeline as model_id Also fixes the endpoints' user dependency: the lineage's 'Annotated[RequiredUser, Depends()]' double-wrap breaks on this branch's FastAPI (CurrentUser got re-interpreted as query params -> every request 422'd); switched to the bare 'user: RequiredUser' style the rest of this repo's routes use. Also isort-fixes the u9v0w1x2y3a4 migration touched by the chain repair. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
The generation route wires rate-limit enforcement, but its route-level fail-open path (Redis pool absent -> _get_redis returns None -> rate check skipped) was silent, so ops could not alert on unmetered LLM traffic. Now both fail-open layers log at WARNING: _get_redis narrows to Import/AttributeError and warns on an absent pool, and the route logs the bypass with user/project context when it skips the check. Behaviour is unchanged (still fail-open; DB budget cap in step 5 remains the non-fail-open backstop) -- only observability is added. Extends the redis-unavailable route test to assert the WARNING fires. Closes the PR-4 rate-limiter fail-open review follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…provenance/robustness
BLOCKER: edges and annotations returned empty, validation-failing stubs and
dropped all type-specific payload — the pipeline parsed a single generic
raw["label"] that the edge/annotation prompts never emit. Replace with a
per-type dispatch (_parse_typed):
- edges: parse target_label/target_iri/relationship_type; flag non-controlled
relationship_type and null target with GEN-05 validation errors instead of
silently dropping; label = target_label.
- annotations: parse property_iri/value/lang; carry the focus class IRI;
display label falls back to value.
- parents (HIGH): link an existing parent by its IRI instead of minting a
duplicate node; correct IS-A direction (web links class_iri -> parent).
- siblings: parent to the focus class's shared parents, not to the class
itself; children unchanged.
Move the type-specific fields (property_iri/value/lang/target_iri/
relationship_type) onto the flat base GeneratedSuggestion so list[...] response
serialization carries them and the shape matches the web contract.
HIGH: refuse generation with 400 when config.model is empty (every suggestion
must carry a resolvable model id for provenance — D-08).
HIGH: rewrite the edge/annotation/parent tests with real prompt-output shapes
and assert the payload round-trips (the old fixtures injected a "label" key,
masking the blocker).
MEDIUM: unexpected pipeline exceptions now surface as 500 instead of being
masked as an empty-200; only transient TimeoutError/ConnectionError fail soft.
MEDIUM: NaN/inf confidence -> None (clamping had promoted NaN to 1.0).
LOW: 502 returns a generic client message (provider detail logged server-side
only, not echoed); dedup-unavailable logs an alertable distinct WARNING;
provider param is honestly typed (Optional + runtime guard, no type: ignore).
Full pytest 1688 passed; ruff + mypy(strict) clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
The generation route's Redis-pool-absent bypass now emits the same FAIL_OPEN_EVENT=llm_rate_limiter_fail_open signal (with structured extra) as the limiter-internal fail-open paths, so all three degraded paths alert as one event. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eQ2CoAwpGv9Q8vBhuGGtH
Duplicate detection always searches across ALL branches (DEDUP-08), so the request's branch field was silently ignored. Remove it (nothing sends it — the duplicate-check route lands in PR-6 with no branch consumer) and document why no per-request branch scope exists. The candidate's found-on branch is still reported via DuplicateCandidate.branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eQ2CoAwpGv9Q8vBhuGGtH
damienriehl
force-pushed
the
upstream-queue/cost-controls
branch
from
July 11, 2026 02:56
f8a14d3 to
3df1cb3
Compare
damienriehl
force-pushed
the
upstream-queue/suggestion-generation
branch
from
July 11, 2026 02:56
f5c7bb7 to
5dab893
Compare
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.
PR-5 (5a / api) — Core suggestion generation ⭐
[upstream-queue]— baseupstream-queue/cost-controls(PR-4), UNMERGED. Part of the alea-fork upstream queue; do not merge (dev stays a pure upstream mirror).What
Server-side engine for AI-generated, reviewable ontology suggestions.
POST /projects/{id}/llm/generate-suggestionsruns the full pipeline: ontology context assembly → LLM → parse → validate → duplicate-check → typed suggestions, for all five suggestion types (children, siblings, parents, annotations, edges).POST .../validate-entityvalidates a manually-authored proposal against the VALID-* rules (no LLM).Suggestion provenance (Fr. John scrutiny target)
Every suggestion — across all five types — carries complete provenance:
model(id),prompt_template(dispatch key),confidence(normalized to [0,1]), andprovenance="llm-proposed". The audit log is metadata-only — raw prompt/response text is never persisted (D-08). Pinned bytest_suggestion_generation.py(GEN-08 / GEN-09 / D-08 across all types).Enforcement gate ladder (closes PR-4 rate-limiter fail-open follow-up)
generate-suggestionsenforces, BEFORE any provider/LLM work, in order: role gate (403) → LLM config (400) → rate limit (429) → budget (402). The rate limiter fails open on Redis unavailability (deliberate; the DB budget cap is the non-fail-open backstop), and both fail-open paths now log an alertable WARNING — the route-level absent-pool path (_get_redis) and the service-level Redis-infra-error catch — so ops can alert on unmetered LLM traffic. Fail-open is narrowed to infra errors; a mis-wired client still raises.Duplicate detection
DuplicateCheckServicecomposite scoring (exact 40 / semantic 40 / structural 20), cross-branch semantic search (HNSW index migration), verdict pass/warn/block. Fails soft topasson dedup error. Reviewer-flow hunks deferred to PR-6.Verification
dev(no generation route — this is an unmerged queue PR) and generation needs a paid LLM call (out of scope today). Pipeline behavior verified via mock-provider tests.Stack
Bottom commits are PR-1..PR-4 (base = PR-4 branch). Merge/rebase order: PR-1 → PR-2 → PR-3 → PR-4 → PR-5.
/ce:review outcome (kieran-python, two emphases)
abc9167).f5c7bb7):edgesandannotationsreturned empty, validation-failing stubs and dropped all type-specific payload — the pipeline parsed a single genericraw["label"]that those prompts never emit. Replaced with per-type dispatch (_parse_typed): edges parsetarget_label/target_iri/relationship_type(+ GEN-05 validation errors for non-controlled type / null target); annotations parseproperty_iri/value/langand carry the focus-class IRI. Type-specific fields moved to the flat baseGeneratedSuggestion(subclass fields would be dropped bylist[GeneratedSuggestion]serialization; matches the web contract).config.modelis empty (every suggestion must carry a resolvable model id). Edge/annotation/parent tests rewritten with real prompt-output shapes asserting payload round-trip (old fixtures injectedlabel, masking the blocker).🤖 Generated with Claude Code
https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q