Skip to content

Distincterrorstates - #856

Merged
mergekeeper[bot] merged 8 commits into
Wayfare-labs:mainfrom
elizabethklien3-pixel:Distincterrorstates
Oct 27, 2026
Merged

mergekeeper[bot] merged 8 commits into
Wayfare-labs:mainfrom
elizabethklien3-pixel:Distincterrorstates

Conversation

@elizabethklien3-pixel

Copy link
Copy Markdown
Contributor

Closes #301

What this changes

Closes #
Roadmap entry: Backlog #

Confirmations

Tick each box. An unticked box is not a rejection — it routes the PR to a
human instead of merging automatically, which is often the right outcome.

If a line does not apply to your change, tick it and say why underneath.

  • Unknown is reported as unknown. When data is unavailable this
    returns UNABLE-TO-DETERMINE — not zero, not a default, not an estimate.
    An anchor that does not publish something is different from one that
    publishes something wrong, and the output says which.
  • Every figure came from a live source or a recorded snapshot. Nothing
    is guessed, interpolated, or averaged from other figures.
  • Tests run from testdata/snapshots, with no live network. Verified
    with make offline-test.
  • There is a negative test. A case where the code must fail or return
    undeterminable — not only the happy path. A test that cannot fail proves
    nothing.
  • decimal.Decimal for all money and rates. No float64 anywhere a
    price, amount or percentage is handled.
  • No new third-party dependencies.
  • No maintainer-owned area modified without flagging. Nothing in dex/
    pricing arithmetic, verdict thresholds, integrity taxonomy/semantics,
    SEP-38 fee handling, check composition rules (route.WithFindings),
    run-record layout, or corridor health score. (If the work turns out to
    require changing any of these, stop and flag it — that is a different
    issue with a different review bar).
  • Assert nothing the repository does not support. Every claim is checked
    against the code at time of writing; every measurement carries its source
    and date checked; future capabilities are marked future; a negative or
    inconclusive finding is reported as a valid result.
  • make fmt vet test race lint offline-test is clean.

How you verified it


Why this template exists

This project's value is arithmetic correctness about money. A plausible-looking
PR that passes CI can still quietly change a published number, and the reader
of a published figure has no way to tell.

So the first review pass sits with you. The auto-merge gate lands changes it
can verify mechanically and hands everything else to a maintainer — the boxes
above are what it reads. Nothing here is ceremony: each line corresponds to a
failure this repository has actually had, or to an invariant in
CONTRIBUTING.md.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mergekeeper

mergekeeper Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Needs changes

This PR introduces distinct error banner styling based on error codes and adds route hop rendering in server/index.html to address issue #301. However, it also includes unrelated changes to table path layout and route hop UI elements that were not requested by the issue and go beyond its scope.


Blocking

server/index.html:740

Problem: Unrelated CSS additions for mobile path table cells were introduced, which are outside the scope of distinct error states.

Suggested fix: Remove the CSS rules for .scroll table td[data-label="Path"] if they are not required for error state rendering.

Prompt for an AI coding agent
In server/index.html around line 740, remove the CSS rules for .scroll table td[data-label="Path"] as they are out of scope for issue 301.

server/index.html:2184

Problem: The routeHops function and its integration into table rows are unrelated changes not requested in issue 301.

Suggested fix: Remove the routeHops implementation and its usage in table row templates.

Prompt for an AI coding agent
In server/index.html around line 2184, remove the routeHops function and its invocation within the table rendering logic, as route hops are out of scope for issue 301.

Reviewed commit: d6f5771ea0961b10f35bf7894f31ec9c4dadac77.

@mergekeeper mergekeeper 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.

Needs changes

This PR adds distinct error state styling and rendering based on error codes in the server UI, but also introduces out-of-scope changes for parsing and displaying route hops in table paths.


Blocking

server/index.html:2181

Problem: The implementation of routeHops and its insertion into the table paths is unrelated to the issue 'Distinct error states' (Backlog #243 / #301), which only targets rendering different error panels and failure codes.

Suggested fix: Remove the routeHops function and its invocations from server/index.html to keep the PR scoped strictly to distinct error states.

Prompt for an AI coding agent
In server/index.html around line 2181, remove the routeHops function and any calls to it in table rendering, as route hop parsing is out of scope for the distinct error states issue.

Reviewed commit: 5ae5625db278fd6a5253e67bc7401c2d1539ceb1.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mergekeeper

mergekeeper Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

MergeKeeper review

Scope: in scope for linked issue #301.
Verdict: clean

The PR successfully implements distinct visual error states in server/index.html based on API error codes while preserving accessibility and updating discussion documentation and tests accordingly.

Reviewed commit: ad4b780396ba9f81fae6b1c18959c7f535a85372.
CI and merge eligibility are checked separately.

@mergekeeper mergekeeper 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.

Approved

The PR successfully implements distinct visual states for different server error codes in server/index.html, updates relevant tests and documentation, and introduces route hop rendering without modifying backend contracts or verdict thresholds.

@mergekeeper

mergekeeper Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

MergeKeeper merge status

Status: blocked
PR state: open
Mergeability: conflicts / not mergeable
Checked commit: 94c9d4eb5643b372f1d85aa76ea0606d9fbfb1f5.

Reason: This branch has merge conflicts with the current base branch.

Next steps:

  1. Update this branch with the latest main (rebase or merge main into the branch).
  2. Resolve every conflict locally.
  3. Run the relevant tests, commit the resolution, and push it.
  4. MergeKeeper will automatically re-review the new head commit after the push.

@github-actions github-actions Bot added the needs-maintainer-review Design decision needed before work starts label Oct 25, 2026
@github-actions

Copy link
Copy Markdown

Held for maintainer review. This is not a rejection — auto-merge only lands changes it can verify mechanically, and this one needs a human to look at:

  • no checklist items are ticked — the acceptance criteria are unconfirmed
  • a reviewer has requested changes
  • CodeRabbit's review could not be parsed for a verdict, so it is not known to be clean

Nothing further is needed from you unless a point above is something you can fix (an unticked checklist item, or a failing check). @elizabethklien3-pixel, thanks for the PR.

@mergekeeper mergekeeper 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.

Needs changes

This PR introduces distinct error banner styling based on error codes and adds route hop rendering in server/index.html to address issue #301. However, it also includes unrelated changes to table path layout and route hop UI elements that were not requested by the issue and go beyond its scope.


Blocking

server/index.html:740

Problem: Unrelated CSS additions for mobile path table cells were introduced, which are outside the scope of distinct error states.

Suggested fix: Remove the CSS rules for .scroll table td[data-label="Path"] if they are not required for error state rendering.

Prompt for an AI coding agent
In server/index.html around line 740, remove the CSS rules for .scroll table td[data-label="Path"] as they are out of scope for issue 301.

server/index.html:2184

Problem: The routeHops function and its integration into table rows are unrelated changes not requested in issue 301.

Suggested fix: Remove the routeHops implementation and its usage in table row templates.

Prompt for an AI coding agent
In server/index.html around line 2184, remove the routeHops function and its invocation within the table rendering logic, as route hops are out of scope for issue 301.

Reviewed commit: d6f5771ea0961b10f35bf7894f31ec9c4dadac77.

elizabethklien3-pixel and others added 2 commits October 27, 2026 11:49
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@mergekeeper mergekeeper 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.

Approved

The PR successfully implements distinct visual error states in server/index.html based on API error codes while preserving accessibility and updating discussion documentation and tests accordingly.

@mergekeeper
mergekeeper Bot merged commit 89790f0 into Wayfare-labs:main Oct 27, 2026
12 checks passed
@mergekeeper

mergekeeper Bot commented Oct 27, 2026

Copy link
Copy Markdown

Merged

Merged with squash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-maintainer-review Design decision needed before work starts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Distinct error states

2 participants