From 03753101b6c0132eb2fd98cd2a91ae97667fdae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 09:12:35 +0200 Subject: [PATCH 01/16] docs(markdown): add design spec for convert-markdown integration Reviewed design for routing crawled HTML and binary documents through convert-markdown.ifad.org so body holds Markdown. Records the insertion point, transport choice, config shape, failure semantics and test plan. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- .../2026-09-04-markdown-conversion-design.md | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-04-markdown-conversion-design.md diff --git a/docs/superpowers/specs/2026-09-04-markdown-conversion-design.md b/docs/superpowers/specs/2026-09-04-markdown-conversion-design.md new file mode 100644 index 00000000..91f72c8e --- /dev/null +++ b/docs/superpowers/specs/2026-09-04-markdown-conversion-design.md @@ -0,0 +1,148 @@ +# Markdown conversion via convert-markdown.ifad.org — Design Spec + +Date: 2026-09-04. Status: reviewed (architect review), approved for v1 implementation. + +## Goal + +Crawled HTML pages and binary documents (PDF, DOCX, XLSX, PPTX) are stored in Elasticsearch with `body` holding Markdown produced by the shared IFAD service `convert-markdown.ifad.org`, so every IFAD app gets identical conversions. Plain-text extraction stays as the fallback path. + +## Non-goals (v1) + +- Local HTML→Markdown inside the crawler. +- Skip-unchanged re-crawl logic (needs bulk `update` op; phase 3). +- Separate conversion thread pool (phase 3, only if the service becomes highly parallel). +- Service-side changes (affinity routing, light/heavy executors, cache) — separate track in the `convert-markdown` repo. + +## convert-markdown API contract (existing, do not change) + +- `POST {base_url}/api/v1/convert/upload?wait=<0..60>` — multipart field `file`, filename MUST carry a supported extension (`.pdf .docx .xlsx .pptx .html .htm`), else HTTP 422. Legacy `.doc .xls .ppt` → 422. +- Response JSON (both 200 and 202): `{job_id, status: pending|processing|done|failed, status_url: "/api/v1/jobs/", source_name, markdown?, chars?, pages?, tables?, error?}`. HTTP 200 when `status` is `done` or `failed` within `wait`; 202 otherwise. **Branch on `status`, not HTTP code.** +- `GET {base_url}{status_url}` — same JSON; 404 when the job is unknown/expired. +- `GET {base_url}/api/v1/health` — 200 when ready. +- Service is single-worker with an in-process job store; conversions are serialised. Envoy default per-route timeout may be 15s. + +## Crawler-side design + +### Insertion point + +`Coordinator#process_crawl_result` (`lib/crawler/coordinator.rb`, the `elsif crawl_task.content?` branch, currently lines ~399-402). Conversion runs on the crawl-task thread, after the rule engine and BEFORE `output_crawl_result`. Rationale: `OutputSink::Elasticsearch#write` holds `@queue_lock` across `to_doc` → `DocumentMapper#create_doc`; a slow call there serialises all conversions and other threads get `SinkLockedError` (retry 1s × 120 then dropped). Target diff: + +```ruby +elsif crawl_task.content? + crawl_task_progress(crawl_task, 'converting to markdown') + config.markdown_converter.convert!(crawl_result) + crawl_task_progress(crawl_task, 'ingesting the result') + outcome = output_crawl_result(crawl_result) + extracted_event.merge!(outcome) +end +``` + +When disabled, `convert!` MUST return without touching the crawl result (coordinator specs use RSpec doubles that only respond to `url, canonical_link, extract_links, meta_nofollow?, error?, html?, redirect?`). + +### Config (`lib/crawler/api/config.rb`) + +New top-level key `markdown_conversion` (nested hash, like `elasticsearch`). Add to `CONFIG_FIELDS` and `DEFAULTS`; unknown keys are rejected by `validate_param_names!`. + +```yaml +markdown_conversion: + enabled: false + base_url: https://convert-markdown.ifad.org # required when enabled; http(s) only + wait_seconds: 10 # 0..60, sent as ?wait= + poll_interval: 2 # seconds between polls; back off ×1.5 up to 5s + timeout: 900 # hard per-document deadline (submit+poll), seconds + on_failure: text # text | skip + ca_file: # optional PEM path for Net::HTTP#ca_file +``` + +Validation in a `configure_markdown_conversion!` step (pattern: `configure_extraction_rules!`): when enabled, `base_url` present and http/https; `on_failure` ∈ {text, skip}; `wait_seconds` 0..60; when enabled and output_sink is elasticsearch, `max_body_size < elasticsearch.bulk_api.max_size_bytes` (default 1 MB) or raise ArgumentError with a clear message. Expose `config.markdown_converter` memoised (`@markdown_converter ||= ::Crawler::MarkdownConverter.new(self)`) next to `document_mapper`. + +Also add `application/vnd.openxmlformats-officedocument.spreadsheetml.sheet` to the commented example MIME list in `config/crawler.yml.example` and document the new block there. + +### `Crawler::MarkdownConverter` (`lib/crawler/markdown_converter.rb`, new) + +Public API: + +- `initialize(config)` +- `enabled?` → bool +- `convertible?(crawl_result)` → bool: enabled AND (`crawl_result.html?` OR `crawl_result.content_extractable_file?` with a MIME in the map below). +- `convert!(crawl_result)` → `:converted | :skipped | :failed`. Never raises. Sets `crawl_result.markdown = String` on success, leaves it `nil` otherwise. Increments `stats[:converted]` / `stats[:failed]`. +- `stats` → `{ converted: Integer, failed: Integer }` (thread-safe: use `Concurrent::AtomicFixnum` or a Mutex). +- `healthy?` → bool (GET `/api/v1/health`, 5s timeout). Called once from `Crawler::API::Crawl#start!` when enabled; if false, raise so the crawl fails fast (a silent converter outage must not degrade the whole index to text). + +MIME → extension map (strip `;charset=...` and downcase the content type first): + +| MIME | ext | +|---|---| +| text/html | .html | +| application/pdf | .pdf | +| application/vnd.openxmlformats-officedocument.wordprocessingml.document | .docx | +| application/vnd.openxmlformats-officedocument.spreadsheetml.sheet | .xlsx | +| application/vnd.openxmlformats-officedocument.presentationml.presentation | .pptx | + +Anything else (incl. `application/msword`, `application/vnd.ms-powerpoint`) → not convertible → `:skipped`, and the existing `_attachment` / Tika path handles it. + +Upload filename: `"#{crawl_result.url_hash}#{ext}"`. Never use `ContentExtractableFile#file_name` (it is `File.basename(url)` and includes query strings / lacks extensions). + +Payload: +- HTML: serialise from Jsoup, not raw bytes, so `exclude_tags` and `data-elastic-exclude/include` directives are honoured. Use `crawl_result.parsed_content_excluding_tags(config.exclude_tags)` (memoised, `lib/crawler/data/crawl_result/html.rb`), apply `Crawler::ContentEngine::Transformer.transform!` to its `body` (see `document_body` in html.rb for the existing sequence), set `doc.charset(java.nio.charset.StandardCharsets::UTF_8)` and upload `doc.outerHtml.to_java_bytes`. Do NOT mutate the memoised parsed document that link extraction uses — clone it first (`doc.clone`). +- Binary: `crawl_result.content` bytes as-is (`Success#content`). + +Transport: Ruby `net/http` (NOT httpclient5 Java classes, NOT `java.net.http` — `bin/crawler` sets `https.protocols=SSLv3` JVM-wide which breaks JSSE clients; Net::HTTP goes through jruby-openssl and WebMock can stub it). Hand-built multipart body (boundary from `SecureRandom.hex`), `Content-Type: multipart/form-data; boundary=...`. `open_timeout` 10s, `read_timeout` = `wait_seconds + 20`. `use_ssl` when https; `ca_file` when configured. `User-Agent: "#{config.user_agent}"`. + +Flow: +1. `POST /api/v1/convert/upload?wait=#{wait_seconds}`. +2. Parse JSON. If `status == done` → markdown. If `failed` → `:failed` (log `error`). If `pending`/`processing` → poll `GET base_url + status_url` every `poll_interval` (×1.5 backoff, cap 5s) until `done`/`failed` or the `timeout` deadline (measured from submit start) passes → `:failed`. +3. Retry ONCE (after 1s) on: connection error, `Net::OpenTimeout`/`Net::ReadTimeout`, HTTP 5xx, HTTP 404 on poll. Never retry HTTP 422, 4xx other than 404-on-poll, or `status: failed`. +4. Empty/blank markdown → `:failed`. +5. Apply `Crawler::ContentEngine::Utils.limit_bytesize(markdown, config.max_body_size)` before assigning. +6. Every failure: `system_logger.warn` with url, reason; never raise. + +### `Crawler::Data::CrawlResult::Success` + +Add `attr_accessor :markdown` (nil by default). Crawl results are created and consumed on one task thread; memoising here also means the `SinkLockedError` retry loop does not reconvert. + +### `Crawler::DocumentMapper` (`lib/crawler/document_mapper.rb`) + +- `html_fields`: `body: crawl_result.markdown || crawl_result.document_body(...)` (existing args), plus `body_format: crawl_result.markdown ? 'markdown' : 'text'`, `content_hash: Digest::SHA1.hexdigest(crawl_result.content)`. +- `binary_file_fields`: when `crawl_result.markdown` present → `body: markdown, body_format: 'markdown'`, and OMIT `_attachment`; otherwise unchanged (`_attachment` kept, `body_format: 'text'`). Always `content_hash: crawl_result.content_hash`. +- Add `body_format` and `content_hash` to `Constants::RESERVED_FIELD_NAMES` (`lib/constants.rb`) so extraction rules cannot clobber them. +- Existing spec expectations for docs without markdown must stay green except for the two added fields. + +### `OutputSink::Elasticsearch` (`lib/crawler/output_sink/elasticsearch.rb`) + +`pipeline_params`: when `config.markdown_conversion[:enabled]`, default `_reduce_whitespace` to `false` (the ingest pipeline does `\s+ → ' '` on `body`, destroying markdown). If the user explicitly sets `_reduce_whitespace: true` with markdown enabled, `system_logger.warn` and honour it. Keep `_extract_binary_content: true` (harmless without `_attachment`; enables the Tika fallback). + +### `OutputSink::Console` + +When `crawl_result.respond_to?(:markdown) && crawl_result.markdown`, print the markdown instead of refusing binary content / raw HTML. Two-line change; optional but cheap. + +### Stats + +`Crawler::API::Crawl#print_final_crawl_status` (or wherever ingestion stats are printed, `lib/crawler/api/crawl.rb` ~216-233): when enabled, also log `Markdown conversions: converted=N failed=N`. + +### Docs + +`docs/features/MARKDOWN_CONVERSION.md`: what it does, config block, fallback semantics (`text` vs `skip` and the purge interaction: with `purge_crawl_enabled: true`, a page whose conversion fails under `skip` is not re-indexed and is deleted by the purge because `last_crawled_at` is stale), throughput caveat (service serialises jobs; `threads_per_crawl` bounds in-flight conversions), replica-affinity caveat. + +## Failure semantics summary + +| Situation | on_failure: text | on_failure: skip | +|---|---|---| +| converter healthy, doc converts | body=markdown, body_format=markdown, no `_attachment` | same | +| unsupported MIME (legacy .doc) | body from Tika via `_attachment` (binary) / text (HTML) | same (not a failure) | +| converter error / timeout / `failed` job | HTML: body=text; binary: `_attachment` kept, body_format=text | doc not written; `sink.failure` outcome; warn | +| converter unreachable at crawl start | crawl aborts with clear error | same | + +## Test strategy + +- `spec/lib/crawler/markdown_converter_spec.rb` (WebMock; `spec_helper` already has `disable_net_connect!(allow_localhost: true)`): 200 inline done; 202 pending → poll → done; `failed`; 422; 5xx then retry succeeds; 5xx twice fails; `Net::ReadTimeout`; 404 on poll; overall deadline; MIME→filename incl. `; charset=utf-8` stripping; HTML payload has excluded tags removed and UTF-8 meta and original parsed doc untouched; binary payload byte-identical; disabled → no HTTP call and result untouched; multipart body shape (field `file`, filename, content-type); blank markdown → failed; `max_body_size` truncation; stats counters. +- `spec/lib/crawler/document_mapper_spec.rb`: HTML with/without markdown; binary with markdown has no `_attachment`; `body_format`, `content_hash` present. +- `spec/lib/crawler/coordinator_spec.rb`: converter invoked once before `sink.write` for content results; not for denied/redirect; disabled path leaves existing double-based specs green; `skip` policy returns failure outcome without writing. +- `spec/lib/crawler/output_sink/elasticsearch_spec.rb`: `_reduce_whitespace` default flips; explicit true warns. +- `spec/lib/crawler/api/config_spec.rb`: defaults, unknown key rejected, `base_url` required when enabled, `on_failure` enum, bulk-size validation. +- `spec/integration/markdown_conversion_spec.rb`: `FauxCrawl` site with an HTML page and a PDF, converter stubbed via WebMock at `http://converter.test`; assert file-sink docs have markdown bodies and `body_format`; second scenario with 5xx asserts text fallback. +- Tests run in Docker (no local JRuby): image `crawler-ci` built from `Dockerfile`; run `script/rspec ` inside a container with the repo mounted (see plan for exact command). + +## Upstream hygiene + +Fork `ifad/crawler`, branch `feature/markdown-conversion` off `upstream/1.0`. Keep the diff surgical: all logic in the new `markdown_converter.rb`; coordinator change is 3 lines. Conventional commits, small, one per task. From 0f58ceecc4a2b17dbd93f9907f87456e9d395e8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 09:53:26 +0200 Subject: [PATCH 02/16] docs(markdown): add implementation plan for convert-markdown integration Nine TDD tasks derived from the design spec, validated by tech-lead review. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- .../plans/2026-09-04-markdown-conversion.md | 2428 +++++++++++++++++ 1 file changed, 2428 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-04-markdown-conversion.md diff --git a/docs/superpowers/plans/2026-09-04-markdown-conversion.md b/docs/superpowers/plans/2026-09-04-markdown-conversion.md new file mode 100644 index 00000000..d493e6ef --- /dev/null +++ b/docs/superpowers/plans/2026-09-04-markdown-conversion.md @@ -0,0 +1,2428 @@ +# Markdown Conversion Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Crawled HTML pages and PDF/DOCX/XLSX/PPTX files are stored with `body` holding Markdown produced by the shared `convert-markdown.ifad.org` service, with plain-text extraction as the fallback. + +**Architecture:** A new `Crawler::MarkdownConverter` (config-driven, `net/http` transport, WebMock-testable) is called from `Coordinator#process_crawl_result` on the crawl-task thread, after the rule engine and before the output sink. It sets `crawl_result.markdown`; `DocumentMapper` prefers that over text/`_attachment` and adds `body_format` + `content_hash`. The Elasticsearch sink stops collapsing whitespace when markdown is enabled, and `Crawl#start!` fails fast if the converter is unhealthy. + +**Tech Stack:** JRuby 9.4, Ruby `net/http` (through jruby-openssl), Jsoup 1.23.2 (`org.jsoup`), `concurrent-ruby` (`Concurrent::AtomicFixnum`), RSpec + FactoryBot + WebMock, Faux test sites, rubocop (line length 120, `Metrics/MethodLength` 15, `Metrics/AbcSize` 20, `Metrics/CyclomaticComplexity` 8, `Metrics/ClassLength` 200). + +**Spec:** `docs/superpowers/specs/2026-09-04-markdown-conversion-design.md` + +## Global Constraints + +- Branch `feature/markdown-conversion` off `upstream/1.0`; conventional commits, one per task, prefixes `feat(markdown):`, `test(markdown):`, `docs(markdown):`. Do not commit anything else. +- All logic lives in the new `lib/crawler/markdown_converter.rb`; the coordinator change stays minimal (one call site plus one small helper). +- `convert!` never raises. When disabled it MUST return without calling any method on the crawl result (coordinator specs use strict RSpec doubles that only respond to `url, canonical_link, extract_links, meta_nofollow?, error?, html?, redirect?`). +- Transport is Ruby `net/http` only. NOT httpclient5 Java classes, NOT `java.net.http` (`bin/crawler` sets `https.protocols=SSLv3` JVM-wide, which breaks JSSE clients). Multipart bodies are hand-built, binary-encoded (`ASCII-8BIT`), `Content-Length` set by Net::HTTP from `body.bytesize`. +- Converter API contract (do not change): `POST {base_url}/api/v1/convert/upload?wait=<0..60>` multipart field `file` with a filename carrying `.pdf .docx .xlsx .pptx .html .htm`; JSON `{job_id, status: pending|processing|done|failed, status_url, markdown?, error?}`; branch on `status`, not HTTP code; `GET {base_url}{status_url}`; `GET {base_url}/api/v1/health`. +- Retry ONCE (after 1 s) on connection errors, `Net::OpenTimeout`/`Net::ReadTimeout`, HTTP 5xx, HTTP 404 while polling. Never retry HTTP 422, other 4xx, or `status: failed`. +- Poll `poll_interval` seconds, backoff ×1.5, cap 5 s; hard `timeout` deadline measured from submit start. +- `open_timeout` 10 s, `read_timeout` = `wait_seconds + 20`, health check 5 s, `User-Agent: config.user_agent` on every request (health check included), `use_ssl` for https, `ca_file` when configured, `Net::HTTP.new(host, port, nil)` so env `HTTP_PROXY`/`HTTPS_PROXY` are ignored. +- Every file starts with the Elastic license header comment and `# frozen_string_literal: true` (copy from any existing `lib/` file). +- Tests run in Docker (no local JRuby, ~10 s overhead per run). The image is `crawler-dev`: build `crawler-ci` from the repo `Dockerfile`, then layer `bundle config unset without && bundle install` on top of it and tag the result `crawler-dev`. **Test command** (run from the repo root; every task's Run step refers to this): + + ```bash + docker run --rm --user root -v "$PWD:/home/app" -w /home/app -e HOME=/home/app -e IS_DOCKER=1 crawler-dev \ + -c "bundle config unset without >/dev/null; JRUBY_OPTS=--debug bundle exec rspec " + # lint: same, with `bundle exec rubocop ` instead of the rspec command + ``` + +--- + +## File Structure + +| File | Responsibility | +|---|---| +| `lib/crawler/api/config.rb` (modify) | `markdown_conversion` nested config: field, nested defaults, validation, `config.markdown_converter` factory | +| `lib/crawler/markdown_converter.rb` (create) | Everything conversion-related: enablement, MIME map, filename, payload builders, multipart transport, polling, retry, health check, stats | +| `lib/crawler/data/crawl_result/success.rb` (modify) | `attr_accessor :markdown` | +| `lib/constants.rb` (modify) | reserve `body_format`, `content_hash` | +| `lib/crawler/coordinator.rb` (modify) | call converter before the sink; `on_failure: skip` outcome | +| `lib/crawler/api/crawl.rb` (modify) | health check at crawl start; conversion stats log line | +| `lib/errors.rb` (modify) | `Errors::MarkdownConverterUnavailableError` | +| `lib/crawler/document_mapper.rb` (modify) | `body`/`body_format`/`content_hash`; omit `_attachment` when markdown present | +| `lib/crawler/output_sink/elasticsearch.rb` (modify) | `_reduce_whitespace` default flips to `false` when markdown enabled; warn when explicitly `true` | +| `lib/crawler/output_sink/console.rb` (modify) | print markdown when present | +| `spec/support/faux/faux_crawl.rb` (modify) | pass `markdown_conversion:` option through to the crawl config | +| `config/crawler.yml.example`, `docs/features/MARKDOWN_CONVERSION.md`, `README.md` (modify/create) | documentation | + +Specs: `spec/lib/crawler/api/config_spec.rb`, `spec/lib/crawler/data/crawl_result/success_spec.rb` (new), `spec/lib/constants_spec.rb` (new), `spec/lib/crawler/markdown_converter_spec.rb` (new), `spec/lib/crawler/coordinator_spec.rb`, `spec/lib/crawler/api/crawl_spec.rb`, `spec/lib/crawler/document_mapper_spec.rb`, `spec/lib/crawler/output_sink/elasticsearch_spec.rb`, `spec/lib/crawler/output_sink/console_spec.rb` (new), `spec/integration/markdown_conversion_spec.rb` (new). + +Existing helpers you will reuse: +- Factories (`spec/factories/crawl_results.rb`): `FactoryBot.build(:html_crawl_result, url:, content:)` and `FactoryBot.build(:content_extractable_file_crawl_result, url:, content:, content_length:, content_type:)`. `FactoryBot::Syntax::Methods` is included, so `build(...)` also works. +- WebMock is enabled in `spec/spec_helper.rb` (`WebMock.disable_net_connect!(allow_localhost: true)`); use `stub_request`, `a_request`, `have_been_requested`. +- `FauxCrawl.run(site, options)` (`spec/support/faux/faux_crawl.rb`) runs a full crawl against a local Faux site using the `:mock` sink and returns a `ResultsCollection` of crawl results (`results.crawl`, `results.crawl_config`). +- `mock_response(...)` + `have_only_these_results` matchers (`spec/support/mock_response.rb`, `spec/support/crawl_response_matchers.rb`). + +--- + +### Task 1: Config block, validation, and `config.markdown_converter` factory + +**Files:** +- Modify: `lib/crawler/api/config.rb` (CONFIG_FIELDS at lines 67-68, DEFAULTS at lines 143-213, `initialize` at lines 231-257, `document_mapper` at lines 549-551) +- Create: `lib/crawler/markdown_converter.rb` (config-facing skeleton; Tasks 3-4 extend it) +- Test: `spec/lib/crawler/api/config_spec.rb` + +**Interfaces:** +- Consumes: `ES::BulkQueue::DEFAULT_SIZE_THRESHOLD` (`lib/es/bulk_queue.rb:18`, 1 MB), `Addressable::URI.parse`. +- Produces: + - `Crawler::API::Config#markdown_conversion` → `Hash` with symbol keys, always fully populated (a nil block means all defaults; any other non-hash raises): `{ enabled: Boolean, base_url: String|nil (no trailing slash), wait_seconds: Integer 0..60, poll_interval: Numeric > 0, timeout: Numeric > 0, on_failure: 'text'|'skip', ca_file: String|nil }`. + - `Crawler::API::Config#markdown_converter` → memoised `Crawler::MarkdownConverter`. + - `Crawler::MarkdownConverter.new(config)`; `#enabled?` → `true|false`; `#skip_on_failure?` → `true|false`; `#stats` → `{ converted: Integer, failed: Integer }`; `#settings` → the hash above; `#config`. + - Validation raises `ArgumentError` with messages: `Unexpected markdown_conversion options: [...]`, `markdown_conversion.enabled must be true or false`, `markdown_conversion.base_url is required when markdown_conversion.enabled is true`, `markdown_conversion.base_url "..." must be an http(s) URL`, `markdown_conversion.on_failure must be one of text, skip`, `markdown_conversion.wait_seconds must be an integer between 0 and 60`, `markdown_conversion. must be a positive number`, and the bulk-size message shown in Step 3. + +- [ ] **Step 1: Write the failing config tests** + +Append this context inside `describe '#initialize' do ... end` in `spec/lib/crawler/api/config_spec.rb`, right before the existing `describe '#configure_http_header_service!'` block (line 363): + +```ruby + context 'when configuring markdown conversion' do + let(:base_params) { { domains: [{ url: 'https://example.com' }], output_sink: :console } } + + it 'defaults to disabled with the documented defaults' do + config = Crawler::API::Config.new(base_params) + expect(config.markdown_conversion).to eq( + enabled: false, + base_url: nil, + wait_seconds: 10, + poll_interval: 2, + timeout: 900, + on_failure: 'text', + ca_file: nil + ) + end + + it 'treats a nil markdown_conversion block as all defaults' do + config = Crawler::API::Config.new(base_params.merge(markdown_conversion: nil)) + expect(config.markdown_conversion).to include(enabled: false, on_failure: 'text') + end + + it 'rejects a non-hash markdown_conversion value' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: 'yes')) + end.to raise_error(ArgumentError, 'markdown_conversion must be a hash') + end + + it 'rejects an unknown top-level key' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversions: { enabled: true })) + end.to raise_error(ArgumentError, /Unexpected configuration options.*markdown_conversions/) + end + + it 'rejects unknown nested keys' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { enabled: false, retries: 3 })) + end.to raise_error(ArgumentError, /Unexpected markdown_conversion options.*retries/) + end + + it 'merges user settings over the defaults and strips a trailing slash from base_url' do + config = Crawler::API::Config.new( + base_params.merge(markdown_conversion: { enabled: true, base_url: 'http://converter.test/', wait_seconds: 5 }) + ) + expect(config.markdown_conversion).to eq( + enabled: true, + base_url: 'http://converter.test', + wait_seconds: 5, + poll_interval: 2, + timeout: 900, + on_failure: 'text', + ca_file: nil + ) + end + + it 'requires enabled to be a boolean' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { enabled: 'yes' })) + end.to raise_error(ArgumentError, 'markdown_conversion.enabled must be true or false') + end + + it 'requires base_url when enabled' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { enabled: true })) + end.to raise_error(ArgumentError, /base_url is required when markdown_conversion.enabled is true/) + end + + it 'requires an http(s) base_url when enabled' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { enabled: true, base_url: 'ftp://x' })) + end.to raise_error(ArgumentError, 'markdown_conversion.base_url "ftp://x" must be an http(s) URL') + end + + it 'rejects an unparseable base_url' do + params = base_params.merge(markdown_conversion: { enabled: true, base_url: 'http://exa mple' }) + expect do + Crawler::API::Config.new(params) + end.to raise_error(ArgumentError, /markdown_conversion.base_url is not a valid URL/) + end + + it 'does not require base_url when disabled' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { enabled: false })) + end.not_to raise_error + end + + it 'validates on_failure' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { on_failure: 'retry' })) + end.to raise_error(ArgumentError, 'markdown_conversion.on_failure must be one of text, skip') + end + + it 'accepts on_failure as a symbol and normalizes it to a string' do + config = Crawler::API::Config.new(base_params.merge(markdown_conversion: { on_failure: :skip })) + expect(config.markdown_conversion[:on_failure]).to eq('skip') + end + + it 'validates the wait_seconds range' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { wait_seconds: 61 })) + end.to raise_error(ArgumentError, 'markdown_conversion.wait_seconds must be an integer between 0 and 60') + end + + it 'validates poll_interval and timeout are positive numbers' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { poll_interval: 0 })) + end.to raise_error(ArgumentError, 'markdown_conversion.poll_interval must be a positive number') + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { timeout: '900' })) + end.to raise_error(ArgumentError, 'markdown_conversion.timeout must be a positive number') + end + + context 'with the elasticsearch sink' do + let(:es_params) do + { + domains: [{ url: 'https://example.com' }], + output_sink: :elasticsearch, + output_index: 'my-index', + elasticsearch: { host: 'http://localhost', port: 9200 }, + markdown_conversion: { enabled: true, base_url: 'http://converter.test' } + } + end + + it 'rejects a max_body_size that is not smaller than the bulk request size limit (default 1 MB)' do + expect do + Crawler::API::Config.new(es_params) + end.to raise_error( + ArgumentError, + /max_body_size \(5242880\) is not smaller than elasticsearch.bulk_api.max_size_bytes \(1048576\)/ + ) + end + + it 'accepts a smaller max_body_size' do + expect { Crawler::API::Config.new(es_params.merge(max_body_size: 500_000)) }.not_to raise_error + end + + it 'honours a larger configured bulk_api.max_size_bytes' do + params = es_params.merge( + elasticsearch: { host: 'http://localhost', port: 9200, bulk_api: { max_size_bytes: 10_000_000 } } + ) + expect { Crawler::API::Config.new(params) }.not_to raise_error + end + + it 'does not validate the body size when markdown conversion is disabled' do + params = es_params.merge(markdown_conversion: { enabled: false }) + expect { Crawler::API::Config.new(params) }.not_to raise_error + end + end + + it 'exposes a memoised markdown converter' do + config = Crawler::API::Config.new(base_params) + expect(config.markdown_converter).to be_a(Crawler::MarkdownConverter) + expect(config.markdown_converter).to equal(config.markdown_converter) + expect(config.markdown_converter.enabled?).to be(false) + expect(config.markdown_converter.skip_on_failure?).to be(false) + expect(config.markdown_converter.stats).to eq(converted: 0, failed: 0) + end + end +``` + +- [ ] **Step 2: Run the config spec to verify it fails** + +Run (see Global Constraints test command): rspec spec/lib/crawler/api/config_spec.rb +Expected: the new examples FAIL (`Unexpected configuration options: [:markdown_conversion]` and `undefined method 'markdown_converter'`); all pre-existing examples PASS. + +- [ ] **Step 3: Add the config field, nested defaults, validation, and factory** + +In `lib/crawler/api/config.rb`: + +(a) Add a `require_dependency` for the bulk queue and the converter next to the existing ones (after line 16): + +```ruby +require_dependency(File.join(__dir__, '..', 'markdown_converter')) +require_dependency(File.join(__dir__, '..', '..', 'es', 'bulk_queue')) +``` + +(b) In `CONFIG_FIELDS`, right after `:elasticsearch, # Elasticsearch connection settings` (line 68), add: + +```ruby + # Markdown conversion settings (nested hash, see docs/features/MARKDOWN_CONVERSION.md) + :markdown_conversion, +``` + +(c) Right after `EXTRACTION_RULES_FIELDS = %i[url_filters rules].freeze` (line 139), add: + +```ruby + MARKDOWN_CONVERSION_FIELDS = %i[enabled base_url wait_seconds poll_interval timeout on_failure ca_file].freeze + MARKDOWN_CONVERSION_DEFAULTS = { + enabled: false, + base_url: nil, + wait_seconds: 10, + poll_interval: 2, + timeout: 900, + on_failure: 'text', + ca_file: nil + }.freeze + MARKDOWN_ON_FAILURE_POLICIES = %w[text skip].freeze +``` + +(d) In `DEFAULTS`, right after `full_html_extraction_enabled: false,` (line 208), add: + +```ruby + markdown_conversion: MARKDOWN_CONVERSION_DEFAULTS, +``` + +(e) In `initialize`, after `configure_exclude_tags!` (line 256), add: + +```ruby + configure_markdown_conversion! +``` + +(f) Right after the `configure_extraction_rules!` method (ends at line 490), add: + +```ruby + # `markdown_conversion:` with no children parses as nil in YAML and means "all defaults" + def configure_markdown_conversion! + unless markdown_conversion.nil? || markdown_conversion.is_a?(Hash) + raise ArgumentError, 'markdown_conversion must be a hash' + end + + settings = MARKDOWN_CONVERSION_DEFAULTS.merge((markdown_conversion || {}).symbolize_keys) + extra_keys = settings.keys - MARKDOWN_CONVERSION_FIELDS + raise ArgumentError, "Unexpected markdown_conversion options: #{extra_keys.inspect}" if extra_keys.any? + + validate_markdown_enabled!(settings) + validate_markdown_on_failure!(settings) + validate_markdown_wait_seconds!(settings) + validate_markdown_durations!(settings) + if settings[:enabled] + validate_markdown_base_url!(settings) + validate_markdown_body_size! + end + + @markdown_conversion = settings + end + + def validate_markdown_enabled!(settings) + return if [true, false].include?(settings[:enabled]) + + raise ArgumentError, 'markdown_conversion.enabled must be true or false' + end + + def validate_markdown_on_failure!(settings) + settings[:on_failure] = settings[:on_failure].to_s + return if MARKDOWN_ON_FAILURE_POLICIES.include?(settings[:on_failure]) + + raise ArgumentError, + "markdown_conversion.on_failure must be one of #{MARKDOWN_ON_FAILURE_POLICIES.join(', ')}" + end + + def validate_markdown_wait_seconds!(settings) + wait = settings[:wait_seconds] + return if wait.is_a?(Integer) && wait.between?(0, 60) + + raise ArgumentError, 'markdown_conversion.wait_seconds must be an integer between 0 and 60' + end + + def validate_markdown_durations!(settings) + %i[poll_interval timeout].each do |key| + value = settings[key] + next if value.is_a?(Numeric) && value.positive? + + raise ArgumentError, "markdown_conversion.#{key} must be a positive number" + end + end + + def validate_markdown_base_url!(settings) + base_url = settings[:base_url].to_s.strip + if base_url.empty? + raise ArgumentError, 'markdown_conversion.base_url is required when markdown_conversion.enabled is true' + end + + scheme = Addressable::URI.parse(base_url).scheme + unless %w[http https].include?(scheme) + raise ArgumentError, "markdown_conversion.base_url #{base_url.inspect} must be an http(s) URL" + end + + settings[:base_url] = base_url.chomp('/') + rescue Addressable::URI::InvalidURIError => e + raise ArgumentError, "markdown_conversion.base_url is not a valid URL (#{e.message})" + end + + # A markdown body must fit into a single bulk request, otherwise the ES sink can never flush it + def validate_markdown_body_size! + return unless output_sink.to_s == 'elasticsearch' + + bulk_max = elasticsearch&.dig(:bulk_api, :max_size_bytes) || ES::BulkQueue::DEFAULT_SIZE_THRESHOLD + return if max_body_size < bulk_max + + raise ArgumentError, <<~MSG.squish + markdown_conversion is enabled but max_body_size (#{max_body_size}) is not smaller than + elasticsearch.bulk_api.max_size_bytes (#{bulk_max}); a single markdown document could exceed one + bulk request. Lower max_body_size or raise elasticsearch.bulk_api.max_size_bytes. + MSG + end +``` + +(g) Right after the `document_mapper` method (line 551), add: + +```ruby + def markdown_converter + @markdown_converter ||= ::Crawler::MarkdownConverter.new(self) + end +``` + +Create `lib/crawler/markdown_converter.rb`: + +```ruby +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +require 'concurrent/atomic/atomic_fixnum' + +module Crawler + # Converts crawled HTML pages and binary documents (PDF, DOCX, XLSX, PPTX) to Markdown + # through the shared convert-markdown service. See docs/features/MARKDOWN_CONVERSION.md. + class MarkdownConverter + attr_reader :config, :settings + + delegate :system_logger, to: :config + + def initialize(config) + @config = config + @settings = config.markdown_conversion + @converted = Concurrent::AtomicFixnum.new(0) + @failed = Concurrent::AtomicFixnum.new(0) + end + + def enabled? + settings[:enabled] == true + end + + def skip_on_failure? + settings[:on_failure] == 'skip' + end + + # Thread-safe counters, printed at the end of the crawl + def stats + { converted: @converted.value, failed: @failed.value } + end + end +end +``` + +- [ ] **Step 4: Run the config spec to verify it passes** + +Run (see Global Constraints test command): rspec spec/lib/crawler/api/config_spec.rb +Expected: all examples PASS. + +- [ ] **Step 5: Lint** + +Run (see Global Constraints test command): rubocop lib/crawler/api/config.rb lib/crawler/markdown_converter.rb spec/lib/crawler/api/config_spec.rb +Expected: `no offenses detected`. If `Metrics/ClassLength` fires on `Config` (it is already `# rubocop:disable Metrics/ClassLength` on line 27, so it should not), keep that existing disable. + +- [ ] **Step 6: Commit** + +```bash +git add lib/crawler/api/config.rb lib/crawler/markdown_converter.rb spec/lib/crawler/api/config_spec.rb +git commit -m "feat(markdown): add markdown_conversion config block and converter factory" +``` + +--- + +### Task 2: `CrawlResult::Success#markdown` accessor and reserved field names + +**Files:** +- Modify: `lib/crawler/data/crawl_result/success.rb:18` +- Modify: `lib/constants.rb:11-37` +- Test: `spec/lib/crawler/data/crawl_result/success_spec.rb` (create), `spec/lib/constants_spec.rb` (create) + +**Interfaces:** +- Produces: `Crawler::Data::CrawlResult::Success#markdown` → `String|nil` (default `nil`), `#markdown=(String)`. Available on `HTML` and `ContentExtractableFile` results. `Constants::RESERVED_FIELD_NAMES` includes `'body_format'` and `'content_hash'`. + +- [ ] **Step 1: Write the failing tests** + +Create `spec/lib/crawler/data/crawl_result/success_spec.rb`: + +```ruby +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +RSpec.describe(Crawler::Data::CrawlResult::Success) do + describe '#markdown' do + it 'is nil by default on HTML results' do + expect(FactoryBot.build(:html_crawl_result).markdown).to be_nil + end + + it 'is nil by default on content extractable file results' do + expect(FactoryBot.build(:content_extractable_file_crawl_result).markdown).to be_nil + end + + it 'can be assigned after a successful conversion' do + result = FactoryBot.build(:html_crawl_result) + result.markdown = '# Title' + expect(result.markdown).to eq('# Title') + end + end +end +``` + +Create `spec/lib/constants_spec.rb`: + +```ruby +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +RSpec.describe(Constants) do + it 'reserves the document fields written by markdown conversion' do + expect(Constants::RESERVED_FIELD_NAMES).to include('body_format', 'content_hash') + end +end +``` + +- [ ] **Step 2: Run them to verify they fail** + +Run (see Global Constraints test command): rspec spec/lib/crawler/data/crawl_result/success_spec.rb spec/lib/constants_spec.rb +Expected: FAIL with `undefined method 'markdown'` and the `include` mismatch. + +- [ ] **Step 3: Implement** + +In `lib/crawler/data/crawl_result/success.rb`, replace line 18 (`attr_reader :content`) with: + +```ruby + attr_reader :content + + # Markdown produced by Crawler::MarkdownConverter, nil until a conversion succeeds. + # Memoised on the result so the SinkLockedError retry loop in the coordinator does not reconvert. + attr_accessor :markdown +``` + +In `lib/constants.rb`, inside `RESERVED_FIELD_NAMES`, add `body_format` after `body` and `content_hash` after it, so the list reads: + +```ruby + body_content + body + body_format + content_hash + domains +``` + +- [ ] **Step 4: Run the tests (plus the reserved-name rule spec, which iterates the constant)** + +Run (see Global Constraints test command): rspec spec/lib/crawler/data/crawl_result/success_spec.rb spec/lib/constants_spec.rb spec/lib/crawler/data/extraction/rule_spec.rb spec/lib/crawler/data/crawl_result/html_spec.rb +Expected: all PASS. + +- [ ] **Step 5: Lint** + +Run (see Global Constraints test command): rubocop lib/crawler/data/crawl_result/success.rb lib/constants.rb spec/lib/crawler/data/crawl_result/success_spec.rb spec/lib/constants_spec.rb +Expected: `no offenses detected`. + +- [ ] **Step 6: Commit** + +```bash +git add lib/crawler/data/crawl_result/success.rb lib/constants.rb spec/lib/crawler/data/crawl_result/success_spec.rb spec/lib/constants_spec.rb +git commit -m "feat(markdown): add Success#markdown accessor and reserve body_format/content_hash" +``` + +--- + +### Task 3: `MarkdownConverter` core — MIME map, `convertible?`, filename, payload builders + +**Files:** +- Modify: `lib/crawler/markdown_converter.rb` (created in Task 1) +- Test: `spec/lib/crawler/markdown_converter_spec.rb` (create) + +**Interfaces:** +- Consumes: `CrawlResult::HTML#parsed_content` / `#parsed_content_excluding_tags(Array)` (`lib/crawler/data/crawl_result/html.rb:25-40`, both memoised Jsoup `Document`s), `Crawler::ContentEngine::Transformer.transform!(element)` (`lib/crawler/content_engine/transformer.rb:20`), `config.exclude_tags` (Hash of domain URL string → Array of tag names, `lib/crawler/api/config.rb:377-393`), `crawl_result.url_hash`, `crawl_result.content_type`, `Success#content`. +- Produces: + - `MarkdownConverter::MIME_EXTENSIONS` → frozen `Hash` (5 entries). + - `#convertible?(crawl_result)` → `true|false`; returns `false` without touching the result when disabled. + - `#upload_filename(crawl_result)` → `"#{url_hash}.html|.pdf|.docx|.xlsx|.pptx"`. + - `#html_payload(crawl_result)` → `ASCII-8BIT` `String` of UTF-8 HTML bytes (exclusions applied, `` present, memoised documents untouched). + - `#binary_payload(crawl_result)` → `ASCII-8BIT` `String`, byte-identical to `crawl_result.content`. + - private `#normalized_content_type(crawl_result)` → e.g. `'application/pdf'` (charset stripped, downcased) — used by Task 4's multipart builder. + +- [ ] **Step 1: Write the failing tests** + +Create `spec/lib/crawler/markdown_converter_spec.rb`: + +```ruby +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +RSpec.describe(Crawler::MarkdownConverter) do + let(:domain) { 'http://example.com' } + let(:base_url) { 'http://converter.test' } + let(:exclude_tags) { nil } + let(:markdown_settings) { { enabled: true, base_url:, wait_seconds: 10, poll_interval: 0.01, timeout: 5 } } + let(:config) do + Crawler::API::Config.new( + domains: [{ url: domain, exclude_tags: }.compact], + output_sink: :console, + markdown_conversion: markdown_settings + ) + end + let(:converter) { described_class.new(config) } + + let(:html) do + <<~HTML + + Hi + +

Héllo

+
secret
+
footer text
+ + + HTML + end + let(:html_result) { FactoryBot.build(:html_crawl_result, url: "#{domain}/page", content: html) } + let(:pdf_bytes) { "%PDF-1.4\n\xFF\xFE binary".b } + let(:pdf_result) do + FactoryBot.build( + :content_extractable_file_crawl_result, + url: "#{domain}/doc.pdf?v=2", + content: pdf_bytes, + content_type: 'application/pdf' + ) + end + + let(:docx_type) { 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' } + let(:xlsx_type) { 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' } + let(:pptx_type) { 'application/vnd.openxmlformats-officedocument.presentationml.presentation' } + + def file_result(content_type) + FactoryBot.build(:content_extractable_file_crawl_result, url: "#{domain}/file", content_type:) + end + + describe '#convertible?' do + it 'is true for HTML results' do + expect(converter.convertible?(html_result)).to be(true) + end + + it 'is true for supported binary MIME types' do + expect(converter.convertible?(pdf_result)).to be(true) + expect(converter.convertible?(file_result(docx_type))).to be(true) + expect(converter.convertible?(file_result(xlsx_type))).to be(true) + expect(converter.convertible?(file_result(pptx_type))).to be(true) + end + + it 'strips charset parameters and ignores case when matching the MIME type' do + xlsx = 'Application/vnd.openxmlformats-officedocument.spreadsheetml.sheet; charset=binary' + expect(converter.convertible?(file_result(xlsx))).to be(true) + end + + it 'is false for legacy office types and other MIME types' do + expect(converter.convertible?(file_result('application/msword'))).to be(false) + expect(converter.convertible?(file_result('application/vnd.ms-powerpoint'))).to be(false) + expect(converter.convertible?(file_result('image/png'))).to be(false) + end + + context 'when disabled' do + let(:markdown_settings) { { enabled: false } } + + it 'is false and does not call anything on the crawl result' do + strict_result = double(:crawl_result) # raises on any message + expect(converter.convertible?(strict_result)).to be(false) + end + end + end + + describe '#upload_filename' do + it 'uses the URL hash plus the extension for the MIME type, never the URL basename' do + expect(converter.upload_filename(pdf_result)).to eq("#{pdf_result.url_hash}.pdf") + expect(converter.upload_filename(html_result)).to eq("#{html_result.url_hash}.html") + end + + it 'maps every supported MIME type to its extension' do + expect(converter.upload_filename(file_result('application/pdf; charset=utf-8'))).to end_with('.pdf') + expect(converter.upload_filename(file_result(docx_type))).to end_with('.docx') + expect(converter.upload_filename(file_result(xlsx_type))).to end_with('.xlsx') + expect(converter.upload_filename(file_result(pptx_type))).to end_with('.pptx') + end + end + + describe '#html_payload' do + let(:exclude_tags) { %w[footer] } + + it 'serialises the Jsoup document as UTF-8 bytes with a charset meta tag' do + payload = converter.html_payload(html_result) + expect(payload.encoding).to eq(Encoding::BINARY) + expect(payload).to include(''.b) + expect(payload).to include('Héllo'.b) + end + + it 'removes excluded tags and data-elastic-exclude content' do + payload = converter.html_payload(html_result) + expect(payload).not_to include('footer text'.b) + expect(payload).not_to include('secret'.b) + end + + it 'leaves the memoised parsed documents untouched' do + converter.html_payload(html_result) + expect(html_result.parsed_content.select('footer').size).to eq(1) + expect(html_result.parsed_content.select('[data-elastic-exclude]').size).to eq(1) + expect(html_result.parsed_content_excluding_tags(%w[footer]).select('[data-elastic-exclude]').size).to eq(1) + end + + context 'without exclude tags configured' do + let(:exclude_tags) { nil } + + it 'keeps the footer but still honours data-elastic-exclude' do + payload = converter.html_payload(html_result) + expect(payload).to include('footer text'.b) + expect(payload).not_to include('secret'.b) + end + end + end + + describe '#binary_payload' do + it 'returns the content bytes unchanged' do + payload = converter.binary_payload(pdf_result) + expect(payload).to eq(pdf_bytes) + expect(payload.encoding).to eq(Encoding::BINARY) + end + end +end +``` + +- [ ] **Step 2: Run the spec to verify it fails** + +Run (see Global Constraints test command): rspec spec/lib/crawler/markdown_converter_spec.rb +Expected: FAIL with `undefined method 'convertible?'` (and friends). + +- [ ] **Step 3: Implement the core** + +Replace the whole `class MarkdownConverter ... end` body in `lib/crawler/markdown_converter.rb` with: + +```ruby + class MarkdownConverter + # Response Content-Type (charset stripped, downcased) -> upload extension required by the converter. + # Anything else (incl. legacy application/msword, application/vnd.ms-powerpoint) is not convertible. + MIME_EXTENSIONS = { + 'text/html' => '.html', + 'application/pdf' => '.pdf', + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' => '.docx', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' => '.xlsx', + 'application/vnd.openxmlformats-officedocument.presentationml.presentation' => '.pptx' + }.freeze + HTML_EXTENSION = '.html' + + attr_reader :config, :settings + + delegate :system_logger, to: :config + + def initialize(config) + @config = config + @settings = config.markdown_conversion + @converted = Concurrent::AtomicFixnum.new(0) + @failed = Concurrent::AtomicFixnum.new(0) + end + + def enabled? + settings[:enabled] == true + end + + def skip_on_failure? + settings[:on_failure] == 'skip' + end + + # Thread-safe counters, printed at the end of the crawl + def stats + { converted: @converted.value, failed: @failed.value } + end + + # Must not call anything on the crawl result when disabled (coordinator specs use strict doubles) + def convertible?(crawl_result) + return false unless enabled? + return true if crawl_result.html? + + crawl_result.content_extractable_file? && !extension_for(crawl_result).nil? + end + + # The converter picks its parser from the extension, so the filename is synthetic: + # ContentExtractableFile#file_name is File.basename(url) and may carry query strings or no extension. + def upload_filename(crawl_result) + "#{crawl_result.url_hash}#{extension_for(crawl_result)}" + end + + # Serialise from Jsoup (not raw bytes) so exclude_tags and data-elastic-exclude/include are honoured. + # The memoised documents are also used for link extraction, so work on a deep clone. + def html_payload(crawl_result) + doc = source_document(crawl_result).clone + Crawler::ContentEngine::Transformer.transform!(doc.body) + doc.charset(java.nio.charset.StandardCharsets::UTF_8) + String.from_java_bytes(doc.outerHtml.to_java_bytes) + end + + def binary_payload(crawl_result) + crawl_result.content.b + end + + private + + def extension_for(crawl_result) + return HTML_EXTENSION if crawl_result.html? + + MIME_EXTENSIONS[normalized_content_type(crawl_result)] + end + + def normalized_content_type(crawl_result) + crawl_result.content_type.to_s.split(';').first.to_s.strip.downcase + end + + # Mirrors CrawlResult::HTML#get_body_tag: parsed_content_excluding_tags([]) would call Jsoup select('') + def source_document(crawl_result) + tags = tags_to_exclude(crawl_result) + return crawl_result.parsed_content if tags.empty? + + crawl_result.parsed_content_excluding_tags(tags) + end + + def tags_to_exclude(crawl_result) + exclude_tags = config.exclude_tags || {} + exclude_tags.fetch(crawl_result.url.site, nil) || exclude_tags.fetch(crawl_result.url.to_s, []) + end + end +``` + +- [ ] **Step 4: Run the spec to verify it passes** + +Run (see Global Constraints test command): rspec spec/lib/crawler/markdown_converter_spec.rb +Expected: all PASS. If `''` is not found, print `payload` in the failing example: Jsoup 1.23 `Document#charset(Charset)` inserts `` into `` (`ensureMetaCharsetElement`); the attribute value is `Charset#displayName`, i.e. exactly `UTF-8`. + +- [ ] **Step 5: Lint** + +Run (see Global Constraints test command): rubocop lib/crawler/markdown_converter.rb spec/lib/crawler/markdown_converter_spec.rb +Expected: `no offenses detected`. + +- [ ] **Step 6: Commit** + +```bash +git add lib/crawler/markdown_converter.rb spec/lib/crawler/markdown_converter_spec.rb +git commit -m "feat(markdown): add MarkdownConverter MIME map, filename and payload builders" +``` + +--- + +### Task 4: `MarkdownConverter` transport — multipart POST, polling, retry, health, truncation, stats + +**Files:** +- Modify: `lib/crawler/markdown_converter.rb` +- Test: `spec/lib/crawler/markdown_converter_spec.rb` + +**Interfaces:** +- Consumes: Task 3 methods; `Crawler::ContentEngine::Utils.limit_bytesize(string, limit)` (`lib/crawler/content_engine/utils.rb:106`, appends `…` when truncating); `config.max_body_size`, `config.user_agent`; `Success#markdown=`. +- Produces: + - `#convert!(crawl_result)` → `:converted | :skipped | :failed`. Never raises. On `:converted` sets `crawl_result.markdown` (truncated to `config.max_body_size` bytes) and increments `stats[:converted]`; on `:failed` leaves `markdown` nil, increments `stats[:failed]`, logs `system_logger.warn("Markdown conversion failed for #{url}: #{reason}")`. `:skipped` touches nothing. + - `#healthy?` → `true|false` (GET `/api/v1/health`, 5 s open/read timeout, any exception → `false` + warn). + - `#multipart_body(crawl_result)` → `[body (ASCII-8BIT String), content_type_header (String)]`. + - Internal errors `MarkdownConverter::RetryableError` and `MarkdownConverter::ConversionError` (both `StandardError`); not part of the public contract but visible in logs. + - Sleeping goes through `Kernel#sleep` on the converter instance (`sleep(RETRY_DELAY)`, `sleep(interval)`) so specs can stub it with `allow(converter).to receive(:sleep)`. + +- [ ] **Step 1: Write the failing transport tests** + +Append inside the top-level `RSpec.describe(Crawler::MarkdownConverter) do` block of `spec/lib/crawler/markdown_converter_spec.rb` (after `describe '#binary_payload'`): + +```ruby + describe '#convert!' do + let(:upload_url) { "#{base_url}/api/v1/convert/upload?wait=10" } + let(:status_url) { "#{base_url}/api/v1/jobs/job-1" } + let(:json_headers) { { 'Content-Type' => 'application/json' } } + + def job_json(status, extra = {}) + { job_id: 'job-1', status:, status_url: '/api/v1/jobs/job-1', source_name: 'x' }.merge(extra).to_json + end + + def json_response(status, extra = {}) + { status: 200, body: job_json(status, extra), headers: json_headers } + end + + def pending_response + { status: 202, body: job_json('pending'), headers: json_headers } + end + + before do + # Keep polling and retries instant; timing is asserted through the arguments passed to sleep + allow(converter).to receive(:sleep) + allow(config.system_logger).to receive(:warn) + end + + it 'stores the markdown when the upload returns status done inline' do + stub_request(:post, upload_url).to_return(json_response('done', markdown: '# Hi')) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(html_result.markdown).to eq('# Hi') + expect(converter.stats).to eq(converted: 1, failed: 0) + expect(converter).not_to have_received(:sleep) + end + + it 'sends a multipart upload with the file field, hashed filename, part content type and user agent' do + disposition = "Content-Disposition: form-data; name=\"file\"; filename=\"#{pdf_result.url_hash}.pdf\"\r\n".b + stub = stub_request(:post, upload_url).with do |request| + body = request.body.b # compare bytes regardless of the encoding tag WebMock assigns + request.headers['Content-Type'].start_with?('multipart/form-data; boundary=') && + request.headers['User-Agent'] == config.user_agent && + body.include?(disposition) && + body.include?("Content-Type: application/pdf\r\n\r\n".b + pdf_bytes + "\r\n".b) + end.to_return(json_response('done', markdown: '# PDF')) + + expect(converter.convert!(pdf_result)).to eq(:converted) + expect(stub).to have_been_requested.once + end + + context 'with exclude tags configured' do + let(:exclude_tags) { %w[footer] } + + it 'uploads the pre-processed UTF-8 HTML document' do + stub = stub_request(:post, upload_url).with do |request| + body = request.body.b + body.include?("filename=\"#{html_result.url_hash}.html\"".b) && + body.include?("Content-Type: text/html; charset=utf-8\r\n\r\n".b) && + body.include?(''.b) && + body.include?('Héllo'.b) && + !body.include?('footer text'.b) && + !body.include?('secret'.b) + end.to_return(json_response('done', markdown: '# HTML')) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(stub).to have_been_requested.once + end + end + + it 'polls the status URL with backoff until the job is done' do + stub_request(:post, upload_url).to_return(pending_response) + poll = stub_request(:get, status_url).to_return( + json_response('processing'), + json_response('processing'), + json_response('done', markdown: '# Polled') + ) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(html_result.markdown).to eq('# Polled') + expect(poll).to have_been_requested.times(3) + expect(converter).to have_received(:sleep).with(0.01).ordered + expect(converter).to have_received(:sleep).with(be_within(1e-9).of(0.015)).ordered + expect(converter).to have_received(:sleep).with(be_within(1e-9).of(0.0225)).ordered + end + + context 'with a large poll interval' do + let(:markdown_settings) { super().merge(poll_interval: 4) } + + it 'caps the backoff at 5 seconds' do + stub_request(:post, upload_url).to_return(pending_response) + stub_request(:get, status_url).to_return( + json_response('processing'), + json_response('processing'), + json_response('done', markdown: '# Capped') + ) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(converter).to have_received(:sleep).with(4.0).ordered + expect(converter).to have_received(:sleep).with(5.0).ordered + expect(converter).to have_received(:sleep).with(5.0).ordered + end + end + + it 'fails without retrying when the converter reports status failed' do + stub = stub_request(:post, upload_url).to_return(json_response('failed', error: 'unsupported document')) + + expect(converter.convert!(pdf_result)).to eq(:failed) + expect(pdf_result.markdown).to be_nil + expect(converter.stats).to eq(converted: 0, failed: 1) + expect(stub).to have_been_requested.once + expect(converter).not_to have_received(:sleep) + expect(config.system_logger).to have_received(:warn) + .with(%r{Markdown conversion failed for http://example.com/doc.pdf\?v=2: .*unsupported document}) + end + + it 'fails without retrying on HTTP 422' do + stub = stub_request(:post, upload_url).to_return(status: 422, body: '{"detail":"unsupported extension"}') + + expect(converter.convert!(pdf_result)).to eq(:failed) + expect(stub).to have_been_requested.once + expect(converter).not_to have_received(:sleep) + expect(config.system_logger).to have_received(:warn).with(/HTTP 422 from converter/) + end + + it 'fails without retrying on HTTP 404 from the upload endpoint' do + stub = stub_request(:post, upload_url).to_return(status: 404, body: 'not found') + + expect(converter.convert!(pdf_result)).to eq(:failed) + expect(stub).to have_been_requested.once + end + + it 'retries once after 1s on HTTP 5xx and succeeds' do + stub = stub_request(:post, upload_url).to_return( + { status: 503, body: 'upstream unavailable' }, + json_response('done', markdown: '# Second try') + ) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(html_result.markdown).to eq('# Second try') + expect(stub).to have_been_requested.twice + expect(converter).to have_received(:sleep).with(1).once + end + + it 'fails after two HTTP 5xx responses' do + stub = stub_request(:post, upload_url).to_return(status: 503, body: 'upstream unavailable') + + expect(converter.convert!(html_result)).to eq(:failed) + expect(html_result.markdown).to be_nil + expect(stub).to have_been_requested.twice + expect(config.system_logger).to have_received(:warn).with(/HTTP 503 from converter/) + end + + it 'retries once on a connection error' do + stub = stub_request(:post, upload_url) + stub.to_raise(Errno::ECONNREFUSED).then.to_return(json_response('done', markdown: '# Reconnected')) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(stub).to have_been_requested.twice + expect(converter).to have_received(:sleep).with(1).once + end + + it 'retries once on an open timeout' do + stub = stub_request(:post, upload_url).to_timeout.then.to_return(json_response('done', markdown: '# Late')) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(stub).to have_been_requested.twice + end + + it 'fails after two read timeouts' do + stub = stub_request(:post, upload_url).to_raise(Net::ReadTimeout) + + expect(converter.convert!(html_result)).to eq(:failed) + expect(stub).to have_been_requested.twice + expect(config.system_logger).to have_received(:warn).with(/Net::ReadTimeout/) + end + + it 'resubmits once when polling returns HTTP 404 (job expired)' do + upload = stub_request(:post, upload_url).to_return( + pending_response, + json_response('done', markdown: '# Resubmitted') + ) + stub_request(:get, status_url).to_return(status: 404, body: '{"detail":"unknown job"}') + + expect(converter.convert!(html_result)).to eq(:converted) + expect(html_result.markdown).to eq('# Resubmitted') + expect(upload).to have_been_requested.twice + end + + context 'when the job never finishes' do + let(:markdown_settings) { super().merge(timeout: 0.2, poll_interval: 0.05) } + + before { allow(converter).to receive(:sleep).and_call_original } + + it 'gives up at the deadline without retrying' do + upload = stub_request(:post, upload_url).to_return(pending_response) + poll = stub_request(:get, status_url).to_return(json_response('processing')) + + expect(converter.convert!(html_result)).to eq(:failed) + expect(upload).to have_been_requested.once + expect(poll).to have_been_requested.at_least_once + expect(config.system_logger).to have_received(:warn).with(/timed out after 0.2s waiting for job job-1/) + end + end + + it 'fails when the markdown is blank' do + stub_request(:post, upload_url).to_return(json_response('done', markdown: " \n\t")) + + expect(converter.convert!(html_result)).to eq(:failed) + expect(html_result.markdown).to be_nil + expect(config.system_logger).to have_received(:warn).with(/empty markdown/) + end + + it 'fails when the converter returns invalid JSON' do + stub_request(:post, upload_url).to_return(status: 200, body: 'oops') + + expect(converter.convert!(html_result)).to eq(:failed) + expect(config.system_logger).to have_received(:warn).with(/invalid JSON from converter/) + end + + context 'with a small max_body_size' do + let(:config) do + Crawler::API::Config.new( + domains: [{ url: domain }], + output_sink: :console, + max_body_size: 10, + markdown_conversion: markdown_settings + ) + end + + it 'truncates the markdown to max_body_size bytes with an omission marker' do + stub_request(:post, upload_url).to_return(json_response('done', markdown: 'a' * 50)) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(html_result.markdown).to eq("#{'a' * 7}…") + expect(html_result.markdown.bytesize).to eq(10) + end + end + + it 'skips unsupported MIME types without any HTTP call' do + result = file_result('application/msword') + + expect(converter.convert!(result)).to eq(:skipped) + expect(result.markdown).to be_nil + expect(a_request(:any, /converter.test/)).not_to have_been_made + expect(converter.stats).to eq(converted: 0, failed: 0) + end + + context 'when disabled' do + let(:markdown_settings) { { enabled: false } } + + it 'returns :skipped without any HTTP call or touching the result' do + strict_result = double(:crawl_result) + + expect(converter.convert!(strict_result)).to eq(:skipped) + expect(a_request(:any, /converter.test/)).not_to have_been_made + expect(converter.stats).to eq(converted: 0, failed: 0) + end + end + + it 'counts conversions and failures across calls' do + stub_request(:post, upload_url).to_return( + json_response('done', markdown: '# One'), + json_response('failed', error: 'boom') + ) + + converter.convert!(html_result) + converter.convert!(pdf_result) + expect(converter.stats).to eq(converted: 1, failed: 1) + end + + context 'over https with a custom CA file' do + let(:base_url) { 'https://converter.test' } + let(:markdown_settings) { super().merge(ca_file: '/etc/ssl/converter-ca.pem') } + + it 'configures the HTTP client with TLS, the CA file and the documented timeouts' do + clients = [] + allow(Net::HTTP).to receive(:new).and_wrap_original do |original, *args| + original.call(*args).tap { |client| clients << client } + end + stub_request(:post, upload_url).to_return(json_response('done', markdown: '# TLS')) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(clients.size).to eq(1) + expect(clients.first.use_ssl?).to be(true) + expect(clients.first.ca_file).to eq('/etc/ssl/converter-ca.pem') + expect(clients.first.open_timeout).to eq(10) + expect(clients.first.read_timeout).to eq(30) + end + end + end + + describe '#healthy?' do + let(:health_url) { "#{base_url}/api/v1/health" } + + before { allow(config.system_logger).to receive(:warn) } + + it 'is true when the health endpoint returns 200 and sends the crawler user agent' do + stub = stub_request(:get, health_url) + .with(headers: { 'User-Agent' => config.user_agent }) + .to_return(status: 200, body: '{"status":"ok"}') + expect(converter.healthy?).to be(true) + expect(stub).to have_been_requested.once + end + + it 'is false on a non-200 response' do + stub_request(:get, health_url).to_return(status: 503) + expect(converter.healthy?).to be(false) + end + + it 'is false and warns when the service is unreachable' do + stub_request(:get, health_url).to_raise(Errno::ECONNREFUSED) + expect(converter.healthy?).to be(false) + expect(config.system_logger).to have_received(:warn).with(/health check failed.*ECONNREFUSED/) + end + + it 'is false on a timeout' do + stub_request(:get, health_url).to_timeout + expect(converter.healthy?).to be(false) + end + + it 'uses a 5 second timeout' do + clients = [] + allow(Net::HTTP).to receive(:new).and_wrap_original do |original, *args| + original.call(*args).tap { |client| clients << client } + end + stub_request(:get, health_url).to_return(status: 200) + + converter.healthy? + expect(clients.first.open_timeout).to eq(5) + expect(clients.first.read_timeout).to eq(5) + end + end +``` + +- [ ] **Step 2: Run the spec to verify the new examples fail** + +Run (see Global Constraints test command): rspec spec/lib/crawler/markdown_converter_spec.rb +Expected: `#convert!` and `#healthy?` examples FAIL with `undefined method 'convert!'` / `'healthy?'`; Task 3 examples still PASS. + +- [ ] **Step 3: Implement the transport** + +In `lib/crawler/markdown_converter.rb`: + +(a) Add these requires after `require 'concurrent/atomic/atomic_fixnum'`: + +```ruby +require 'json' +require 'net/http' +require 'securerandom' +require 'uri' +``` + +(b) Immediately after `HTML_EXTENSION = '.html'`, add: + +```ruby + # Raised internally for errors worth one retry (network, 5xx, expired job); never escapes convert! + class RetryableError < StandardError; end + # Raised internally for definitive failures (4xx, status: failed, empty markdown, deadline) + class ConversionError < StandardError; end + + UPLOAD_PATH = '/api/v1/convert/upload' + HEALTH_PATH = '/api/v1/health' + FINISHED_STATUSES = %w[done failed].freeze + OPEN_TIMEOUT = 10 + HEALTH_TIMEOUT = 5 + READ_TIMEOUT_MARGIN = 20 # read_timeout = wait_seconds + this + RETRY_DELAY = 1 + POLL_BACKOFF = 1.5 + MAX_POLL_INTERVAL = 5 + RETRYABLE_ERRORS = [SystemCallError, SocketError, EOFError, Net::OpenTimeout, Net::ReadTimeout].freeze +``` + +(c) Add these public methods right after `binary_payload` (before `private`): + +```ruby + # Returns :converted | :skipped | :failed and never raises. + # On success sets crawl_result.markdown (limited to config.max_body_size bytes). + def convert!(crawl_result) + return :skipped unless convertible?(crawl_result) + + markdown = with_one_retry(crawl_result) { run_conversion(crawl_result) } + crawl_result.markdown = Crawler::ContentEngine::Utils.limit_bytesize(markdown, config.max_body_size) + @converted.increment + :converted + rescue StandardError => e + @failed.increment + system_logger.warn("Markdown conversion failed for #{crawl_result.url}: #{e.message}") + :failed + end + + # GET /api/v1/health with a short timeout; called once at crawl start + def healthy? + uri = URI.parse("#{settings[:base_url]}#{HEALTH_PATH}") + request = Net::HTTP::Get.new(uri.request_uri) + request['User-Agent'] = config.user_agent + response = http_client(uri, open_timeout: HEALTH_TIMEOUT, read_timeout: HEALTH_TIMEOUT).request(request) + response.code.to_i == 200 + rescue StandardError => e + system_logger.warn("Markdown converter health check failed (#{settings[:base_url]}): #{e.class}: #{e.message}") + false + end + + # Hand-built multipart/form-data body. Everything is appended as ASCII-8BIT so binary payloads + # concatenate without Encoding::CompatibilityError; Net::HTTP sets Content-Length from body.bytesize. + def multipart_body(crawl_result) + boundary = SecureRandom.hex(16) + payload = crawl_result.html? ? html_payload(crawl_result) : binary_payload(crawl_result) + part_type = crawl_result.html? ? 'text/html; charset=utf-8' : normalized_content_type(crawl_result) + head = [ + "--#{boundary}\r\n", + "Content-Disposition: form-data; name=\"file\"; filename=\"#{upload_filename(crawl_result)}\"\r\n", + "Content-Type: #{part_type}\r\n\r\n" + ].join.b + tail = "\r\n--#{boundary}--\r\n".b + [head + payload + tail, "multipart/form-data; boundary=#{boundary}"] + end +``` + +(d) Add these private methods at the end of the `private` section (after `tags_to_exclude`): + +```ruby + def with_one_retry(crawl_result) + attempts = 0 + begin + attempts += 1 + yield + rescue RetryableError => e + raise e if attempts > 1 + + system_logger.warn( + "Markdown conversion for #{crawl_result.url} failed (#{e.message}), retrying once in #{RETRY_DELAY}s" + ) + sleep(RETRY_DELAY) + retry + end + end + + def run_conversion(crawl_result) + deadline = monotonic_now + settings[:timeout] + job = submit(crawl_result) + job = poll_until_finished(job, deadline) unless finished?(job) + raise ConversionError, "converter reported status 'failed': #{job['error']}" if job['status'] == 'failed' + + markdown = job['markdown'].to_s + raise ConversionError, 'converter returned empty markdown' if markdown.strip.empty? + + markdown + end + + def submit(crawl_result) + uri = URI.parse("#{settings[:base_url]}#{UPLOAD_PATH}?wait=#{settings[:wait_seconds]}") + request = Net::HTTP::Post.new(uri.request_uri) + body, content_type = multipart_body(crawl_result) + request['Content-Type'] = content_type + request.body = body + parse_job(perform(uri, request)) + end + + def poll_until_finished(job, deadline) + if job['status_url'].blank? + raise ConversionError, "converter returned status #{job['status'].inspect} without a status_url" + end + + uri = URI.parse("#{settings[:base_url]}#{job['status_url']}") + interval = settings[:poll_interval].to_f + loop do + if monotonic_now >= deadline + raise ConversionError, "timed out after #{settings[:timeout]}s waiting for job #{job['job_id']}" + end + + sleep(interval) + job = parse_job(perform(uri, Net::HTTP::Get.new(uri.request_uri)), polling: true) + return job if finished?(job) + + interval = [interval * POLL_BACKOFF, MAX_POLL_INTERVAL].min + end + end + + def perform(uri, request) + request['User-Agent'] = config.user_agent + http_client(uri, open_timeout: OPEN_TIMEOUT, read_timeout: settings[:wait_seconds] + READ_TIMEOUT_MARGIN) + .request(request) + rescue *RETRYABLE_ERRORS => e + raise RetryableError, "#{e.class}: #{e.message}" + end + + # p_addr = nil disables Net::HTTP's implicit HTTP_PROXY/HTTPS_PROXY env proxying; the crawler's own + # http_proxy_* settings only apply to the Java HTTP client used for crawling, not to converter calls. + def http_client(uri, open_timeout:, read_timeout:) + Net::HTTP.new(uri.host, uri.port, nil).tap do |http| + http.use_ssl = uri.scheme == 'https' + http.open_timeout = open_timeout + http.read_timeout = read_timeout + http.ca_file = settings[:ca_file] if settings[:ca_file].present? + end + end + + # Branch on the JSON status, not the HTTP code (200 and 202 both carry a job document) + def parse_job(response, polling: false) + raise_on_http_error!(response, polling) + JSON.parse(response.body) + rescue JSON::ParserError => e + raise ConversionError, "invalid JSON from converter: #{e.message}" + end + + def raise_on_http_error!(response, polling) + code = response.code.to_i + raise RetryableError, "HTTP #{code} from converter" if code >= 500 + raise RetryableError, 'HTTP 404 from converter while polling (job expired)' if polling && code == 404 + return if [200, 202].include?(code) + + raise ConversionError, "HTTP #{code} from converter: #{response.body.to_s[0, 200]}" + end + + def finished?(job) + FINISHED_STATUSES.include?(job['status']) + end + + def monotonic_now + Process.clock_gettime(Process::CLOCK_MONOTONIC) + end +``` + +- [ ] **Step 4: Run the spec to verify it passes** + +Run (see Global Constraints test command): rspec spec/lib/crawler/markdown_converter_spec.rb +Expected: all PASS. The deadline example takes ~0.2 s of real time; everything else is instant because `sleep` is stubbed. + +- [ ] **Step 5: Lint** + +Run (see Global Constraints test command): rubocop lib/crawler/markdown_converter.rb spec/lib/crawler/markdown_converter_spec.rb +Expected: `no offenses detected`. If rubocop reports `Metrics/ClassLength` on `MarkdownConverter` (limit 200 code lines; the file should land around 170), change the class line to `class MarkdownConverter # rubocop:disable Metrics/ClassLength` (same pattern as `Coordinator`, `lib/crawler/coordinator.rb:19`). + +- [ ] **Step 6: Commit** + +```bash +git add lib/crawler/markdown_converter.rb spec/lib/crawler/markdown_converter_spec.rb +git commit -m "feat(markdown): add converter transport with polling, retry, health check and stats" +``` + +--- + +### Task 5: Coordinator hook, `on_failure: skip`, health check at crawl start, stats line + +**Files:** +- Modify: `lib/crawler/coordinator.rb:400-404` (content branch of `process_crawl_result`) and add a private helper before `output_crawl_result` (line 529) +- Modify: `lib/crawler/api/crawl.rb:83-111` (`start!`) and private methods after `print_final_crawl_status` (line 238) +- Modify: `lib/errors.rb` +- Test: `spec/lib/crawler/coordinator_spec.rb` (inside `describe '#process_crawl_result'`, lines 162-444), `spec/lib/crawler/api/crawl_spec.rb` + +**Interfaces:** +- Consumes: `config.markdown_converter.convert!(crawl_result)` → `:converted|:skipped|:failed`, `#skip_on_failure?`, `#enabled?`, `#healthy?`, `#stats` (Tasks 1, 4); `sink.failure(message)` → `{ outcome: :failure, message: }` (`lib/crawler/output_sink/base.rb:62`); `config.markdown_conversion[:base_url]`. +- Produces: + - `Errors::MarkdownConverterUnavailableError < StandardError` (`lib/errors.rb`). + - `Coordinator#convert_and_output_crawl_result(crawl_task, crawl_result)` (private) → outcome hash from the sink, or `sink.failure("Markdown conversion failed for #{url} and on_failure is 'skip'; document not ingested")`. + - `Crawl#start!` raises/handles `Errors::MarkdownConverterUnavailableError` before `coordinator.run_crawl!`; outcome `:failure` with message `Markdown converter at is not healthy (GET /api/v1/health did not return 200); aborting the crawl so the index is not degraded to plain text`. + - Log line at crawl end (when enabled): `system_logger.info("Markdown conversions: converted=N failed=N")`. + +- [ ] **Step 1: Write the failing coordinator tests** + +In `spec/lib/crawler/coordinator_spec.rb`, add these contexts inside `describe '#process_crawl_result' do`, right before its closing `end` (after the `'when crawl result is an error'` context, line 443): + +```ruby + context 'when markdown conversion is disabled (default)' do + it 'calls the converter but leaves the crawl result untouched and writes it to the sink' do + # crawl_result is a strict double: any message outside the ones declared above raises + expect(crawl_config.markdown_converter).to receive(:convert!).with(crawl_result).and_call_original + expect(crawl.sink).to receive(:write).with(crawl_result).and_call_original + process_crawl_result + end + end + + context 'when markdown conversion is enabled' do + let(:on_failure) { 'text' } + let(:conversion_result) { :converted } + let(:crawl_configuration) do + super().merge( + output_sink: :mock, + markdown_conversion: { enabled: true, base_url: 'http://converter.test', on_failure: } + ) + end + let(:converter) { crawl_config.markdown_converter } + + before do + allow(converter).to receive(:convert!).and_return(conversion_result) + end + + it 'converts the result exactly once before writing it to the sink' do + expect(converter).to receive(:convert!).with(crawl_result).once.ordered.and_return(:converted) + expect(crawl.sink).to receive(:write).with(crawl_result).ordered.and_call_original + process_crawl_result + end + + context 'when conversion fails and on_failure is text' do + let(:conversion_result) { :failed } + + it 'still writes the result and reports success' do + expect(crawl.sink).to receive(:write).with(crawl_result).and_call_original + expect(events).to receive(:url_extracted).with( + hash_including(url: crawl_result.url, outcome: :success, message: 'Successfully ingested crawl result') + ) + coordinator.send(:process_crawl_result, crawl_task, crawl_result) + end + end + + context 'when conversion fails and on_failure is skip' do + let(:on_failure) { 'skip' } + let(:conversion_result) { :failed } + let(:skip_message) do + "Markdown conversion failed for http://example.com and on_failure is 'skip'; document not ingested" + end + + it 'does not write the result and reports a failure outcome' do + expect(crawl.sink).not_to receive(:write) + expect(system_logger).to receive(:warn).with(skip_message) + expect(events).to receive(:url_extracted).with( + hash_including(url: crawl_result.url, type: :allowed, outcome: :failure, message: skip_message) + ) + coordinator.send(:process_crawl_result, crawl_task, crawl_result) + end + end + + context 'when conversion is skipped (unsupported MIME type) and on_failure is skip' do + let(:on_failure) { 'skip' } + let(:conversion_result) { :skipped } + + it 'writes the result normally' do + expect(crawl.sink).to receive(:write).with(crawl_result).and_call_original + process_crawl_result + end + end + + context 'when the crawl result is a redirect' do + let(:crawl_result) do + double( + :crawl_result, + url:, + redirect_chain: [], + location: Crawler::Data::URL.parse('http://example.com/redirected'), + error?: false, + redirect?: true + ) + end + + it 'does not call the converter' do + allow(coordinator).to receive(:add_urls_to_backlog) + expect(converter).not_to receive(:convert!) + process_crawl_result + end + end + + context 'when the rule engine denies the result' do + let(:rule_engine) do + double( + :rule_engine, + output_crawl_result_outcome: double( + :output_crawl_result_outcome, denied?: true, deny_reason: 'blocked', message: 'nope' + ), + discover_url_outcome: double(:discover_url_outcome, denied?: false) + ) + end + let(:crawl_result) { double(:crawl_result, url:, error?: true, redirect?: false) } + + it 'does not call the converter' do + expect(converter).not_to receive(:convert!) + process_crawl_result + end + end + end +``` + +Note: the `output_sink: :mock` override matters — the top-level `crawl_configuration` leaves `output_sink` at its `:elasticsearch` default, and Task 1's bulk-size validation would reject the default 5 MB `max_body_size` when markdown is enabled. + +- [ ] **Step 2: Write the failing crawl tests** + +In `spec/lib/crawler/api/crawl_spec.rb`, add right after the `it 'has a output sink' do ... end` example (line 78): + +```ruby + it 'does not check converter health when markdown conversion is disabled' do + expect(crawl_config.markdown_converter).not_to receive(:healthy?) + subject.start! + end + + context 'with markdown conversion enabled' do + let(:crawl_config) do + Crawler::API::Config.new( + domains: [{ url: }], + output_sink: :mock, + results_collection: ResultsCollection.new, + markdown_conversion: { enabled: true, base_url: 'http://converter.test' } + ) + end + let(:converter) { crawl_config.markdown_converter } + let(:system_logger) { crawl_config.system_logger } + + before do + allow(converter).to receive(:convert!).and_return(:converted) + allow(system_logger).to receive(:info).and_call_original + allow(system_logger).to receive(:error).and_call_original + end + + context 'when the converter is healthy' do + before { allow(converter).to receive(:healthy?).and_return(true) } + + it 'checks health once, runs the crawl and logs the conversion stats' do + subject.start! + + expect(converter).to have_received(:healthy?).once + expect(subject.outcome).to eq(:success) + expect(system_logger).to have_received(:info).with('Markdown conversions: converted=0 failed=0') + end + end + + context 'when the converter is unhealthy' do + before { allow(converter).to receive(:healthy?).and_return(false) } + + it 'aborts the crawl with a clear failure outcome before crawling anything' do + expect(subject.coordinator).not_to receive(:run_crawl!) + + subject.start! + + expect(subject.outcome).to eq(:failure) + expect(subject.outcome_message).to eq( + 'Markdown converter at http://converter.test is not healthy (GET /api/v1/health did not return 200); ' \ + 'aborting the crawl so the index is not degraded to plain text' + ) + expect(system_logger).to have_received(:error).with(/is not healthy/) + expect(subject.config.event_logger.mock_events).to include( + hash_including('event.action' => 'crawl-end', 'event.outcome' => 'failure') + ) + end + end + end +``` + +- [ ] **Step 3: Run both specs to verify the new examples fail** + +Run (see Global Constraints test command): rspec spec/lib/crawler/coordinator_spec.rb spec/lib/crawler/api/crawl_spec.rb +Expected: the `markdown conversion is enabled` coordinator examples FAIL (`convert!` never received / sink written on skip); the unhealthy crawl example FAILS (outcome `:success`); the healthy example FAILS on the missing stats log line. Pre-existing examples PASS. + +- [ ] **Step 4: Implement** + +(a) `lib/errors.rb` — add at the end of `class Errors` (after `ExitIfUnableToCreateIndex`): + +```ruby + + # Raised at crawl start when markdown conversion is enabled but the converter's health endpoint + # does not answer 200. The crawl aborts instead of silently degrading the whole index to plain text. + class MarkdownConverterUnavailableError < StandardError; end +``` + +(b) `lib/crawler/coordinator.rb` — replace lines 400-404: + +```ruby + elsif crawl_task.content? + crawl_task_progress(crawl_task, 'ingesting the result') + outcome = output_crawl_result(crawl_result) + extracted_event.merge!(outcome) + end +``` + +with: + +```ruby + elsif crawl_task.content? + outcome = convert_and_output_crawl_result(crawl_task, crawl_result) + extracted_event.merge!(outcome) + end +``` + +and add this private method right before `# Outputs the results of a single URL processing to an output module configured for the crawl` (line 528): + +```ruby + # Converts the crawl result to markdown (no-op when disabled) and then sends it to the output sink. + # Runs on the crawl-task thread, before the sink, because the Elasticsearch sink holds its queue lock + # while mapping documents and a slow conversion there would serialise every thread. + def convert_and_output_crawl_result(crawl_task, crawl_result) + crawl_task_progress(crawl_task, 'converting to markdown') if config.markdown_converter.enabled? + conversion = config.markdown_converter.convert!(crawl_result) + + if conversion == :failed && config.markdown_converter.skip_on_failure? + message = "Markdown conversion failed for #{crawl_result.url} and on_failure is 'skip'; document not ingested" + system_logger.warn(message) + return sink.failure(message) + end + + crawl_task_progress(crawl_task, 'ingesting the result') + output_crawl_result(crawl_result) + end +``` + +(c) `lib/crawler/api/crawl.rb`: + +Add after `require 'concurrent'` (line 10): + +```ruby + +require_dependency File.join(__dir__, '..', '..', 'errors') +``` + +Replace `start!` (lines 83-111) with: + +```ruby + def start! # rubocop:disable Metrics/AbcSize, Metrics/MethodLength + events.crawl_start( + url_queue_items: crawl_queue.length, + seen_urls: seen_urls.count + ) + verify_markdown_converter! + ingestion_stats = coordinator.run_crawl! + + record_overall_outcome(coordinator.crawl_results) + rescue Errors::MarkdownConverterUnavailableError => e + system_logger.error(e.message) + record_outcome(outcome: :failure, message: e.message) + rescue StandardError => e + log_exception(e, 'Unexpected error while running the crawl') + record_outcome( + outcome: :failure, + message: 'Unexpected error while running the crawl, check system logs for details' + ) + ensure + # Execute hooks to either save the state or clean up after the crawl. + # The actual cleanup and persistence implementation depends on specific UrlQueue and SeenUrls classes + if allow_resume? + system_logger.info('Not removing the crawl queue to allow the crawl to resume later') + crawl_queue.save + seen_urls.save + else + system_logger.info('Releasing resources used by the crawl...') + crawl_queue.delete + seen_urls.clear + print_final_crawl_status + log_markdown_conversion_stats + print_crawl_ingestion_results(ingestion_stats) if config.output_sink.to_s == 'elasticsearch' + end + end +``` + +Add these private methods right after `print_final_crawl_status` (after line 238): + +```ruby + # Fail fast when the converter is down: a silent outage would degrade the whole index to plain text. + # Calling config.markdown_converter here also warms the `@markdown_converter ||=` memo on the main + # thread before the task pool starts (the memo is not thread-safe); keep that call unconditional so a + # future `return unless enabled?` guard cannot leave the pool threads racing to build separate + # instances and splitting the stats across them. + def verify_markdown_converter! + converter = config.markdown_converter + return unless converter.enabled? + return if converter.healthy? + + raise Errors::MarkdownConverterUnavailableError, <<~MSG.squish + Markdown converter at #{config.markdown_conversion[:base_url]} is not healthy + (GET /api/v1/health did not return 200); aborting the crawl so the index is not degraded to plain text + MSG + end + + def log_markdown_conversion_stats + return unless config.markdown_converter.enabled? + + stats = config.markdown_converter.stats + system_logger.info("Markdown conversions: converted=#{stats[:converted]} failed=#{stats[:failed]}") + end +``` + +- [ ] **Step 5: Run both specs to verify they pass** + +Run (see Global Constraints test command): rspec spec/lib/crawler/coordinator_spec.rb spec/lib/crawler/api/crawl_spec.rb +Expected: all PASS, including every pre-existing `#process_crawl_result` example that uses strict doubles. + +Tester note (before merge, outside the unit suite): `bin/crawler` sets `https.protocols=SSLv3` JVM-wide, which breaks JSSE-based HTTP clients. The unit suite cannot prove Net::HTTP (jruby-openssl) is unaffected, so run one real `bin/crawler crawl ` with `markdown_conversion.enabled: true` against the HTTPS converter (or a one-off `config.markdown_converter.healthy?` call inside the `bin/crawler` runtime) and confirm the health check passes and at least one document lands with `body_format: markdown`. + +- [ ] **Step 6: Lint** + +Run (see Global Constraints test command): rubocop lib/crawler/coordinator.rb lib/crawler/api/crawl.rb lib/errors.rb spec/lib/crawler/coordinator_spec.rb spec/lib/crawler/api/crawl_spec.rb +Expected: `no offenses detected` (`Coordinator` already disables the Metrics cops file-wide at line 16; `start!` already disables AbcSize/MethodLength). + +- [ ] **Step 7: Commit** + +```bash +git add lib/crawler/coordinator.rb lib/crawler/api/crawl.rb lib/errors.rb spec/lib/crawler/coordinator_spec.rb spec/lib/crawler/api/crawl_spec.rb +git commit -m "feat(markdown): convert crawl results before ingestion, fail fast on unhealthy converter" +``` + +--- + +### Task 6: DocumentMapper — markdown body, `body_format`, `content_hash`, no `_attachment` with markdown + +**Files:** +- Modify: `lib/crawler/document_mapper.rb:68-87` +- Test: `spec/lib/crawler/document_mapper_spec.rb` + +**Interfaces:** +- Consumes: `Success#markdown` (Task 2), `ContentExtractableFile#content_hash` (`lib/crawler/data/crawl_result/content_extractable_file.rb:29`), `Digest::SHA1`. +- Produces: HTML docs gain `body_format: 'markdown'|'text'` and `content_hash: `; `body` is the markdown when present. Binary docs gain `body_format`, `content_hash: crawl_result.content_hash`; with markdown → `body: markdown` and NO `_attachment`; without → unchanged (`_attachment` kept, `body_format: 'text'`). + +- [ ] **Step 1: Update the expected results and add the failing markdown examples** + +In `spec/lib/crawler/document_mapper_spec.rb`: + +(a) HTML `expected_result` (lines 52-71): add two entries after `body: crawl_result.document_body,`: + +```ruby + body: crawl_result.document_body, + body_format: 'text', + content_hash: Digest::SHA1.hexdigest(content), +``` + +(b) Binary `expected_result` (lines 179-194): add after `content_type:,`: + +```ruby + content_type:, + content_hash: crawl_result.content_hash, + body_format: 'text', +``` + +(c) Add inside `context 'when crawl result is HTML' do`, right after the `'creates a doc with HTML fields'` example (line 77): + +```ruby + context 'when the crawl result carries markdown' do + let(:markdown) { "# A website!\n\nChosen 1\n\n- Chosen 2" } + let(:expected_result_markdown) { expected_result.merge(body: markdown, body_format: 'markdown') } + + before { crawl_result.markdown = markdown } + + it 'uses the markdown as the body and marks the format' do + result = subject.create_doc(crawl_result) + + expect(result).to eq(expected_result_markdown) + end + end +``` + +(d) Add inside `context 'when crawl result is a binary file' do`, right after the `'creates a doc with binary file fields'` example (line 200): + +```ruby + context 'when the crawl result carries markdown' do + let(:markdown) { '# A PDF for ants' } + let(:expected_result_markdown) do + expected_result.except(:_attachment).merge(body: markdown, body_format: 'markdown') + end + + before { crawl_result.markdown = markdown } + + it 'uses the markdown as the body and omits the attachment' do + result = subject.create_doc(crawl_result) + + expect(result).to eq(expected_result_markdown) + expect(result).not_to have_key(:_attachment) + end + end +``` + +- [ ] **Step 2: Run the spec to verify it fails** + +Run (see Global Constraints test command): rspec spec/lib/crawler/document_mapper_spec.rb +Expected: every `create_doc` example FAILS on the missing `body_format`/`content_hash` keys. + +- [ ] **Step 3: Implement** + +In `lib/crawler/document_mapper.rb`, add `require 'digest'` after `# frozen_string_literal: true` (line 7, leave a blank line between), then replace `html_fields` and `binary_file_fields` (lines 68-87) with: + +```ruby + def html_fields(crawl_result) # rubocop:disable Metrics/AbcSize + remove_empty_values( + title: crawl_result.document_title(limit: config.max_title_size), + body: html_body(crawl_result), + body_format: body_format(crawl_result), + content_hash: Digest::SHA1.hexdigest(crawl_result.content.to_s), # content may be nil (response.rb:47) + meta_keywords: crawl_result.meta_keywords(limit: config.max_keywords_size), + meta_description: crawl_result.meta_description(limit: config.max_description_size), + links: crawl_result.links(limit: config.max_indexed_links_count), + headings: crawl_result.headings(limit: config.max_headings_count), + full_html: crawl_result.full_html(enabled: config.full_html_extraction_enabled) + ) + end + + # With markdown present the ingest attachment processor is not needed, so `_attachment` is omitted + def binary_file_fields(crawl_result) + fields = { + file_name: crawl_result.file_name, + content_length: crawl_result.content_length, + content_type: crawl_result.content_type, + content_hash: crawl_result.content_hash, + body_format: body_format(crawl_result) + } + if crawl_result.markdown + fields[:body] = crawl_result.markdown + else + fields[:_attachment] = crawl_result.base64_encoded_content + end + remove_empty_values(fields) + end + + def html_body(crawl_result) + return crawl_result.markdown if crawl_result.markdown + + crawl_result.document_body(limit: config.max_body_size, exclude_tags: config.exclude_tags) + end + + def body_format(crawl_result) + crawl_result.markdown ? 'markdown' : 'text' + end +``` + +- [ ] **Step 4: Run the spec to verify it passes** + +Run (see Global Constraints test command): rspec spec/lib/crawler/document_mapper_spec.rb spec/lib/crawler/output_sink/file_spec.rb spec/lib/crawler/output_sink/elasticsearch_spec.rb +Expected: all PASS (the sink specs use `hash_including`, so the two new keys do not break them). + +- [ ] **Step 5: Lint** + +Run (see Global Constraints test command): rubocop lib/crawler/document_mapper.rb spec/lib/crawler/document_mapper_spec.rb +Expected: `no offenses detected`. + +- [ ] **Step 6: Commit** + +```bash +git add lib/crawler/document_mapper.rb spec/lib/crawler/document_mapper_spec.rb +git commit -m "feat(markdown): map markdown into body with body_format and content_hash" +``` + +--- + +### Task 7: Elasticsearch sink `_reduce_whitespace` default + warning; console sink prints markdown + +**Files:** +- Modify: `lib/crawler/output_sink/elasticsearch.rb:228-232` (`pipeline_params`, top of `private`) +- Modify: `lib/crawler/output_sink/console.rb:14-25` +- Test: `spec/lib/crawler/output_sink/elasticsearch_spec.rb` (inside `describe '#initialize'`, after the `'when elasticsearch.pipeline_params are changed'` context, line ~358), `spec/lib/crawler/output_sink/console_spec.rb` (create) + +**Interfaces:** +- Consumes: `config.markdown_conversion[:enabled]` (Task 1), `Success#markdown` (Task 2). +- Produces: `OutputSink::Elasticsearch#pipeline_params` → `DEFAULT_PIPELINE_PARAMS` merged with user params, except `_reduce_whitespace` defaults to `false` when markdown conversion is enabled; explicit `_reduce_whitespace: true` with markdown enabled is honoured and logs one `system_logger.warn`. `OutputSink::Console#write` prints `crawl_result.markdown` when present. + +- [ ] **Step 1: Write the failing tests** + +In `spec/lib/crawler/output_sink/elasticsearch_spec.rb`, add right after the `'when elasticsearch.pipeline_params are changed'` context (its closing `end`, before `context 'when elasticsearch.pipeline_enabled is false'` that checks `overrides the specified default params`): + +```ruby + context 'when markdown conversion is enabled' do + let(:pipeline_params) { {} } + let(:config) do + Crawler::API::Config.new( + domains:, + output_sink: 'elasticsearch', + output_index: index_name, + max_body_size: 500_000, + elasticsearch: { + host: 'http://localhost', + port: 1234, + api_key: 'key', + pipeline_params: + }, + markdown_conversion: { enabled: true, base_url: 'http://converter.test' } + ) + end + + it 'defaults _reduce_whitespace to false and keeps binary extraction on' do + expect(subject.pipeline_params).to eq( + _reduce_whitespace: false, + _run_ml_inference: true, + _extract_binary_content: true + ) + expect(system_logger).not_to have_received(:warn) + end + + context 'when _reduce_whitespace is explicitly true' do + let(:pipeline_params) { { _reduce_whitespace: true } } + + it 'honours the setting and warns that markdown will be mangled' do + expect(subject.pipeline_params[:_reduce_whitespace]).to be(true) + expect(system_logger).to have_received(:warn).with( + 'elasticsearch.pipeline_params._reduce_whitespace is true while markdown_conversion is enabled; ' \ + 'the ingest pipeline will collapse whitespace and break markdown formatting in `body`' + ).once + end + end + end +``` + +Create `spec/lib/crawler/output_sink/console_spec.rb`: + +```ruby +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +RSpec.describe(Crawler::OutputSink::Console) do + let(:config) { Crawler::API::Config.new(domains: [{ url: 'http://example.com' }], output_sink: :console) } + let(:sink) { described_class.new(config) } + + it 'prints the raw content of HTML results' do + result = FactoryBot.build(:html_crawl_result, content: '

raw html

') + expect { sink.write(result) }.to output(%r{

raw html

}).to_stdout + end + + it 'prints a placeholder instead of binary content' do + result = FactoryBot.build(:content_extractable_file_crawl_result) + expect { sink.write(result) }.to output(%r{Content extractable file \(content type: application/pdf}).to_stdout + end + + it 'prints the markdown when the result has been converted' do + result = FactoryBot.build(:content_extractable_file_crawl_result) + result.markdown = '# From the converter' + expect { sink.write(result) }.to output(/# From the converter/).to_stdout + expect { sink.write(result) }.not_to output(/Content extractable file/).to_stdout + end + + it 'returns a success outcome' do + result = FactoryBot.build(:html_crawl_result) + outcome = nil + expect { outcome = sink.write(result) }.to output.to_stdout + expect(outcome).to eq(outcome: :success, message: 'Successfully ingested crawl result') + end +end +``` + +- [ ] **Step 2: Run both specs to verify they fail** + +Run (see Global Constraints test command): rspec spec/lib/crawler/output_sink/elasticsearch_spec.rb spec/lib/crawler/output_sink/console_spec.rb +Expected: the two new ES examples FAIL (`_reduce_whitespace` is `true`, no warning); the console markdown example FAILS (placeholder printed instead of markdown). + +- [ ] **Step 3: Implement** + +`lib/crawler/output_sink/elasticsearch.rb` — replace `pipeline_params` (lines 228-230) with: + +```ruby + def pipeline_params + @pipeline_params ||= default_pipeline_params.merge(es_config[:pipeline_params] || {}).tap do |params| + warn_about_whitespace_reduction(params) + end + end +``` + +and add these right after `private` (line 232): + +```ruby + def markdown_conversion_enabled? + config.markdown_conversion[:enabled] == true + end + + # The default ingest pipeline collapses whitespace in `body` (\s+ -> ' '), which destroys markdown + def default_pipeline_params + return DEFAULT_PIPELINE_PARAMS unless markdown_conversion_enabled? + + DEFAULT_PIPELINE_PARAMS.merge(_reduce_whitespace: false) + end + + def warn_about_whitespace_reduction(params) + return unless markdown_conversion_enabled? && params[:_reduce_whitespace] == true + + system_logger.warn( + 'elasticsearch.pipeline_params._reduce_whitespace is true while markdown_conversion is enabled; ' \ + 'the ingest pipeline will collapse whitespace and break markdown formatting in `body`' + ) + end +``` + +`lib/crawler/output_sink/console.rb` — replace `write` (lines 14-25) with: + +```ruby + def write(crawl_result) + puts "# #{crawl_result.id}, #{crawl_result.url}, #{crawl_result.status_code}" + + if crawl_result.respond_to?(:markdown) && crawl_result.markdown + puts crawl_result.markdown + elsif crawl_result.content_extractable_file? + puts "** [Content extractable file (content type: #{crawl_result.content_type}, " \ + "content length: #{crawl_result.content.bytesize})] **" + else + puts crawl_result.content + end + + success + end +``` + +- [ ] **Step 4: Run both specs to verify they pass** + +Run (see Global Constraints test command): rspec spec/lib/crawler/output_sink/elasticsearch_spec.rb spec/lib/crawler/output_sink/console_spec.rb +Expected: all PASS. + +- [ ] **Step 5: Lint** + +Run (see Global Constraints test command): rubocop lib/crawler/output_sink/elasticsearch.rb lib/crawler/output_sink/console.rb spec/lib/crawler/output_sink/elasticsearch_spec.rb spec/lib/crawler/output_sink/console_spec.rb +Expected: `no offenses detected`. + +- [ ] **Step 6: Commit** + +```bash +git add lib/crawler/output_sink/elasticsearch.rb lib/crawler/output_sink/console.rb spec/lib/crawler/output_sink/elasticsearch_spec.rb spec/lib/crawler/output_sink/console_spec.rb +git commit -m "feat(markdown): keep markdown whitespace in the ES pipeline and print markdown on the console sink" +``` + +--- + +### Task 8: Integration spec — Faux site + WebMock-stubbed converter + +**Files:** +- Modify: `spec/support/faux/faux_crawl.rb:46-73` (options) and `:152-176` (`configure_crawl`) +- Test: `spec/integration/markdown_conversion_spec.rb` (create) + +**Interfaces:** +- Consumes: everything from Tasks 1-7; `FauxCrawl.run(site, options)` returning a `ResultsCollection` (`results.crawl_config`, `results.crawl`); `Crawler::DocumentMapper#create_doc`. +- Produces: `FauxCrawl` accepts `markdown_conversion: Hash` and forwards it verbatim into `Crawler::API::Config.new`. (No new production interfaces.) + +Why the mock sink: `FauxCrawl` hard-codes `output_sink: :mock`, which collects `CrawlResult` objects. The spec's "file-sink docs" assertion is made equivalently by running each collected result through `results.crawl_config.document_mapper.create_doc` — the same mapper every real sink calls via `OutputSink::Base#to_doc`. + +- [ ] **Step 1: Extend FauxCrawl to forward `markdown_conversion`** + +In `spec/support/faux/faux_crawl.rb` (excluded from rubocop): + +(a) Add `:markdown_conversion` to the `attr_reader` list (lines 46-48): + +```ruby + attr_reader :options, :sites, :site_containers, :timeouts, :content_extraction, :default_encoding, :crawl_id, + :url_queue, :auth, :user_agent, :url, :seed_urls, :sitemap_urls, :domain_allowlist, :results, + :expect_success, :markdown_conversion +``` + +(b) In `initialize`, after `@content_extraction = ...` (line 64), add: + +```ruby + @markdown_conversion = options[:markdown_conversion] +``` + +(c) In `configure_crawl`, after `config[:default_encoding] = default_encoding if default_encoding` (line 175), add: + +```ruby + config[:markdown_conversion] = markdown_conversion if markdown_conversion +``` + +- [ ] **Step 2: Write the failing integration spec** + +Create `spec/integration/markdown_conversion_spec.rb`: + +```ruby +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +RSpec.describe 'Markdown conversion' do + let(:converter_url) { 'http://converter.test' } + let(:health_url) { "#{converter_url}/api/v1/health" } + let(:upload_url) { "#{converter_url}/api/v1/convert/upload?wait=10" } + let(:json_headers) { { 'Content-Type' => 'application/json' } } + let(:markdown_conversion) do + { enabled: true, base_url: converter_url, wait_seconds: 10, poll_interval: 0.01, timeout: 5 } + end + + let(:site) do + Faux.site do + page '/' do + body do + text { '

Home

' } + link_to '/pdf' + text { '
site footer
' } + end + end + + page '/pdf' do + headers 'Content-Type' => 'application/pdf' + end + end + end + + def done_json(markdown) + { job_id: 'job', status: 'done', status_url: '/api/v1/jobs/job', source_name: 'x', markdown: }.to_json + end + + def run_crawl + FauxCrawl.run( + site, + content_extraction: { enabled: true, mime_types: %w[application/pdf] }, + markdown_conversion: + ) + end + + # Same mapping every real sink performs through OutputSink::Base#to_doc + def docs_by_url(results) + mapper = results.crawl_config.document_mapper + results.each_with_object({}) { |result, docs| docs[result.url.to_s] = mapper.create_doc(result) } + end + + before do + stub_request(:get, health_url).to_return(status: 200, body: '{"status":"ok"}', headers: json_headers) + end + + context 'when the converter is healthy' do + before do + stub_request(:post, upload_url) + .with { |request| request.body.include?('.html"') } + .to_return(status: 200, body: done_json("# Home\n\nsite footer"), headers: json_headers) + stub_request(:post, upload_url) + .with { |request| request.body.include?('.pdf"') } + .to_return(status: 200, body: done_json('# PDF document'), headers: json_headers) + end + + it 'stores markdown bodies for the HTML page and the PDF' do + results = run_crawl + + expect(results).to have_only_these_results [ + mock_response(url: 'http://127.0.0.1:9393/', status_code: 200), + mock_response(url: 'http://127.0.0.1:9393/pdf', status_code: 200) + ] + + docs = docs_by_url(results) + expect(docs['http://127.0.0.1:9393/']).to include(body: "# Home\n\nsite footer", body_format: 'markdown') + expect(docs['http://127.0.0.1:9393/']).to have_key(:content_hash) + expect(docs['http://127.0.0.1:9393/pdf']).to include(body: '# PDF document', body_format: 'markdown') + expect(docs['http://127.0.0.1:9393/pdf']).not_to have_key(:_attachment) + expect(results.crawl_config.markdown_converter.stats).to eq(converted: 2, failed: 0) + expect(a_request(:get, health_url)).to have_been_made.once + end + end + + context 'when the converter keeps answering 5xx' do + before do + stub_request(:post, upload_url).to_return(status: 503, body: 'upstream unavailable') + end + + it 'falls back to text extraction and keeps the binary attachment' do + results = run_crawl + + docs = docs_by_url(results) + expect(docs['http://127.0.0.1:9393/']).to include(body_format: 'text') + expect(docs['http://127.0.0.1:9393/'][:body]).to include('Home') + expect(docs['http://127.0.0.1:9393/pdf']).to include(body_format: 'text') + expect(docs['http://127.0.0.1:9393/pdf']).to have_key(:_attachment) + # two documents, each tried twice (initial attempt + one retry) + expect(a_request(:post, upload_url)).to have_been_made.times(4) + expect(results.crawl_config.markdown_converter.stats).to eq(converted: 0, failed: 2) + end + end + + context 'when the converter health check fails' do + before do + stub_request(:get, health_url).to_return(status: 503) + end + + it 'aborts the crawl before fetching anything' do + expect { run_crawl }.to raise_error(RuntimeError, /Test Crawl failed!.*is not healthy/) + expect(a_request(:post, upload_url)).not_to have_been_made + end + end +end +``` + +- [ ] **Step 3: Run the integration spec** + +Run (see Global Constraints test command): rspec spec/integration/markdown_conversion_spec.rb +Expected: all three examples PASS on the first run (Tasks 1-7 are complete). The 5xx scenario takes ~2 s of real retry sleeps. If the first run fails instead, treat it as a real integration bug: read the crawl system log in the rspec output, fix the production code (not the spec), and re-run. + +Tester note: `WebMock::NetConnectNotAllowedError` inherits from `Exception`, not `StandardError`, so a stub mismatch (wrong query string, unexpected filename, missing health stub) is not caught by `convert!` — it kills the crawl-task pool thread and surfaces only as a `have_only_these_results` count mismatch or a missing doc. When an example fails, search the rspec output for `NetConnectNotAllowedError` first. + +Then run the existing integration spec to make sure `FauxCrawl` still works without the new option: + +Run (see Global Constraints test command): rspec spec/integration/content_extraction_spec.rb +Expected: PASS. + +- [ ] **Step 4: Lint** + +Run (see Global Constraints test command): rubocop spec/integration/markdown_conversion_spec.rb +Expected: `no offenses detected` (`spec/support/faux/**/*` is excluded in `.rubocop.yml`). + +- [ ] **Step 5: Run the full suite once** + +Run (see Global Constraints test command): rspec (no arguments, whole suite) +Expected: 0 failures. + +- [ ] **Step 6: Commit** + +```bash +git add spec/support/faux/faux_crawl.rb spec/integration/markdown_conversion_spec.rb +git commit -m "test(markdown): add end-to-end crawl spec with a stubbed converter" +``` + +--- + +### Task 9: Configuration example and feature documentation + +**Files:** +- Modify: `config/crawler.yml.example:204-213` (binary content block) and append a new block after the `elasticsearch` section (after line 267, before the sink-lock settings) +- Create: `docs/features/MARKDOWN_CONVERSION.md` +- Modify: `README.md:241` (feature list) + +**Interfaces:** +- Consumes: the config shape from Task 1, failure semantics from Tasks 5-7. +- Produces: documentation only. + +- [ ] **Step 1: Update `config/crawler.yml.example`** + +(a) In the binary content extraction MIME list (lines 208-213), add the spreadsheet type so the list reads: + +```yaml +#binary_content_extraction_mime_types: +# - application/pdf +# - application/msword +# - application/vnd.openxmlformats-officedocument.wordprocessingml.document +# - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet +# - application/vnd.ms-powerpoint +# - application/vnd.openxmlformats-officedocument.presentationml.presentation +``` + +(b) Insert this block right after the `elasticsearch:` section (after line 267 `# max_size_bytes: 1_048_576`) and before `## The interval in seconds to wait before retrying to acquire the sink lock.`: + +```yaml + +## ------------------------------- Markdown Conversion ------------------------- +# +## Convert crawled HTML pages and PDF/DOCX/XLSX/PPTX files to Markdown through the shared +## convert-markdown service before indexing. `body` then holds Markdown and `body_format` +## is set to `markdown`; documents that cannot be converted fall back to plain text +## (`body_format: text`). See docs/features/MARKDOWN_CONVERSION.md for details. +## Binary files still need `binary_content_extraction_enabled` and their MIME types listed above. +#markdown_conversion: +# enabled: false +# base_url: https://convert-markdown.ifad.org # required when enabled; http(s) only +# wait_seconds: 10 # 0..60, how long the service may hold the upload request before answering +# poll_interval: 2 # seconds between status polls; backs off x1.5 up to 5s +# timeout: 900 # hard per-document deadline (submit + polling), in seconds +# on_failure: text # text: index the plain-text fallback | skip: do not index the document +# ca_file: # optional PEM bundle used to verify the converter's TLS certificate +# +## NOTE: when enabled with the elasticsearch sink, `max_body_size` must be smaller than +## `elasticsearch.bulk_api.max_size_bytes` (default 1_048_576), e.g. `max_body_size: 900_000`. +``` + +- [ ] **Step 2: Create `docs/features/MARKDOWN_CONVERSION.md`** (outer fence uses four backticks because the document itself contains ```yaml fences) + +````markdown +# Markdown Conversion + +The crawler can send every crawled HTML page and supported binary document (PDF, DOCX, XLSX, PPTX) to the shared +IFAD `convert-markdown` service and index the returned Markdown in the `body` field. Every IFAD application then +gets identical conversions, and Markdown keeps headings, lists and tables that plain-text extraction flattens. + +Plain-text extraction remains the fallback: if a document cannot be converted, the crawler indexes what it +would have indexed before this feature existed. + +## Using this feature + +1. Make sure the converter is reachable from the crawler (`GET {base_url}/api/v1/health` must answer 200). +2. Enable the feature in the crawler configuration: + +```yaml +markdown_conversion: + enabled: true + base_url: https://convert-markdown.ifad.org + wait_seconds: 10 + poll_interval: 2 + timeout: 900 + on_failure: text + # ca_file: /etc/ssl/certs/ifad-ca.pem + +# Binary files are only downloaded when binary content extraction is on +binary_content_extraction_enabled: true +binary_content_extraction_mime_types: + - application/pdf + - application/vnd.openxmlformats-officedocument.wordprocessingml.document + - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet + - application/vnd.openxmlformats-officedocument.presentationml.presentation + +# Required with the elasticsearch sink: a markdown body must fit into one bulk request +max_body_size: 900_000 +``` + +| Setting | Default | Meaning | +|---|---|---| +| `enabled` | `false` | Turn the feature on. | +| `base_url` | – | Converter base URL, `http` or `https`. Required when enabled. | +| `wait_seconds` | `10` | Sent as `?wait=` on upload (0..60). The service answers inline when the job finishes within this window. | +| `poll_interval` | `2` | Seconds between status polls; backs off ×1.5 per poll, capped at 5 s. | +| `timeout` | `900` | Hard per-document deadline in seconds (upload + polling). | +| `on_failure` | `text` | `text`: index the plain-text fallback. `skip`: do not index the document at all. | +| `ca_file` | – | PEM bundle to verify the converter's certificate (passed to `Net::HTTP#ca_file`). | + +At crawl start the crawler calls the health endpoint once. If it does not answer 200 the crawl aborts with +`Markdown converter at is not healthy ...` instead of silently indexing the whole site as plain text. + +## What gets indexed + +| Document | Converted | Not converted (fallback) | +|---|---|---| +| HTML page | `body`: Markdown, `body_format: markdown` | `body`: extracted text, `body_format: text` | +| PDF / DOCX / XLSX / PPTX | `body`: Markdown, `body_format: markdown`, no `_attachment` | `_attachment` kept (Tika via the ingest pipeline), `body_format: text` | +| Other binary types (e.g. legacy `.doc`, `.ppt`) | never sent to the converter | unchanged behaviour | + +Every document also gets `content_hash` (SHA-1 of the fetched bytes). `body_format` and `content_hash` are +reserved field names and cannot be overwritten by extraction rules. + +HTML is uploaded after the crawler's own pre-processing, so `exclude_tags` and the +`data-elastic-exclude` / `data-elastic-include` directives are honoured in the Markdown as well. + +With the Elasticsearch sink, the ingest pipeline parameter `_reduce_whitespace` defaults to `false` while this +feature is enabled, because the default pipeline collapses all whitespace in `body` and would destroy the +Markdown structure. Setting it to `true` explicitly is honoured but logs a warning. + +## Failure semantics + +| Situation | `on_failure: text` | `on_failure: skip` | +|---|---|---| +| Converter healthy, document converts | Markdown body | Markdown body | +| Unsupported MIME type | fallback (not counted as a failure) | fallback (not counted as a failure) | +| Converter error, timeout, or `status: failed` | fallback body, `body_format: text`, warning in the system log | document not written, `url-extracted` event with `outcome: failure`, warning in the system log | +| Converter unreachable at crawl start | crawl aborts | crawl aborts | + +Transient errors (connection errors, timeouts, HTTP 5xx, HTTP 404 while polling an expired job) are retried +once after one second. HTTP 422 (unsupported file), other 4xx responses and `status: failed` are not retried. + +The system log ends with `Markdown conversions: converted=N failed=N`. + +### `skip` and purge crawls + +With `purge_crawl_enabled: true` (the default) the purge stage deletes every document whose `last_crawled_at` +is older than the crawl start. A page whose conversion failed under `on_failure: skip` is not re-indexed, so its +`last_crawled_at` stays stale and the purge crawl deletes it from the index. Use `skip` only when a missing +document is preferable to a plain-text one. + +## Throughput and deployment caveats + +- The converter is single-worker with an in-process job store: conversions are serialised. Each crawl thread + waits for its own document, so `threads_per_crawl` bounds the number of in-flight conversions; large PDFs + block one crawl thread for the whole conversion (up to `timeout`). +- Because the job store is in-process, polling must reach the same replica that accepted the upload. Behind a + load balancer without session affinity, polls can hit another replica and answer 404; the crawler resubmits + once, then falls back. Run a single replica or enable affinity routing. +- The service front-end may enforce its own per-request timeout (Envoy defaults to 15 s); keep `wait_seconds` + below it so long conversions go through the polling path instead of failing at the proxy. +- A conversion in progress ignores a crawl shutdown request: the task thread finishes (or times out) its + current upload/poll cycle first, so a slow job can hold a task thread for up to `timeout` seconds. +- Converter calls go straight to `base_url`: they use neither the crawler's `http_proxy_*` settings nor the + `HTTP_PROXY`/`HTTPS_PROXY` environment variables. +```` + +- [ ] **Step 3: Link the feature from the README** + +In `README.md`, right after line 241 (`- [Binary content extraction](docs/features/BINARY_CONTENT_EXTRACTION.md) - Extract text from PDFs, DOCX files`), add: + +```markdown +- [Markdown conversion](docs/features/MARKDOWN_CONVERSION.md) - Index HTML pages and office documents as Markdown through the shared convert-markdown service +``` + +- [ ] **Step 4: Verify the example config still loads and lint stays clean** + +Run (see Global Constraints test command): rspec spec/lib/crawler/cli/ spec/lib/crawler/api/config_spec.rb +Expected: PASS (the CLI specs load YAML fixtures; the example file is comment-only so nothing changes at runtime, this just guards against a stray uncommented line). + +Run (see Global Constraints test command): rubocop lib spec +Expected: `no offenses detected`. + +- [ ] **Step 5: Commit** + +```bash +git add config/crawler.yml.example docs/features/MARKDOWN_CONVERSION.md README.md +git commit -m "docs(markdown): document the markdown_conversion feature and config block" +``` + +--- + +## Self-review notes (already applied) + +- **Spec coverage:** insertion point (Task 5), disabled-path double constraint (Tasks 3-5), config block + validation + factory (Task 1), MIME map / filename / Jsoup payload / binary payload (Task 3), net/http multipart transport, flow, retry rules, blank markdown, `limit_bytesize`, warn-never-raise, `healthy?`, stats (Task 4), `Success#markdown` + reserved names (Task 2), DocumentMapper (Task 6), ES `_reduce_whitespace` + console (Task 7), stats line + health check at start (Task 5), docs + yml example + xlsx MIME (Task 9), integration spec (Task 8). Every WebMock branch listed in the spec's test strategy has an example in Task 4. +- **Resolved ambiguities:** (1) the spec says `parsed_content_excluding_tags(config.exclude_tags)`, but that method takes an `Array` of tags while `config.exclude_tags` is a per-domain `Hash`, and `Jsoup#select('')` raises on an empty list — Task 3 mirrors `HTML#get_body_tag` (`html.rb:220-232`) instead. (2) `on_failure: skip` needs a branch the spec's 3-line diff does not show — Task 5 adds one small private helper rather than inlining it. (3) `FauxCrawl` only supports the `:mock` sink — Task 8 asserts on `DocumentMapper#create_doc` output of the collected results, which is what every real sink writes. (4) The retry deadline is measured per attempt (each attempt gets `timeout` from its own submit), so the worst case per document is `2 × timeout + 1 s`. (5) Enabling markdown with the ES sink and the default `max_body_size` (5 MB) is rejected by design (spec requires `max_body_size < bulk max_size_bytes`); every spec that enables markdown with the ES sink therefore sets `max_body_size: 500_000`, and specs that do not need ES use `output_sink: :mock`/`:console`. (6) Tech-lead review additions: `Net::HTTP.new(host, port, nil)` disables env proxying, the health check sends the `User-Agent`, a nil `markdown_conversion:` block means defaults, unparseable `base_url` raises `ArgumentError`, `content_hash` hashes `content.to_s`, and `crawl_task_progress('converting to markdown')` is only emitted when enabled. +- **Type consistency:** `convert!` returns `:converted|:skipped|:failed` (Tasks 4, 5, 8); `stats` returns `{ converted:, failed: }` (Tasks 1, 4, 5, 8); `skip_on_failure?` (Tasks 1, 5); `markdown_conversion` hash keys are identical in Tasks 1, 4, 7, 9; `Errors::MarkdownConverterUnavailableError` (Task 5 only); `raise_on_http_error!(response, polling)` is only called from `parse_job` (Task 4); DocumentMapper field names `body_format`, `content_hash` match `Constants::RESERVED_FIELD_NAMES` (Tasks 2, 6, 8, 9). From 9d094afae47e0db3b788e51a6f7968c4375b1d65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 09:56:17 +0200 Subject: [PATCH 03/16] feat(markdown): add markdown_conversion config block and converter factory Adds the markdown_conversion nested config block (default disabled), its validation (booleans, on_failure enum, wait_seconds range, positive poll_interval/timeout, http(s) base_url, and an ES bulk-size guard), and a memoised Config#markdown_converter factory backed by a new Crawler::MarkdownConverter skeleton class. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- lib/crawler/api/config.rb | 109 +++++++++++++++++++ lib/crawler/markdown_converter.rb | 39 +++++++ spec/lib/crawler/api/config_spec.rb | 158 ++++++++++++++++++++++++++++ 3 files changed, 306 insertions(+) create mode 100644 lib/crawler/markdown_converter.rb diff --git a/lib/crawler/api/config.rb b/lib/crawler/api/config.rb index c6959ebf..e04f582f 100644 --- a/lib/crawler/api/config.rb +++ b/lib/crawler/api/config.rb @@ -14,6 +14,8 @@ require_dependency(File.join(__dir__, '..', 'data', 'extraction', 'ruleset')) require_dependency(File.join(__dir__, '..', 'document_mapper')) require_dependency(File.join(__dir__, '..', 'utils')) +require_dependency(File.join(__dir__, '..', 'markdown_converter')) +require_dependency(File.join(__dir__, '..', '..', 'es', 'bulk_queue')) java_import java.io.ByteArrayInputStream java_import java.security.cert.CertificateFactory @@ -67,6 +69,9 @@ class Config # rubocop:disable Metrics/ClassLength # Elasticsearch settings :elasticsearch, # Elasticsearch connection settings + # Markdown conversion settings (nested hash, see docs/features/MARKDOWN_CONVERSION.md) + :markdown_conversion, + # HTTP header settings :http_header_service, # Service to determine the HTTP headers used for requests made from the crawler. :http_auth_allowed, # If HTTP auth is permitted for non-HTTPS URLs. @@ -138,6 +143,18 @@ class Config # rubocop:disable Metrics/ClassLength EXTRACTION_RULES_FIELDS = %i[url_filters rules].freeze + MARKDOWN_CONVERSION_FIELDS = %i[enabled base_url wait_seconds poll_interval timeout on_failure ca_file].freeze + MARKDOWN_CONVERSION_DEFAULTS = { + enabled: false, + base_url: nil, + wait_seconds: 10, + poll_interval: 2, + timeout: 900, + on_failure: 'text', + ca_file: nil + }.freeze + MARKDOWN_ON_FAILURE_POLICIES = %w[text skip].freeze + # Please note: These defaults are used the `Crawler::HttpUtils::Config` class. # Make sure to check those before renaming or removing any defaults. DEFAULTS = { @@ -206,6 +223,7 @@ class Config # rubocop:disable Metrics/ClassLength crawl_rules: {}, purge_crawl_enabled: true, full_html_extraction_enabled: false, + markdown_conversion: MARKDOWN_CONVERSION_DEFAULTS, # Sink lock retry settings sink_lock_retry_interval: 1, @@ -254,6 +272,7 @@ def initialize(params = {}) configure_sitemap_urls! configure_extraction_rules! configure_exclude_tags! + configure_markdown_conversion! end def to_s @@ -489,6 +508,92 @@ def configure_extraction_rules! end end + # `markdown_conversion:` with no children parses as nil in YAML and means "all defaults" + def configure_markdown_conversion! + unless markdown_conversion.nil? || markdown_conversion.is_a?(Hash) + raise ArgumentError, 'markdown_conversion must be a hash' + end + + settings = MARKDOWN_CONVERSION_DEFAULTS.merge((markdown_conversion || {}).symbolize_keys) + extra_keys = settings.keys - MARKDOWN_CONVERSION_FIELDS + raise ArgumentError, "Unexpected markdown_conversion options: #{extra_keys.inspect}" if extra_keys.any? + + validate_markdown_settings!(settings) + + @markdown_conversion = settings + end + + def validate_markdown_settings!(settings) + validate_markdown_enabled!(settings) + validate_markdown_on_failure!(settings) + validate_markdown_wait_seconds!(settings) + validate_markdown_durations!(settings) + return unless settings[:enabled] + + validate_markdown_base_url!(settings) + validate_markdown_body_size! + end + + def validate_markdown_enabled!(settings) + return if [true, false].include?(settings[:enabled]) + + raise ArgumentError, 'markdown_conversion.enabled must be true or false' + end + + def validate_markdown_on_failure!(settings) + settings[:on_failure] = settings[:on_failure].to_s + return if MARKDOWN_ON_FAILURE_POLICIES.include?(settings[:on_failure]) + + raise ArgumentError, + "markdown_conversion.on_failure must be one of #{MARKDOWN_ON_FAILURE_POLICIES.join(', ')}" + end + + def validate_markdown_wait_seconds!(settings) + wait = settings[:wait_seconds] + return if wait.is_a?(Integer) && wait.between?(0, 60) + + raise ArgumentError, 'markdown_conversion.wait_seconds must be an integer between 0 and 60' + end + + def validate_markdown_durations!(settings) + %i[poll_interval timeout].each do |key| + value = settings[key] + next if value.is_a?(Numeric) && value.positive? + + raise ArgumentError, "markdown_conversion.#{key} must be a positive number" + end + end + + def validate_markdown_base_url!(settings) + base_url = settings[:base_url].to_s.strip + if base_url.empty? + raise ArgumentError, 'markdown_conversion.base_url is required when markdown_conversion.enabled is true' + end + + scheme = Addressable::URI.parse(base_url).scheme + unless %w[http https].include?(scheme) + raise ArgumentError, "markdown_conversion.base_url #{base_url.inspect} must be an http(s) URL" + end + + settings[:base_url] = base_url.chomp('/') + rescue Addressable::URI::InvalidURIError => e + raise ArgumentError, "markdown_conversion.base_url is not a valid URL (#{e.message})" + end + + # A markdown body must fit into a single bulk request, otherwise the ES sink can never flush it + def validate_markdown_body_size! + return unless output_sink.to_s == 'elasticsearch' + + bulk_max = elasticsearch&.dig(:bulk_api, :max_size_bytes) || ES::BulkQueue::DEFAULT_SIZE_THRESHOLD + return if max_body_size < bulk_max + + raise ArgumentError, <<~MSG.squish + markdown_conversion is enabled but max_body_size (#{max_body_size}) is not smaller than + elasticsearch.bulk_api.max_size_bytes (#{bulk_max}); a single markdown document could exceed one + bulk request. Lower max_body_size or raise elasticsearch.bulk_api.max_size_bytes. + MSG + end + def configure_logging!(log_level, event_logs_to_file_enabled, system_logs_to_file_enabled) # set up log directory if it doesn't exist if event_logs_to_file_enabled || system_logs_to_file_enabled @@ -549,6 +654,10 @@ def stats def document_mapper @document_mapper ||= ::Crawler::DocumentMapper.new(self) end + + def markdown_converter + @markdown_converter ||= ::Crawler::MarkdownConverter.new(self) + end end end end diff --git a/lib/crawler/markdown_converter.rb b/lib/crawler/markdown_converter.rb new file mode 100644 index 00000000..2a5bd010 --- /dev/null +++ b/lib/crawler/markdown_converter.rb @@ -0,0 +1,39 @@ +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +require 'concurrent/atomic/atomic_fixnum' + +module Crawler + # Converts crawled HTML pages and binary documents (PDF, DOCX, XLSX, PPTX) to Markdown + # through the shared convert-markdown service. See docs/features/MARKDOWN_CONVERSION.md. + class MarkdownConverter + attr_reader :config, :settings + + delegate :system_logger, to: :config + + def initialize(config) + @config = config + @settings = config.markdown_conversion + @converted = Concurrent::AtomicFixnum.new(0) + @failed = Concurrent::AtomicFixnum.new(0) + end + + def enabled? + settings[:enabled] == true + end + + def skip_on_failure? + settings[:on_failure] == 'skip' + end + + # Thread-safe counters, printed at the end of the crawl + def stats + { converted: @converted.value, failed: @failed.value } + end + end +end diff --git a/spec/lib/crawler/api/config_spec.rb b/spec/lib/crawler/api/config_spec.rb index 9b532ee2..7acd3673 100644 --- a/spec/lib/crawler/api/config_spec.rb +++ b/spec/lib/crawler/api/config_spec.rb @@ -360,6 +360,164 @@ def expect_x509_certificates(certs) end end + context 'when configuring markdown conversion' do + let(:base_params) { { domains: [{ url: 'https://example.com' }], output_sink: :console } } + + it 'defaults to disabled with the documented defaults' do + config = Crawler::API::Config.new(base_params) + expect(config.markdown_conversion).to eq( + enabled: false, + base_url: nil, + wait_seconds: 10, + poll_interval: 2, + timeout: 900, + on_failure: 'text', + ca_file: nil + ) + end + + it 'treats a nil markdown_conversion block as all defaults' do + config = Crawler::API::Config.new(base_params.merge(markdown_conversion: nil)) + expect(config.markdown_conversion).to include(enabled: false, on_failure: 'text') + end + + it 'rejects a non-hash markdown_conversion value' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: 'yes')) + end.to raise_error(ArgumentError, 'markdown_conversion must be a hash') + end + + it 'rejects an unknown top-level key' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversions: { enabled: true })) + end.to raise_error(ArgumentError, /Unexpected configuration options.*markdown_conversions/) + end + + it 'rejects unknown nested keys' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { enabled: false, retries: 3 })) + end.to raise_error(ArgumentError, /Unexpected markdown_conversion options.*retries/) + end + + it 'merges user settings over the defaults and strips a trailing slash from base_url' do + config = Crawler::API::Config.new( + base_params.merge(markdown_conversion: { enabled: true, base_url: 'http://converter.test/', wait_seconds: 5 }) + ) + expect(config.markdown_conversion).to eq( + enabled: true, + base_url: 'http://converter.test', + wait_seconds: 5, + poll_interval: 2, + timeout: 900, + on_failure: 'text', + ca_file: nil + ) + end + + it 'requires enabled to be a boolean' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { enabled: 'yes' })) + end.to raise_error(ArgumentError, 'markdown_conversion.enabled must be true or false') + end + + it 'requires base_url when enabled' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { enabled: true })) + end.to raise_error(ArgumentError, /base_url is required when markdown_conversion.enabled is true/) + end + + it 'requires an http(s) base_url when enabled' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { enabled: true, base_url: 'ftp://x' })) + end.to raise_error(ArgumentError, 'markdown_conversion.base_url "ftp://x" must be an http(s) URL') + end + + it 'rejects an unparseable base_url' do + params = base_params.merge(markdown_conversion: { enabled: true, base_url: 'http://exa mple' }) + expect do + Crawler::API::Config.new(params) + end.to raise_error(ArgumentError, /markdown_conversion.base_url is not a valid URL/) + end + + it 'does not require base_url when disabled' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { enabled: false })) + end.not_to raise_error + end + + it 'validates on_failure' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { on_failure: 'retry' })) + end.to raise_error(ArgumentError, 'markdown_conversion.on_failure must be one of text, skip') + end + + it 'accepts on_failure as a symbol and normalizes it to a string' do + config = Crawler::API::Config.new(base_params.merge(markdown_conversion: { on_failure: :skip })) + expect(config.markdown_conversion[:on_failure]).to eq('skip') + end + + it 'validates the wait_seconds range' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { wait_seconds: 61 })) + end.to raise_error(ArgumentError, 'markdown_conversion.wait_seconds must be an integer between 0 and 60') + end + + it 'validates poll_interval and timeout are positive numbers' do + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { poll_interval: 0 })) + end.to raise_error(ArgumentError, 'markdown_conversion.poll_interval must be a positive number') + expect do + Crawler::API::Config.new(base_params.merge(markdown_conversion: { timeout: '900' })) + end.to raise_error(ArgumentError, 'markdown_conversion.timeout must be a positive number') + end + + context 'with the elasticsearch sink' do + let(:es_params) do + { + domains: [{ url: 'https://example.com' }], + output_sink: :elasticsearch, + output_index: 'my-index', + elasticsearch: { host: 'http://localhost', port: 9200 }, + markdown_conversion: { enabled: true, base_url: 'http://converter.test' } + } + end + + it 'rejects a max_body_size that is not smaller than the bulk request size limit (default 1 MB)' do + expect do + Crawler::API::Config.new(es_params) + end.to raise_error( + ArgumentError, + /max_body_size \(5242880\) is not smaller than elasticsearch.bulk_api.max_size_bytes \(1048576\)/ + ) + end + + it 'accepts a smaller max_body_size' do + expect { Crawler::API::Config.new(es_params.merge(max_body_size: 500_000)) }.not_to raise_error + end + + it 'honours a larger configured bulk_api.max_size_bytes' do + params = es_params.merge( + elasticsearch: { host: 'http://localhost', port: 9200, bulk_api: { max_size_bytes: 10_000_000 } } + ) + expect { Crawler::API::Config.new(params) }.not_to raise_error + end + + it 'does not validate the body size when markdown conversion is disabled' do + params = es_params.merge(markdown_conversion: { enabled: false }) + expect { Crawler::API::Config.new(params) }.not_to raise_error + end + end + + it 'exposes a memoised markdown converter' do + config = Crawler::API::Config.new(base_params) + expect(config.markdown_converter).to be_a(Crawler::MarkdownConverter) + expect(config.markdown_converter).to equal(config.markdown_converter) + expect(config.markdown_converter.enabled?).to be(false) + expect(config.markdown_converter.skip_on_failure?).to be(false) + expect(config.markdown_converter.stats).to eq(converted: 0, failed: 0) + end + end + describe '#configure_http_header_service!' do context 'when no auth configuration is provided' do let(:domains) { [{ url: 'https://example1.com' }, { url: 'https://example2.com' }] } From fec8d2ec656195cb07ac06deb4e8cfaf8e37da20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 10:01:05 +0200 Subject: [PATCH 04/16] feat(markdown): add Success#markdown accessor and reserve body_format/content_hash Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- lib/constants.rb | 2 ++ lib/crawler/data/crawl_result/success.rb | 4 +++ spec/lib/constants_spec.rb | 13 ++++++++++ .../crawler/data/crawl_result/success_spec.rb | 25 +++++++++++++++++++ 4 files changed, 44 insertions(+) create mode 100644 spec/lib/constants_spec.rb create mode 100644 spec/lib/crawler/data/crawl_result/success_spec.rb diff --git a/lib/constants.rb b/lib/constants.rb index 801c983c..dfcd3d0a 100644 --- a/lib/constants.rb +++ b/lib/constants.rb @@ -19,6 +19,8 @@ module Constants additional_urls body_content body + body_format + content_hash domains headings last_crawled_at diff --git a/lib/crawler/data/crawl_result/success.rb b/lib/crawler/data/crawl_result/success.rb index 3b928448..5b874d8b 100644 --- a/lib/crawler/data/crawl_result/success.rb +++ b/lib/crawler/data/crawl_result/success.rb @@ -17,6 +17,10 @@ class Success < Base attr_reader :content + # Markdown produced by Crawler::MarkdownConverter, nil until a conversion succeeds. + # Memoised on the result so the SinkLockedError retry loop in the coordinator does not reconvert. + attr_accessor :markdown + def initialize(status_code:, content:, **kwargs) super(status_code:, **kwargs) diff --git a/spec/lib/constants_spec.rb b/spec/lib/constants_spec.rb new file mode 100644 index 00000000..2d52f10f --- /dev/null +++ b/spec/lib/constants_spec.rb @@ -0,0 +1,13 @@ +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +RSpec.describe(Constants) do + it 'reserves the document fields written by markdown conversion' do + expect(Constants::RESERVED_FIELD_NAMES).to include('body_format', 'content_hash') + end +end diff --git a/spec/lib/crawler/data/crawl_result/success_spec.rb b/spec/lib/crawler/data/crawl_result/success_spec.rb new file mode 100644 index 00000000..d13fe803 --- /dev/null +++ b/spec/lib/crawler/data/crawl_result/success_spec.rb @@ -0,0 +1,25 @@ +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +RSpec.describe(Crawler::Data::CrawlResult::Success) do + describe '#markdown' do + it 'is nil by default on HTML results' do + expect(FactoryBot.build(:html_crawl_result).markdown).to be_nil + end + + it 'is nil by default on content extractable file results' do + expect(FactoryBot.build(:content_extractable_file_crawl_result).markdown).to be_nil + end + + it 'can be assigned after a successful conversion' do + result = FactoryBot.build(:html_crawl_result) + result.markdown = '# Title' + expect(result.markdown).to eq('# Title') + end + end +end From 81d930e163666d1baa286d0ab5a59de78045c5a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 10:04:14 +0200 Subject: [PATCH 05/16] feat(markdown): add MarkdownConverter MIME map, filename and payload builders Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- lib/crawler/markdown_converter.rb | 63 +++++++++ spec/lib/crawler/markdown_converter_spec.rb | 142 ++++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 spec/lib/crawler/markdown_converter_spec.rb diff --git a/lib/crawler/markdown_converter.rb b/lib/crawler/markdown_converter.rb index 2a5bd010..c81e8994 100644 --- a/lib/crawler/markdown_converter.rb +++ b/lib/crawler/markdown_converter.rb @@ -12,6 +12,17 @@ module Crawler # Converts crawled HTML pages and binary documents (PDF, DOCX, XLSX, PPTX) to Markdown # through the shared convert-markdown service. See docs/features/MARKDOWN_CONVERSION.md. class MarkdownConverter + # Response Content-Type (charset stripped, downcased) -> upload extension required by the converter. + # Anything else (incl. legacy application/msword, application/vnd.ms-powerpoint) is not convertible. + MIME_EXTENSIONS = { + 'text/html' => '.html', + 'application/pdf' => '.pdf', + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' => '.docx', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' => '.xlsx', + 'application/vnd.openxmlformats-officedocument.presentationml.presentation' => '.pptx' + }.freeze + HTML_EXTENSION = '.html' + attr_reader :config, :settings delegate :system_logger, to: :config @@ -35,5 +46,57 @@ def skip_on_failure? def stats { converted: @converted.value, failed: @failed.value } end + + # Must not call anything on the crawl result when disabled (coordinator specs use strict doubles) + def convertible?(crawl_result) + return false unless enabled? + return true if crawl_result.html? + + crawl_result.content_extractable_file? && !extension_for(crawl_result).nil? + end + + # The converter picks its parser from the extension, so the filename is synthetic: + # ContentExtractableFile#file_name is File.basename(url) and may carry query strings or no extension. + def upload_filename(crawl_result) + "#{crawl_result.url_hash}#{extension_for(crawl_result)}" + end + + # Serialise from Jsoup (not raw bytes) so exclude_tags and data-elastic-exclude/include are honoured. + # The memoised documents are also used for link extraction, so work on a deep clone. + def html_payload(crawl_result) + doc = source_document(crawl_result).clone + Crawler::ContentEngine::Transformer.transform!(doc.body) + doc.charset(java.nio.charset.StandardCharsets::UTF_8) + String.from_java_bytes(doc.outerHtml.to_java_bytes) + end + + def binary_payload(crawl_result) + crawl_result.content.b + end + + private + + def extension_for(crawl_result) + return HTML_EXTENSION if crawl_result.html? + + MIME_EXTENSIONS[normalized_content_type(crawl_result)] + end + + def normalized_content_type(crawl_result) + crawl_result.content_type.to_s.split(';').first.to_s.strip.downcase + end + + # Mirrors CrawlResult::HTML#get_body_tag: parsed_content_excluding_tags([]) would call Jsoup select('') + def source_document(crawl_result) + tags = tags_to_exclude(crawl_result) + return crawl_result.parsed_content if tags.empty? + + crawl_result.parsed_content_excluding_tags(tags) + end + + def tags_to_exclude(crawl_result) + exclude_tags = config.exclude_tags || {} + exclude_tags.fetch(crawl_result.url.site, nil) || exclude_tags.fetch(crawl_result.url.to_s, []) + end end end diff --git a/spec/lib/crawler/markdown_converter_spec.rb b/spec/lib/crawler/markdown_converter_spec.rb new file mode 100644 index 00000000..0ac5df32 --- /dev/null +++ b/spec/lib/crawler/markdown_converter_spec.rb @@ -0,0 +1,142 @@ +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +RSpec.describe(Crawler::MarkdownConverter) do + let(:domain) { 'http://example.com' } + let(:base_url) { 'http://converter.test' } + let(:exclude_tags) { nil } + let(:markdown_settings) { { enabled: true, base_url:, wait_seconds: 10, poll_interval: 0.01, timeout: 5 } } + let(:config) do + Crawler::API::Config.new( + domains: [{ url: domain, exclude_tags: }.compact], + output_sink: :console, + markdown_conversion: markdown_settings + ) + end + let(:converter) { described_class.new(config) } + + let(:html) do + <<~HTML + + Hi + +

Héllo

+
secret
+
footer text
+ + + HTML + end + let(:html_result) { FactoryBot.build(:html_crawl_result, url: "#{domain}/page", content: html) } + let(:pdf_bytes) { "%PDF-1.4\n\xFF\xFE binary".b } + let(:pdf_result) do + FactoryBot.build( + :content_extractable_file_crawl_result, + url: "#{domain}/doc.pdf?v=2", + content: pdf_bytes, + content_type: 'application/pdf' + ) + end + + let(:docx_type) { 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' } + let(:xlsx_type) { 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' } + let(:pptx_type) { 'application/vnd.openxmlformats-officedocument.presentationml.presentation' } + + def file_result(content_type) + FactoryBot.build(:content_extractable_file_crawl_result, url: "#{domain}/file", content_type:) + end + + describe '#convertible?' do + it 'is true for HTML results' do + expect(converter.convertible?(html_result)).to be(true) + end + + it 'is true for supported binary MIME types' do + expect(converter.convertible?(pdf_result)).to be(true) + expect(converter.convertible?(file_result(docx_type))).to be(true) + expect(converter.convertible?(file_result(xlsx_type))).to be(true) + expect(converter.convertible?(file_result(pptx_type))).to be(true) + end + + it 'strips charset parameters and ignores case when matching the MIME type' do + xlsx = 'Application/vnd.openxmlformats-officedocument.spreadsheetml.sheet; charset=binary' + expect(converter.convertible?(file_result(xlsx))).to be(true) + end + + it 'is false for legacy office types and other MIME types' do + expect(converter.convertible?(file_result('application/msword'))).to be(false) + expect(converter.convertible?(file_result('application/vnd.ms-powerpoint'))).to be(false) + expect(converter.convertible?(file_result('image/png'))).to be(false) + end + + context 'when disabled' do + let(:markdown_settings) { { enabled: false } } + + it 'is false and does not call anything on the crawl result' do + strict_result = double(:crawl_result) # raises on any message + expect(converter.convertible?(strict_result)).to be(false) + end + end + end + + describe '#upload_filename' do + it 'uses the URL hash plus the extension for the MIME type, never the URL basename' do + expect(converter.upload_filename(pdf_result)).to eq("#{pdf_result.url_hash}.pdf") + expect(converter.upload_filename(html_result)).to eq("#{html_result.url_hash}.html") + end + + it 'maps every supported MIME type to its extension' do + expect(converter.upload_filename(file_result('application/pdf; charset=utf-8'))).to end_with('.pdf') + expect(converter.upload_filename(file_result(docx_type))).to end_with('.docx') + expect(converter.upload_filename(file_result(xlsx_type))).to end_with('.xlsx') + expect(converter.upload_filename(file_result(pptx_type))).to end_with('.pptx') + end + end + + describe '#html_payload' do + let(:exclude_tags) { %w[footer] } + + it 'serialises the Jsoup document as UTF-8 bytes with a charset meta tag' do + payload = converter.html_payload(html_result) + expect(payload.encoding).to eq(Encoding::BINARY) + expect(payload).to include(''.b) + expect(payload).to include('Héllo'.b) + end + + it 'removes excluded tags and data-elastic-exclude content' do + payload = converter.html_payload(html_result) + expect(payload).not_to include('footer text'.b) + expect(payload).not_to include('secret'.b) + end + + it 'leaves the memoised parsed documents untouched' do + converter.html_payload(html_result) + expect(html_result.parsed_content.select('footer').size).to eq(1) + expect(html_result.parsed_content.select('[data-elastic-exclude]').size).to eq(1) + expect(html_result.parsed_content_excluding_tags(%w[footer]).select('[data-elastic-exclude]').size).to eq(1) + end + + context 'without exclude tags configured' do + let(:exclude_tags) { nil } + + it 'keeps the footer but still honours data-elastic-exclude' do + payload = converter.html_payload(html_result) + expect(payload).to include('footer text'.b) + expect(payload).not_to include('secret'.b) + end + end + end + + describe '#binary_payload' do + it 'returns the content bytes unchanged' do + payload = converter.binary_payload(pdf_result) + expect(payload).to eq(pdf_bytes) + expect(payload.encoding).to eq(Encoding::BINARY) + end + end +end From be688e4901c7d37eed3792302239870912a4b887 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 10:13:01 +0200 Subject: [PATCH 06/16] feat(markdown): add converter transport with polling, retry, health check and stats Adds the HTTP side of MarkdownConverter: a hand-built multipart upload to POST /api/v1/convert/upload?wait=N, status polling with 1.5x backoff capped at 5s against a hard deadline, one retry after 1s for network errors, 5xx and an expired job (404 while polling), a 5s /api/v1/health check, markdown truncation to max_body_size and thread-safe converted/failed counters. convert! never raises; it returns :converted, :skipped or :failed. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- lib/crawler/markdown_converter.rb | 175 +++++++++++ spec/lib/crawler/markdown_converter_spec.rb | 331 ++++++++++++++++++++ 2 files changed, 506 insertions(+) diff --git a/lib/crawler/markdown_converter.rb b/lib/crawler/markdown_converter.rb index c81e8994..5da2b253 100644 --- a/lib/crawler/markdown_converter.rb +++ b/lib/crawler/markdown_converter.rb @@ -7,6 +7,10 @@ # frozen_string_literal: true require 'concurrent/atomic/atomic_fixnum' +require 'json' +require 'net/http' +require 'securerandom' +require 'uri' module Crawler # Converts crawled HTML pages and binary documents (PDF, DOCX, XLSX, PPTX) to Markdown @@ -23,6 +27,22 @@ class MarkdownConverter }.freeze HTML_EXTENSION = '.html' + # Raised internally for errors worth one retry (network, 5xx, expired job); never escapes convert! + class RetryableError < StandardError; end + # Raised internally for definitive failures (4xx, status: failed, empty markdown, deadline) + class ConversionError < StandardError; end + + UPLOAD_PATH = '/api/v1/convert/upload' + HEALTH_PATH = '/api/v1/health' + FINISHED_STATUSES = %w[done failed].freeze + OPEN_TIMEOUT = 10 + HEALTH_TIMEOUT = 5 + READ_TIMEOUT_MARGIN = 20 # read_timeout = wait_seconds + this + RETRY_DELAY = 1 + POLL_BACKOFF = 1.5 + MAX_POLL_INTERVAL = 5 + RETRYABLE_ERRORS = [SystemCallError, SocketError, EOFError, Net::OpenTimeout, Net::ReadTimeout].freeze + attr_reader :config, :settings delegate :system_logger, to: :config @@ -74,6 +94,48 @@ def binary_payload(crawl_result) crawl_result.content.b end + # Returns :converted | :skipped | :failed and never raises. + # On success sets crawl_result.markdown (limited to config.max_body_size bytes). + def convert!(crawl_result) + return :skipped unless convertible?(crawl_result) + + markdown = with_one_retry(crawl_result) { run_conversion(crawl_result) } + crawl_result.markdown = Crawler::ContentEngine::Utils.limit_bytesize(markdown, config.max_body_size) + @converted.increment + :converted + rescue StandardError => e + @failed.increment + system_logger.warn("Markdown conversion failed for #{crawl_result.url}: #{e.message}") + :failed + end + + # GET /api/v1/health with a short timeout; called once at crawl start + def healthy? + uri = URI.parse("#{settings[:base_url]}#{HEALTH_PATH}") + request = Net::HTTP::Get.new(uri.request_uri) + request['User-Agent'] = config.user_agent + response = http_client(uri, open_timeout: HEALTH_TIMEOUT, read_timeout: HEALTH_TIMEOUT).request(request) + response.code.to_i == 200 + rescue StandardError => e + system_logger.warn("Markdown converter health check failed (#{settings[:base_url]}): #{e.class}: #{e.message}") + false + end + + # Hand-built multipart/form-data body. Everything is appended as ASCII-8BIT so binary payloads + # concatenate without Encoding::CompatibilityError; Net::HTTP sets Content-Length from body.bytesize. + def multipart_body(crawl_result) + boundary = SecureRandom.hex(16) + payload = crawl_result.html? ? html_payload(crawl_result) : binary_payload(crawl_result) + part_type = crawl_result.html? ? 'text/html; charset=utf-8' : normalized_content_type(crawl_result) + head = [ + "--#{boundary}\r\n", + "Content-Disposition: form-data; name=\"file\"; filename=\"#{upload_filename(crawl_result)}\"\r\n", + "Content-Type: #{part_type}\r\n\r\n" + ].join.b + tail = "\r\n--#{boundary}--\r\n".b + [head + payload + tail, "multipart/form-data; boundary=#{boundary}"] + end + private def extension_for(crawl_result) @@ -98,5 +160,118 @@ def tags_to_exclude(crawl_result) exclude_tags = config.exclude_tags || {} exclude_tags.fetch(crawl_result.url.site, nil) || exclude_tags.fetch(crawl_result.url.to_s, []) end + + def with_one_retry(crawl_result) + attempts = 0 + begin + attempts += 1 + yield + rescue RetryableError => e + raise e if attempts > 1 + + system_logger.warn( + "Markdown conversion for #{crawl_result.url} failed (#{e.message}), retrying once in #{RETRY_DELAY}s" + ) + sleep(RETRY_DELAY) + retry + end + end + + def run_conversion(crawl_result) + deadline = monotonic_now + settings[:timeout] + job = submit(crawl_result) + job = poll_until_finished(job, deadline) unless finished?(job) + raise ConversionError, "converter reported status 'failed': #{job['error']}" if job['status'] == 'failed' + + markdown = job['markdown'].to_s + raise ConversionError, 'converter returned empty markdown' if markdown.strip.empty? + + markdown + end + + def submit(crawl_result) + uri = URI.parse("#{settings[:base_url]}#{UPLOAD_PATH}?wait=#{settings[:wait_seconds]}") + request = Net::HTTP::Post.new(uri.request_uri) + body, content_type = multipart_body(crawl_result) + request['Content-Type'] = content_type + request.body = body + parse_job(perform(uri, request)) + end + + def poll_until_finished(job, deadline) + uri = status_uri(job) + interval = settings[:poll_interval].to_f + loop do + check_deadline!(job, deadline) + sleep(interval) + job = poll_status(uri) + return job if finished?(job) + + interval = [interval * POLL_BACKOFF, MAX_POLL_INTERVAL].min + end + end + + def status_uri(job) + if job['status_url'].blank? + raise ConversionError, "converter returned status #{job['status'].inspect} without a status_url" + end + + URI.parse("#{settings[:base_url]}#{job['status_url']}") + end + + # The deadline runs from the start of the submit and is never retried + def check_deadline!(job, deadline) + return if monotonic_now < deadline + + raise ConversionError, "timed out after #{settings[:timeout]}s waiting for job #{job['job_id']}" + end + + def poll_status(uri) + parse_job(perform(uri, Net::HTTP::Get.new(uri.request_uri)), polling: true) + end + + def perform(uri, request) + request['User-Agent'] = config.user_agent + http_client(uri, open_timeout: OPEN_TIMEOUT, read_timeout: settings[:wait_seconds] + READ_TIMEOUT_MARGIN) + .request(request) + rescue *RETRYABLE_ERRORS => e + raise RetryableError, "#{e.class}: #{e.message}" + end + + # p_addr = nil disables Net::HTTP's implicit HTTP_PROXY/HTTPS_PROXY env proxying; the crawler's own + # http_proxy_* settings only apply to the Java HTTP client used for crawling, not to converter calls. + def http_client(uri, open_timeout:, read_timeout:) + Net::HTTP.new(uri.host, uri.port, nil).tap do |http| + http.use_ssl = uri.scheme == 'https' + http.open_timeout = open_timeout + http.read_timeout = read_timeout + http.ca_file = settings[:ca_file] if settings[:ca_file].present? + end + end + + # Branch on the JSON status, not the HTTP code (200 and 202 both carry a job document) + def parse_job(response, polling: false) + raise_on_http_error!(response, polling) + JSON.parse(response.body) + rescue JSON::ParserError => e + raise ConversionError, "invalid JSON from converter: #{e.message}" + end + + def raise_on_http_error!(response, polling) + code = response.code.to_i + raise RetryableError, "HTTP #{code} from converter" if code >= 500 + raise RetryableError, 'HTTP 404 from converter while polling (job expired)' if polling && code == 404 + return if [200, 202].include?(code) + + raise ConversionError, "HTTP #{code} from converter: #{response.body.to_s[0, 200]}" + end + + def finished?(job) + FINISHED_STATUSES.include?(job['status']) + end + + def monotonic_now + Process.clock_gettime(Process::CLOCK_MONOTONIC) + end end end diff --git a/spec/lib/crawler/markdown_converter_spec.rb b/spec/lib/crawler/markdown_converter_spec.rb index 0ac5df32..8fb30620 100644 --- a/spec/lib/crawler/markdown_converter_spec.rb +++ b/spec/lib/crawler/markdown_converter_spec.rb @@ -139,4 +139,335 @@ def file_result(content_type) expect(payload.encoding).to eq(Encoding::BINARY) end end + + describe '#convert!' do + let(:upload_url) { "#{base_url}/api/v1/convert/upload?wait=10" } + let(:status_url) { "#{base_url}/api/v1/jobs/job-1" } + let(:json_headers) { { 'Content-Type' => 'application/json' } } + + def job_json(status, extra = {}) + { job_id: 'job-1', status:, status_url: '/api/v1/jobs/job-1', source_name: 'x' }.merge(extra).to_json + end + + def json_response(status, extra = {}) + { status: 200, body: job_json(status, extra), headers: json_headers } + end + + def pending_response + { status: 202, body: job_json('pending'), headers: json_headers } + end + + before do + # Keep polling and retries instant; timing is asserted through the arguments passed to sleep + allow(converter).to receive(:sleep) + allow(config.system_logger).to receive(:warn) + end + + it 'stores the markdown when the upload returns status done inline' do + stub_request(:post, upload_url).to_return(json_response('done', markdown: '# Hi')) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(html_result.markdown).to eq('# Hi') + expect(converter.stats).to eq(converted: 1, failed: 0) + expect(converter).not_to have_received(:sleep) + end + + it 'sends a multipart upload with the file field, hashed filename, part content type and user agent' do + disposition = "Content-Disposition: form-data; name=\"file\"; filename=\"#{pdf_result.url_hash}.pdf\"\r\n".b + stub = stub_request(:post, upload_url).with do |request| + body = request.body.b # compare bytes regardless of the encoding tag WebMock assigns + request.headers['Content-Type'].start_with?('multipart/form-data; boundary=') && + request.headers['User-Agent'] == config.user_agent && + body.include?(disposition) && + body.include?("Content-Type: application/pdf\r\n\r\n".b + pdf_bytes + "\r\n".b) + end.to_return(json_response('done', markdown: '# PDF')) + + expect(converter.convert!(pdf_result)).to eq(:converted) + expect(stub).to have_been_requested.once + end + + context 'with exclude tags configured' do + let(:exclude_tags) { %w[footer] } + + it 'uploads the pre-processed UTF-8 HTML document' do + stub = stub_request(:post, upload_url).with do |request| + body = request.body.b + body.include?("filename=\"#{html_result.url_hash}.html\"".b) && + body.include?("Content-Type: text/html; charset=utf-8\r\n\r\n".b) && + body.include?(''.b) && + body.include?('Héllo'.b) && + !body.include?('footer text'.b) && + !body.include?('secret'.b) + end.to_return(json_response('done', markdown: '# HTML')) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(stub).to have_been_requested.once + end + end + + it 'polls the status URL with backoff until the job is done' do + stub_request(:post, upload_url).to_return(pending_response) + poll = stub_request(:get, status_url).to_return( + json_response('processing'), + json_response('processing'), + json_response('done', markdown: '# Polled') + ) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(html_result.markdown).to eq('# Polled') + expect(poll).to have_been_requested.times(3) + expect(converter).to have_received(:sleep).with(0.01).ordered + expect(converter).to have_received(:sleep).with(be_within(1e-9).of(0.015)).ordered + expect(converter).to have_received(:sleep).with(be_within(1e-9).of(0.0225)).ordered + end + + context 'with a large poll interval' do + let(:markdown_settings) { super().merge(poll_interval: 4) } + + it 'caps the backoff at 5 seconds' do + # `have_received(...).with(5.0)` defaults to "exactly once", so record the arguments instead + intervals = [] + allow(converter).to receive(:sleep) { |seconds| intervals << seconds } + stub_request(:post, upload_url).to_return(pending_response) + stub_request(:get, status_url).to_return( + json_response('processing'), + json_response('processing'), + json_response('done', markdown: '# Capped') + ) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(intervals).to eq([4.0, 5.0, 5.0]) + end + end + + it 'fails without retrying when the converter reports status failed' do + stub = stub_request(:post, upload_url).to_return(json_response('failed', error: 'unsupported document')) + + expect(converter.convert!(pdf_result)).to eq(:failed) + expect(pdf_result.markdown).to be_nil + expect(converter.stats).to eq(converted: 0, failed: 1) + expect(stub).to have_been_requested.once + expect(converter).not_to have_received(:sleep) + expect(config.system_logger).to have_received(:warn) + .with(%r{Markdown conversion failed for http://example.com/doc.pdf\?v=2: .*unsupported document}) + end + + it 'fails without retrying on HTTP 422' do + stub = stub_request(:post, upload_url).to_return(status: 422, body: '{"detail":"unsupported extension"}') + + expect(converter.convert!(pdf_result)).to eq(:failed) + expect(stub).to have_been_requested.once + expect(converter).not_to have_received(:sleep) + expect(config.system_logger).to have_received(:warn).with(/HTTP 422 from converter/) + end + + it 'fails without retrying on HTTP 404 from the upload endpoint' do + stub = stub_request(:post, upload_url).to_return(status: 404, body: 'not found') + + expect(converter.convert!(pdf_result)).to eq(:failed) + expect(stub).to have_been_requested.once + end + + it 'retries once after 1s on HTTP 5xx and succeeds' do + stub = stub_request(:post, upload_url).to_return( + { status: 503, body: 'upstream unavailable' }, + json_response('done', markdown: '# Second try') + ) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(html_result.markdown).to eq('# Second try') + expect(stub).to have_been_requested.twice + expect(converter).to have_received(:sleep).with(1).once + end + + it 'fails after two HTTP 5xx responses' do + stub = stub_request(:post, upload_url).to_return(status: 503, body: 'upstream unavailable') + + expect(converter.convert!(html_result)).to eq(:failed) + expect(html_result.markdown).to be_nil + expect(stub).to have_been_requested.twice + # warned twice: once announcing the retry, once for the final failure + expect(config.system_logger).to have_received(:warn).with(/HTTP 503 from converter/).at_least(:once) + end + + it 'retries once on a connection error' do + stub = stub_request(:post, upload_url) + stub.to_raise(Errno::ECONNREFUSED).then.to_return(json_response('done', markdown: '# Reconnected')) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(stub).to have_been_requested.twice + expect(converter).to have_received(:sleep).with(1).once + end + + it 'retries once on an open timeout' do + stub = stub_request(:post, upload_url).to_timeout.then.to_return(json_response('done', markdown: '# Late')) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(stub).to have_been_requested.twice + end + + it 'fails after two read timeouts' do + stub = stub_request(:post, upload_url).to_raise(Net::ReadTimeout) + + expect(converter.convert!(html_result)).to eq(:failed) + expect(stub).to have_been_requested.twice + expect(config.system_logger).to have_received(:warn).with(/Net::ReadTimeout/).at_least(:once) + end + + it 'resubmits once when polling returns HTTP 404 (job expired)' do + upload = stub_request(:post, upload_url).to_return( + pending_response, + json_response('done', markdown: '# Resubmitted') + ) + stub_request(:get, status_url).to_return(status: 404, body: '{"detail":"unknown job"}') + + expect(converter.convert!(html_result)).to eq(:converted) + expect(html_result.markdown).to eq('# Resubmitted') + expect(upload).to have_been_requested.twice + end + + context 'when the job never finishes' do + let(:markdown_settings) { super().merge(timeout: 0.2, poll_interval: 0.05) } + + before { allow(converter).to receive(:sleep).and_call_original } + + it 'gives up at the deadline without retrying' do + upload = stub_request(:post, upload_url).to_return(pending_response) + poll = stub_request(:get, status_url).to_return(json_response('processing')) + + expect(converter.convert!(html_result)).to eq(:failed) + expect(upload).to have_been_requested.once + expect(poll).to have_been_requested.at_least_once + expect(config.system_logger).to have_received(:warn).with(/timed out after 0.2s waiting for job job-1/) + end + end + + it 'fails when the markdown is blank' do + stub_request(:post, upload_url).to_return(json_response('done', markdown: " \n\t")) + + expect(converter.convert!(html_result)).to eq(:failed) + expect(html_result.markdown).to be_nil + expect(config.system_logger).to have_received(:warn).with(/empty markdown/) + end + + it 'fails when the converter returns invalid JSON' do + stub_request(:post, upload_url).to_return(status: 200, body: 'oops') + + expect(converter.convert!(html_result)).to eq(:failed) + expect(config.system_logger).to have_received(:warn).with(/invalid JSON from converter/) + end + + context 'with a small max_body_size' do + let(:config) do + Crawler::API::Config.new( + domains: [{ url: domain }], + output_sink: :console, + max_body_size: 10, + markdown_conversion: markdown_settings + ) + end + + it 'truncates the markdown to max_body_size bytes with an omission marker' do + stub_request(:post, upload_url).to_return(json_response('done', markdown: 'a' * 50)) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(html_result.markdown).to eq("#{'a' * 7}…") + expect(html_result.markdown.bytesize).to eq(10) + end + end + + it 'skips unsupported MIME types without any HTTP call' do + result = file_result('application/msword') + + expect(converter.convert!(result)).to eq(:skipped) + expect(result.markdown).to be_nil + expect(a_request(:any, /converter.test/)).not_to have_been_made + expect(converter.stats).to eq(converted: 0, failed: 0) + end + + context 'when disabled' do + let(:markdown_settings) { { enabled: false } } + + it 'returns :skipped without any HTTP call or touching the result' do + strict_result = double(:crawl_result) + + expect(converter.convert!(strict_result)).to eq(:skipped) + expect(a_request(:any, /converter.test/)).not_to have_been_made + expect(converter.stats).to eq(converted: 0, failed: 0) + end + end + + it 'counts conversions and failures across calls' do + stub_request(:post, upload_url).to_return( + json_response('done', markdown: '# One'), + json_response('failed', error: 'boom') + ) + + converter.convert!(html_result) + converter.convert!(pdf_result) + expect(converter.stats).to eq(converted: 1, failed: 1) + end + + context 'over https with a custom CA file' do + let(:base_url) { 'https://converter.test' } + let(:markdown_settings) { super().merge(ca_file: '/etc/ssl/converter-ca.pem') } + + it 'configures the HTTP client with TLS, the CA file and the documented timeouts' do + clients = [] + allow(Net::HTTP).to receive(:new).and_wrap_original do |original, *args| + original.call(*args).tap { |client| clients << client } + end + stub_request(:post, upload_url).to_return(json_response('done', markdown: '# TLS')) + + expect(converter.convert!(html_result)).to eq(:converted) + expect(clients.size).to eq(1) + expect(clients.first.use_ssl?).to be(true) + expect(clients.first.ca_file).to eq('/etc/ssl/converter-ca.pem') + expect(clients.first.open_timeout).to eq(10) + expect(clients.first.read_timeout).to eq(30) + end + end + end + + describe '#healthy?' do + let(:health_url) { "#{base_url}/api/v1/health" } + + before { allow(config.system_logger).to receive(:warn) } + + it 'is true when the health endpoint returns 200 and sends the crawler user agent' do + stub = stub_request(:get, health_url) + .with(headers: { 'User-Agent' => config.user_agent }) + .to_return(status: 200, body: '{"status":"ok"}') + expect(converter.healthy?).to be(true) + expect(stub).to have_been_requested.once + end + + it 'is false on a non-200 response' do + stub_request(:get, health_url).to_return(status: 503) + expect(converter.healthy?).to be(false) + end + + it 'is false and warns when the service is unreachable' do + stub_request(:get, health_url).to_raise(Errno::ECONNREFUSED) + expect(converter.healthy?).to be(false) + expect(config.system_logger).to have_received(:warn).with(/health check failed.*ECONNREFUSED/) + end + + it 'is false on a timeout' do + stub_request(:get, health_url).to_timeout + expect(converter.healthy?).to be(false) + end + + it 'uses a 5 second timeout' do + clients = [] + allow(Net::HTTP).to receive(:new).and_wrap_original do |original, *args| + original.call(*args).tap { |client| clients << client } + end + stub_request(:get, health_url).to_return(status: 200) + + converter.healthy? + expect(clients.first.open_timeout).to eq(5) + expect(clients.first.read_timeout).to eq(5) + end + end end From c9d74bbbd74d4149816f805d13f5d05614bee4bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 13:40:03 +0200 Subject: [PATCH 07/16] fix(markdown): validate converter status_url before polling The status_url in a converter job document is server-controlled and was concatenated onto base_url unchecked, so a response of "@evil.host/x" parsed as host evil.host with the configured base_url demoted to userinfo and the poll left the intended service. Accept only an absolute path and require the parsed URI to keep the configured host with no userinfo; both failures raise ConversionError naming the offending status_url and are not retried. Also pins the final-failure log assertions to their exact message instead of at_least(:once), and asserts p_addr is nil on the constructed Net::HTTP. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- lib/crawler/markdown_converter.rb | 20 ++++++++++-- spec/lib/crawler/markdown_converter_spec.rb | 35 +++++++++++++++++++-- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/lib/crawler/markdown_converter.rb b/lib/crawler/markdown_converter.rb index 5da2b253..76baefa6 100644 --- a/lib/crawler/markdown_converter.rb +++ b/lib/crawler/markdown_converter.rb @@ -15,7 +15,7 @@ module Crawler # Converts crawled HTML pages and binary documents (PDF, DOCX, XLSX, PPTX) to Markdown # through the shared convert-markdown service. See docs/features/MARKDOWN_CONVERSION.md. - class MarkdownConverter + class MarkdownConverter # rubocop:disable Metrics/ClassLength # Response Content-Type (charset stripped, downcased) -> upload extension required by the converter. # Anything else (incl. legacy application/msword, application/vnd.ms-powerpoint) is not convertible. MIME_EXTENSIONS = { @@ -211,12 +211,26 @@ def poll_until_finished(job, deadline) end end + # status_url is server-controlled, so it must not be able to move the poll to another service: + # only an absolute path is accepted and the parsed URI has to stay on the configured host. + # "@evil.host/x" would otherwise parse as host evil.host with base_url as the userinfo. def status_uri(job) - if job['status_url'].blank? + status_url = job['status_url'].to_s + if status_url.blank? raise ConversionError, "converter returned status #{job['status'].inspect} without a status_url" end - URI.parse("#{settings[:base_url]}#{job['status_url']}") + unless status_url.start_with?('/') + raise ConversionError, "converter returned a non-absolute status_url: #{status_url.inspect}" + end + + URI.parse("#{settings[:base_url]}#{status_url}").tap { |uri| validate_status_host!(uri, status_url) } + end + + def validate_status_host!(uri, status_url) + return if uri.userinfo.nil? && uri.host == URI.parse(settings[:base_url]).host + + raise ConversionError, "converter returned an off-host status_url: #{status_url.inspect}" end # The deadline runs from the start of the submit and is never retried diff --git a/spec/lib/crawler/markdown_converter_spec.rb b/spec/lib/crawler/markdown_converter_spec.rb index 8fb30620..e637ebae 100644 --- a/spec/lib/crawler/markdown_converter_spec.rb +++ b/spec/lib/crawler/markdown_converter_spec.rb @@ -286,8 +286,9 @@ def pending_response expect(converter.convert!(html_result)).to eq(:failed) expect(html_result.markdown).to be_nil expect(stub).to have_been_requested.twice - # warned twice: once announcing the retry, once for the final failure - expect(config.system_logger).to have_received(:warn).with(/HTTP 503 from converter/).at_least(:once) + # the retry line reads "Markdown conversion for failed (...)", so only the final failure matches + expect(config.system_logger).to have_received(:warn) + .with(/Markdown conversion failed for .*HTTP 503 from converter/) end it 'retries once on a connection error' do @@ -311,7 +312,7 @@ def pending_response expect(converter.convert!(html_result)).to eq(:failed) expect(stub).to have_been_requested.twice - expect(config.system_logger).to have_received(:warn).with(/Net::ReadTimeout/).at_least(:once) + expect(config.system_logger).to have_received(:warn).with(/Markdown conversion failed for .*Net::ReadTimeout/) end it 'resubmits once when polling returns HTTP 404 (job expired)' do @@ -326,6 +327,31 @@ def pending_response expect(upload).to have_been_requested.twice end + it 'refuses to poll a status_url that smuggles another host through the userinfo' do + upload = stub_request(:post, upload_url) + .to_return(status: 202, body: job_json('pending', status_url: '@evil.host/x'), headers: json_headers) + + expect(converter.convert!(html_result)).to eq(:failed) + expect(html_result.markdown).to be_nil + expect(upload).to have_been_requested.once + expect(a_request(:any, %r{//evil.host})).not_to have_been_made + expect(config.system_logger).to have_received(:warn).with(/status_url/) + end + + it 'refuses to poll an absolute status_url pointing at another host' do + upload = stub_request(:post, upload_url).to_return( + status: 202, + body: job_json('pending', status_url: 'http://evil.host/x'), + headers: json_headers + ) + + expect(converter.convert!(html_result)).to eq(:failed) + expect(html_result.markdown).to be_nil + expect(upload).to have_been_requested.once + expect(a_request(:any, %r{//evil.host})).not_to have_been_made + expect(config.system_logger).to have_received(:warn).with(/status_url/) + end + context 'when the job never finishes' do let(:markdown_settings) { super().merge(timeout: 0.2, poll_interval: 0.05) } @@ -414,13 +440,16 @@ def pending_response it 'configures the HTTP client with TLS, the CA file and the documented timeouts' do clients = [] + new_args = [] allow(Net::HTTP).to receive(:new).and_wrap_original do |original, *args| + new_args << args original.call(*args).tap { |client| clients << client } end stub_request(:post, upload_url).to_return(json_response('done', markdown: '# TLS')) expect(converter.convert!(html_result)).to eq(:converted) expect(clients.size).to eq(1) + expect(new_args.first[2]).to be_nil # p_addr = nil disables Net::HTTP's env proxying expect(clients.first.use_ssl?).to be(true) expect(clients.first.ca_file).to eq('/etc/ssl/converter-ca.pem') expect(clients.first.open_timeout).to eq(10) From 84f1581a9510fedc4f88b8bf87b227c3065a43ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 13:47:56 +0200 Subject: [PATCH 08/16] feat(markdown): convert crawl results before ingestion, fail fast on unhealthy converter Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- lib/crawler/api/crawl.rb | 30 ++++++++ lib/crawler/coordinator.rb | 20 ++++- lib/errors.rb | 4 + spec/lib/crawler/api/crawl_spec.rb | 56 ++++++++++++++ spec/lib/crawler/coordinator_spec.rb | 107 +++++++++++++++++++++++++++ 5 files changed, 215 insertions(+), 2 deletions(-) diff --git a/lib/crawler/api/crawl.rb b/lib/crawler/api/crawl.rb index 5db0ca85..c9c8d461 100644 --- a/lib/crawler/api/crawl.rb +++ b/lib/crawler/api/crawl.rb @@ -9,6 +9,8 @@ require 'bson' require 'concurrent' +require_dependency File.join(__dir__, '..', '..', 'errors') + module Crawler module API # This represents a crawl job. Individual crawls can be started and then tracked given an @@ -85,9 +87,13 @@ def start! # rubocop:disable Metrics/AbcSize, Metrics/MethodLength url_queue_items: crawl_queue.length, seen_urls: seen_urls.count ) + verify_markdown_converter! ingestion_stats = coordinator.run_crawl! record_overall_outcome(coordinator.crawl_results) + rescue Errors::MarkdownConverterUnavailableError => e + system_logger.error(e.message) + record_outcome(outcome: :failure, message: e.message) rescue StandardError => e log_exception(e, 'Unexpected error while running the crawl') record_outcome( @@ -106,6 +112,7 @@ def start! # rubocop:disable Metrics/AbcSize, Metrics/MethodLength crawl_queue.delete seen_urls.clear print_final_crawl_status + log_markdown_conversion_stats print_crawl_ingestion_results(ingestion_stats) if config.output_sink.to_s == 'elasticsearch' end end @@ -237,6 +244,29 @@ def print_final_crawl_status # rubocop:disable Metrics/AbcSize puts "- Average response time (seconds): #{crawl_status[:avg_response_time_msec] / 1000}" end + # Fail fast when the converter is down: a silent outage would degrade the whole index to plain text. + # Calling config.markdown_converter here also warms the `@markdown_converter ||=` memo on the main + # thread before the task pool starts (the memo is not thread-safe); keep that call unconditional so a + # future `return unless enabled?` guard cannot leave the pool threads racing to build separate + # instances and splitting the stats across them. + def verify_markdown_converter! + converter = config.markdown_converter + return unless converter.enabled? + return if converter.healthy? + + raise Errors::MarkdownConverterUnavailableError, <<~MSG.squish + Markdown converter at #{config.markdown_conversion[:base_url]} is not healthy + (GET /api/v1/health did not return 200); aborting the crawl so the index is not degraded to plain text + MSG + end + + def log_markdown_conversion_stats + return unless config.markdown_converter.enabled? + + stats = config.markdown_converter.stats + system_logger.info("Markdown conversions: converted=#{stats[:converted]} failed=#{stats[:failed]}") + end + def print_extracted_links(result) return unless result.is_a?(Crawler::Data::CrawlResult::HTML) diff --git a/lib/crawler/coordinator.rb b/lib/crawler/coordinator.rb index eb68ea80..7a0fe5b7 100644 --- a/lib/crawler/coordinator.rb +++ b/lib/crawler/coordinator.rb @@ -398,8 +398,7 @@ def process_crawl_result(crawl_task, crawl_result) crawl_task_progress(crawl_task, 'skipping ingestion of redirect') extracted_event[:message] = "Crawler was redirected to #{crawl_result.location}" elsif crawl_task.content? - crawl_task_progress(crawl_task, 'ingesting the result') - outcome = output_crawl_result(crawl_result) + outcome = convert_and_output_crawl_result(crawl_task, crawl_result) extracted_event.merge!(outcome) end @@ -525,6 +524,23 @@ def extract_links(crawl_result, crawl_depth:) end end + # Converts the crawl result to markdown (no-op when disabled) and then sends it to the output sink. + # Runs on the crawl-task thread, before the sink, because the Elasticsearch sink holds its queue lock + # while mapping documents and a slow conversion there would serialise every thread. + def convert_and_output_crawl_result(crawl_task, crawl_result) + crawl_task_progress(crawl_task, 'converting to markdown') if config.markdown_converter.enabled? + conversion = config.markdown_converter.convert!(crawl_result) + + if conversion == :failed && config.markdown_converter.skip_on_failure? + message = "Markdown conversion failed for #{crawl_result.url} and on_failure is 'skip'; document not ingested" + system_logger.warn(message) + return sink.failure(message) + end + + crawl_task_progress(crawl_task, 'ingesting the result') + output_crawl_result(crawl_result) + end + # Outputs the results of a single URL processing to an output module configured for the crawl def output_crawl_result(crawl_result) retries = 0 diff --git a/lib/errors.rb b/lib/errors.rb index c1020b4d..6e619573 100644 --- a/lib/errors.rb +++ b/lib/errors.rb @@ -29,4 +29,8 @@ class ExitIfESConnectionError < SystemExit; end # against the output_index value, and will continue if the index is found. # If it is not found, this error will be raised, which causes a system exit to occur. class ExitIfUnableToCreateIndex < SystemExit; end + + # Raised at crawl start when markdown conversion is enabled but the converter's health endpoint + # does not answer 200. The crawl aborts instead of silently degrading the whole index to plain text. + class MarkdownConverterUnavailableError < StandardError; end end diff --git a/spec/lib/crawler/api/crawl_spec.rb b/spec/lib/crawler/api/crawl_spec.rb index 97d988f9..4c15f69d 100644 --- a/spec/lib/crawler/api/crawl_spec.rb +++ b/spec/lib/crawler/api/crawl_spec.rb @@ -77,6 +77,62 @@ expect(subject.sink).to be_a(Crawler::OutputSink::Base) end + it 'does not check converter health when markdown conversion is disabled' do + expect(crawl_config.markdown_converter).not_to receive(:healthy?) + subject.start! + end + + context 'with markdown conversion enabled' do + let(:crawl_config) do + Crawler::API::Config.new( + domains: [{ url: }], + output_sink: :mock, + results_collection: ResultsCollection.new, + markdown_conversion: { enabled: true, base_url: 'http://converter.test' } + ) + end + let(:converter) { crawl_config.markdown_converter } + let(:system_logger) { crawl_config.system_logger } + + before do + allow(converter).to receive(:convert!).and_return(:converted) + allow(system_logger).to receive(:info).and_call_original + allow(system_logger).to receive(:error).and_call_original + end + + context 'when the converter is healthy' do + before { allow(converter).to receive(:healthy?).and_return(true) } + + it 'checks health once, runs the crawl and logs the conversion stats' do + subject.start! + + expect(converter).to have_received(:healthy?).once + expect(subject.outcome).to eq(:success) + expect(system_logger).to have_received(:info).with('Markdown conversions: converted=0 failed=0') + end + end + + context 'when the converter is unhealthy' do + before { allow(converter).to receive(:healthy?).and_return(false) } + + it 'aborts the crawl with a clear failure outcome before crawling anything' do + expect(subject.coordinator).not_to receive(:run_crawl!) + + subject.start! + + expect(subject.outcome).to eq(:failure) + expect(subject.outcome_message).to eq( + 'Markdown converter at http://converter.test is not healthy (GET /api/v1/health did not return 200); ' \ + 'aborting the crawl so the index is not degraded to plain text' + ) + expect(system_logger).to have_received(:error).with(/is not healthy/) + expect(subject.config.event_logger.mock_events).to include( + hash_including('event.action' => 'crawl-end', 'event.outcome' => 'failure') + ) + end + end + end + it 'starts without error' do expect { subject.start! }.to_not raise_error expect(subject.config.event_logger.mock_events).to include( diff --git a/spec/lib/crawler/coordinator_spec.rb b/spec/lib/crawler/coordinator_spec.rb index fb2b2b9a..49a4e0b8 100644 --- a/spec/lib/crawler/coordinator_spec.rb +++ b/spec/lib/crawler/coordinator_spec.rb @@ -441,6 +441,113 @@ def process_crawl_result process_crawl_result end end + + context 'when markdown conversion is disabled (default)' do + it 'calls the converter but leaves the crawl result untouched and writes it to the sink' do + # crawl_result is a strict double: any message outside the ones declared above raises + expect(crawl_config.markdown_converter).to receive(:convert!).with(crawl_result).and_call_original + expect(crawl.sink).to receive(:write).with(crawl_result).and_call_original + process_crawl_result + end + end + + context 'when markdown conversion is enabled' do + let(:on_failure) { 'text' } + let(:conversion_result) { :converted } + let(:crawl_configuration) do + super().merge( + output_sink: :mock, + markdown_conversion: { enabled: true, base_url: 'http://converter.test', on_failure: } + ) + end + let(:converter) { crawl_config.markdown_converter } + + before do + allow(converter).to receive(:convert!).and_return(conversion_result) + end + + it 'converts the result exactly once before writing it to the sink' do + expect(converter).to receive(:convert!).with(crawl_result).once.ordered.and_return(:converted) + expect(crawl.sink).to receive(:write).with(crawl_result).ordered.and_call_original + process_crawl_result + end + + context 'when conversion fails and on_failure is text' do + let(:conversion_result) { :failed } + + it 'still writes the result and reports success' do + expect(crawl.sink).to receive(:write).with(crawl_result).and_call_original + expect(events).to receive(:url_extracted).with( + hash_including(url: crawl_result.url, outcome: :success, message: 'Successfully ingested crawl result') + ) + coordinator.send(:process_crawl_result, crawl_task, crawl_result) + end + end + + context 'when conversion fails and on_failure is skip' do + let(:on_failure) { 'skip' } + let(:conversion_result) { :failed } + let(:skip_message) do + "Markdown conversion failed for http://example.com and on_failure is 'skip'; document not ingested" + end + + it 'does not write the result and reports a failure outcome' do + expect(crawl.sink).not_to receive(:write) + expect(system_logger).to receive(:warn).with(skip_message) + expect(events).to receive(:url_extracted).with( + hash_including(url: crawl_result.url, type: :allowed, outcome: :failure, message: skip_message) + ) + coordinator.send(:process_crawl_result, crawl_task, crawl_result) + end + end + + context 'when conversion is skipped (unsupported MIME type) and on_failure is skip' do + let(:on_failure) { 'skip' } + let(:conversion_result) { :skipped } + + it 'writes the result normally' do + expect(crawl.sink).to receive(:write).with(crawl_result).and_call_original + process_crawl_result + end + end + + context 'when the crawl result is a redirect' do + let(:crawl_result) do + double( + :crawl_result, + url:, + redirect_chain: [], + location: Crawler::Data::URL.parse('http://example.com/redirected'), + error?: false, + redirect?: true + ) + end + + it 'does not call the converter' do + allow(coordinator).to receive(:add_urls_to_backlog) + expect(converter).not_to receive(:convert!) + process_crawl_result + end + end + + context 'when the rule engine denies the result' do + let(:rule_engine) do + double( + :rule_engine, + output_crawl_result_outcome: double( + :output_crawl_result_outcome, denied?: true, deny_reason: 'blocked', message: 'nope' + ), + discover_url_outcome: double(:discover_url_outcome, denied?: false) + ) + end + let(:crawl_result) { double(:crawl_result, url:, error?: true, redirect?: false) } + + it 'does not call the converter' do + expect(converter).not_to receive(:convert!) + process_crawl_result + end + end + end end #------------------------------------------------------------------------------------------------- From 58c933149b061e542061ef96f26b428a9c6efbb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 13:53:40 +0200 Subject: [PATCH 09/16] feat(markdown): map markdown into body with body_format and content_hash DocumentMapper now emits body_format and content_hash for both HTML and binary documents. When Success#markdown is present, it becomes the body (body_format: 'markdown'); binary docs then omit _attachment since the ingest attachment processor is unneeded. Otherwise behavior is unchanged (body_format: 'text', _attachment kept for binary docs). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- lib/crawler/document_mapper.rb | 30 ++++++++++++++++++--- spec/lib/crawler/document_mapper_spec.rb | 33 ++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/lib/crawler/document_mapper.rb b/lib/crawler/document_mapper.rb index 40c44a9b..224cb2d5 100644 --- a/lib/crawler/document_mapper.rb +++ b/lib/crawler/document_mapper.rb @@ -6,6 +6,8 @@ # frozen_string_literal: true +require 'digest' + module Crawler class DocumentMapper class UnsupportedCrawlResultError < StandardError; end @@ -68,7 +70,9 @@ def core_fields(crawl_result) def html_fields(crawl_result) # rubocop:disable Metrics/AbcSize remove_empty_values( title: crawl_result.document_title(limit: config.max_title_size), - body: crawl_result.document_body(limit: config.max_body_size, exclude_tags: config.exclude_tags), + body: html_body(crawl_result), + body_format: body_format(crawl_result), + content_hash: Digest::SHA1.hexdigest(crawl_result.content.to_s), # content may be nil (response.rb:47) meta_keywords: crawl_result.meta_keywords(limit: config.max_keywords_size), meta_description: crawl_result.meta_description(limit: config.max_description_size), links: crawl_result.links(limit: config.max_indexed_links_count), @@ -77,13 +81,31 @@ def html_fields(crawl_result) # rubocop:disable Metrics/AbcSize ) end + # With markdown present the ingest attachment processor is not needed, so `_attachment` is omitted def binary_file_fields(crawl_result) - remove_empty_values( + fields = { file_name: crawl_result.file_name, content_length: crawl_result.content_length, content_type: crawl_result.content_type, - _attachment: crawl_result.base64_encoded_content - ) + content_hash: crawl_result.content_hash, + body_format: body_format(crawl_result) + } + if crawl_result.markdown + fields[:body] = crawl_result.markdown + else + fields[:_attachment] = crawl_result.base64_encoded_content + end + remove_empty_values(fields) + end + + def html_body(crawl_result) + return crawl_result.markdown if crawl_result.markdown + + crawl_result.document_body(limit: config.max_body_size, exclude_tags: config.exclude_tags) + end + + def body_format(crawl_result) + crawl_result.markdown ? 'markdown' : 'text' end def url_components(url) diff --git a/spec/lib/crawler/document_mapper_spec.rb b/spec/lib/crawler/document_mapper_spec.rb index e8c40f13..ebb52827 100644 --- a/spec/lib/crawler/document_mapper_spec.rb +++ b/spec/lib/crawler/document_mapper_spec.rb @@ -55,6 +55,8 @@ last_crawled_at: crawl_result.start_time.rfc3339, title:, body: crawl_result.document_body, + body_format: 'text', + content_hash: Digest::SHA1.hexdigest(content), meta_keywords:, meta_description:, number_value_tag: meta_tag_number, @@ -76,6 +78,19 @@ expect(result).to eq(expected_result) end + context 'when the crawl result carries markdown' do + let(:markdown) { "# A website!\n\nChosen 1\n\n- Chosen 2" } + let(:expected_result_markdown) { expected_result.merge(body: markdown, body_format: 'markdown') } + + before { crawl_result.markdown = markdown } + + it 'uses the markdown as the body and marks the format' do + result = subject.create_doc(crawl_result) + + expect(result).to eq(expected_result_markdown) + end + end + context 'when extraction rules are present' do let(:config_params) do { @@ -183,6 +198,8 @@ file_name:, content_length:, content_type:, + content_hash: crawl_result.content_hash, + body_format: 'text', _attachment: crawl_result.base64_encoded_content, url: file_url.to_s, url_scheme: file_url.scheme, @@ -199,6 +216,22 @@ expect(result).to eq(expected_result) end + context 'when the crawl result carries markdown' do + let(:markdown) { '# A PDF for ants' } + let(:expected_result_markdown) do + expected_result.except(:_attachment).merge(body: markdown, body_format: 'markdown') + end + + before { crawl_result.markdown = markdown } + + it 'uses the markdown as the body and omits the attachment' do + result = subject.create_doc(crawl_result) + + expect(result).to eq(expected_result_markdown) + expect(result).not_to have_key(:_attachment) + end + end + context 'when extraction rules are present' do let(:config_params) do { From 709450d81e25757024ef33abfec510e2f18768cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 13:58:05 +0200 Subject: [PATCH 10/16] feat(markdown): keep markdown whitespace in the ES pipeline and print markdown on the console sink _reduce_whitespace now defaults to false when markdown_conversion is enabled, since the default ingest pipeline collapses whitespace in `body` and would destroy markdown formatting. An explicit _reduce_whitespace: true is still honoured, with a one-time warning. The console sink now prints crawl_result.markdown when present, instead of the raw HTML/binary-content placeholder. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- lib/crawler/output_sink/console.rb | 4 +- lib/crawler/output_sink/elasticsearch.rb | 24 ++++++++++- spec/lib/crawler/output_sink/console_spec.rb | 36 +++++++++++++++++ .../crawler/output_sink/elasticsearch_spec.rb | 40 +++++++++++++++++++ 4 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 spec/lib/crawler/output_sink/console_spec.rb diff --git a/lib/crawler/output_sink/console.rb b/lib/crawler/output_sink/console.rb index 879b8ac5..a7542559 100644 --- a/lib/crawler/output_sink/console.rb +++ b/lib/crawler/output_sink/console.rb @@ -14,7 +14,9 @@ class Console < OutputSink::Base def write(crawl_result) puts "# #{crawl_result.id}, #{crawl_result.url}, #{crawl_result.status_code}" - if crawl_result.content_extractable_file? + if crawl_result.respond_to?(:markdown) && crawl_result.markdown + puts crawl_result.markdown + elsif crawl_result.content_extractable_file? puts "** [Content extractable file (content type: #{crawl_result.content_type}, " \ "content length: #{crawl_result.content.bytesize})] **" else diff --git a/lib/crawler/output_sink/elasticsearch.rb b/lib/crawler/output_sink/elasticsearch.rb index 4c0501f7..6b421075 100644 --- a/lib/crawler/output_sink/elasticsearch.rb +++ b/lib/crawler/output_sink/elasticsearch.rb @@ -226,11 +226,33 @@ def pipeline_enabled? end def pipeline_params - @pipeline_params ||= DEFAULT_PIPELINE_PARAMS.merge(es_config[:pipeline_params] || {}) + @pipeline_params ||= default_pipeline_params.merge(es_config[:pipeline_params] || {}).tap do |params| + warn_about_whitespace_reduction(params) + end end private + def markdown_conversion_enabled? + config.markdown_conversion[:enabled] == true + end + + # The default ingest pipeline collapses whitespace in `body` (\s+ -> ' '), which destroys markdown + def default_pipeline_params + return DEFAULT_PIPELINE_PARAMS unless markdown_conversion_enabled? + + DEFAULT_PIPELINE_PARAMS.merge(_reduce_whitespace: false) + end + + def warn_about_whitespace_reduction(params) + return unless markdown_conversion_enabled? && params[:_reduce_whitespace] == true + + system_logger.warn( + 'elasticsearch.pipeline_params._reduce_whitespace is true while markdown_conversion is enabled; ' \ + 'the ingest pipeline will collapse whitespace and break markdown formatting in `body`' + ) + end + def assign_default_pipeline(version, build_flavor) # any 9.x or serverless project should use the v2 pipeline if version.split('.').first == '9' || build_flavor == 'serverless' diff --git a/spec/lib/crawler/output_sink/console_spec.rb b/spec/lib/crawler/output_sink/console_spec.rb new file mode 100644 index 00000000..20ec1c2c --- /dev/null +++ b/spec/lib/crawler/output_sink/console_spec.rb @@ -0,0 +1,36 @@ +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +RSpec.describe(Crawler::OutputSink::Console) do + let(:config) { Crawler::API::Config.new(domains: [{ url: 'http://example.com' }], output_sink: :console) } + let(:sink) { described_class.new(config) } + + it 'prints the raw content of HTML results' do + result = FactoryBot.build(:html_crawl_result, content: '

raw html

') + expect { sink.write(result) }.to output(%r{

raw html

}).to_stdout + end + + it 'prints a placeholder instead of binary content' do + result = FactoryBot.build(:content_extractable_file_crawl_result) + expect { sink.write(result) }.to output(%r{Content extractable file \(content type: application/pdf}).to_stdout + end + + it 'prints the markdown when the result has been converted' do + result = FactoryBot.build(:content_extractable_file_crawl_result) + result.markdown = '# From the converter' + expect { sink.write(result) }.to output(/# From the converter/).to_stdout + expect { sink.write(result) }.not_to output(/Content extractable file/).to_stdout + end + + it 'returns a success outcome' do + result = FactoryBot.build(:html_crawl_result) + outcome = nil + expect { outcome = sink.write(result) }.to output.to_stdout + expect(outcome).to eq(outcome: :success, message: 'Successfully ingested crawl result') + end +end diff --git a/spec/lib/crawler/output_sink/elasticsearch_spec.rb b/spec/lib/crawler/output_sink/elasticsearch_spec.rb index 261ade87..63f2bfcd 100644 --- a/spec/lib/crawler/output_sink/elasticsearch_spec.rb +++ b/spec/lib/crawler/output_sink/elasticsearch_spec.rb @@ -357,6 +357,46 @@ end end + context 'when markdown conversion is enabled' do + let(:pipeline_params) { {} } + let(:config) do + Crawler::API::Config.new( + domains:, + output_sink: 'elasticsearch', + output_index: index_name, + max_body_size: 500_000, + elasticsearch: { + host: 'http://localhost', + port: 1234, + api_key: 'key', + pipeline_params: + }, + markdown_conversion: { enabled: true, base_url: 'http://converter.test' } + ) + end + + it 'defaults _reduce_whitespace to false and keeps binary extraction on' do + expect(subject.pipeline_params).to eq( + _reduce_whitespace: false, + _run_ml_inference: true, + _extract_binary_content: true + ) + expect(system_logger).not_to have_received(:warn) + end + + context 'when _reduce_whitespace is explicitly true' do + let(:pipeline_params) { { _reduce_whitespace: true } } + + it 'honours the setting and warns that markdown will be mangled' do + expect(subject.pipeline_params[:_reduce_whitespace]).to be(true) + expect(system_logger).to have_received(:warn).with( + 'elasticsearch.pipeline_params._reduce_whitespace is true while markdown_conversion is enabled; ' \ + 'the ingest pipeline will collapse whitespace and break markdown formatting in `body`' + ).once + end + end + end + context 'when elasticsearch.pipeline_enabled is false' do let(:config) do Crawler::API::Config.new( From 596afd6c00d0fa838ad60243ad69adbb2a1ad6f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 14:04:57 +0200 Subject: [PATCH 11/16] test(markdown): add end-to-end crawl spec with a stubbed converter Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- spec/integration/markdown_conversion_spec.rb | 113 +++++++++++++++++++ spec/support/faux/faux_crawl.rb | 4 +- 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 spec/integration/markdown_conversion_spec.rb diff --git a/spec/integration/markdown_conversion_spec.rb b/spec/integration/markdown_conversion_spec.rb new file mode 100644 index 00000000..db089ecc --- /dev/null +++ b/spec/integration/markdown_conversion_spec.rb @@ -0,0 +1,113 @@ +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License 2.0; +# you may not use this file except in compliance with the Elastic License 2.0. +# + +# frozen_string_literal: true + +RSpec.describe 'Markdown conversion' do + let(:converter_url) { 'http://converter.test' } + let(:health_url) { "#{converter_url}/api/v1/health" } + let(:upload_url) { "#{converter_url}/api/v1/convert/upload?wait=10" } + let(:json_headers) { { 'Content-Type' => 'application/json' } } + let(:markdown_conversion) do + { enabled: true, base_url: converter_url, wait_seconds: 10, poll_interval: 0.01, timeout: 5 } + end + + let(:site) do + Faux.site do + page '/' do + body do + text { '

Home

' } + link_to '/pdf' + text { '
site footer
' } + end + end + + page '/pdf' do + headers 'Content-Type' => 'application/pdf' + end + end + end + + def done_json(markdown) + { job_id: 'job', status: 'done', status_url: '/api/v1/jobs/job', source_name: 'x', markdown: }.to_json + end + + def run_crawl + FauxCrawl.run( + site, + content_extraction: { enabled: true, mime_types: %w[application/pdf] }, + markdown_conversion: + ) + end + + # Same mapping every real sink performs through OutputSink::Base#to_doc + def docs_by_url(results) + mapper = results.crawl_config.document_mapper + results.each_with_object({}) { |result, docs| docs[result.url.to_s] = mapper.create_doc(result) } + end + + before do + stub_request(:get, health_url).to_return(status: 200, body: '{"status":"ok"}', headers: json_headers) + end + + context 'when the converter is healthy' do + before do + stub_request(:post, upload_url) + .with { |request| request.body.include?('.html"') } + .to_return(status: 200, body: done_json("# Home\n\nsite footer"), headers: json_headers) + stub_request(:post, upload_url) + .with { |request| request.body.include?('.pdf"') } + .to_return(status: 200, body: done_json('# PDF document'), headers: json_headers) + end + + it 'stores markdown bodies for the HTML page and the PDF' do + results = run_crawl + + expect(results).to have_only_these_results [ + mock_response(url: 'http://127.0.0.1:9393/', status_code: 200), + mock_response(url: 'http://127.0.0.1:9393/pdf', status_code: 200) + ] + + docs = docs_by_url(results) + expect(docs['http://127.0.0.1:9393/']).to include(body: "# Home\n\nsite footer", body_format: 'markdown') + expect(docs['http://127.0.0.1:9393/']).to have_key(:content_hash) + expect(docs['http://127.0.0.1:9393/pdf']).to include(body: '# PDF document', body_format: 'markdown') + expect(docs['http://127.0.0.1:9393/pdf']).not_to have_key(:_attachment) + expect(results.crawl_config.markdown_converter.stats).to eq(converted: 2, failed: 0) + expect(a_request(:get, health_url)).to have_been_made.once + end + end + + context 'when the converter keeps answering 5xx' do + before do + stub_request(:post, upload_url).to_return(status: 503, body: 'upstream unavailable') + end + + it 'falls back to text extraction and keeps the binary attachment' do + results = run_crawl + + docs = docs_by_url(results) + expect(docs['http://127.0.0.1:9393/']).to include(body_format: 'text') + expect(docs['http://127.0.0.1:9393/'][:body]).to include('Home') + expect(docs['http://127.0.0.1:9393/pdf']).to include(body_format: 'text') + expect(docs['http://127.0.0.1:9393/pdf']).to have_key(:_attachment) + # two documents, each tried twice (initial attempt + one retry) + expect(a_request(:post, upload_url)).to have_been_made.times(4) + expect(results.crawl_config.markdown_converter.stats).to eq(converted: 0, failed: 2) + end + end + + context 'when the converter health check fails' do + before do + stub_request(:get, health_url).to_return(status: 503) + end + + it 'aborts the crawl before fetching anything' do + expect { run_crawl }.to raise_error(RuntimeError, /Test Crawl failed!.*is not healthy/) + expect(a_request(:post, upload_url)).not_to have_been_made + end + end +end diff --git a/spec/support/faux/faux_crawl.rb b/spec/support/faux/faux_crawl.rb index 3726327d..603308d8 100644 --- a/spec/support/faux/faux_crawl.rb +++ b/spec/support/faux/faux_crawl.rb @@ -45,7 +45,7 @@ def self.crawl_site(&block) attr_reader :options, :sites, :site_containers, :timeouts, :content_extraction, :default_encoding, :crawl_id, :url_queue, :auth, :user_agent, :url, :seed_urls, :sitemap_urls, :domain_allowlist, :results, - :expect_success + :expect_success, :markdown_conversion delegate :crawl, to: :results @@ -62,6 +62,7 @@ def initialize(*sites) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength @sitemap_urls = coerce_to_absolute_urls(options[:sitemap_urls] || []) @domain_allowlist = seed_urls.map { |url| Crawler::Data::URL.parse(url).site } @content_extraction = options.fetch(:content_extraction, { enabled: false, mime_types: [] }) + @markdown_conversion = options[:markdown_conversion] @default_encoding = options[:default_encoding] @timeouts = options.fetch(:timeouts, {}).slice( :connect_timeout, :socket_timeout, :request_timeout @@ -173,6 +174,7 @@ def configure_crawl # rubocop:disable Metrics/AbcSize, Metrics/MethodLength } config.merge!(timeouts) config[:default_encoding] = default_encoding if default_encoding + config[:markdown_conversion] = markdown_conversion if markdown_conversion # Add crawl rules for Enterprise Search tests if enterprise_search? From 3a415dcc6ca1dc9e1eaeef7eb0b9465d07553f7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 14:10:03 +0200 Subject: [PATCH 12/16] docs(markdown): document the markdown_conversion feature and config block Adds config/crawler.yml.example markdown_conversion block and xlsx MIME type, docs/features/MARKDOWN_CONVERSION.md, and a README link, matching the paths already referenced by lib/crawler/api/config.rb and lib/crawler/markdown_converter.rb. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- README.md | 1 + config/crawler.yml.example | 20 ++++++ docs/features/MARKDOWN_CONVERSION.md | 102 +++++++++++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 docs/features/MARKDOWN_CONVERSION.md diff --git a/README.md b/README.md index de706758..3e130f45 100644 --- a/README.md +++ b/README.md @@ -239,6 +239,7 @@ GET /web-crawl-test/_search - [Crawl lifecycle](docs/ADVANCED.md#crawl-lifecycle) - Understand how the crawler discovers, queues, and indexes content across two stages: the primary crawl and the purge crawl - [Extraction rules](docs/features/EXTRACTION_RULES.md) - Define how crawler extracts content from HTML - [Binary content extraction](docs/features/BINARY_CONTENT_EXTRACTION.md) - Extract text from PDFs, DOCX files +- [Markdown conversion](docs/features/MARKDOWN_CONVERSION.md) - Index HTML pages and office documents as Markdown through the shared convert-markdown service - [Crawler directives](docs/features/CRAWLER_DIRECTIVES.md) - Use robots.txt, meta tags, or embedded data attributes to guide discovery and content extraction - [Scheduling](docs/features/SCHEDULING.md) - Automate crawls with cron scheduling - [Ingest pipelines](docs/features/INGEST_PIPELINES.md) - Elasticsearch ingest pipeline integration diff --git a/config/crawler.yml.example b/config/crawler.yml.example index 469b5a85..4cc71a10 100644 --- a/config/crawler.yml.example +++ b/config/crawler.yml.example @@ -209,6 +209,7 @@ # - application/pdf # - application/msword # - application/vnd.openxmlformats-officedocument.wordprocessingml.document +# - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet # - application/vnd.ms-powerpoint # - application/vnd.openxmlformats-officedocument.presentationml.presentation # @@ -266,6 +267,25 @@ # max_items: 10 # max_size_bytes: 1_048_576 +## ------------------------------- Markdown Conversion ------------------------- +# +## Convert crawled HTML pages and PDF/DOCX/XLSX/PPTX files to Markdown through the shared +## convert-markdown service before indexing. `body` then holds Markdown and `body_format` +## is set to `markdown`; documents that cannot be converted fall back to plain text +## (`body_format: text`). See docs/features/MARKDOWN_CONVERSION.md for details. +## Binary files still need `binary_content_extraction_enabled` and their MIME types listed above. +#markdown_conversion: +# enabled: false +# base_url: https://convert-markdown.ifad.org # required when enabled; http(s) only +# wait_seconds: 10 # 0..60, how long the service may hold the upload request before answering +# poll_interval: 2 # seconds between status polls; backs off x1.5 up to 5s +# timeout: 900 # hard per-document deadline (submit + polling), in seconds +# on_failure: text # text: index the plain-text fallback | skip: do not index the document +# ca_file: # optional PEM bundle used to verify the converter's TLS certificate +# +## NOTE: when enabled with the elasticsearch sink, `max_body_size` must be smaller than +## `elasticsearch.bulk_api.max_size_bytes` (default 1_048_576), e.g. `max_body_size: 900_000`. + ## The interval in seconds to wait before retrying to acquire the sink lock. #sink_lock_retry_interval: 1 diff --git a/docs/features/MARKDOWN_CONVERSION.md b/docs/features/MARKDOWN_CONVERSION.md new file mode 100644 index 00000000..f030e224 --- /dev/null +++ b/docs/features/MARKDOWN_CONVERSION.md @@ -0,0 +1,102 @@ +# Markdown Conversion + +The crawler can send every crawled HTML page and supported binary document (PDF, DOCX, XLSX, PPTX) to the shared +IFAD `convert-markdown` service and index the returned Markdown in the `body` field. Every IFAD application then +gets identical conversions, and Markdown keeps headings, lists and tables that plain-text extraction flattens. + +Plain-text extraction remains the fallback: if a document cannot be converted, the crawler indexes what it +would have indexed before this feature existed. + +## Using this feature + +1. Make sure the converter is reachable from the crawler (`GET {base_url}/api/v1/health` must answer 200). +2. Enable the feature in the crawler configuration: + +```yaml +markdown_conversion: + enabled: true + base_url: https://convert-markdown.ifad.org + wait_seconds: 10 + poll_interval: 2 + timeout: 900 + on_failure: text + # ca_file: /etc/ssl/certs/ifad-ca.pem + +# Binary files are only downloaded when binary content extraction is on +binary_content_extraction_enabled: true +binary_content_extraction_mime_types: + - application/pdf + - application/vnd.openxmlformats-officedocument.wordprocessingml.document + - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet + - application/vnd.openxmlformats-officedocument.presentationml.presentation + +# Required with the elasticsearch sink: a markdown body must fit into one bulk request +max_body_size: 900_000 +``` + +| Setting | Default | Meaning | +|---|---|---| +| `enabled` | `false` | Turn the feature on. | +| `base_url` | – | Converter base URL, `http` or `https`. Required when enabled. | +| `wait_seconds` | `10` | Sent as `?wait=` on upload (0..60). The service answers inline when the job finishes within this window. | +| `poll_interval` | `2` | Seconds between status polls; backs off ×1.5 per poll, capped at 5 s. | +| `timeout` | `900` | Hard per-document deadline in seconds (upload + polling). | +| `on_failure` | `text` | `text`: index the plain-text fallback. `skip`: do not index the document at all. | +| `ca_file` | – | PEM bundle to verify the converter's certificate (passed to `Net::HTTP#ca_file`). | + +At crawl start the crawler calls the health endpoint once. If it does not answer 200 the crawl aborts with +`Markdown converter at is not healthy ...` instead of silently indexing the whole site as plain text. + +## What gets indexed + +| Document | Converted | Not converted (fallback) | +|---|---|---| +| HTML page | `body`: Markdown, `body_format: markdown` | `body`: extracted text, `body_format: text` | +| PDF / DOCX / XLSX / PPTX | `body`: Markdown, `body_format: markdown`, no `_attachment` | `_attachment` kept (Tika via the ingest pipeline), `body_format: text` | +| Other binary types (e.g. legacy `.doc`, `.ppt`) | never sent to the converter | unchanged behaviour | + +Every document also gets `content_hash` (SHA-1 of the fetched bytes). `body_format` and `content_hash` are +reserved field names and cannot be overwritten by extraction rules. + +HTML is uploaded after the crawler's own pre-processing, so `exclude_tags` and the +`data-elastic-exclude` / `data-elastic-include` directives are honoured in the Markdown as well. + +With the Elasticsearch sink, the ingest pipeline parameter `_reduce_whitespace` defaults to `false` while this +feature is enabled, because the default pipeline collapses all whitespace in `body` and would destroy the +Markdown structure. Setting it to `true` explicitly is honoured but logs a warning. + +## Failure semantics + +| Situation | `on_failure: text` | `on_failure: skip` | +|---|---|---| +| Converter healthy, document converts | Markdown body | Markdown body | +| Unsupported MIME type | fallback (not counted as a failure) | fallback (not counted as a failure) | +| Converter error, timeout, or `status: failed` | fallback body, `body_format: text`, warning in the system log | document not written, `url-extracted` event with `outcome: failure`, warning in the system log | +| Converter unreachable at crawl start | crawl aborts | crawl aborts | + +Transient errors (connection errors, timeouts, HTTP 5xx, HTTP 404 while polling an expired job) are retried +once after one second. HTTP 422 (unsupported file), other 4xx responses and `status: failed` are not retried. + +The system log ends with `Markdown conversions: converted=N failed=N`. + +### `skip` and purge crawls + +With `purge_crawl_enabled: true` (the default) the purge stage deletes every document whose `last_crawled_at` +is older than the crawl start. A page whose conversion failed under `on_failure: skip` is not re-indexed, so its +`last_crawled_at` stays stale and the purge crawl deletes it from the index. Use `skip` only when a missing +document is preferable to a plain-text one. + +## Throughput and deployment caveats + +- The converter is single-worker with an in-process job store: conversions are serialised. Each crawl thread + waits for its own document, so `threads_per_crawl` bounds the number of in-flight conversions; large PDFs + block one crawl thread for the whole conversion (up to `timeout`). +- Because the job store is in-process, polling must reach the same replica that accepted the upload. Behind a + load balancer without session affinity, polls can hit another replica and answer 404; the crawler resubmits + once, then falls back. Run a single replica or enable affinity routing. +- The service front-end may enforce its own per-request timeout (Envoy defaults to 15 s); keep `wait_seconds` + below it so long conversions go through the polling path instead of failing at the proxy. +- A conversion in progress ignores a crawl shutdown request: the task thread finishes (or times out) its + current upload/poll cycle first, so a slow job can hold a task thread for up to `timeout` seconds. +- Converter calls go straight to `base_url`: they use neither the crawler's `http_proxy_*` settings nor the + `HTTP_PROXY`/`HTTPS_PROXY` environment variables. From 5824e69638c7181217918bf53303a24e98bf91a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 14:32:33 +0200 Subject: [PATCH 13/16] feat(markdown): add converter circuit breaker and health-check retry A converter that dies mid-crawl used to cost every remaining document a full upload, a retry and possibly the whole per-document timeout before falling back to plain text. MarkdownConverter now counts consecutive failures and opens a circuit breaker after 20 of them: for the next 60 seconds convert! returns :failed without any HTTP call, so on_failure keeps deciding as before (text -> plain-text body, skip -> not indexed) and the failures still show up in the final stats line. Exactly one thread re-probes /health after the cooldown; success closes the breaker, failure re-arms it. A successful conversion resets the counter. healthy? now retries once after RETRY_DELAY so a single blip neither aborts a crawl nor keeps the breaker open, and the per-document retry announcement drops from warn to debug: only the final failure of a document deserves a warning. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- lib/crawler/markdown_converter.rb | 107 ++++++++++++++++--- spec/lib/crawler/markdown_converter_spec.rb | 109 ++++++++++++++++++-- 2 files changed, 198 insertions(+), 18 deletions(-) diff --git a/lib/crawler/markdown_converter.rb b/lib/crawler/markdown_converter.rb index 76baefa6..f63666c9 100644 --- a/lib/crawler/markdown_converter.rb +++ b/lib/crawler/markdown_converter.rb @@ -42,6 +42,9 @@ class ConversionError < StandardError; end POLL_BACKOFF = 1.5 MAX_POLL_INTERVAL = 5 RETRYABLE_ERRORS = [SystemCallError, SocketError, EOFError, Net::OpenTimeout, Net::ReadTimeout].freeze + # Consecutive failures that open the circuit breaker, and how long it stays open before re-probing + CIRCUIT_BREAKER_THRESHOLD = 20 + CIRCUIT_BREAKER_COOLDOWN = 60 attr_reader :config, :settings @@ -52,6 +55,10 @@ def initialize(config) @settings = config.markdown_conversion @converted = Concurrent::AtomicFixnum.new(0) @failed = Concurrent::AtomicFixnum.new(0) + @consecutive_failures = Concurrent::AtomicFixnum.new(0) + @circuit_open = false + @circuit_retry_at = 0.0 + @circuit_mutex = Mutex.new end def enabled? @@ -96,29 +103,29 @@ def binary_payload(crawl_result) # Returns :converted | :skipped | :failed and never raises. # On success sets crawl_result.markdown (limited to config.max_body_size bytes). + # While the circuit breaker is open no HTTP call is made at all: the result is :failed, so + # on_failure decides as usual (text -> plain-text body, skip -> document not ingested). def convert!(crawl_result) return :skipped unless convertible?(crawl_result) + return fail_fast_on_open_circuit if circuit_open? markdown = with_one_retry(crawl_result) { run_conversion(crawl_result) } crawl_result.markdown = Crawler::ContentEngine::Utils.limit_bytesize(markdown, config.max_body_size) @converted.increment + @consecutive_failures.value = 0 :converted rescue StandardError => e - @failed.increment system_logger.warn("Markdown conversion failed for #{crawl_result.url}: #{e.message}") - :failed + record_conversion_failure end - # GET /api/v1/health with a short timeout; called once at crawl start + # GET /api/v1/health with a short timeout; called at crawl start and when re-probing the circuit + # breaker. Retried once after RETRY_DELAY so a single blip does not abort a whole crawl. def healthy? - uri = URI.parse("#{settings[:base_url]}#{HEALTH_PATH}") - request = Net::HTTP::Get.new(uri.request_uri) - request['User-Agent'] = config.user_agent - response = http_client(uri, open_timeout: HEALTH_TIMEOUT, read_timeout: HEALTH_TIMEOUT).request(request) - response.code.to_i == 200 - rescue StandardError => e - system_logger.warn("Markdown converter health check failed (#{settings[:base_url]}): #{e.class}: #{e.message}") - false + return true if health_check_ok? + + sleep(RETRY_DELAY) + health_check_ok? end # Hand-built multipart/form-data body. Everything is appended as ASCII-8BIT so binary payloads @@ -138,6 +145,82 @@ def multipart_body(crawl_result) private + def health_check_ok? + uri = URI.parse("#{settings[:base_url]}#{HEALTH_PATH}") + request = Net::HTTP::Get.new(uri.request_uri) + request['User-Agent'] = config.user_agent + response = http_client(uri, open_timeout: HEALTH_TIMEOUT, read_timeout: HEALTH_TIMEOUT).request(request) + response.code.to_i == 200 + rescue StandardError => e + system_logger.warn("Markdown converter health check failed (#{settings[:base_url]}): #{e.class}: #{e.message}") + false + end + + # Counted like any other failure, but without the per-document warning: the breaker already said it once. + def fail_fast_on_open_circuit + @failed.increment + :failed + end + + def record_conversion_failure + @failed.increment + open_circuit! if @consecutive_failures.increment >= CIRCUIT_BREAKER_THRESHOLD + :failed + end + + # A converter outage would otherwise cost every document its full retry and timeout budget. After + # CIRCUIT_BREAKER_THRESHOLD consecutive failures conversions are short-circuited for + # CIRCUIT_BREAKER_COOLDOWN seconds, then a single thread re-probes /health. + def circuit_open? + return false unless @circuit_open + return true if monotonic_now < @circuit_retry_at + + # try_lock: only one thread probes, the others keep short-circuiting instead of queueing behind it + with_circuit_lock(default: true) { probe_circuit } + end + + def with_circuit_lock(default:) + return default unless @circuit_mutex.try_lock + + begin + yield + ensure + @circuit_mutex.unlock + end + end + + # Returns whether the breaker is still open. Called with the circuit mutex held. + def probe_circuit + return false unless @circuit_open + return true if monotonic_now < @circuit_retry_at + return close_circuit! if healthy? + + @circuit_retry_at = monotonic_now + CIRCUIT_BREAKER_COOLDOWN + true + end + + def close_circuit! + @circuit_open = false + @consecutive_failures.value = 0 + system_logger.info('Markdown converter is healthy again, circuit breaker closed') + false + end + + def open_circuit! + with_circuit_lock(default: nil) { open_circuit_now! } + end + + def open_circuit_now! + return if @circuit_open + + @circuit_retry_at = monotonic_now + CIRCUIT_BREAKER_COOLDOWN + @circuit_open = true + system_logger.error( + "Markdown converter circuit breaker opened after #{@consecutive_failures.value} consecutive failures; " \ + "skipping conversions for #{CIRCUIT_BREAKER_COOLDOWN}s" + ) + end + def extension_for(crawl_result) return HTML_EXTENSION if crawl_result.html? @@ -169,7 +252,7 @@ def with_one_retry(crawl_result) rescue RetryableError => e raise e if attempts > 1 - system_logger.warn( + system_logger.debug( "Markdown conversion for #{crawl_result.url} failed (#{e.message}), retrying once in #{RETRY_DELAY}s" ) sleep(RETRY_DELAY) diff --git a/spec/lib/crawler/markdown_converter_spec.rb b/spec/lib/crawler/markdown_converter_spec.rb index e637ebae..96fd3ae9 100644 --- a/spec/lib/crawler/markdown_converter_spec.rb +++ b/spec/lib/crawler/markdown_converter_spec.rb @@ -286,7 +286,7 @@ def pending_response expect(converter.convert!(html_result)).to eq(:failed) expect(html_result.markdown).to be_nil expect(stub).to have_been_requested.twice - # the retry line reads "Markdown conversion for failed (...)", so only the final failure matches + # the retry announcement is logged at debug level, so warn only ever carries the final failure expect(config.system_logger).to have_received(:warn) .with(/Markdown conversion failed for .*HTTP 503 from converter/) end @@ -423,6 +423,91 @@ def pending_response end end + describe 'the circuit breaker' do + let(:health_url) { "#{base_url}/api/v1/health" } + let(:threshold) { described_class::CIRCUIT_BREAKER_THRESHOLD } + # A settable monotonic clock so the 60s cooldown can be crossed without sleeping + let(:clock) { [0.0] } + + before do + allow(converter).to receive(:monotonic_now) { clock.first } + allow(config.system_logger).to receive(:error) + allow(config.system_logger).to receive(:info) + end + + def trip_breaker! + stub_request(:post, upload_url).to_return(status: 422, body: 'nope') + threshold.times { converter.convert!(html_result) } + end + + it 'opens after the threshold and then fails without any HTTP call' do + trip_breaker! + + expect(a_request(:post, upload_url)).to have_been_made.times(threshold) + expect(config.system_logger).to have_received(:error).with( + "Markdown converter circuit breaker opened after #{threshold} consecutive failures; " \ + 'skipping conversions for 60s' + ).once + + expect(converter.convert!(html_result)).to eq(:failed) + expect(converter.convert!(pdf_result)).to eq(:failed) + expect(html_result.markdown).to be_nil + expect(pdf_result.markdown).to be_nil + expect(a_request(:post, upload_url)).to have_been_made.times(threshold) + expect(converter.stats).to eq(converted: 0, failed: threshold + 2) + end + + it 're-probes after the cooldown and closes the breaker when the converter is healthy' do + trip_breaker! + health = stub_request(:get, health_url).to_return(status: 200) + stub_request(:post, upload_url).to_return(json_response('done', markdown: '# Back')) + + clock[0] += described_class::CIRCUIT_BREAKER_COOLDOWN + 1 + expect(converter.convert!(html_result)).to eq(:converted) + expect(html_result.markdown).to eq('# Back') + expect(health).to have_been_requested.once + expect(config.system_logger).to have_received(:info).with(/circuit breaker closed/) + end + + it 'stays open and re-arms the cooldown when the probe fails' do + trip_breaker! + health = stub_request(:get, health_url).to_return(status: 503) + + clock[0] += described_class::CIRCUIT_BREAKER_COOLDOWN + 1 + expect(converter.convert!(html_result)).to eq(:failed) + expect(health).to have_been_requested.twice # the health check itself retries once + + expect(converter.convert!(html_result)).to eq(:failed) + expect(health).to have_been_requested.twice # cooldown re-armed, so no new probe + expect(a_request(:post, upload_url)).to have_been_made.times(threshold) + end + + it 'resets the consecutive failure count after a successful conversion' do + stub_request(:post, upload_url).to_return(status: 422, body: 'nope') + (threshold - 1).times { converter.convert!(html_result) } + stub_request(:post, upload_url).to_return(json_response('done', markdown: '# Ok')) + expect(converter.convert!(html_result)).to eq(:converted) + + stub_request(:post, upload_url).to_return(status: 422, body: 'nope') + (threshold - 1).times { converter.convert!(html_result) } + + expect(a_request(:post, upload_url)).to have_been_made.times((threshold * 2) - 1) + expect(config.system_logger).not_to have_received(:error) + end + + context 'when disabled' do + let(:markdown_settings) { { enabled: false } } + + it 'never opens' do + strict_result = double(:crawl_result) + + (threshold + 5).times { expect(converter.convert!(strict_result)).to eq(:skipped) } + expect(converter.stats).to eq(converted: 0, failed: 0) + expect(config.system_logger).not_to have_received(:error) + end + end + end + it 'counts conversions and failures across calls' do stub_request(:post, upload_url).to_return( json_response('done', markdown: '# One'), @@ -461,7 +546,10 @@ def pending_response describe '#healthy?' do let(:health_url) { "#{base_url}/api/v1/health" } - before { allow(config.system_logger).to receive(:warn) } + before do + allow(config.system_logger).to receive(:warn) + allow(converter).to receive(:sleep) + end it 'is true when the health endpoint returns 200 and sends the crawler user agent' do stub = stub_request(:get, health_url) @@ -471,20 +559,29 @@ def pending_response expect(stub).to have_been_requested.once end - it 'is false on a non-200 response' do - stub_request(:get, health_url).to_return(status: 503) + it 'is false after two non-200 responses' do + stub = stub_request(:get, health_url).to_return(status: 503) expect(converter.healthy?).to be(false) + expect(stub).to have_been_requested.twice + end + + it 'retries once after a second and succeeds when the second attempt returns 200' do + stub = stub_request(:get, health_url).to_return({ status: 503 }, { status: 200 }) + expect(converter.healthy?).to be(true) + expect(stub).to have_been_requested.twice + expect(converter).to have_received(:sleep).with(1).once end it 'is false and warns when the service is unreachable' do stub_request(:get, health_url).to_raise(Errno::ECONNREFUSED) expect(converter.healthy?).to be(false) - expect(config.system_logger).to have_received(:warn).with(/health check failed.*ECONNREFUSED/) + expect(config.system_logger).to have_received(:warn).with(/health check failed.*ECONNREFUSED/).twice end it 'is false on a timeout' do - stub_request(:get, health_url).to_timeout + stub = stub_request(:get, health_url).to_timeout expect(converter.healthy?).to be(false) + expect(stub).to have_been_requested.twice end it 'uses a 5 second timeout' do From 6d71fc396504ae558d0f36b1b343abd616efd29f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 14:32:49 +0200 Subject: [PATCH 14/16] fix(markdown): harden converter parsing, logging and config redaction - parse_job: a valid JSON body that is not an object (`null`, an array) reached job['status'] and blew up with a NoMethodError inside the generic rescue; it now fails as a plain ConversionError. - raise_on_http_error!: the error body is remote input, so inspect it before interpolating - a body with newlines could otherwise forge entries in the system log. - html_payload: exclude_tags may remove the element itself, which made Transformer.transform! raise on a nil tag; the remaining document is now uploaded as-is. - require_dependency the ContentEngine modules the converter uses instead of relying on another file having loaded them first. - Config::SENSITIVE_FIELDS gains markdown_conversion, so a converter base_url (which can carry credentials) is redacted from Config#to_s. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- lib/crawler/api/config.rb | 1 + lib/crawler/markdown_converter.rb | 15 +++++++++--- spec/lib/crawler/api/config_spec.rb | 14 +++++++++++ spec/lib/crawler/markdown_converter_spec.rb | 26 +++++++++++++++++++++ 4 files changed, 53 insertions(+), 3 deletions(-) diff --git a/lib/crawler/api/config.rb b/lib/crawler/api/config.rb index e04f582f..824878df 100644 --- a/lib/crawler/api/config.rb +++ b/lib/crawler/api/config.rb @@ -237,6 +237,7 @@ class Config # rubocop:disable Metrics/ClassLength http_proxy_username http_proxy_password elasticsearch + markdown_conversion ].freeze # Specific processed configuration options diff --git a/lib/crawler/markdown_converter.rb b/lib/crawler/markdown_converter.rb index f63666c9..1652c38e 100644 --- a/lib/crawler/markdown_converter.rb +++ b/lib/crawler/markdown_converter.rb @@ -12,6 +12,9 @@ require 'securerandom' require 'uri' +require_dependency File.join(__dir__, 'content_engine', 'transformer') +require_dependency File.join(__dir__, 'content_engine', 'utils') + module Crawler # Converts crawled HTML pages and binary documents (PDF, DOCX, XLSX, PPTX) to Markdown # through the shared convert-markdown service. See docs/features/MARKDOWN_CONVERSION.md. @@ -92,7 +95,9 @@ def upload_filename(crawl_result) # The memoised documents are also used for link extraction, so work on a deep clone. def html_payload(crawl_result) doc = source_document(crawl_result).clone - Crawler::ContentEngine::Transformer.transform!(doc.body) + # exclude_tags may have removed the element itself; upload the remaining document as-is + body = doc.body + Crawler::ContentEngine::Transformer.transform!(body) if body doc.charset(java.nio.charset.StandardCharsets::UTF_8) String.from_java_bytes(doc.outerHtml.to_java_bytes) end @@ -349,7 +354,10 @@ def http_client(uri, open_timeout:, read_timeout:) # Branch on the JSON status, not the HTTP code (200 and 202 both carry a job document) def parse_job(response, polling: false) raise_on_http_error!(response, polling) - JSON.parse(response.body) + job = JSON.parse(response.body) + raise ConversionError, 'converter returned a non-object JSON body' unless job.is_a?(Hash) + + job rescue JSON::ParserError => e raise ConversionError, "invalid JSON from converter: #{e.message}" end @@ -360,7 +368,8 @@ def raise_on_http_error!(response, polling) raise RetryableError, 'HTTP 404 from converter while polling (job expired)' if polling && code == 404 return if [200, 202].include?(code) - raise ConversionError, "HTTP #{code} from converter: #{response.body.to_s[0, 200]}" + # inspected: the body is remote input and must not be able to forge newlines into the system log + raise ConversionError, "HTTP #{code} from converter: #{response.body.to_s[0, 200].inspect}" end def finished?(job) diff --git a/spec/lib/crawler/api/config_spec.rb b/spec/lib/crawler/api/config_spec.rb index 7acd3673..c4bae68c 100644 --- a/spec/lib/crawler/api/config_spec.rb +++ b/spec/lib/crawler/api/config_spec.rb @@ -557,4 +557,18 @@ def expect_x509_certificates(certs) end end end + + describe '#to_s' do + it 'redacts sensitive settings, including the markdown converter block' do + config = Crawler::API::Config.new( + domains: [{ url: 'https://example.com' }], + output_sink: :console, + markdown_conversion: { enabled: true, base_url: 'https://converter.example.com' } + ) + + expect(config.to_s).to include('elasticsearch=[redacted]') + expect(config.to_s).to include('markdown_conversion=[redacted]') + expect(config.to_s).not_to include('converter.example.com') + end + end end diff --git a/spec/lib/crawler/markdown_converter_spec.rb b/spec/lib/crawler/markdown_converter_spec.rb index 96fd3ae9..8521e9e5 100644 --- a/spec/lib/crawler/markdown_converter_spec.rb +++ b/spec/lib/crawler/markdown_converter_spec.rb @@ -130,6 +130,17 @@ def file_result(content_type) expect(payload).not_to include('secret'.b) end end + + context 'when the exclusions removed the body element' do + let(:exclude_tags) { %w[body] } + + it 'still serialises the document instead of raising' do + payload = converter.html_payload(html_result) + expect(payload).to be_a(String) + expect(payload).to include(''.b) + expect(payload).not_to include('Héllo'.b) + end + end end describe '#binary_payload' do @@ -383,6 +394,21 @@ def pending_response expect(config.system_logger).to have_received(:warn).with(/invalid JSON from converter/) end + it 'fails when the converter returns a valid JSON body that is not an object' do + stub_request(:post, upload_url).to_return(status: 200, body: 'null', headers: json_headers) + + expect(converter.convert!(html_result)).to eq(:failed) + expect(html_result.markdown).to be_nil + expect(config.system_logger).to have_received(:warn).with(/converter returned a non-object JSON body/) + end + + it 'escapes the error body so it cannot forge log lines' do + stub_request(:post, upload_url).to_return(status: 400, body: "oops\nWARN: fake log line") + + expect(converter.convert!(html_result)).to eq(:failed) + expect(config.system_logger).to have_received(:warn).with(/HTTP 400 from converter: "oops\\nWARN: fake log line"/) + end + context 'with a small max_body_size' do let(:config) do Crawler::API::Config.new( From e122fe4664dd74c7dc45e6ceb00caf2bcb9f6ee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 14:32:49 +0200 Subject: [PATCH 15/16] refactor(markdown): tidy crawl stats logging and coordinator helper log_markdown_conversion_stats shadowed the delegated `stats` method with a local; rename it. The line is now also printed on the resumable shutdown path (so the documented "log ends with Markdown conversions:" holds) and suppressed when the crawl aborted on the start-up health check, where there is nothing to report. start_url_test! runs verify_markdown_converter! too: a URL test used to report plain-text bodies without ever asking the converter, and it also warms the markdown_converter memo on the main thread. The duplicated crawl-start event and unexpected-error handling move into helpers to keep both entry points inside the metrics limits. Also assign the converter once in Coordinator#convert_and_output_crawl_result and fix the stale comment claiming Jsoup's .clone is shallow - it is a deep copy, which is what MarkdownConverter#html_payload relies on. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- lib/crawler/api/crawl.rb | 52 ++++++++++++++++----------- lib/crawler/coordinator.rb | 7 ++-- lib/crawler/data/crawl_result/html.rb | 4 +-- spec/lib/crawler/api/crawl_spec.rb | 26 ++++++++++++++ 4 files changed, 63 insertions(+), 26 deletions(-) diff --git a/lib/crawler/api/crawl.rb b/lib/crawler/api/crawl.rb index c9c8d461..c2f25a76 100644 --- a/lib/crawler/api/crawl.rb +++ b/lib/crawler/api/crawl.rb @@ -44,6 +44,9 @@ def initialize(config) # When set to +true+, the shutdown process will stop gracefully while preserving # the state of the crawl, which should allow us to resume the crawl later as needed. @allow_resume = false + + # Set once the crawl got past its pre-flight checks; the final stats are only worth printing then. + @crawl_started = false end delegate :system_logger, :events, :stats, to: :config @@ -83,11 +86,9 @@ def coordinator # Starts a new crawl described by the given config. The job is started immediately. def start! # rubocop:disable Metrics/AbcSize, Metrics/MethodLength - events.crawl_start( - url_queue_items: crawl_queue.length, - seen_urls: seen_urls.count - ) + emit_crawl_start_event verify_markdown_converter! + @crawl_started = true ingestion_stats = coordinator.run_crawl! record_overall_outcome(coordinator.crawl_results) @@ -95,11 +96,7 @@ def start! # rubocop:disable Metrics/AbcSize, Metrics/MethodLength system_logger.error(e.message) record_outcome(outcome: :failure, message: e.message) rescue StandardError => e - log_exception(e, 'Unexpected error while running the crawl') - record_outcome( - outcome: :failure, - message: 'Unexpected error while running the crawl, check system logs for details' - ) + fail_crawl_on_unexpected_error(e) ensure # Execute hooks to either save the state or clean up after the crawl. # The actual cleanup and persistence implementation depends on specific UrlQueue and SeenUrls classes @@ -107,6 +104,7 @@ def start! # rubocop:disable Metrics/AbcSize, Metrics/MethodLength system_logger.info('Not removing the crawl queue to allow the crawl to resume later') crawl_queue.save seen_urls.save + log_markdown_conversion_stats else system_logger.info('Releasing resources used by the crawl...') crawl_queue.delete @@ -119,21 +117,19 @@ def start! # rubocop:disable Metrics/AbcSize, Metrics/MethodLength # Starts a crawl of a single URL, specifically for the UrlTest CLI command def start_url_test!(endpoint) # rubocop:disable Metrics/AbcSize - events.crawl_start( - url_queue_items: crawl_queue.length, - seen_urls: seen_urls.count - ) + emit_crawl_start_event # Use the File sink regardless of what is set in the config @sink = Crawler::OutputSink::File.new(config) + verify_markdown_converter! + @crawl_started = true coordinator.run_urltest_crawl!(endpoint) record_overall_outcome(coordinator.crawl_results) print_url_test_results(coordinator.url_test_results) + rescue Errors::MarkdownConverterUnavailableError => e + system_logger.error(e.message) + record_outcome(outcome: :failure, message: e.message) rescue StandardError => e - log_exception(e, 'Unexpected error while running the crawl') - record_outcome( - outcome: :failure, - message: 'Unexpected error while running the crawl, check system logs for details' - ) + fail_crawl_on_unexpected_error(e) ensure crawl_queue.delete seen_urls.clear @@ -244,6 +240,17 @@ def print_final_crawl_status # rubocop:disable Metrics/AbcSize puts "- Average response time (seconds): #{crawl_status[:avg_response_time_msec] / 1000}" end + def emit_crawl_start_event + events.crawl_start(url_queue_items: crawl_queue.length, seen_urls: seen_urls.count) + end + + # Both entry points bail out the same way, so the message stays identical for crawl and urltest + def fail_crawl_on_unexpected_error(exception) + log_exception(exception, 'Unexpected error while running the crawl') + message = 'Unexpected error while running the crawl, check system logs for details' + record_outcome(outcome: :failure, message:) + end + # Fail fast when the converter is down: a silent outage would degrade the whole index to plain text. # Calling config.markdown_converter here also warms the `@markdown_converter ||=` memo on the main # thread before the task pool starts (the memo is not thread-safe); keep that call unconditional so a @@ -260,11 +267,14 @@ def verify_markdown_converter! MSG end + # Only after the crawl actually started: a crawl aborted by verify_markdown_converter! has nothing + # to report. The local is not called `stats` because that name is delegated to the config. def log_markdown_conversion_stats - return unless config.markdown_converter.enabled? + return unless @crawl_started && config.markdown_converter.enabled? - stats = config.markdown_converter.stats - system_logger.info("Markdown conversions: converted=#{stats[:converted]} failed=#{stats[:failed]}") + conversion_stats = config.markdown_converter.stats + counts = "converted=#{conversion_stats[:converted]} failed=#{conversion_stats[:failed]}" + system_logger.info("Markdown conversions: #{counts}") end def print_extracted_links(result) diff --git a/lib/crawler/coordinator.rb b/lib/crawler/coordinator.rb index 7a0fe5b7..0cd1a138 100644 --- a/lib/crawler/coordinator.rb +++ b/lib/crawler/coordinator.rb @@ -528,10 +528,11 @@ def extract_links(crawl_result, crawl_depth:) # Runs on the crawl-task thread, before the sink, because the Elasticsearch sink holds its queue lock # while mapping documents and a slow conversion there would serialise every thread. def convert_and_output_crawl_result(crawl_task, crawl_result) - crawl_task_progress(crawl_task, 'converting to markdown') if config.markdown_converter.enabled? - conversion = config.markdown_converter.convert!(crawl_result) + converter = config.markdown_converter + crawl_task_progress(crawl_task, 'converting to markdown') if converter.enabled? + conversion = converter.convert!(crawl_result) - if conversion == :failed && config.markdown_converter.skip_on_failure? + if conversion == :failed && converter.skip_on_failure? message = "Markdown conversion failed for #{crawl_result.url} and on_failure is 'skip'; document not ingested" system_logger.warn(message) return sink.failure(message) diff --git a/lib/crawler/data/crawl_result/html.rb b/lib/crawler/data/crawl_result/html.rb index 0fe062c8..005d7d71 100644 --- a/lib/crawler/data/crawl_result/html.rb +++ b/lib/crawler/data/crawl_result/html.rb @@ -30,8 +30,8 @@ def parsed_content_excluding_tags(exclude_tags) # we don't cache per different exclude_tags list because we use # the same exclude_tags per CrawlResult, i.e. for every call @parsed_content_excluding_tags ||= begin - # parsing a fresh copy rather than using parsed_content, as it's a mutable object - # and .clone only does a shallow copy + # parsing a fresh copy rather than using parsed_content, as it's a mutable object that + # callers keep using (Jsoup's .clone is a deep copy, so cloning it would work as well) doc = Jsoup.parse(content) selector = exclude_tags.join(', ') doc.select(selector).remove diff --git a/spec/lib/crawler/api/crawl_spec.rb b/spec/lib/crawler/api/crawl_spec.rb index 4c15f69d..68324170 100644 --- a/spec/lib/crawler/api/crawl_spec.rb +++ b/spec/lib/crawler/api/crawl_spec.rb @@ -110,6 +110,21 @@ expect(subject.outcome).to eq(:success) expect(system_logger).to have_received(:info).with('Markdown conversions: converted=0 failed=0') end + + it 'logs the conversion stats on the resumable shutdown path as well' do + allow(subject).to receive(:allow_resume?).and_return(true) + + subject.start! + + expect(system_logger).to have_received(:info).with('Markdown conversions: converted=0 failed=0') + end + + it 'checks health before a url test crawl as well' do + subject.start_url_test!("#{url}/website") + + expect(converter).to have_received(:healthy?).once + expect(subject.outcome).to eq(:success) + end end context 'when the converter is unhealthy' do @@ -126,10 +141,21 @@ 'aborting the crawl so the index is not degraded to plain text' ) expect(system_logger).to have_received(:error).with(/is not healthy/) + expect(system_logger).not_to have_received(:info).with(/Markdown conversions:/) expect(subject.config.event_logger.mock_events).to include( hash_including('event.action' => 'crawl-end', 'event.outcome' => 'failure') ) end + + it 'aborts a url test crawl the same way' do + expect(subject.coordinator).not_to receive(:run_urltest_crawl!) + + subject.start_url_test!("#{url}/website") + + expect(subject.outcome).to eq(:failure) + expect(subject.outcome_message).to match(/is not healthy/) + expect(system_logger).to have_received(:error).with(/is not healthy/) + end end end From 6d7ade85f3c39c277999fb6ba56411dcd1fd301e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lle=C3=AFr=20Borr=C3=A0s=20Metje?= Date: Fri, 4 Sep 2026 14:32:49 +0200 Subject: [PATCH 16/16] docs(markdown): document disabled-mode fields and circuit breaker Explain that body_format and content_hash are written to every document even when the feature is disabled, and that both names stay reserved for extraction rules, so the index shape never depends on the converter and a later re-crawl can skip unchanged content. Document the circuit breaker, the health check running for both crawl and urltest, and the retry announcement now being a debug line. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XMpa3bomxH6WW3GBBfmoVZ --- docs/features/MARKDOWN_CONVERSION.md | 45 ++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/docs/features/MARKDOWN_CONVERSION.md b/docs/features/MARKDOWN_CONVERSION.md index f030e224..6dcf1c04 100644 --- a/docs/features/MARKDOWN_CONVERSION.md +++ b/docs/features/MARKDOWN_CONVERSION.md @@ -44,8 +44,11 @@ max_body_size: 900_000 | `on_failure` | `text` | `text`: index the plain-text fallback. `skip`: do not index the document at all. | | `ca_file` | – | PEM bundle to verify the converter's certificate (passed to `Net::HTTP#ca_file`). | -At crawl start the crawler calls the health endpoint once. If it does not answer 200 the crawl aborts with +At crawl start the crawler calls the health endpoint once (twice if the first call fails: the check is retried +once after a second). If it still does not answer 200 the crawl aborts with `Markdown converter at is not healthy ...` instead of silently indexing the whole site as plain text. +This applies to both `bin/crawler crawl` and `bin/crawler urltest`, so a URL test never reports a plain-text +body while pretending the converter was consulted. ## What gets indexed @@ -58,6 +61,19 @@ At crawl start the crawler calls the health endpoint once. If it does not answer Every document also gets `content_hash` (SHA-1 of the fetched bytes). `body_format` and `content_hash` are reserved field names and cannot be overwritten by extraction rules. +### What changes even when the feature is disabled + +Two fields are added to every indexed document regardless of `markdown_conversion.enabled`, for HTML pages and +binary documents alike: + +- `body_format` — `text` while the feature is off (and for any document that was not converted). +- `content_hash` — SHA-1 of the fetched bytes. + +Both names are reserved for extraction rules in every crawl, enabled or not. This is deliberate: the fields are +written unconditionally so the shape of an index never depends on whether the converter happened to be on, and +so a later re-crawl can compare `content_hash` and skip content that has not changed. Turning the feature on +later therefore does not require a mapping change or a full re-index to make the fields appear. + HTML is uploaded after the crawler's own pre-processing, so `exclude_tags` and the `data-elastic-exclude` / `data-elastic-include` directives are honoured in the Markdown as well. @@ -76,8 +92,31 @@ Markdown structure. Setting it to `true` explicitly is honoured but logs a warni Transient errors (connection errors, timeouts, HTTP 5xx, HTTP 404 while polling an expired job) are retried once after one second. HTTP 422 (unsupported file), other 4xx responses and `status: failed` are not retried. +The retry itself is announced at `debug` level; only the final failure of a document is logged as a warning, +so one line per failed document reaches a normally configured log. + +The system log ends with `Markdown conversions: converted=N failed=N` (also on a resumable shutdown; a crawl +that aborted on the start-up health check prints nothing, since it converted nothing). + +### Circuit breaker + +The converter can go down mid-crawl. Without a circuit breaker every remaining document would still pay for a +full upload, a retry and possibly the `timeout` deadline before falling back. After **20 consecutive failures** +the crawler opens a circuit breaker, logs + +``` +Markdown converter circuit breaker opened after 20 consecutive failures; skipping conversions for 60s +``` -The system log ends with `Markdown conversions: converted=N failed=N`. +once, and for the next **60 seconds** `convert!` returns a failure immediately without any HTTP call. Failure +handling is unchanged in that state: with `on_failure: text` the documents get their plain-text bodies and +`body_format: text`, with `on_failure: skip` they are not indexed at all — and they are counted in the `failed` +total of the final stats line. After the cooldown the next document to be converted triggers a single health +check (one thread probes, the others keep short-circuiting): if it passes, the breaker closes with an `info` +line and normal conversion resumes; if it fails, the cooldown is re-armed for another 60 seconds. A single +successful conversion also resets the consecutive-failure counter, so isolated failures never open the breaker. +There is no configuration for this; a crawl that starts against a healthy converter degrades to plain text +rather than stalling. ### `skip` and purge crawls @@ -98,5 +137,7 @@ document is preferable to a plain-text one. below it so long conversions go through the polling path instead of failing at the proxy. - A conversion in progress ignores a crawl shutdown request: the task thread finishes (or times out) its current upload/poll cycle first, so a slow job can hold a task thread for up to `timeout` seconds. +- A converter that dies mid-crawl does not slow the crawl down for long: after 20 consecutive failures the + circuit breaker (see [Circuit breaker](#circuit-breaker)) short-circuits conversions for 60 seconds at a time. - Converter calls go straight to `base_url`: they use neither the crawler's `http_proxy_*` settings nor the `HTTP_PROXY`/`HTTPS_PROXY` environment variables.