Skip to content

approach.md

Latest commit

 

History

History
390 lines (330 loc) · 20.9 KB

approach.md

File metadata and controls

390 lines (330 loc) · 20.9 KB
Raw

Approach & Decision Log — Tri9Trace

0. Honest note up front

The CT-200 v1/v2 manuals (data/ct200_manual.md, data/ct200_manual_v2.md) were authored for this build. The provided manual files were not present in the repo or anywhere on disk when the build started, and the assignment needs them to demonstrate parser irregularities, the v1->v2 diff, and staleness. Rather than stop, I authored a realistic regulatory-style manual and a v2 with a diff deliberately constructed to exercise every matcher branch (unchanged / text-edited / renamed / moved / split / merged / new / ambiguous / deleted) and every staleness category (fresh / text-stale / structure-stale / intent-stale), including the dangerous "safety threshold rewritten, heading intact" case. This is a reasonable, documented default, not a silent guess: the parser irregularities are therefore real irregularities present in the actual file (I put them there deliberately and the tests target them), and the v2 diff is the actual diff of the file the system runs on. The downside (acknowledged): I did not get to test against a third-party-supplied manual's specific formatting quirks, so the validation pass (section 9) uses a real open-source standards document to close that gap partially.

1. Data model

Two stores, per TRD:

  • SQLite (SQLAlchemy): Document, DocumentVersion, Node, NodeVersion, Selection, SelectionItem (app/models.py).
  • MongoDB Atlas: one generations collection document per generation — test cases, raw prompt/response attempts, validation failures, full provenance.

Identity split (the key idea): Node is the durable logical identity that survives across versions; NodeVersion is the per-version snapshot with its own content_hash, parent_node_version_id, order_index, and a change_type relative to the node's prior version. This makes "same logical node across v1->v2" a simple join instead of re-deriving identity in every endpoint.

SelectionItem pins to a specific NodeVersion (node + version), not just a Node. That is what makes a stored selection resolve to the exact text the user picked even after the document is re-ingested — proven by tests/test_selection.py::test_selection_pin_survives_reingest.

change_type enum: unchanged | text-edited | moved | renamed | split | merged | new | ambiguous. deleted is deliberately not a stored value: a node is "deleted in version N" when its logical Node has a NodeVersion in N-1 but none in N. Deletion is derived at query time (app/ingest.py::deleted_in_version), not stored on a row that does not exist. The changes API surfaces it via deleted_in_to.

Migration: create_all, not Alembic. Justification in app/database.py: the schema is small and co-evolves with the code in one repo; Alembic's value (coordinated migrations across deployed instances with live data) does not pay off here. Break-even is the moment a column rename touches already-stored ingests — that is when I would switch.

2. Parsing decisions (app/parser.py)

markdown-it-py token stream identifies headings + levels (a # inside a code block is correctly NOT a heading — the advantage over a regex). For each heading, the body is the raw source lines between that heading and the next heading at any level, taken verbatim. That single rule means tables, embedded HTML, lists, and anything else markdown-it emits as block tokens are preserved exactly and never silently dropped — they are simply the text between two headings.

Irregularities found by a full manual read-through of data/ct200_manual.md (each has a dedicated test in tests/test_parser.py targeting the actual occurrence):

  1. Preamble before the first heading (title block + revision line). No preceding heading to attach to -> kept in a synthetic level-0 root node rather than discarded.
  2. Duplicate headings at the same level under the same parent (### Power Requirements twice under ## Installation, different bodies) -> two distinct node records, shared parent, distinct hashes, correct sibling order.
  3. Irregular heading depth jump H2 -> H4 (#### Components directly under ## Device Overview, no H3) -> parented to the H2, no missing H3 invented.
  4. Embedded raw HTML block (<div class="note">…</div>) inside a body -> kept verbatim.
  5. Markdown table (## Specifications is a pure-table section) -> kept verbatim, including the |---| separator.

How found: read-through first, then confirmed against parser output (notes/section2_irregularities.md). Not test-driven discovery — I read the file, listed what was non-uniform, then wrote tests for those.

Known parser limitation (knowingly left unhandled — see decision log Q3): the manual uses descriptive, un-numbered headings. A real regulatory manual often uses auto-incremented numbered headings (## 5. Specifications); inserting a section renumbers every following heading, which would make the matcher see a wave of "renamed" nodes. The matcher does not separate a section's semantic title from its auto-incremented number. This is the single biggest "would break first on a real numbered manual" gap.

3. Version-matching strategy (app/matcher.py)

Layered, exactly per TRD section 3, in order:

  1. exact content_hash -> unchanged
  2. normalized heading identical AND parent-chain path identical -> text-edited
  3. body similarity >= 0.6 AND heading similarity >= 0.6 -> same node, then moved if the parent path (excluding the node's own heading) changed, else renamed if the heading changed
  4. split / merge heuristic (phase B), candidates logged, never auto-resolved
  5. no match -> new (v2 side); prev node with no v2 match -> deleted (derived)
  6. can't confidently place -> ambiguous

Thresholds (SIM_BODY=0.6, SIM_HEADING=0.6, PARTIAL=0.4, MERGE_CONF=0.5, SPLIT_SEC=0.5) were calibrated against the actual v1->v2 similarity ratios (notes/section3_thresholds.md): confident matches sit well above SIM_* genuine merge partners above PARTIAL, while coincidental short-body word overlap (e.g. Maintenance Procedures vs FAQ at 0.34) sits below PARTIAL.

Split detection is per previous node (not per new node): a prev node P that was matched to a primary M but not fully preserved (M is not unchanged) is split only if some still-unmatched new node shares

= SPLIT_SEC body with P. This avoids the false-split storm where any short new node overlapping a matched prev on common words gets called a split — a real bug I hit and fixed during the build (the first version flagged ~half the tree as split).

Known failure mode (TRD section 3, demonstrated in this data): a heading rewritten AND restructured AND with materially different body text in the same edit gets misclassified as new + deleted rather than moved+renamed, because no single signal crosses its threshold. In the CT-200 diff this is the ## Troubleshooting + ## FAQ -> ## Troubleshooting and FAQ rewrite: body similarity to Troubleshooting is only ~0.50 (heavy paraphrase) and to FAQ ~0.34, so it is surfaced as ambiguous (not a clean merge) with its candidate, and both originals are derived-deleted. This is surfaced, not hidden.

4. LLM prompt design + retry (app/llm.py, app/generation.py)

  • System prompt (TRD section 6, verbatim intent): generate QA test-case drafts for a medical-device requirements section, output ONLY valid JSON matching the schema, each case concrete and executable (specific trigger + specific expected result).
  • User prompt: reconstructed selected node text (headings + bodies in document order) + the JSON schema. response_format={"type":"json_object"} is requested to nudge valid JSON.
  • Pydantic schema: TestCase{title, trigger, expected_result, source_node_ids} and GenerationResult{test_cases: 3-5} with a field_validator enforcing the 3-5 bound.
  • Retry: on ValidationError, retry ONCE with the validation error message appended to the prompt ("your previous response failed validation because: ..."). On second failure: store the raw failure (both attempts), return HTTP 422, mark the generation failed — never fabricate.
  • Idempotency: by (selection_id, sha256(sorted resolved node_version_ids)). Same input returns the same stored output unless ?force=true. Tradeoff: you cannot "just try again for a better answer" without the explicit flag — a deliberate choice (TRD section 6) so demo/testing drift is debuggable.
  • Provenance stored on every generation (success or failure): prompt_template_version, schema_version, model_id, raw attempts, timestamp, and per-source-node {node_version_id, node_id, version_number, heading_text, body_text, content_hash}.

A real successful generation (5 cases from v1 Safety Limits) and the failure/retry -> 422 path were run against real Groq + real MongoDB Atlas (notes/section6_demo.md, scripts/generate_demo.py).

5. Staleness detection (app/staleness.py)

Per TRD section 7, at retrieval time, per source node:

  1. look up the node's current NodeVersion in the document's latest version;
  2. content_hash unchanged -> fresh;
  3. changed and change_type in {moved, split, merged, ambiguous, new}, or the node has no snapshot in the current version (deleted) -> structure-stale;
  4. changed and change_type in {text-edited, renamed} -> one intent-stale LLM call per test case: still holds -> text-stale; any no-longer-holds -> intent-stale;
  5. recall bias: unparseable / ambiguous intent output defaults to holds=false -> intent-stale, never silently fresh.

Overall generation staleness = worst-case across source nodes (fresh < text-stale < structure-stale < intent-stale).

Recall-biased policy, stated explicitly (PRD differentiator #5): thresholds and the intent check are tuned to over-flag rather than under-flag. A false "stale" costs a reviewer a glance; a missed stale flag is the actual danger (patient safety in the real-world analog). The known cost is false positives — e.g. in the real Groq run (notes/section7_demo.md) the intent judge flagged the temperature test cases as no-longer-holds with shaky reasoning (it even mis-stated "45 C is within 10-40 C"). The overall intent-stale verdict was still correct (the threshold/load cases genuinely broke), but per-test-case accuracy is only as good as that one extra LLM call. This is the soft spot named in TRD section 7: a one-word typo and a changed pressure threshold both produce text-edited at the matcher layer; the intent check is the only thing that tells them apart, and it is not perfect.

The cosmetic case in the real diff (Warnings: "near open flame" -> "near an open flame") is correctly classified text-stale, not over-flagged intent-stale, when the judge says it still holds (tests/test_staleness.py::test_text_stale_on_cosmetic_warnings_change).

6. Retrieval (app/api/retrieval.py)

GET /generations/{id}/staleness and GET /generations?selection_id= / ?node_id= both call compute_staleness directly — staleness is inlined in retrieval, not computed separately and left unused (PRD success criterion).

Cost note (decision log Q2): the list endpoint runs an intent LLM call per text-edited source node per generation. For a handful of generations that is fine (and tests use a fake LLM), but at scale this is O(generations x text-edited nodes x test cases) LLM calls on a GET — unacceptable in production. See "what I'd do differently".

7. Validation pass (PRD section 7 / TRD section 9)

Document chosen: the OpenAPI Specification README (data/validation_openapi_readme.md), fetched from https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/README.md Why: it is a real, freely-available, open-source standards specification document (regulatory/standards-style), and its formatting is deliberately different from the CT-200 manual — heavy inline links and images, badges, bulleted lists, a thematic-break ---, long paragraphs — so it is a real robustness check that the parser does not just work on the assignment's specific formatting.

Result of running it through app.parser.parse_markdown:

  • Worked: parsed cleanly into 6 nodes (1 H1 + 5 H2). Heading levels, parent links, and sibling order all correct. Bodies preserved verbatim, including paragraphs dense with inline links/images, the bulleted Participation list, and the trailing --- thematic break. Nothing was dropped. No preamble node (the H1 is the first line).
  • Nothing broke on this document.
  • Gap exposed (knowingly left unhandled, no feature built): the parser is heading-driven. A real regulatory document that uses numbered sections without markdown headings (e.g. a raw GPL-style license plain text, or a doc whose structure is 1. / 1.1. paragraphs rather than #/##) would collapse to a single node (a preamble root with the whole document as its body), because markdown-it sees no headings. This is the same limitation as the numbered-heading identity problem in section 2. Per the instructions, no new feature was built to accommodate it; it is recorded here as a known boundary of the parser's applicability.

Reproduce:

uv run python -c "from app.parser import parse_markdown; ns=parse_markdown(open('data/validation_openapi_readme.md',encoding='utf-8').read()); print(len(ns), 'nodes'); [print(n.level, n.heading_text) for n in ns]"

8. What I'd do differently with more time

  • Separate a heading's semantic title from its auto-incremented number so numbered regulatory manuals don't turn every insertion into a rename wave. (The biggest real-world gap.)
  • Handle setext-style headings explicitly in tests (markdown-it supports them, but I did not exercise them).
  • Make staleness asynchronous/cached: compute structural + hash staleness synchronously (cheap), and run intent checks as a background job with cached verdicts keyed by (old_hash, new_hash, test_case_hash, model_id), so the retrieval list endpoint is not O(LLM-calls).
  • Replace the difflib body ratio with token-overlap / TF-IDF so heavy paraphrases (the Troubleshooting&FAQ case) match more reliably without lowering the confident-match threshold.
  • Add an ingest HTTP endpoint with auth + idempotency instead of a library call, for real operational use.
  • Use Alembic once the schema stabilises and there is stored data to migrate.

Decision log (required questions)

Q1 — What is the one part most likely to silently give wrong results without erroring, and how would you catch it?

The node matcher (app/matcher.py). It runs silently and stamps a change_type on every node; a misclassification (e.g. a real moved+renamed edit misread as new+deleted, or a coincidental word overlap misread as a merge) produces no error — just a wrong label that then drives wrong staleness downstream. It is exactly the "looks valid but no longer matches intent" failure the PRD names.

How I catch it today: explicit unit tests on the actual v1->v2 diff (tests/test_ingest.py) assert the classification of every change_type, and notes/section3_thresholds.md records the measured similarity ratios so threshold choices are auditable. How I'd catch it more rigorously with more time: a golden-diff regression fixture — a curated set of (v1, v2, expected_change_types) triples checked into the repo, run on every change, so a threshold tweak that flips a classification fails the build loudly instead of silently shifting behaviour.

Q2 — Where did you choose simplicity over correctness because of time, and what breaks first in production?

Two places:

  1. Staleness on the retrieval list endpoint is computed synchronously and calls the LLM per text-edited node per test case. Simple and correct for a handful of generations; what breaks first in production is performance/cost — a GET /generations?selection_id= over many generations becomes dozens of Groq calls and multi-second latency, and one flaky LLM call can 500 a read. Fix: compute cheap structural/hash staleness synchronously, run intent checks async with cached verdicts.
  2. difflib.SequenceMatcher character ratio for body similarity. Simple and dependency-free; what breaks first is heavy paraphrase — a rewritten section with the same meaning scores low and gets misclassified new/deleted (the Troubleshooting&FAQ case in this very data). Fix: token-overlap / embeddings similarity.

Q3 — Name one input you did not handle, and what the system does when it sees it.

A markdown document whose structure is carried by numbered paragraphs rather than #/## headings (e.g. a raw license plain-text, or 1./1.1.-structured regulatory text with no ATX/setext headings). The system does not error — markdown-it finds no heading tokens, so parse_markdown produces a single synthetic level-0 preamble node whose body is the entire document. Ingestion then stores one NodeVersion. Nothing crashes, but the tree is degenerate (one node) and browse/search are useless. This is a stated boundary of the parser's applicability (section 7), knowingly left unhandled per the validation-pass instructions (no new feature built), and it is the same root limitation as the numbered-heading identity problem in section 2.

9. Staleness policy and false-positive cost (PRD success criterion)

Stated plainly: the system is recall-biased — it is deliberately tuned to over-flag borderline cases as stale rather than under-flag them. A false "stale" costs a reviewer a few seconds to glance and dismiss; a missed stale flag (a test case that looks valid but no longer matches a changed safety threshold) is the actual danger. The explicit costs:

  • The intent judge can produce false intent-stale positives (the temperature cases in the real run were over-flagged with flawed reasoning). The overall verdict was still correct, but per-test-case reasons are not authoritative.
  • The matcher can produce false structure-stale / ambiguous on heavy paraphrase (Troubleshooting&FAQ) — flagged rather than guessed.
  • Unparseable intent output is treated as stale, not fresh.

These are accepted, documented tradeoffs, not hidden behaviour.

Commit history note (Section 9 final review)

The git history is nine real incremental commits, one per section, in order (Section 1 scaffolding -> Section 2 parser -> Section 3 matcher -> Section 4 browse -> Section 5 selection -> Section 6 generation -> Section 7 staleness -> Section 8 retrieval -> Section 9 docs/validation). It reads as incremental work, not one dump.

Bonus — compliance-mapping report layer (beyond the 9 sections)

Clearly an add-on: it reuses app/staleness.py (compute_staleness) and app/storage.py (find_by_node) and adds no columns to the core data model. New code: app/compliance.py (taxonomy + classifier + classification cache) and app/api/compliance.py (report builder + endpoint).

  • Taxonomy: 8 fixed categories (label + one-line description) covering IEC 62304 lifecycle/maintenance, ISO 14971, FDA 21 CFR 820.30, IEC 62366, IEC 60601-1, IEC 60601-1-8, and SOUP. A 9th sentinel unclassified is the fallback, not a real category.
  • Classifier: one LLM call per test case; system prompt lists the 8 categories; output validated by a strict Pydantic model (category must be exactly one of the 8 labels, confidence in [0,1]). On validation failure: one retry with the error fed back (same pattern as the generation retry), then a safe fallback to unclassified/0.0 — a bad classification never raises or loses the report.
  • Cache: classifications are keyed by (generation_id, test_case_index) in a swappable store (in-memory for tests, MongoDB classifications collection for real use), so repeat report requests do not re-call the LLM for a test case already classified.
  • Report status per node: Uncovered (no generation), Partially covered (generation exists but failed / no test cases), Covered (has test cases and worst-case staleness fresh), Stale (has test cases and any non-fresh staleness). Staleness is computed only over generations that actually have test cases, so a failed generation on a text-edited node does not falsely force a covered node to Stale.
  • Aggregates: functional_coverage_pct = (Covered+Stale+Partially covered) / total; stale_coverage_pct = Stale / (Covered+Stale) (0.0 when the denominator is 0). review_backlog = nodes whose current change_type != unchanged AND whose most-recent generation was pinned to an older version. category_breakdown counts unique classified test cases (deduped by generation_id + index) per category with average confidence.
  • Disclaimer: present verbatim on every response.

What I'd do differently here with more time: (1) persist classifications with a content hash key (test_case text + model_id) so identical test cases across generations share a classification, not just same generation+index; (2) compute staleness once per generation in the report (currently cached within a single report build, which is enough); (3) make the report incremental/async for large corpora since it is O(test cases) LLM calls.