Skip to content

handle RTL language rendering part for OBS #481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
RevantCI merged 2 commits into from
May 11, 2026
Merged

handle RTL language rendering part for OBS #481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f3f6e8
handle RTL language rendering part for OBS
KetanKBaboo May 11, 2026
5769c59
handle BOM in windows OS for OBS md file
RevantCI May 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
195 changes: 102 additions & 93 deletions frontend/src/components/DetailsDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,108 +1,117 @@
import { useMemo, useState } from "react";
import {
Dialog,
DialogContent,
DialogHeader,
DialogTitle,
DialogFooter,
DialogClose,
Dialog,
DialogContent,
DialogHeader,
DialogTitle,
DialogFooter,
DialogClose,
} from "@/components/ui/dialog";
import { Button } from "@/components/ui/button";
import { MoreHorizontal } from "lucide-react";

type DetailsCellProps = {
text?: string | null;
limit?: number;
title?: string;
triggerVariant?: "text" | "icon";
justify?: "between" | "start" | "center"
text?: string | null;
limit?: number;
title?: string;
triggerVariant?: "text" | "icon";
justify?: "between" | "start" | "center";
};

export function DetailsDialog({
text,
limit = 25,
title = "Details",
triggerVariant = "text",
justify = "between"
text,
limit = 25,
title = "Details",
triggerVariant = "text",
justify = "between",
}: DetailsCellProps) {
const [open, setOpen] = useState(false);
const value = text ?? "";

const isTruncated = value.length > limit;
const wrapperJustifyClass = justify === "start" ? "justify-start" : justify === "center" ? "justify-center" : "justify-between"

const display = useMemo(() => {
if (!value) return "";

if (!isTruncated) return value;

if (triggerVariant === "text") {
return value.slice(0, limit) + "...";
}

return value.slice(0, limit);
}, [value, limit, isTruncated, triggerVariant]);

const handleOpen = () => setOpen(true);

return (
<>
<div className={`flex flex-row items-center ${wrapperJustifyClass} gap-1 w-full`}>
<div className="min-w-0 whitespace-nowrap overflow-hidden truncate">
{display}
</div>


{isTruncated && (
<>
{triggerVariant === "text" && (
<button
type="button"
onClick={handleOpen}
className="text-xs underline shrink-0"
>
See More
</button>
)}

{triggerVariant === "icon" && (
<button
type="button"
onClick={handleOpen}
className="shrink-0 inline-flex items-center justify-center rounded-md
const [open, setOpen] = useState(false);
const value = text ?? "";

const isTruncated = value.length > limit;
const wrapperJustifyClass =
justify === "start"
? "justify-start"
: justify === "center"
? "justify-center"
: "justify-between";

const display = useMemo(() => {
if (!value) return "";

if (!isTruncated) return value;

if (triggerVariant === "text") {
return value.slice(0, limit) + "...";
}

return value.slice(0, limit);
}, [value, limit, isTruncated, triggerVariant]);

const handleOpen = () => setOpen(true);

return (
<>
<div
dir="auto"
className={`flex flex-row items-center ${wrapperJustifyClass} gap-1 w-full`}
>
<div
dir="auto"
className="min-w-0 whitespace-nowrap overflow-hidden truncate text-start"
>
{display}
</div>

{isTruncated && (
<>
{triggerVariant === "text" && (
<button
type="button"
onClick={handleOpen}
className="text-xs underline shrink-0"
>
See More
</button>
)}

{triggerVariant === "icon" && (
<button
type="button"
onClick={handleOpen}
className="shrink-0 inline-flex items-center justify-center rounded-md
hover:bg-gray-200 transition-all group mt-1 p-1"
title="See More"
>
<MoreHorizontal className="w-5 h-5 text-gray-600 group-hover:text-gray-900" />
</button>
)}
</>
)}
title="See More"
>
<MoreHorizontal className="w-5 h-5 text-gray-600 group-hover:text-gray-900" />
</button>
)}
</>
)}
Comment on lines +66 to +90
Copy link
Copy Markdown

@codeant-ai codeant-ai Bot May 11, 2026

Choose a reason for hiding this comment

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

Suggestion: The fallback "No details available." is unreachable for empty values because the dialog trigger is rendered only when text is truncated. For empty text, isTruncated is false, so users cannot open the dialog and never see the fallback message. Render a trigger for empty values (or show the fallback inline) so the intended empty-state behavior is actually visible. [incomplete implementation]

Severity Level: Major ⚠️ 
- ⚠️ OBS upload preview hides empty story descriptions silently.
- ⚠️ Users never see intended "No details available" message.
Steps of Reproduction ✅ 
1. Open the OBS admin page implemented in `frontend/src/pages/OBS.tsx` and trigger the
upload flow handled by `OBSDataAction`'s `handleFiles` function at lines 114–165 by
clicking the upload button when there is no OBS data.

2. Upload an ISL OBS CSV file where at least one row has `storyNo`, `title`, and `url`
populated but an empty `description` column; `parseISLOBSCSV` in
`frontend/src/utils/obsParser.ts` (lines 40–63) maps this row into an `OBSStory` with
`text` set to an empty string.

3. After parsing, `OBSDataAction` sets `rows` and opens `OBSUploadPreviewDialog` (OBS.tsx
lines 139–143 and 193–204), which renders its preview table in
`frontend/src/components/OBSUploadPreviewDialog.tsx` using a `DetailsDialog` for the
"Text" column at lines 72–80 with `text={row.original.text}` and `limit={40}`.

4. For the row with empty `text`, `DetailsDialog` in
`frontend/src/components/DetailsDialog.tsx` computes `value = text ?? ""` (line 29),
`isTruncated = value.length > limit` (line 31) which is `false` for `""`, and `display`
returns `""` because `!value` (line 40); since `isTruncated` is false, the trigger block
guarded by `{isTruncated && (...)}` (lines 66–90) is not rendered, so `handleOpen` (line
51) is never invoked and the dialog never opens, meaning the fallback `{value || "No
details available."}` (lines 101–103) is unreachable for this empty-state row.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** frontend/src/components/DetailsDialog.tsx
**Line:** 66:90
**Comment:**
	*Incomplete Implementation: The fallback `"No details available."` is unreachable for empty values because the dialog trigger is rendered only when text is truncated. For empty text, `isTruncated` is false, so users cannot open the dialog and never see the fallback message. Render a trigger for empty values (or show the fallback inline) so the intended empty-state behavior is actually visible.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

</div>

<Dialog open={open} onOpenChange={setOpen}>
<DialogContent className="sm:max-w-2xl w-[90vw] max-w-[900px] p-0 overflow-hidden">
<div className="flex flex-col max-h-[80vh]">
<DialogHeader className="px-6 pt-6">
<DialogTitle dir="auto">{title}</DialogTitle>
<div className="mt-1 border-b border-gray-300" />
</DialogHeader>

<div className="px-6 py-4 overflow-auto text-justify">
<div dir="auto" className="whitespace-pre-wrap break-words">
{value || "No details available."}
</div>
</div>


<Dialog open={open} onOpenChange={setOpen}>
<DialogContent className="sm:max-w-2xl w-[90vw] max-w-[900px] p-0 overflow-hidden">
<div className="flex flex-col max-h-[80vh]">
<DialogHeader className="px-6 pt-6">
<DialogTitle>{title}</DialogTitle>
<div className="mt-1 border-b border-gray-300" />
</DialogHeader>

<div className="px-6 py-4 overflow-auto text-justify">
<div className="whitespace-pre-wrap break-words">
{value || "No details available."}
</div>
</div>

<DialogFooter className="px-6 pb-6 pt-3">
<DialogClose asChild>
<Button>Close</Button>
</DialogClose>
</DialogFooter>
</div>
</DialogContent>
</Dialog>
</>
);
<DialogFooter className="px-6 pb-6 pt-3">
<DialogClose asChild>
<Button>Close</Button>
</DialogClose>
</DialogFooter>
</div>
</DialogContent>
</Dialog>
</>
);
}
3 changes: 2 additions & 1 deletion frontend/src/components/OBSViewDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ export const OBSViewDialog = ({
className="flex-1 overflow-auto border rounded"
>
{!editMode && (
<div className="p-3 bg-gray-50">
<div dir="auto" className="p-3 bg-gray-50">
{previewMode ? (
<ReactMarkdown
components={{
Expand Down Expand Up @@ -641,6 +641,7 @@ export const OBSViewDialog = ({
{/* EDIT MODE */}
{editMode && (
<div
dir="auto"
ref={editableRef}
className="p-4 bg-white outline-none min-h-full whitespace-pre-wrap font-mono text-sm tracking-wide"
contentEditable
Expand Down
72 changes: 46 additions & 26 deletions frontend/src/utils/obsParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,39 @@ import type { OBSStory } from "./types";

const ISL_HEADERS = ["storyNo", "title", "url", "description"];

// Supports:
// # 1. Creation
// # 1۔ تخلیق
const OBS_HEADING_REGEX = /^#+\s*(\d+)\s*[^\p{L}\p{N}]\s*(.+)$/u;

const cleanHeader = (header: string) =>
header
.replace(/^\uFEFF/, "")
.trim()
.toLowerCase();

export const isISLOBSCSV = (headers: string[]) =>
Copy link
Copy Markdown

@codeant-ai codeant-ai Bot May 11, 2026

Choose a reason for hiding this comment

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

🔴 Architect Review — CRITICAL

CSV header normalization is only applied during validation (isISLOBSCSV), but parseISLOBSCSV still reads hard-coded keys (row.storyNo, row.title, etc.). For CSVs whose headers differ by case/spacing/BOM (which now pass validation via cleanHeader), the parsed row fields become undefined/NaN, so the preview and subsequent upload operate on invalid OBS data.

Suggestion: Normalize headers at parse time (e.g., via Papa.parse transformHeader or a canonical header-to-field mapping) so row access uses the normalized keys, and add row-level validation to reject records with invalid story_no/title/url/description with a clear import error instead of uploading bad data.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.

**Path:** frontend/src/utils/obsParser.ts
**Line:** 17:62
**Comment:**
	*CRITICAL: CSV header normalization is only applied during validation (`isISLOBSCSV`), but `parseISLOBSCSV` still reads hard-coded keys (`row.storyNo`, `row.title`, etc.). For CSVs whose headers differ by case/spacing/BOM (which now pass validation via `cleanHeader`), the parsed row fields become `undefined`/`NaN`, so the preview and subsequent upload operate on invalid OBS data.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

Copy link
Copy Markdown

@codeant-ai codeant-ai Bot May 11, 2026

Choose a reason for hiding this comment

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

🟠 Architect Review — HIGH

CSV header normalization via cleanHeader is only applied during header validation in isISLOBSCSV, not to the actual row keys. For headers that cleanHeader newly accepts (e.g. with BOM or leading/trailing spaces), parseISLOBSCSV still reads row.storyNo, row.title, row.url, and row.description based on the unnormalized header names, so those properties are undefined and story_no becomes NaN, causing failed or corrupted uploads for exactly the CSVs this change is meant to support.

Suggestion: Normalize headers at parse time (for example by using Papa.parse's transformHeader with the same cleanHeader logic) and build OBSStory objects from these canonical keys so header validation and row extraction stay consistent for all accepted header variants.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.

**Path:** frontend/src/utils/obsParser.ts
**Line:** 17:62
**Comment:**
	*HIGH: CSV header normalization via `cleanHeader` is only applied during header validation in `isISLOBSCSV`, not to the actual row keys. For headers that `cleanHeader` newly accepts (e.g. with BOM or leading/trailing spaces), `parseISLOBSCSV` still reads `row.storyNo`, `row.title`, `row.url`, and `row.description` based on the unnormalized header names, so those properties are `undefined` and `story_no` becomes `NaN`, causing failed or corrupted uploads for exactly the CSVs this change is meant to support.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

ISL_HEADERS.every((h) =>
headers.map((x) => x.toLowerCase()).includes(h.toLowerCase()),
);
ISL_HEADERS.every((h) => headers.map(cleanHeader).includes(h.toLowerCase()));
Copy link
Copy Markdown

@codeant-ai codeant-ai Bot May 11, 2026

Choose a reason for hiding this comment

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

Suggestion: Header validation now accepts case/spacing/BOM variants, but row extraction still reads fixed keys (row.storyNo, row.title, etc.). This creates a contract mismatch: files that pass validation can still parse into NaN/undefined fields at runtime. Normalize the row keys (or remap headers from meta.fields) before reading values so accepted header variants are actually supported. [api mismatch]

Severity Level: Critical 🚨 
- ❌ ISL OBS CSV uploads create stories with NaN story numbers.
- ❌ Backend receives malformed OBS stories undermining data integrity.
- ⚠️ Preview table shows blank titles/text for affected CSV rows.
Steps of Reproduction ✅ 
1. In `frontend/src/pages/OBS.tsx:114-145`, a user without existing OBS data clicks the
upload icon, selects a single `.csv` file, and `handleFiles()` detects `hasCSV` as true
and calls `parseISLOBSCSV(arr[0])`.

2. The CSV file's header row contains variants like `" StoryNo"` or `"StoryNo"` (different
casing/spacing/BOM) instead of the exact `"storyNo"`. Papa.parse (see
`frontend/src/utils/obsParser.ts:40-45`) parses with `header: true`, so `meta.fields`
contains these raw header strings and each `row` object is keyed with the same raw header
names.

3. `isISLOBSCSV()` at `frontend/src/utils/obsParser.ts:17-18` maps `headers` through
`cleanHeader()` (lines 11-15), stripping BOM, trimming, and lowercasing, so the normalized
headers include `"storyno"`, `"title"`, `"url"`, `"description"`, and the CSV is accepted
as valid.

4. Still in `parseISLOBSCSV()` (lines 56-62), the mapper reads `row.storyNo`, `row.title`,
`row.url`, and `row.description`, but because the actual keys are e.g. `" StoryNo"` or
`"StoryNo"`, these properties are `undefined`. `Number(row.storyNo)` becomes `NaN`, and
`title`, `url`, and `text` fields become `undefined` in the resulting `OBSStory[]`.

5. Back in `frontend/src/pages/OBS.tsx:20-23`, the parsed rows (with `story_no: NaN`,
`title/text: undefined`) are saved into `rows` state and shown in `OBSUploadPreviewDialog`
(see `frontend/src/components/OBSUploadPreviewDialog.tsx:20-27,46-83`), and then uploaded
via `handleUpload()` (lines 30-37), sending malformed `OBSStory` objects to the backend.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** frontend/src/utils/obsParser.ts
**Line:** 18:18
**Comment:**
	*Api Mismatch: Header validation now accepts case/spacing/BOM variants, but row extraction still reads fixed keys (`row.storyNo`, `row.title`, etc.). This creates a contract mismatch: files that pass validation can still parse into `NaN`/`undefined` fields at runtime. Normalize the row keys (or remap headers from `meta.fields`) before reading values so accepted header variants are actually supported.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment on lines 17 to +18
Copy link
Copy Markdown

@codeant-ai codeant-ai Bot May 11, 2026

Choose a reason for hiding this comment

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

🔴 Architect Review — CRITICAL

Header validation now normalizes casing/whitespace/BOM (isISLOBSCSV), but row extraction still reads raw keys storyNo, title, url, description from Papa rows. CSVs whose headers differ in case or include leading/trailing spaces/BOM will pass validation but produce undefined/NaN fields during import.

Suggestion: Normalize headers at parse time (e.g., via Papa's transformHeader that strips BOM, trims, and lowercases) and access row fields using these normalized keys, rejecting rows that are missing required normalized fields before upload.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.

**Path:** frontend/src/utils/obsParser.ts
**Line:** 17:18
**Comment:**
	*CRITICAL: Header validation now normalizes casing/whitespace/BOM (`isISLOBSCSV`), but row extraction still reads raw keys `storyNo`, `title`, `url`, `description` from Papa rows. CSVs whose headers differ in case or include leading/trailing spaces/BOM will pass validation but produce undefined/NaN fields during import.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

Comment on lines 17 to +18
Copy link
Copy Markdown

@codeant-ai codeant-ai Bot May 11, 2026

Choose a reason for hiding this comment

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

Suggestion: The header check now accepts normalized CSV headers (trimmed/lowercased/BOM-stripped), but the parser still reads rows using fixed property names like row.storyNo. When a file passes validation with headers such as " storyNo " or different casing, parsed fields become undefined/NaN at runtime. Normalize header keys before row mapping (or map columns by resolved header names) so accepted files are parsed correctly. [api mismatch]

Severity Level: Critical 🚨 
- ❌ ISL OBS CSVs with BOM/spaces parse into NaN/undefined data.
- ❌ OBS upload preview shows invalid story numbers and empty fields.
- ❌ Backend receives malformed OBS stories through `uploadOBS`.
- ⚠️ DetailsDialog receives undefined text, compounding empty-state issues.
Steps of Reproduction ✅ 
1. Prepare an ISL OBS CSV where the first header has a BOM or spaces, e.g. `"storyNo
,Title,Url,Description"` (note the hidden BOM / space before `storyNo`). PapaParse will
expose headers as `meta.fields` with the raw names including BOM/spacing.

2. In `frontend/src/utils/obsParser.ts:11-18`, `isISLOBSCSV()` is called from
`parseISLOBSCSV()` (line 47). It transforms `headers` with `cleanHeader()` (stripping BOM,
trimming, lowercasing) and successfully validates the file because `"storyNo "` normalizes
to `"storyno"`, which matches `ISL_HEADERS`.

3. Still in `parseISLOBSCSV()` at lines 56-62, the mapping uses fixed property names
`row.storyNo`, `row.title`, `row.url`, and `row.description`. However, PapaParse keys
`row` by the raw header names (e.g. `"storyNo "`), so `row.storyNo` and the other fields
are `undefined`, producing `story_no: Number(undefined)` (`NaN`) and `title`, `url`,
`text` as `undefined`.

4. In `frontend/src/pages/OBS.tsx:32-44`, `handleFiles()` calls `parseISLOBSCSV(arr[0])`
and passes the resulting `OBSStory[]` (containing `NaN` and `undefined` values) to
`OBSUploadPreviewDialog` (`rows` prop) and later to `uploadOBS` via
`OBSUploadPreviewDialog.handleUpload()`
(`frontend/src/components/OBSUploadPreviewDialog.tsx:30-39` and
`frontend/src/utils/api.ts:14-19`), leading to a preview table with broken data and
potentially invalid payloads sent to the backend.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** frontend/src/utils/obsParser.ts
**Line:** 17:18
**Comment:**
	*Api Mismatch: The header check now accepts normalized CSV headers (trimmed/lowercased/BOM-stripped), but the parser still reads rows using fixed property names like `row.storyNo`. When a file passes validation with headers such as `" storyNo "` or different casing, parsed fields become `undefined`/`NaN` at runtime. Normalize header keys before row mapping (or map columns by resolved header names) so accepted files are parsed correctly.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


function extractHeadingData(headingLine: string): {
story_no: number;
title: string;
} {
const normalized = headingLine.normalize("NFC");

const match = normalized.match(OBS_HEADING_REGEX);
Comment on lines +24 to +26
Copy link
Copy Markdown

@codeant-ai codeant-ai Bot May 11, 2026

Choose a reason for hiding this comment

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

Suggestion: The heading parser validates candidates using l.trim().startsWith("#") but then matches the untrimmed headingLine against a start-anchored regex. Headings with leading whitespace or a UTF-8 BOM will be detected as headings and then rejected as invalid, causing valid markdown uploads/edits to fail. Normalize and trim (and strip BOM) before applying the regex. [logic error]

Severity Level: Critical 🚨 
- ❌ OBS markdown uploads fail for indented heading lines.
- ❌ OBS editor rejects valid stories with indented headings.
- ⚠️ Users see confusing "Invalid OBS heading" errors.
Steps of Reproduction ✅ 
1. Open the OBS upload page implemented in `frontend/src/pages/OBS.tsx` and select one or
more `.md` files via the file input that triggers `handleFiles` (see `OBS.tsx:42-49` for
the input and `handleFiles` wiring).

2. Ensure at least one markdown file has its story heading indented, e.g. first line `" #
1. Creation"` (leading spaces before `#`) or similar; this is read in `parseOBSMarkdown`
at `frontend/src/utils/obsParser.ts:70-75`, where `lines.find((l) =>
l.trim().startsWith("#"))` will correctly detect this indented line as a heading
candidate.

3. The original, untrimmed line (still containing leading spaces) is passed as
`headingLine` into `extractHeadingData` at `obsParser.ts:81` and normalized via `const
normalized = headingLine.normalize("NFC");` at `obsParser.ts:24` but never trimmed or
BOM-stripped before `normalized.match(OBS_HEADING_REGEX)` at `obsParser.ts:26`, so the
start-anchored regex `/^#+\s*(\d+)\s*[.۔]\s*(.+)$/u` fails to match because the string
starts with whitespace, not `#`.

4. `extractHeadingData` then throws `new Error("Invalid OBS heading. Expected '# 1. Title'
or '# 1۔ عنوان'")` at `obsParser.ts:28-31`, causing `parseOBSMarkdownFiles` at
`obsParser.ts:90-96` to reject; this rejection is caught in `handleFiles` in
`OBS.tsx:26-33`, which shows `toast.error(e.message)` and prevents preview/upload, even
though the heading format is semantically valid apart from leading whitespace.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** frontend/src/utils/obsParser.ts
**Line:** 24:26
**Comment:**
	*Logic Error: The heading parser validates candidates using `l.trim().startsWith("#")` but then matches the untrimmed `headingLine` against a start-anchored regex. Headings with leading whitespace or a UTF-8 BOM will be detected as headings and then rejected as invalid, causing valid markdown uploads/edits to fail. Normalize and trim (and strip BOM) before applying the regex.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


Comment on lines +24 to +27
Copy link
Copy Markdown

@codeant-ai codeant-ai Bot May 11, 2026

Choose a reason for hiding this comment

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

🟠 Architect Review — HIGH

Heading detection uses l.trim().startsWith("#"), but extractHeadingData matches the untrimmed line against a ^#+-anchored regex, so headings with leading spaces (e.g., " # 1. Title") are detected but then rejected, leaving leading-space support incomplete.

Suggestion: Normalize the heading line (e.g., trim leading whitespace) before applying OBS_HEADING_REGEX in extractHeadingData so detection and extraction accept the same set of headings in both markdown upload and metadata edit flows.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.

**Path:** frontend/src/utils/obsParser.ts
**Line:** 24:27
**Comment:**
	*HIGH: Heading detection uses `l.trim().startsWith("#")`, but `extractHeadingData` matches the untrimmed line against a `^#+`-anchored regex, so headings with leading spaces (e.g., "   # 1. Title") are detected but then rejected, leaving leading-space support incomplete.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

if (!match) {
throw new Error(
"Invalid OBS heading. Expected '# 1. Title' or '# 1۔ عنوان'",
);
}

return {
story_no: Number(match[1]),
title: match[2].trim(),
};
}

export function parseISLOBSCSV(file: File): Promise<OBSStory[]> {
return new Promise((resolve, reject) => {
Expand Down Expand Up @@ -40,23 +69,16 @@ export function parseISLOBSCSV(file: File): Promise<OBSStory[]> {

export async function parseOBSMarkdown(file: File): Promise<OBSStory> {
const content = await file.text();
const lines = content.split("\n");

// markdown heading
const headingLine = lines.find((l) => l.startsWith("#"));
const lines = normalizeContent(content).split("\n");

const headingLine = lines.find((l) => l.trim().startsWith("#"));

if (!headingLine) {
throw new Error("Invalid OBS markdown: missing heading");
}

const match = headingLine.match(/^#+\s*(\d+)[.\s]+(.+)/);

if (!match) {
throw new Error("Invalid OBS heading. Expected format: '# 1. Title'");
}

const story_no = Number(match[1]);
const title = match[2].trim();
const { story_no, title } = extractHeadingData(headingLine);

Comment on lines +75 to 82
Copy link
Copy Markdown

@codeant-ai codeant-ai Bot May 11, 2026

Choose a reason for hiding this comment

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

🟠 Architect Review — HIGH

OBS markdown heading detection now finds lines with leading spaces using l.trim().startsWith("#"), but passes the untrimmed line into extractHeadingData, whose regex (^#+) requires # at the start of the string; headings like " # 1. Title" are still rejected instead of being accepted as intended.

Suggestion: Trim (or otherwise normalize) the heading line before calling extractHeadingData, so the regex sees # at the start, and ensure both file parsing and metadata extraction use this same normalization path for consistent behavior.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.

**Path:** frontend/src/utils/obsParser.ts
**Line:** 75:82
**Comment:**
	*HIGH: OBS markdown heading detection now finds lines with leading spaces using `l.trim().startsWith("#")`, but passes the untrimmed line into `extractHeadingData`, whose regex (`^#+`) requires `#` at the start of the string; headings like `"   # 1. Title"` are still rejected instead of being accepted as intended.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

return {
story_no,
Expand All @@ -77,21 +99,19 @@ export function extractOBSMetadataFromMarkdown(markdown: string): {
story_no: number;
title: string;
} {
const lines = markdown.split("\n");
const headingLine = lines.find((l) => l.startsWith("#"));
const lines = normalizeContent(markdown).split("\n");

const headingLine = lines.find((l) => l.trim().startsWith("#"));

if (!headingLine) {
throw new Error("Invalid OBS markdown: missing heading");
}

const match = headingLine.match(/^#+\s*(\d+)[.\s]+(.+)/);

if (!match) {
throw new Error("Invalid OBS heading. Expected format: '# 1. Title'");
}

return {
story_no: Number(match[1]),
title: match[2].trim(),
};
return extractHeadingData(headingLine);
}

const normalizeContent = (content: string) =>
content
.replace(/^\uFEFF/, "")
.replace(/\r\n/g, "\n")
.replace(/\r/g, "\n");
Loading