-
Notifications
You must be signed in to change notification settings - Fork 11
feat(cascade): heading-nav, two-tier retrieval routing, and large-read gate #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
tkcoding
wants to merge
10
commits into
constructorfabric:main
Choose a base branch
from
tkcoding:jit-retrieval-cascade-gate
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
d5e2be7
feat(toc,doc-index): add JIT-retrieval readiness checks and cached do…
512975d
feat(doc-index): infer real section granularity, hash sections for st…
66b9cc2
fix(doc-index): resolve CodeRabbit review findings on PR #109
9cf51f4
fix(doc-index): resolve second CodeRabbit review round on PR #109
42ae3d1
feat(tfidf,okf): add TF-IDF scoring and a local OKF bundle
1d0c082
fix(tfidf,okf,doc-index): resolve Spec Coverage failure on PR #110
36aaf83
fix(tfidf): resolve SonarCloud Security Rating failure on PR #110
7dd694c
fix(doc-index,okf,toc): resolve CodeRabbit final review round on PR #110
bbfc925
feat(cascade): add heading-nav, two-tier retrieval routing, and read-…
7af88ac
fix(cascade,okf,tfidf,decision-log,toc): resolve CodeRabbit findings …
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Large diffs are not rendered by default.
Oops, something went wrong.
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| """Studio retrieve command — route a query against a Markdown file through | ||
| the two-tier JIT-retrieval cascade (heading-nav + TF-IDF, falling back to an | ||
| OKF-vs-baseline choice), and report the routing decision. | ||
|
|
||
| Thin CLI wrapper around ``studio.utils.cascade``. | ||
|
|
||
| @cpt-flow:cpt-studio-flow-traceability-validation-validate:p1 | ||
| """ | ||
|
|
||
| import argparse | ||
| from typing import List | ||
|
|
||
| from ..utils.cascade import route_query | ||
| from ..utils.ui import ui | ||
|
|
||
|
|
||
| # @cpt-begin:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-cmd | ||
| def cmd_retrieve(argv: List[str]) -> int: | ||
| """Route a query against a Markdown file through the JIT-retrieval cascade.""" | ||
| p = argparse.ArgumentParser( | ||
| prog="cfs retrieve", | ||
| description="Route a query through the two-tier JIT-retrieval cascade and report the decision.", | ||
| ) | ||
| p.add_argument("file", help="Markdown file path") | ||
| p.add_argument("query", help="Query text") | ||
| p.add_argument( | ||
| "--margin-threshold", type=float, default=None, | ||
| help="Enable a numeric TF-IDF margin cutoff for a large-margin Tier 1 resolution " | ||
| "(default: disabled -- only an unambiguous score counts)", | ||
| ) | ||
| p.add_argument( | ||
| "--expected-future-queries", type=int, default=None, | ||
| help="Expected future query volume against this document, for the OKF-vs-baseline break-even math", | ||
| ) | ||
| args = p.parse_args(argv) | ||
|
|
||
| filepath = ui.require_existing_file(args.file) | ||
| if filepath is None: | ||
| return 2 | ||
|
|
||
| result = route_query( | ||
| filepath, args.query, | ||
| margin_threshold=args.margin_threshold, | ||
| expected_future_queries=args.expected_future_queries, | ||
| ) | ||
|
|
||
| output = {"file": str(filepath), **result} | ||
| ui.result(output, human_fn=_human_retrieve) | ||
| return 0 | ||
| # @cpt-end:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-cmd | ||
|
|
||
|
|
||
| # @cpt-begin:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-cmd-format | ||
| def _human_retrieve(data: dict) -> None: | ||
| ui.header("Retrieve") | ||
| ui.substep(f"query: {data['query']!r}") | ||
| ui.substep(f"tier: {data['tier']} ({data['reason']})") | ||
| for c in data["candidates"]: | ||
| ui.substep(f" [{c['line_start']}-{c['line_end']}] {c['heading']}") | ||
| if "tier2" in data: | ||
| tier2 = data["tier2"] | ||
| ui.substep(f"tier 2 recommendation: {tier2['recommendation']} ({tier2['reason']})") | ||
| if tier2.get("okf_needs_rebuild"): | ||
| ui.substep(" OKF bundle exists but is stale/missing for this candidate -- needs a rebuild") | ||
| if "read_gate" in data and data["read_gate"]["needs_confirmation"]: | ||
| gate = data["read_gate"] | ||
| ui.substep(f"read gate: needs confirmation ({gate['total_lines']} lines > {gate['threshold']})") | ||
| ui.blank() | ||
| # @cpt-end:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-cmd-format | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| """Studio doc-index command — build/reuse a cached structural index for a | ||
| Markdown file, so heading-based JIT retrieval reads a file's structure once, | ||
| not once per query. | ||
|
|
||
| Thin CLI wrapper around ``studio.utils.doc_index``. | ||
|
|
||
| @cpt-flow:cpt-studio-flow-traceability-validation-validate:p1 | ||
| """ | ||
|
|
||
| import argparse | ||
| from typing import List | ||
|
|
||
| from ..utils.doc_index import get_or_build_doc_index | ||
| from ..utils.ui import ui | ||
|
|
||
|
|
||
| # @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cmd | ||
| def cmd_doc_index(argv: List[str]) -> int: | ||
| """Build (or reuse the cached) structural index for a Markdown file.""" | ||
| p = argparse.ArgumentParser( | ||
| prog="cfs doc-index", | ||
| description=( | ||
| "Build or reuse a cached heading/section index for a Markdown file, " | ||
| "so navigation reads the file's structure once, not once per query." | ||
| ), | ||
| ) | ||
| p.add_argument("file", help="Markdown file path") | ||
| p.add_argument( | ||
| "--rebuild", | ||
| action="store_true", | ||
| help="Force a fresh build even if a valid cached index exists", | ||
| ) | ||
| args = p.parse_args(argv) | ||
|
|
||
| filepath = ui.require_existing_file(args.file) | ||
| if filepath is None: | ||
| return 2 | ||
|
|
||
| index = get_or_build_doc_index(filepath, force_rebuild=args.rebuild) | ||
|
|
||
| output = { | ||
| "file": str(filepath), | ||
| "cache_hit": index["cache_hit"], | ||
| "total_lines": index["total_lines"], | ||
| "section_count": len(index["sections"]), | ||
| "sections": index["sections"], | ||
| "section_level": index["section_level"], | ||
| "retrieval_section_count": len(index["retrieval_sections"]), | ||
| "retrieval_sections": index["retrieval_sections"], | ||
| } | ||
| ui.result(output, human_fn=_human_doc_index) | ||
| return 0 | ||
| # @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cmd | ||
|
|
||
|
|
||
| # @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cmd-format | ||
| def _human_doc_index(data: dict) -> None: | ||
| ui.header("Doc Index") | ||
| hit = "cache hit — reused existing index" if data["cache_hit"] else "cache miss — built fresh index" | ||
| ui.substep(hit) | ||
| ui.substep(f"{data['section_count']} section(s), {data['total_lines']} total lines") | ||
| for s in data["sections"]: | ||
| summary = f" — {s['summary']}" if s.get("summary") else "" | ||
| ui.substep(f" H{s['level']} [{s['line_start']}-{s['line_end']}] {s['heading']}{summary}") | ||
| ui.blank() | ||
|
|
||
| level = data["section_level"] | ||
| ui.step(f"Retrieval sections (level {level}, {data['retrieval_section_count']} section(s))" if level is not None | ||
| else "Retrieval sections (no headings — none inferred)") | ||
| for s in data["retrieval_sections"]: | ||
| summary = f" — {s['summary']}" if s.get("summary") else "" | ||
| ui.substep(f" [{s['line_start']}-{s['line_end']}] {s['heading']} ({s['hash'][:12]}){summary}") | ||
| ui.blank() | ||
| # @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cmd-format |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| """Studio heading-nav command — grep a Markdown file's retrieval sections | ||
| for a query's literal text, for inspecting/benchmarking the JIT-retrieval | ||
| mechanical gate independent of any cascade routing logic built on top of it. | ||
|
|
||
| Thin CLI wrapper around ``studio.utils.heading_nav``. | ||
|
|
||
| @cpt-flow:cpt-studio-flow-traceability-validation-validate:p1 | ||
| """ | ||
|
|
||
| import argparse | ||
| from typing import List | ||
|
|
||
| from ..utils.heading_nav import find_sections | ||
| from ..utils.ui import ui | ||
|
|
||
|
|
||
| # @cpt-begin:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-cmd | ||
| def cmd_heading_nav(argv: List[str]) -> int: | ||
| """Find a Markdown file's retrieval sections containing a query literally.""" | ||
| p = argparse.ArgumentParser( | ||
| prog="cfs heading-nav", | ||
| description="Find a Markdown file's retrieval sections containing a query's literal text.", | ||
| ) | ||
| p.add_argument("file", help="Markdown file path") | ||
| p.add_argument("query", help="Query text to search for, literally") | ||
| args = p.parse_args(argv) | ||
|
|
||
| filepath = ui.require_existing_file(args.file) | ||
| if filepath is None: | ||
| return 2 | ||
|
|
||
| result = find_sections(filepath, args.query) | ||
|
|
||
| output = { | ||
| "file": str(filepath), | ||
| "query": args.query, | ||
| "matches": result["matches"], | ||
| "first_match": result["first_match"], | ||
| } | ||
| ui.result(output, human_fn=_human_heading_nav) | ||
| return 0 | ||
| # @cpt-end:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-cmd | ||
|
|
||
|
|
||
| # @cpt-begin:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-cmd-format | ||
| def _human_heading_nav(data: dict) -> None: | ||
| ui.header("Heading-Nav Search") | ||
| ui.substep(f"query: {data['query']!r}") | ||
| if not data["matches"]: | ||
| ui.substep("(no matches -- this method has no semantic fallback)") | ||
| ui.blank() | ||
| return | ||
| for entry in data["matches"]: | ||
| ui.substep(f" {entry['hit_count']}x [{entry['line_start']}-{entry['line_end']}] {entry['heading']}") | ||
| ui.blank() | ||
| # @cpt-end:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-cmd-format |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| """Studio okf-status command — report an OKF bundle's state for a Markdown | ||
| file: which concept files exist, are stale, or are missing entirely, | ||
| relative to the document's current retrieval sections. | ||
|
|
||
| Read-only. Writing a concept file is an external caller's job (an agent | ||
| that has actually produced a summary) via | ||
| ``studio.utils.okf.write_concept_file`` -- this command never invokes an | ||
| LLM itself. | ||
|
|
||
| Thin CLI wrapper around ``studio.utils.okf``. | ||
|
|
||
| @cpt-flow:cpt-studio-flow-traceability-validation-validate:p1 | ||
| """ | ||
|
|
||
| import argparse | ||
| from typing import List | ||
|
|
||
| from ..utils.okf import get_okf_status | ||
| from ..utils.ui import ui | ||
|
|
||
|
|
||
| # @cpt-begin:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-cmd | ||
| def cmd_okf_status(argv: List[str]) -> int: | ||
| """Report an OKF bundle's state for a Markdown file.""" | ||
| p = argparse.ArgumentParser( | ||
| prog="cfs okf-status", | ||
| description="Report which OKF concept files exist, are stale, or are missing for a Markdown file.", | ||
| ) | ||
| p.add_argument("file", help="Markdown file path") | ||
| args = p.parse_args(argv) | ||
|
|
||
| filepath = ui.require_existing_file(args.file) | ||
| if filepath is None: | ||
| return 2 | ||
|
|
||
| status = get_okf_status(filepath) | ||
| output = {"file": str(filepath), **status} | ||
| ui.result(output, human_fn=_human_okf_status) | ||
| return 0 | ||
| # @cpt-end:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-cmd | ||
|
|
||
|
|
||
| # @cpt-begin:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-cmd-format | ||
| def _human_okf_status(data: dict) -> None: | ||
| ui.header("OKF Status") | ||
| if not data["available"]: | ||
| ui.substep("no Studio directory found -- OKF is unavailable for this file") | ||
| ui.blank() | ||
| return | ||
| ui.substep(f"bundle: {data['bundle_dir']}") | ||
| counts: dict = {} | ||
| for entry in data["entries"]: | ||
| counts[entry["status"]] = counts.get(entry["status"], 0) + 1 | ||
| summary = ", ".join(f"{count} {status}" for status, count in sorted(counts.items())) or "no sections" | ||
| ui.substep(summary) | ||
| for entry in data["entries"]: | ||
| ui.substep(f" [{entry['status']:>7}] [{entry['line_start']}-{entry['line_end']}] {entry['heading']}") | ||
| ui.blank() | ||
| # @cpt-end:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-cmd-format |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--margin-thresholdaccepts values that defeat the cascade's own documented safety marginSeverity: Major
Problem
commands/cascade.py's--margin-thresholdargument usestype=floatwith no validator, so any float — negative, zero, or non-finite — reachesutils/cascade.py'sroute_tier1(). The module's own docstring states the design basis explicitly: real measured data showed only an infinite margin was safe on a correct pick, while finite margins of 1.06x-1.58x still occurred on two independently wrong picks — "unambiguous is safe, anything finite isn't yet proven safe." A negative or near-zero--margin-thresholdmakestfidf_result["margin"] >= margin_thresholdtrue for virtually any margin, soagree_large_marginfires and Tier 1 returns a confident"resolved"instead of escalating — exactly the case the module's design notes say is unsafe. (Ananthreshold does not have this effect: NaN comparisons are always false, somargin >= nannever fires and Tier 1 simply falls back to the conservativeunambiguous-only path — that is a fail-safe no-op, not a defeat of the escalation logic.) The codebase already has an established pattern for exactly this hazard:commands/eval.py's_compliance_argrejects any value wherenot math.isfinite(parsed)before accepting it as an argparse type.cascade.pydoes not reuse or mirror that pattern.How to reproduce
cfs cascade <file> <query> --margin-threshold -1.unambiguous).route_tier1returns{"tier": "resolved", ...}instead of{"tier": "escalate", "reason": "diffuse_margin", ...}.Expected behavior
A negative, zero, or otherwise out-of-range
--margin-thresholdshould be rejected at the CLI boundary, consistent with the project's own stated design intent that no finite margin has yet been proven safe.Actual behavior
Any float, including negative values, is silently accepted and used directly in the safety-relevant comparison, letting an operator (accidentally or otherwise) force Tier 1 to resolve confidently on cases the module's own design notes classify as unsafe.
Impact
Silently weakens the one documented safety guard in the cascade's decision logic, with no error or warning at invocation time. Given this is optional/off-by-default, impact is limited to callers who explicitly pass the flag, but there is nothing stopping an unvalidated or accidentally-negative value from being passed.
Suggested correction
Add a small validator function for
--margin-threshold(mirroring_compliance_argincommands/eval.py) that requiresmath.isfinite(value) and value > 0, and use it as the argparsetype=.How to verify
Add a unit test asserting the CLI rejects
--margin-threshold -1,--margin-threshold 0with a clear argparse error, and that a valid positive finite value still passes through toroute_tier1unchanged.