Conversation
…e disappears, the reinit will either properly restore it or throw a clear error rather than silently failing and falling back to html-to-md. 1. Replaced unreliable sleep with proper page load detection 2. Actually verify TurndownService after loading script 3. Clean up old WebView and verify after reinit
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2170b58362
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let webView = createWebView(userAgent: options.userAgent) | ||
| self.webView = webView | ||
|
|
||
| defer { | ||
| self.webView?.stopLoading() | ||
| self.webView = nil |
There was a problem hiding this comment.
When two convertToMarkdown(url:) calls run concurrently on the same Demark instance (possible because @MainActor allows reentrancy while the first call is suspended), ConversionRuntime reuses a single URLLoadingRuntime instance. In loadAndExtract, the new call overwrites self.webView, and the first call’s defer then executes self.webView?.stopLoading() on whatever is current. This means the earlier call can stop the later in‑flight load, leading to unexpected timeouts or urlNavigationFailed errors. Consider cleaning up the local webView instead of the shared property, or instantiate a fresh URLLoadingRuntime per request to avoid cross‑cancellation (see ConversionRuntime.urlLoadingRuntime in Demark.swift lines 19–25).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06f3fd42ed
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return try await withTaskCancellationHandler { | ||
| try await performLoad(webView: webView, url: url, options: options) | ||
| } onCancel: { | ||
| Task { @MainActor in | ||
| webView.stopLoading() | ||
| } |
There was a problem hiding this comment.
If the caller cancels the task, the cancellation handler only calls webView.stopLoading() and never resumes the continuation. WKWebView doesn’t guarantee a failure callback when stopLoading() is invoked, so the withCheckedThrowingContinuation can stay suspended until the timeout task fires (default 30s) or even complete successfully if navigation finishes quickly. This means cancellation can be delayed and surfaces as a timeout or success rather than a CancellationError. Consider explicitly resuming the continuation with cancellation in the handler or wiring cancellation into the delegate.
Useful? React with 👍 / 👎.
| delegate.timeoutTask = Task { | ||
| try? await Task.sleep(nanoseconds: UInt64(options.timeout * 1_000_000_000)) | ||
| delegate.handleTimeout() |
There was a problem hiding this comment.
Casting options.timeout directly to UInt64 will trap if a caller supplies a negative timeout (e.g., computed or user-provided values). That would crash the process rather than throwing a DemarkError. The same pattern is used for idleDelay later in the file. Consider validating/clamping to >= 0 and failing gracefully so invalid options don’t take down the app.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eefb6443fa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| delegate.timeoutTask = Task { | ||
| try? await Task.sleep(nanoseconds: UInt64(max(0, options.timeout) * 1_000_000_000)) | ||
| delegate.handleTimeout() |
There was a problem hiding this comment.
If a caller sets URLLoadingOptions(timeout: .infinity) (or .nan) to disable timeouts, max(0, options.timeout) * 1_000_000_000 becomes non‑finite and UInt64(...) traps at runtime, crashing before any load occurs. Consider checking options.timeout.isFinite and skipping the timeout task (or clamping to a safe upper bound) before converting to UInt64.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5199e77f8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| logger: logger, | ||
| continuation: continuation | ||
| ) | ||
| self.navigationDelegate = delegate | ||
| webView.navigationDelegate = delegate |
There was a problem hiding this comment.
The runtime stores only one navigationDelegate and overwrites it for every load; because WKWebView.navigationDelegate is weak, any concurrent loadAndExtract call will drop the previous delegate and its continuation can never resume, causing the earlier convertToMarkdown task to hang or time out. This also contradicts the “supports concurrent calls” comment—each request needs its own strong delegate reference (e.g., keyed by webView) that is cleared on completion rather than a single shared property.
Useful? React with 👍 / 👎.
…rence to its delegate in the dictionary, preventing premature deallocation when multiple requests run simultaneously.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d1de29474
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let request = URLRequest(url: url) | ||
| webView.load(request) |
There was a problem hiding this comment.
The request uses the default URLRequest timeout interval, so if a caller sets URLLoadingOptions.timeout higher than the system default, the underlying load can still time out early and surface as urlNavigationFailed, even though the API claims it will wait up to the configured maximum. This makes long timeouts ineffective for slow sites; consider applying options.timeout to the request (or documenting the cap) so the load duration matches the option.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Codex Review: Didn't find any major issues. You're on a roll.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…and > 0, so the underlying load honors longer timeouts.
1 participant
convertToMarkdown(url:options:loadingOptions:)loadsURLs in aWebView, waits for JS execution, and converts rendered HTML to MarkdownURLLoadingOptionsfor timeout, idle detection, content selector, and custom user agentFix
TurndownServiceinitialization that caused intermittent "TurndownService is not available" errorsdocument.readyStatepollingTurndownServiceis actually available before marking initializedWebViewbefore reinitializingExample