Skip to content

fix(kegg): bound final pathway download attempt - #25

Merged
FuqingZh merged 7 commits into
mainfrom
codex/kegg-pathway-attempt-budget
Aug 20, 2026
Merged

fix(kegg): bound final pathway download attempt#25
FuqingZh merged 7 commits into
mainfrom
codex/kegg-pathway-attempt-budget

Conversation

@FuqingZh

Copy link
Copy Markdown
Owner

Summary

  • add an opt-in zero-default httpx option to skip metadata probing while retaining resume behavior
  • make the final KEGG PATHWAY attempt GET-only so request timeouts stay within --max-attempts
  • lock request sequences, final 206 resume, ignored-Range behavior, and unchanged shared defaults with tests

Follow-up to #24 and its exact-head review thread PRRT_kwDORj9T486atjmT.

Tests

  • GOCACHE=/tmp/biofetch-test go test ./internal/shared/httpx ./internal/kegg
  • GOCACHE=/tmp/biofetch-race go test -race ./internal/shared/httpx ./internal/kegg
  • canonical AGENTS.md gate (test, vet, race, build, tidy diff, mod verify, notices check)

@FuqingZh

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc3760b171

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/shared/httpx/httpx.go Outdated
@FuqingZh

Copy link
Copy Markdown
Owner Author

@codex review

@FuqingZh

Copy link
Copy Markdown
Owner Author

Addressed the follow-up exact-candidate P1 at 5cccef3: open-ended 206 resumes now validate the declared range and actual body length; short reads wrap io.ErrUnexpectedEOF, and all append failures truncate the partial back to its original length. Added chunked short/long/complete, unknown-total, and PATHWAY rollback/request-budget regressions. Focused normal/race tests and the full canonical gate pass.

@codex review

@FuqingZh

Copy link
Copy Markdown
Owner Author

Addressed the exact-head overlong-body P2 at 87c06fb: append resumes reject known Content-Length mismatches before opening the partial and bound unknown-length reads with io.LimitReader(expected+1) before progress wrapping. The overlong regression uses a 1 MiB source and proves at most 11 bytes are read for a 10-byte range, with one request and rollback. Focused normal/race and the full canonical gate pass.

@codex review

@FuqingZh

Copy link
Copy Markdown
Owner Author

Addressed the progress-wrapper bypass at 6b61066: progressReader now wraps the already limited reader. The same 1 MiB overlong fixture runs with a non-nil progress callback and proves at most expected+1 bytes are read, with one request and partial rollback. Focused normal/race and the full canonical gate pass.

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6b61066abb

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/kegg/pathway.go
@FuqingZh

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a24b1a9509

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/shared/httpx/httpx_test.go Outdated
@FuqingZh

Copy link
Copy Markdown
Owner Author

Addressed the independent destructive-boundary review at 4fee727: exact partial directories and empty output paths fail closed with zero requests; regular files/symlinks are unlinked non-recursively; only explicit .parts scratch uses recursive removal. Directory/empty-path sentinels and symlink target preservation are covered.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 4fee7273aa

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@FuqingZh
FuqingZh merged commit 1eb23e8 into main Aug 20, 2026
3 checks passed
@FuqingZh
FuqingZh deleted the codex/kegg-pathway-attempt-budget branch August 20, 2026 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant