fix(s31): close 1 review BLOCKER — per-call retriever + run_portfolio forwards semantic_retrieval
This commit is contained in:
parent
969d450b31
commit
fc69285f2c
3 changed files with 231 additions and 57 deletions
|
|
@ -388,16 +388,23 @@ async def run_project(
|
|||
# post-generation into a discarded SessionContext (step 7 below), so a prior verdict could not
|
||||
# reach the next hypothesis. Bundle-driven path with a populated store only; the road path is
|
||||
# untouched (its post-hoc, proposal-keyed retrieval below is unchanged).
|
||||
# S3.1 opt-in: install the hybrid ranker on the RESOLVED store, immediately before the fold it
|
||||
# is meant to affect. Anchoring matters — set at the verdict_dir-conditional resolution above
|
||||
# it would miss every run without an inbox, and set after the fold it would rank nothing that
|
||||
# reaches this prompt. Flag off => store.retriever stays None => StructuralRetriever default.
|
||||
if semantic_retrieval and store is not None:
|
||||
store.retriever = HybridRanker(FakeEmbedder(), similarity, SEMANTIC_WEIGHT_DEFAULT)
|
||||
# S3.1 opt-in: build the hybrid ranker as a LOCAL, then pass it explicitly at each retrieval
|
||||
# this run performs. It is deliberately not assigned to ``store.retriever``: the store is
|
||||
# caller-owned (``run_portfolio`` threads one store across every project, and a library caller
|
||||
# may reuse theirs), so a store-global assignment leaked this run's opt-in into every later use
|
||||
# of that object — including a subsequent run with the flag OFF. Flag off => ranker stays None
|
||||
# => ``retrieve`` falls through to the StructuralRetriever default.
|
||||
ranker = (
|
||||
HybridRanker(FakeEmbedder(), similarity, SEMANTIC_WEIGHT_DEFAULT)
|
||||
if semantic_retrieval
|
||||
else None
|
||||
)
|
||||
|
||||
if bundle_dir is not None and store is not None and store.verdicts:
|
||||
expel_query = bundle_candidate_features(bundle_dir)
|
||||
fewshot = ExpeLContextProvider(store, expel_query, k=top_k).format_fewshot()
|
||||
fewshot = ExpeLContextProvider(
|
||||
store, expel_query, k=top_k, retriever=ranker
|
||||
).format_fewshot()
|
||||
gen_context = f"{fewshot}\n\n{gen_context}"
|
||||
|
||||
# 5. Structured candidate -> blocking validation on the NUMBERS; token bound = the meter.
|
||||
|
|
@ -457,10 +464,10 @@ async def run_project(
|
|||
# is NOT what reaches the prompt.
|
||||
store = store if store is not None else VerdictStore(verdicts=[])
|
||||
features = _features_of(proposal)
|
||||
provider = ExpeLContextProvider(store, features, k=top_k)
|
||||
provider = ExpeLContextProvider(store, features, k=top_k, retriever=ranker)
|
||||
sctx = SessionContext(input_messages=[], instructions=[])
|
||||
await provider.before_run(agent=None, session=None, context=sctx, state={})
|
||||
retrieved = store.retrieve(features, k=top_k) if store.verdicts else []
|
||||
retrieved = store.retrieve(features, k=top_k, retriever=ranker) if store.verdicts else []
|
||||
|
||||
# 8. Layer-2 (out-of-band): capture the durable verdict + persist; B11 notify is a stub.
|
||||
verdict = capture_verdict(features, verdict_input["decision"], verdict_input["rationale"])
|
||||
|
|
@ -571,10 +578,9 @@ async def run_portfolio(
|
|||
projects = {p.id: p for p in load_reference_projects()}
|
||||
ids = list(project_ids) if project_ids is not None else list(projects)
|
||||
store = store if store is not None else VerdictStore(verdicts=[])
|
||||
# S3.1: one shared store is threaded across every project, so installing the hybrid here —
|
||||
# before the loop — covers every project's Step-1 fold in this pass.
|
||||
if semantic_retrieval:
|
||||
store.retriever = HybridRanker(FakeEmbedder(), similarity, SEMANTIC_WEIGHT_DEFAULT)
|
||||
# S3.1: the flag is FORWARDED per project rather than installed on the shared store here. The
|
||||
# store is threaded across every project in the pass (and may be caller-owned), so a pre-loop
|
||||
# assignment outlived the pass; forwarding keeps the opt-in scoped to each run's own retrievals.
|
||||
ledger = ledger if ledger is not None else SavingsLedger(entries=[])
|
||||
goals = goals if goals is not None else GoalConfig()
|
||||
portfolio_baseline_ore = _to_ore(sum(projects[p].total_cost for p in ids if p in projects))
|
||||
|
|
@ -625,6 +631,7 @@ async def run_portfolio(
|
|||
max_rounds=max_rounds,
|
||||
max_tokens=max_tokens,
|
||||
top_k=top_k,
|
||||
semantic_retrieval=semantic_retrieval,
|
||||
meter=meter_factory() if meter_factory is not None else None,
|
||||
),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -264,20 +264,30 @@ class VerdictStore:
|
|||
that want a fresh store straight from a folder."""
|
||||
return cls(verdicts=load_verdicts_from_dir(directory))
|
||||
|
||||
def retrieve(self, query: ProposalFeatures, k: int) -> list[Verdict]:
|
||||
def retrieve(
|
||||
self,
|
||||
query: ProposalFeatures,
|
||||
k: int,
|
||||
*,
|
||||
retriever: semretrieval.Retriever | None = None,
|
||||
) -> list[Verdict]:
|
||||
"""Return the top-``k`` verdicts. Deterministic: ties break by verdict id, so ordering
|
||||
is stable across runs.
|
||||
|
||||
Ranking is delegated to ``self.retriever``, defaulting to ``StructuralRetriever`` — the
|
||||
same weighted structural score and ``(-similarity, id)`` key as before the seam existed.
|
||||
A caller that installs a ``HybridRanker`` opts into an additional semantic term."""
|
||||
Ranking resolves in three steps: the explicit ``retriever`` argument, then
|
||||
``self.retriever``, then ``StructuralRetriever`` — the same weighted structural score and
|
||||
``(-similarity, id)`` key as before the seam existed.
|
||||
|
||||
``retriever`` is PER CALL and keyword-only. It exists because the alternative — assigning
|
||||
``store.retriever`` — mutates an object the caller owns, so an opt-in made for one
|
||||
retrieval silently governed every later use of that store (including a subsequent run with
|
||||
the flag OFF). ``self.retriever`` survives as the store-level default for callers that
|
||||
genuinely want a store to rank one way for its whole lifetime; see ``docs/extending.md``."""
|
||||
if k <= 0:
|
||||
raise ValueError(f"k must be positive, got {k}")
|
||||
ranker = (
|
||||
self.retriever
|
||||
if self.retriever is not None
|
||||
else semretrieval.StructuralRetriever(similarity)
|
||||
)
|
||||
ranker = retriever if retriever is not None else self.retriever
|
||||
if ranker is None:
|
||||
ranker = semretrieval.StructuralRetriever(similarity)
|
||||
return ranker.rank(query, self.verdicts, k)
|
||||
|
||||
def add(self, verdict: Verdict) -> None:
|
||||
|
|
@ -298,14 +308,24 @@ class ExpeLContextProvider(ContextProvider):
|
|||
"""Wraps ``VerdictStore.retrieve`` for ExpeL few-shot injection via the GA
|
||||
``ContextProvider`` hook."""
|
||||
|
||||
def __init__(self, store: VerdictStore, query: ProposalFeatures, *, k: int = 3) -> None:
|
||||
def __init__(
|
||||
self,
|
||||
store: VerdictStore,
|
||||
query: ProposalFeatures,
|
||||
*,
|
||||
k: int = 3,
|
||||
retriever: semretrieval.Retriever | None = None,
|
||||
) -> None:
|
||||
super().__init__(source_id="expel-verdictstore")
|
||||
self._store = store
|
||||
self._query = query
|
||||
self._k = k
|
||||
# Per-call ranker, threaded straight through to ``retrieve`` — see its docstring for why
|
||||
# this is a parameter rather than an assignment on the store.
|
||||
self._retriever = retriever
|
||||
|
||||
def format_fewshot(self) -> str:
|
||||
hits = self._store.retrieve(self._query, self._k)
|
||||
hits = self._store.retrieve(self._query, self._k, retriever=self._retriever)
|
||||
body = "\n".join(f"- [{v.id}] {v.decision}: {v.rationale}" for v in hits)
|
||||
return f"Relevant prior verdicts (ExpeL few-shot):\n{body}"
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue