Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[codex] Add line numbers to Oracle text bundles#239

Open
tristanmanchester wants to merge 2 commits into
steipete:mainsteipete/oracle:mainfrom
tristanmanchester:codex/citation-friendly-line-numberstristanmanchester/oracle:codex/citation-friendly-line-numbersCopy head branch name to clipboard
Open

[codex] Add line numbers to Oracle text bundles#239
tristanmanchester wants to merge 2 commits into
steipete:mainsteipete/oracle:mainfrom
tristanmanchester:codex/citation-friendly-line-numberstristanmanchester/oracle:codex/citation-friendly-line-numbersCopy head branch name to clipboard

Conversation

@tristanmanchester
Copy link
Copy Markdown

@tristanmanchester tristanmanchester commented Jun 3, 2026

Summary

Oracle-generated text context now carries stable line numbers, so models can cite bundled source as path:line or path:line-line without guessing whether file citations are accepted. The generated prompt, rendered markdown, browser inline mode, and attachments-bundle.txt all use the numbered view, while raw browser uploads, ZIP entries, legacy createFileSections().sectionText, and the default exported formatFileSection(...) helper stay unmodified for compatibility.

The public formatter contract is preserved: direct formatFileSection(path, content) calls still return raw fenced source, callers can opt into numbered output with formatFileSection(..., { lineNumbers: true }), and generated prompt surfaces use formatFileSections(...) so they do not drift. Token accounting follows the numbered generated view, and the default Oracle system prompt now asks for line-specific file citations when line numbers are present.

Proof

Real local Oracle render from this branch against source files in this repository, using the TypeScript CLI entrypoint so the output reflects the changed source rather than stale built dist:

$ fnm exec --using v24.11.1 node --no-deprecation --import tsx bin/oracle-cli.ts --render --render-plain --prompt "Proof: show numbered Oracle source context" --file src/oracle/markdown.ts src/browser/prompt.ts
🧿 oracle 0.13.0 — Your prompt's adult supervision.
[SYSTEM]
You are Oracle, a focused one-shot problem solver. Emphasize direct answers and cite referenced files as path:line or path:line-line when line numbers are available.

[USER]
Proof: show numbered Oracle source context

### File: src/oracle/markdown.ts
Lines: 1-106
```ts
  1 | import path from "node:path";
  2 | 
  3 | export type FenceLanguage = string | null | undefined;
  4 | 
  5 | export interface FormatFileSectionOptions {
  6 |   lineNumbers?: boolean;
  7 | }
  8 | 
  9 | export interface FormatFileSectionsOptions extends FormatFileSectionOptions {
 10 |   trailingNewline?: boolean;
 11 | }
 12 | 
 13 | export interface FileSectionInput {
 14 |   displayPath: string;
 15 |   content: string;
 16 | }
 17 | 
 18 | const EXT_TO_LANG: Record<string, string> = {
 19 |   ".ts": "ts",
 20 |   ".tsx": "tsx",
```

The command output is clipped here to the relevant proof window, but it demonstrates the generated --render text view includes the repository-relative path, Lines: range, padded line numbers, and numbered blank/source lines. Separately, tests assert raw upload/ZIP compatibility and that formatFileSection(...) remains raw by default.

Validation

  • fnm exec --using v24.11.1 pnpm vitest run tests/oracle/markdown.test.ts tests/oracle/oracleExports.test.ts tests/browser/policies.test.ts tests/browser/prompt.test.ts tests/cli/markdownBundle.test.ts tests/cli/runOracle/prompt-and-utils.test.ts passed: 6 files, 57 tests.
  • fnm exec --using v24.11.1 pnpm run check passed.
  • git diff --check passed.
  • fnm exec --using v24.11.1 pnpm test passed: 133 files passed, 18 skipped; 1084 tests passed, 41 skipped.

Review

Ran compound-engineering:ce-code-review locally and fixed the actionable findings around token-stat assertions, legacy sectionText documentation, shared generated-section rendering, API migration notes, line-specific citation prompting, and the ClawSweeper compatibility/proof blocker. The local review artifact was written to /tmp/compound-engineering/ce-code-review/20260603-054114-d9c6521a/review.json.


Compound Engineering
Codex

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented Jun 3, 2026

Codex review: needs maintainer review before merge. Reviewed June 3, 2026, 1:51 AM ET / 05:51 UTC.

Summary
The PR adds line-numbered generated file-section rendering across CLI/API prompt context, rendered markdown, browser inline/text bundles, token stats, exported formatter helpers, tests, and changelog copy.

Reproducibility: not applicable. this is a feature PR, not a bug report. The PR body supplies a real CLI render proving the new numbered generated context for the central behavior.

Review metrics: 2 noteworthy metrics.

  • Changed files: 17 files, +196/-55. The diff spans formatter, prompt assembly, browser bundle logic, token stats, tests, and changelog, so the generated-output contract needs maintainer review.
  • Validation surface: 6 targeted test files plus full test/check output reported. The PR body reports focused formatter/browser/CLI tests and full suite validation, which materially supports the patch despite the compatibility decision.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Maintainer should explicitly accept the generated text format compatibility change before merge.
  • [P2] Run or request one relevant browser inline/text-bundle smoke if browser-path release confidence is needed.

Risk before merge

  • [P1] Existing consumers that parse generated Oracle prompts, render output, browser inline text, or text bundle attachments may need to tolerate the new Lines: header and N | prefixes.
  • [P1] Numbered file context increases generated text size; the PR updates token accounting, but large-file workflows may hit token thresholds earlier than before.
  • [P1] The PR proof shows a real CLI render and tests cover browser bundle behavior, but no live browser smoke is shown for the browser text path AGENTS.md calls out before release.

Maintainer options:

  1. Accept numbered generated context (recommended)
    If maintainers want line-specific citations as the default generated context behavior, merge with the documented compatibility shift and existing proof/tests.
  2. Ask for one browser-path proof
    If browser generated text is the concern, request a focused browser inline or text-bundle smoke before merge or before release.
  3. Defer to an opt-in path
    If default generated output must stay raw for existing consumers, pause this PR and ask for a flag/API-only approach instead.

Next step before merge

  • [P2] There is no narrow automation repair target; maintainers need to decide whether the intentional generated-output contract change is acceptable.

Security
Cleared: The diff does not change dependencies, CI, secrets, package resolution, or new code-execution surfaces; no concrete security or supply-chain concern was found.

Review details

Best possible solution:

Merge after maintainers explicitly accept numbered generated context as the new output contract and decide whether the browser/bundle smoke evidence is sufficient for release.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this is a feature PR, not a bug report. The PR body supplies a real CLI render proving the new numbered generated context for the central behavior.

Is this the best way to solve the issue?

Yes, with maintainer sign-off: the implementation centralizes generated section rendering while preserving the raw single-section formatter and legacy sectionText. The remaining question is whether the default generated-output contract should change now.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 4490e00b5e23.

Label changes

Label justifications:

  • P2: This is a normal user-facing generated-output improvement with bounded but real compatibility impact.
  • merge-risk: 🚨 compatibility: Generated prompts, render output, browser inline text, and text bundle attachments intentionally change shape for existing consumers.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal output from a real TypeScript CLI --render --render-plain --file run showing numbered file context after the change, with tests supplementing browser and compatibility paths.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output from a real TypeScript CLI --render --render-plain --file run showing numbered file context after the change, with tests supplementing browser and compatibility paths.
Evidence reviewed

What I checked:

  • Current main has raw single-section formatting only: Current main's formatter emits a raw fenced section with no line-number option or multi-section helper, so the requested capability is not already implemented on main. (src/oracle/markdown.ts:45, 4490e00b5e23)
  • Current generated API prompt uses legacy raw sectionText: buildPrompt currently joins createFileSections().sectionText, which is the raw legacy generated text the PR replaces for generated prompt context. (src/oracle/request.ts:19, 4490e00b5e23)
  • Current browser text bundle and inline paths use raw formatFileSection: Browser text bundles and attachment token estimates currently call formatFileSection on each file, while browser inline text is assembled the same way in policies.ts. (src/browser/prompt.ts:85, 4490e00b5e23)
  • Current main lacks line-number support: Search found no lineNumbers, Lines:, path:line prompt copy, or formatFileSections support in current main beyond unrelated variable names. (4490e00b5e23)
  • PR diff is focused on generated formatter adoption: The PR diff adds FormatFileSectionOptions/formatFileSections, keeps formatFileSection raw by default, switches generated prompt/browser/token-stat surfaces to numbered rendering, and updates focused tests. (src/oracle/markdown.ts:45, 8d3d762a5fb9)
  • Repository policy affected proof review: AGENTS.md asks for relevant browser/manual smokes before release and notes browser Markdown preservation checks, so browser-output proof remains a maintainer confidence point even though unit tests cover the shared formatter paths. (AGENTS.md:13, 4490e00b5e23)

Likely related people:

  • Peter Steinberger: Blame and history show Peter introduced the current formatter/prompt/bundle implementation and later browser inline delivery paths that this PR changes. (role: feature-history owner; confidence: high; commits: b9ce3970100a, f6c3c9704934, 2b8290a489a3; files: src/oracle/markdown.ts, src/oracle/request.ts, src/oracle/promptAssembly.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jun 3, 2026
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 3, 2026
@tristanmanchester tristanmanchester marked this pull request as ready for review June 3, 2026 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Morty Proxy This is a proxified and sanitized view of the page, visit original site.