Skip to content

chore(b1): delete the /convert compatibility shim (PR C of 3, register #51) — DO NOT MERGE until N5 passes - #73

Closed
tornidomaroc-web wants to merge 1 commit into
feat/b1a-service-ingest-with-convert-shimfrom
chore/b1c-remove-convert-shim
Closed

chore(b1): delete the /convert compatibility shim (PR C of 3, register #51) — DO NOT MERGE until N5 passes#73
tornidomaroc-web wants to merge 1 commit into
feat/b1a-service-ingest-with-convert-shimfrom
chore/b1c-remove-convert-shim

Conversation

@tornidomaroc-web

Copy link
Copy Markdown
Owner

PR C of 3 in the (b1) sequence. SERVICE-ONLY. DRAFT, and it must stay a draft until N5 passes.

Deletes the deprecated /convert compatibility shim from services/ingestion/main.py. That is the entire code change.


⛔ THIS MUST NOT MERGE UNTIL N5 PASSES

N5 (docs/b1-verification-protocol.md §3, §10) has two halves and both are required:

  1. grep -rn "/convert" src/ returns zero hits, and
  2. the production Vercel deployment's commit SHA is at or after PR B's merge commit.

Neither half is satisfied today. src/app/api/ingest/route.ts:154 still calls ${pythonServiceUrl}/convert, and PR B — the PR that repoints it — does not exist yet.

The second half is the one that cannot be shortcut. A merged PR B that Vercel has not finished deploying leaves production Next still calling /convert against an image that no longer serves it.

Merging this early is an instant production outage, not a degradation. Every upload's POST /convert returns 404, !pyResponse.ok fires, and every ingestion fails until a rebuild lands.

N6 is the post-merge backstop: if POST /convert does not return 404 after this merges, PR C did not deploy (§9 — Railway Watch Paths is empty today; a Watch Paths pattern set between B and C that never matches would let PR C merge green while /convert stays live forever, with no symptom anywhere).


⚠️ AND IT IS NOT OPTIONAL

Once PR B makes uploads work again, the temptation is to leave the shim in place as harmless. It is not.

If PR C is dropped:

  • Register deps: bump @supabase/ssr 0.6.1 -> 0.10.2 (pinned exact) #50's 9-13× return-blob endpoint — full markdown plus every chunk's 1024-float embedding, returned inline — stays live indefinitely, behind a shared bearer token.
  • The "loud 404 on version skew" property that the entire /convert/ingest rename was chosen to buy is silently gone. A version-skewed caller would find a working old endpoint and quietly persist a null markdown_content over a document this service already finished correctly, instead of failing visibly.

The rename only pays for itself once the old name is actually absent. Until this PR lands, (b1) is not done — it is half-done with a regression parked in production.


Why this is stacked on PR A's branch, not on main

Based on main, this PR's diff would carry PR A's five files as well, and one merge would add /ingest and delete /convert simultaneously against the old caller — precisely the single-merge skew window the three-PR split was invented to eliminate, reachable from the green merge button.

Stacked on feat/b1a-service-ingest-with-convert-shim, the diff is exactly the two files below and the bad merge is mechanically unavailable. GitHub retargets this PR to main automatically when PR A merges, keeping this number, URL and body.


The diff, proven rather than asserted

git diff --numstat against PR A's head (c27163a):

4	0	docs/PROGRESS.md
0	48	services/ingestion/main.py

main.py is a pure deletion: 0 insertions, 48 deletions, the @app.post("/convert") handler and nothing else.

Survivors, from an ast walk of the post-deletion module — not from intent:

ROUTES:
  GET  /health  -> health
  POST /ingest  -> ingest
  POST /embed   -> embed_endpoint

FUNCS: _check_auth, _chunk_text, _convert_to_markdown, _embed, _embed_bge_m3,
       _embed_voyage, _load_owned_document, _mark_error, _persist,
       _safe_basename, _user_client, embed_endpoint, health, ingest

_convert_to_markdown is deliberately not deleted even though it sits directly above the shim and reads like part of it — it is the converter /ingest itself calls, so removing it with the handler would break the endpoint this whole sequence exists to ship.

No import is orphaned by the deletion, which is why there is no ride-along cleanup: UploadFile, File, Header, HTTPException, JSONResponse and Form are still used by /ingest, Body by /embed, and shutil / tempfile / MarkItDown by _convert_to_markdown. An unused-import removal here would have made the diff non-pure for zero benefit.


What this PR deliberately does NOT touch


Checks

Both pre-commit guards pass on the staged set:

secrets scan clean (2 files)
dash scan clean (0 user-facing files)

The required checks on this PR are not evidence. tsc and db-types will both pass vacuously — this PR contains zero TypeScript and zero SQL. The ingestion-image job (register #52) is the only check that reads a line of Python here, and it is not a required check.


Register #51. Sequence: PR A (#72, open) → PR B (does not exist yet) → PR C (this).

…#51)

PR C of the three-PR (b1) sequence, SERVICE-ONLY. Removes the deprecated
/convert handler from services/ingestion/main.py. The main.py diff is a pure
deletion: 0 insertions, 48 deletions, that handler and nothing else.

Survivors asserted from the file, not from intent: routes GET /health,
POST /ingest, POST /embed; helpers _check_auth, _convert_to_markdown,
_chunk_text, _embed and the rest. _convert_to_markdown stays because /ingest
calls it. No import is orphaned, so there is no ride-along cleanup.

MUST NOT MERGE until N5 passes: nothing under src/ calls /convert AND the
production Vercel deployment is at or after PR B's merge commit.
src/app/api/ingest/route.ts:154 still calls /convert today and PR B does not
exist yet, so merging now is an instant production outage.

docs/b1-verification-protocol.md is deliberately untouched: N5 and N6 are
filled when the checks actually run, not at open time.

Zero files under src/. No migration, no DB change, no requirements.txt change,
no branch-protection change, no Railway or Vercel dashboard change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
knowflow Ready Ready Preview, Comment Jul 28, 2026 3:28pm

tornidomaroc-web added a commit that referenced this pull request Jul 28, 2026
The row this PR added at docs/PROGRESS.md:150 was a SINGLE-CELL row in a
four-column table: 2 pipes, where the header (| # | Item | Why deferred |
Where addressed |) and every other row from #1 to #53 have 5 or more. Its
Item, Why deferred and Where addressed cells were empty and its entire body
rendered inside the narrow # column.

Repaired here rather than in a later PR for one reason: this PR introduced the
defect and this PR is still open, so fixing it here means the malformed row
never reaches main at all.

Separate commit, no amend and no force-push, so the defect stays visible in
this PR's history, the same way the vacuous-green fix was handled on #71.

ONE FACTUAL CORRECTION rides along, disclosed rather than folded in silently.
The old status narrative opened "PR A of 3 landed 2026-07-26" and marked
"A = ... (landed)". PR A has not landed: gh pr view 72 reports state OPEN with
mergedAt null and no merge commit, and git branch --contains c27163a lists only
the two feature branches, not main. PROGRESS.md:9 already contradicted it,
correctly describing the same work as the active branch. A §4 row asserting a
merge that did not happen is the false-check class registers #39 and #23 exist
to police, so the status cell now reads "opened 2026-07-26; PR A NOT YET MERGED
(PR #72 open as of 2026-07-28)" and Where addressed records A as PR #72, OPEN,
not merged.

Content otherwise preserved and redistributed as the neighbouring rows do:
status in #, the substance in Item, the three-PR rationale in Why deferred, and
the sequence with its gates, closing rule and related registers in Where
addressed.

One line replaced by one line. CRLF preserved. §7 untouched: zero additions and
zero deletions inside it. No change to services/, none to src/, no migration,
no DB change, no branch-protection change, no Railway or Vercel dashboard
change; PR #73 and PR #70 untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tornidomaroc-web added a commit that referenced this pull request Jul 29, 2026
…encies

V3 is the one row §10 assigns to PR A pre-merge ("PR A fills V3 on its own
branch before merge, since the Railway variables are readable now and item 8
gates PR A pre-merge"). It was blank, and §10's own rule is that empty cells
are not pass by default.

Run at 2026-07-29 15:57 UTC. The role claim decodes to anon, not service_role.
The value is a legacy Supabase JWT, three dot-separated segments, so the
sb_publishable_ branch of the check did not apply and was not exercised. The
Railway service carries no SUPABASE_SERVICE_ROLE_KEY; its four user variables
are INGESTION_TOKEN, VOYAGE_API_KEY, SUPABASE_URL and SUPABASE_ANON_KEY.

The key material itself is deliberately absent from this file and this message.
No key value, no JWT, no project ref, no iat, no exp. The role claim and the
absence list ARE the evidence; the secret is not evidence and does not belong
in version control.

TWO DEPENDENCIES ARE RECORDED BELOW THE TABLE, UNSOFTENED, because a protocol
that records more certainty than its evidence carries is the exact failure this
file exists to prevent:

  1. The absence half is a dashboard read performed by the repository owner,
     not by the agent recording the row. No tooling here enumerated Railway's
     variables. "No SUPABASE_SERVICE_ROLE_KEY" is attested, not machine-verified.

  2. Decoding proves the value examined is an anon key. It does not prove it is
     the value the deployed service holds. The decode ran against a copy, and
     nothing in this check ties that copy to the variable Railway injects at
     runtime.

V3 is therefore PASS on the claim as stated and no wider. Deployment column
records "Dashboard read plus local decode; no Railway deployment", the same
shape V11 uses for its CI-runner note, because this row observed no deployed
artifact.

Third separate commit on this branch. No amend, no force-push, so each step
stays legible in the PR history.

One row changed and one note added. Every row in the table keeps 8 pipes. LF
preserved: the file is LF in both the working tree and the blob, and the staged
blob carries zero CR bytes. No other cell in the table touched, no other section
touched, none of services/, none of src/, no migration, no DB change, no
branch-protection change, no Railway or Vercel dashboard change; PR #73 and
PR #70 untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tornidomaroc-web added a commit that referenced this pull request Jul 29, 2026
…PR A of 3, register #51) (#72)

PR A of 3 for register #51. The ingestion service gains `/ingest` and KEEPS
the `/convert` shim, so this merge is additive: nothing that calls `/convert`
today breaks. The shim is deleted separately in PR C (#73), and only after N5
proves nothing calls it.

Three commits, deliberately separate and never amended, so each step stays
legible in this PR's history:

  c27163a  the service change: /ingest added, /convert retained as a shim
  80dc5f7  repair of register row #51 into the four cells §4's header declares
  e6640ba  fill V3 in §10 with the decode result and record its two dependencies

V3 is filled and PASSES. The Railway anon key's `role` claim decodes to `anon`,
not `service_role`; it is a legacy Supabase JWT, so the `sb_publishable_` branch
of the check did not apply. The Railway service carries no
SUPABASE_SERVICE_ROLE_KEY, its four user variables being INGESTION_TOKEN,
VOYAGE_API_KEY, SUPABASE_URL and SUPABASE_ANON_KEY. No key material appears in
this repository: the role claim and the absence list are the evidence, and the
secret is not evidence.

TWO DEPENDENCIES ARE RECORDED BENEATH §10's TABLE AND ARE NOT DISCHARGED BY THIS
MERGE. The absence half is a dashboard read performed by the repository owner,
not by tooling in this repo. And decoding proves the value examined is an anon
key without proving it is the value the deployed service holds. V3 is PASS on
the claim as stated and no wider. A protocol that records more certainty than
its evidence carries is the failure docs/b1-verification-protocol.md exists to
prevent, so the limit is disclosed directly under the row rather than implied.

STILL OPEN AFTER THIS MERGE, by design, not by oversight. V1, V2, V4, V5, N4 and
N9 are only observable once this image is live, and §10 assigns them to a
docs-only PR to main between PR A and PR B. None of them are filled here. PR B
may not merge while any of those rows is blank (V12).

Scope: five files, none under src/. No migration, no DB change, no branch
protection change, no Railway or Vercel settings change. PR #73 and PR #70
untouched.
@tornidomaroc-web
tornidomaroc-web deleted the branch feat/b1a-service-ingest-with-convert-shim July 29, 2026 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant